17 Oct
2018
17 Oct
'18
11:34 a.m.
On 10/17/18 11:29 AM, Guillaume Delhumeau wrote:
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com <mailto:simon.urli@xwiki.com>
More about us at http://www.xwiki.com
--
Guillaume Delhumeau (guillaume.delhumeau(a)xwiki.com
<mailto:guillaume.delhumeau@xwiki.com>)
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
Le mer. 17 oct. 2018 à 10:47, Simon Urli
<simon.urli(a)xwiki.com
<mailto:simon.urli@xwiki.com>> a écrit :
On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:
I'll create an issue in that way.
I'm OK.
<OFF TOPIC>
I'm just thinking about an other particular case:
Imagine you have 3 event listeners (A, B, C):
- A receives the event and perform some actions (saving something
in the
database).
- B receives the event and cancels it
- C don't receive the event because it had been canceled
However, we may want to resend some infos to listener A so it can
rollback
its actions (otherwise we end up with bad info in
the database).
Do we have a strategy for this?
I don't think we have a strategy for that, but we might add a new
method
in EventListener:
onRollback(CancelableEvent canceledEvent, Object source, Object data)
and store somewhere the list of called listener to be able to call
their
rollback method if the event has been cancelled. Should do the
trick, WDYT?
Sounds like a good idea, indeed. Or it could be called onCanceled()
instead of onRollback, since the rollback is what we expect, not the
event that is triggered.
> </OFF TOPIC>
>
Now Thomas just opens the debate about where to put the behaviour
change: I proposed in my PR to put it in AbstractCancelableEvents to
allow users redefine the behaviour if needed by overriding the method.
Thomas proposes
(https://github.com/xwiki/xwiki-commons/pull/49#issuecomment-430555665)
that we put it in DefaultObservationManager as we want the behaviour for
all CancelableEvents, not only those who inherits from
AbstractCancelableEvents. My concern here is that this behaviour
couldn't be changed easily if needed (unless using another kind of event).
WDYT?
Le mer. 17
oct. 2018 à 09:09, Thomas Mortagne
<thomas.mortagne(a)xwiki.com
<mailto:thomas.mortagne@xwiki.com>> a
écrit :
> +1 to stopping event propagation when it's cancelled
> On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com
<mailto:simon.urli@xwiki.com>> wrote:
>>
>> Hi everyone,
>>
>> the current behaviour of the ObservationManager is to always
triggers
>> the listeners if it matches the events.
>> Now regarding the CancelableEvents, the match is only done on
the
type
>> of the event and some given filter rules,
but never with its cancel
>> status: if an event is cancelled, the matching listeners are always
>> triggered.
>>
>> I propose to change that behaviour, to trigger listeners only
if the
>> CancelableEvents are not canceled:
basically, a cancelled event
wouldn't
>> match any listener.
>>
>> My primary reason for wanting that change is that the current
behaviour
>> led to a bad UX: if an event triggers
multiple questions, no
matter if
one is cancelled, all questions will be asked to the
user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com <mailto:simon.urli@xwiki.com>
More about us at http://www.xwiki.com
--
Thomas Mortagne