> On 17 Oct 2018, at 11:37, Thomas Mortagne <thomas.morta...@xwiki.com> wrote:
>
> On Wed, Oct 17, 2018 at 11:29 AM Vincent Massol <vinc...@massol.net> wrote:
>>
>>
>>
>>> On 17 Oct 2018, at 11:08, Simon Urli <simon.u...@xwiki.com> wrote:
>>
>> [snip]
>>
>>>>>>>>> I reused the existing UI which does not look so bad IMO (see the
>>>>>>>>> screenshot in the design page).
>>>>>>>> This is what happens in the AntiSpam app when the event is cancelled
>>>>>>>> (ie when it finds some spam in the doc):
>>>>>>>> https://extensions.xwiki.org/xwiki/bin/download/Extension/AntiSpam%20Tool%20Application/antispam-error.png
>>>>>>>> As you can see it’s not really user-friendly.
>>>>>>>> Maybe you don’t get this because you’re running inside a job? But in
>>>>>>>> this case I don’t understand why you need a listener since you check
>>>>>>>> for XClass before you start the move/rename.
>>>>>>>> I must be missing something.
>>>>>>>
>>>>>>> I don't check before I start the move/rename, I'm checking after the
>>>>>>> job already started. I reuse exactly the same mechanism as the one used
>>>>>>> when refactoring a page that belongs to an extension:
>>>>>>> https://jira.xwiki.org/browse/XWIKI-14591.
>>>>>> Yes and I remember mentioning that for me it’s too late… How do you
>>>>>> rollback the move/rename if you find an XClass after having
>>>>>> renamed/moved 100 pages? And yes there’s the same problem with the other
>>>>>> refactoring (and I already mentioned the problem) but we need to improve
>>>>>> at some point and not continue doing it in a suboptimal way. We need to
>>>>>> put ourselves in the shoes of the user.
>>>>>
>>>>> Actually you don't have to rollback: the listener is catching the event
>>>>> before doing the refactoring, so the user has a chance to edit the list
>>>>> of pages to refactor before the refactor begins.
>>>> Before any page in the set has been moved/renamed?
>>>> Are we running 2 jobs? One job to perform the check on all pages and
>>>> another job to perform the refactoring?
>>>
>>> No, to be precise here we are talking about a MoveJob which extends
>>> AbstractEntityJobWithChecks (same for DeleteJob and RenameJob): when the
>>> job starts a method runInternal() is called, which performs a check by
>>> creating a DeletingEvent (as some documents might be deleted) and
>>> propagating it to the observationManager.
>>>
>>> Then it's catched by the listener and processed: only after this step, the
>>> job will be really started.
>>
>> ok cool, I don’t fully understand the implementation but I don’t need to ;)
>> (I can check the code if I need).
>
> You could summarize it that way: there is two main steps in the
> delete/rename/move jobs
> * preparation: find out the "concerned pages" and associate each of
> them with a boolean indicating if it should be taken into account or
> not, at the end of this step an event is fired to give a chance to any
> listener to modify that list (for example what Simon is doing in his
> listener is injecting a question in the current job and modify the
> list based on the answer)
> * apply: the action is applied to all the pages in the list with true
Thanks for the explanations. It’s clear now.
Thanks
-Vincent
[snip]
