Handle custom events

[reinhardt]

We would like to have the possibility to let custom events trigger an audit log entry. It is already possible to register the appropriate event handler for a custom event, but no audit log entry will be created. This is because AuditActionExecutor hard codes checks for a number of interfaces on the event and discards events that don't match.

I image registering adapters or utilities for events that return an action verb ('added', 'modified', ...) and other hints about how this event needs to be logged. Maybe the way plone.app.contentrules registers rule names for events can be an inspiration.

[ale-rt]

I think that custom events can be handled by registering a more specific adapter:

collective.auditlog/collective/auditlog/action.py

Lines 53 to 57 in 20d5949

	class AuditActionExecutor(object):
	implements(IExecutable)
	adapts(Interface, IAuditAction, Interface)
	def __init__(self, context, element, event):

You can register an adapter that inherits from AuditActionExecutor and is targeting your own content type iface and/or your custom action and/or your custom event iface.

[reinhardt]

I would have to override the whole __call__ method though. Of course I can delegate to it with super(), but I would still have to duplicate a lot of things in my own overriden method, like retrieving the content rule, checking canExecute(), take care of working copy handling, etc. I don't see how this is much different from a patch. I think some refactoring is needed to make this feasible.

[ale-rt]

Well, a patch is effective for every context, action, event combination, a ZCA registered adapter is not.
Also the __call__ for your custom adapter will be super simple (and much more maintainable than the patch) because it will not contain all the if/else that the original method has.
Of course splitting the __call__ in more methods is welcome and will make things even better.

[reinhardt]

because it will not contain all the if/else that the original method has

...but still pretty much all the rest of the original method. See my examples above.

Also, if two packages have custom events and each registers an adapter override, one will "win" and blot out the other. I was hoping to get to some kind of plugin structure where multiple packages could contribute events/actions.

[reinhardt]

Anyway, I'll try to take a first step by refactoring the if/else block a little. That should be helpful in any case.

[ale-rt]

I would just put the if block in other method for a start, this will already simplify things a lot.
That will be easily overridable.

[reinhardt]

Yes.

[ale-rt]

Also, if two packages have custom events and each registers an adapter override, one will "win" and blot out the other. I was hoping to get to some kind of plugin structure where multiple packages could contribute events/actions.

Well, you have to provide the proper interfaces... Patches are even worse from this point of view.
I think we already have enough adapters to satisfy all our needs. I would try this way before introducing some other way of plugging in things.

[reinhardt]

Well, you have to provide the proper interfaces...

How do I do that if I don't happen to control all of the third party packages?

Patches are even worse from this point of view.

No discussion there. The point of this ticket is to get rid of the need to patch.

[ale-rt]

I am sorry I cannot see where the problem is :(
Third party packages will work as well as they are working now and I still do not see the need for a patch.

[reinhardt]

You're right that I can do an adapter override and that this has certain advantages over a patch. I just think that the advantages aren't big enough in this case to justify using an adapter. Currently we already have an overriden adapter in a customer package. I would now create another adapter override in an intermediary package. That would however be ignored because the customer package would still use its own adapter. So I would have to change the customer adapter to derive from the intermediary adapter, or at least in some way make sure the code from both packages is used. If I think that a little further I end up with a stack of dependent adapters, and if I break one of them everything topples over.

[ale-rt]

I think this is not more a thing after #20 is merged.
Reopen (or open a more detailed one) if not