On 17 Oct 2018, at 11:37, Thomas Mortagne <[email protected]> wrote:
On Wed, Oct 17, 2018 at 11:29 AM Vincent Massol <[email protected]> wrote:
On 17 Oct 2018, at 11:08, Simon Urli <[email protected]> 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%20... >> 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]