search

ezsystems / eztags

GNU General Public License v2.0
40 stars 40 forks source link

Event filters #48

Closed bchoquet-heliopsis closed 12 years ago

bchoquet-heliopsis commented 12 years ago

Hi, here are a few more events where it wasn't implemented yet.

I found it better to use standard add/edit/delete events for synonyms manipulation : observers will be able to tell wether a tag is a synonym and if it requires a different processing.

Hope you like it.

gggeek commented 12 years ago

Any reason to use the ezpevent system instead of triggers/operations? Do you know how easy it is to hook up workflows to ezpevents?

bchoquet-heliopsis commented 12 years ago

It is what was already partly implemented in add, edit, delete and merge modules. I just went on with it. As for your second question I'm not sure I understand it, but I definitely find it better to use an observer pattern than the heavy trigger system.

gggeek commented 12 years ago

The trigger system is heavy, but it is also much more featureful (it implements a workflow engine, while ezpevents do not, afaik). I am thinking e.g. about using an existing workflow with generic events in it (eg send a mail, call a webservice, whatever), chained to a tag alteration

dpobel commented 12 years ago

IMO, the ezpEvent is the right way, it's easy to use and I don't see any good reason to implement the trigger system. One question though: why do you test the existence of the ezpEvent class ? to me, it's useless, we can assume it is there (it was added more than one year ago if I remember correctly).

emodric commented 12 years ago

@gggeek Initial ezpEvent filters are there because Smart Tags extension from eZ Market is using them.

@dpobel Testing class existence is for compatibility with eZ Publish 4.3 & 4.4.

dpobel commented 12 years ago

@emodric that was clear for me :-) my question was more: is this compatibility code really needed ?

bchoquet-heliopsis commented 12 years ago

I didn't mean to initiate such a debate :) Let me know if you want me to remove the compatibility tests.

emodric commented 12 years ago

@dpobel Doh, my bad :) I would say that yes, it is needed if I don't wanna piss off some of our clients which still use eZP 4.4 :)

@bchoquet-heliopsis It's fine this way, they should stay. I'll review the pull request in coming days.

bchoquet-heliopsis commented 12 years ago

ok, thanks