collective / collective.elephantvocabulary

zope vocabulary with possibility to hide/show terms
http://pypi.python.org/pypi/collective.elephantvocabulary
GNU General Public License v2.0
1 stars 0 forks source link

Hidden / visible terms can't be removed by removing them from registry #5

Open lukasgraf opened 10 years ago

lukasgraf commented 10 years ago

`Currently, the internal terms lists only get populated from the registry by appending to them.

However, terms never get removed from the internal lists, only appended. This means removing a term from the registry record won't have any effect (until the instance is restarted).

With the current implementation it's not trivial to change this: There's one single list on the vocabulary factory for each of the term types (visible / hidden). This list gets populated with the initial terms passed in to __init__. Later (every __call__), this list gets appended to the terms from the registry.

This means it's not possible to distinguish where terms in those internal lists came from (registry or initialization). So if we were to remove all terms that aren't in the registry record any more, we would potentially remove terms that came in trough initialization, not the registry.

I'm not quite sure what the desired / expected behavior is here. It only matters if both concepts are used in conjunction, passing in terms to __init__ as well as using registry terms. But I would suspect that removing a term from the registry causing initialization terms to be removed would take people by surprise.

So the clean way to address this would probably be to store terms passed in to __init__ in an separate instance attribute, different from the one that's checked against, and compute the value of the attribute that's checked against anew every time. So conceptually something like

hidden_terms = list(set(initial_hidden_terms + hidden_terms_from_registry)
lukasgraf commented 10 years ago

/cc @garbas