date:20181017




> On 17 Oct 2018, at 15:54, Vincent Massol  wrote:
> 
> Hi,
> 
>> On 17 Oct 2018, at 11:20, Vincent Massol  wrote:
>> 
>> Hi,
>> 
>> [snip]
>> 
>>> Process to run DSpot:
>>> 1) Pick a module. Measure coverage and mutation score (or take the value 
>>> there already if they’re in the pom.xml). Same as for Descartes testing.
>>> 2) Run DSpot on the module, see 
>>> https://massol.myxwiki.org/xwiki/bin/view/Blog/TestGenerationDspot for 
>>> explanations
>> 
>> One important detail that I had missed. We need to run Dspot with 
>> “—descartes” on the command line so that it uses Descartes for computing the 
>> mutation score for mutations and only keep tests that increase the mutation 
>> score as reported by Descartes.
> 
> So actually, after speaking with Benjamin, I’ve realized a few things:
> 
> * By default DSpot runs with the PIT selector (PitMutantScoreSelector) which 
> is configured to use the default PIT mutations. This is why we need to run 
> with the PIT selector but configured to use the Descartes mutation, and this 
> is done by specifying --descartes.
> * Now this will optimize the generation of new tests for their increased 
> mutation score. Right now we got 0% all the time on our tests (see 
> https://docs.google.com/spreadsheets/d/1LULpGpsJirmFyvHNstLGv-Gv5DVBdpLTM2hm0jgCKUw/edit#gid=2061481816)
>  and it’s because we didn’t use --descartes. We need to try again or run on 
> new modules with --descartes and see what it gives us. It’s possible it’ll 
> generate even less tests…
> * For the coverage part, there are 2 other selectors that can be used with 
> DSpot to generate tests that all increase the coverage:
> ** "--test-criterion JacocoCoverageSelector": uses jacoco and keep tests that 
> increase the instruction coverage
> ** "--test-criterion CloverCoverageSelector”: uses openclover and keep tests 
> that increase the branch coverage
> 
> So we need to test with the various selectors and see what we get. 

I’ve retested on xwiki-commons-component-default:
1) With —descartes: failure, see 
https://github.com/STAMP-project/dspot/issues/584
2) With jacoco selector: failure, see 
https://github.com/STAMP-project/dspot/issues/586. I’ve manually fixed the 
tests and remove those that didn’t pass. I got only +0.18% jacoco coverage 
increase and -2% descartes mutation score… That’s the problem, we would need a 
selector that optimizes for both. I’ve created 
https://github.com/STAMP-project/dspot/issues/587
3) With clover selector: no tests generated! Opened 
https://github.com/STAMP-project/dspot/issues/588

So my recommendation is to wait for 
https://github.com/STAMP-project/dspot/issues/584 to be fixed and then to use 
—descartes for our measures FTM.

Thanks
-Vincent

PS: Command lines used for reference:

- java -jar 
/Users/vmassol/dev/dspot/dspot/target/dspot-1.1.1-SNAPSHOT-jar-with-dependencies.jar
 --path-to-properties dspot.properties --descartes --verbose 
--generate-new-test-class --with-comment
- java -jar 
/Users/vmassol/dev/dspot/dspot/target/dspot-1.1.1-SNAPSHOT-jar-with-dependencies.jar
 --path-to-properties dspot.properties --test-criterion JacocoCoverageSelector 
--verbose --generate-new-test-class --with-comment
- java -jar 
/Users/vmassol/dev/dspot/dspot/target/dspot-1.1.1-SNAPSHOT-jar-with-dependencies.jar
 --path-to-properties dspot.properties --test-criterion CloverCoverageSelector 
--verbose --generate-new-test-class --with-comment


> 
> If we want to get the best values, we should use --descartes for K03 and 
> either jacoco or clover selector for K01. Now we need to see what tests we 
> get.
> 
> Thanks
> -Vincent
> 
>> 
>>> 3) If DSpot has generated tests, add them to XWiki’s source code in 
>>> src/test/dspot and add the following to the pom of that module:
>>> 
>>> 
>>> 
>>>  
>>>  
>>>org.codehaus.mojo
>>>build-helper-maven-plugin
>>>  
>>> 
>>> 
>>> 
>>> Example: 
>>> https://github.com/xwiki/xwiki-commons/tree/244ee07976c691c335b7f54c48e6308004ba3d82/xwiki-commons-core/xwiki-commons-crypto/xwiki-commons-crypto-cipher
>>> 
>>> Note: The generated tests sometimes need to be modified a bit to pass. 
>>> Personally I’ve only committed tests that were passing and I reported 
>>> issues for those that were not passing.
>>> 
>>> 4) File the various reports:
>>> a) https://github.com/STAMP-project/dspot-usecases-output/tree/master/xwiki 
>>> both for success and failures
>>> b) 
>>> https://docs.google.com/spreadsheets/d/1LULpGpsJirmFyvHNstLGv-Gv5DVBdpLTM2hm0jgCKUw/edit#gid=2061481816
>>> c) for failures, file a github issue at 
>>> https://github.com/STAMP-project/dspot/issues and link to the place on 
>>> https://github.com/STAMP-project/dspot-usecases-output/tree/master/xwiki 
>>> where we put the failing result.
>>> 
>>> Note: The reason we need to report failures too is because DSpot fails a 
>>> lot so we need to show what we have tested
>>> 
>>> Thanks
>>> -Vincent
>>> 
>> 
>> [snip]
>> 
>> Thanks
>> -Vincent

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button

On Wed, Oct 17, 2018 at 7:13 PM Vincent Massol  wrote:

>
>
> > On 17 Oct 2018, at 18:10, Ecaterina Moraru (Valica) 
> wrote:
> >
> > On Wed, Oct 17, 2018 at 6:58 PM Vincent Massol 
> wrote:
> >
> >>
> >>
> >>> On 17 Oct 2018, at 17:46, Ecaterina Moraru (Valica)  >
> >> wrote:
> >>>
> >>> Hi Vincent,
> >>>
> >>> This proposal doesn't apply to the "Install" button, since there are
> >> steps
> >>> where there is not just a single button.
> >>> Each Wiki or Flavor could have their own "Install", so that's why the
> >>> position is related to each type of entity, see an example of multiple
> >>> "Upgrade" buttons
> >>>
> >>
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-wikisStep.png
> >>> or in the case of extensions, see
> >>>
> >>
> https://network.xwiki.com/xwiki/bin/download/DocXE981En/XEAttachments/InvalidExtensions.png
> >>
> >> ok. We could have not button at all on the bottom right for the flavor
> >> screen for ex.
> >>
> >> It’s not clear how VAR1 & 2 would looklike for the flavor screen. No
> >> button in the bottom right corner, right? (which is fine with me).
> >>
> >
> > Well the VAR1 and VAR2 only refer to the Later/Never/Skip actions.
>
> If you check the flavor screen at
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
> you’ll see it has "later" and “never” so I think it applies to this screen
> too, no?
>

In this particular screen, the "later"/ "never" would be replaced by a
"Skip" button, placed in the left side (the opposite from their current
location).


>
> Thanks
> -Vincent
>
> >
> > In theory the "Continue" would still stay there. I don't know how
> mandatory
> > is the presence of the "Continue" button in this wikis case. I guess you
> > could continue the wizard, without doing the upgrades now and continue in
> > each wiki separately. So the "Continue" is still a valid action. The
> issue
> > is that it has the same visual importance as "Upgrade" and it provide an
> > easy get away for the user, so the user might click it and not complete
> the
> > needed steps.
> >
> > Another problem is that we don't have an "Upgrade all" button (
> > https://jira.xwiki.org/browse/XWIKI-15633) so in case we would have more
> > wikis / extensions, we would need to press on each of them. A solution
> > would be to have the "Upgrade all" near the "Continue".
> >
> >
> >>
> >> Thanks
> >> -Vincent
> >>
> >>>
> >>> Thanks,
> >>> Caty
> >>>
> >>> On Wed, Oct 17, 2018 at 6:16 PM Vincent Massol 
> >> wrote:
> >>>
>  Hi Caty,
> 
>  Thanks for this.
> 
>  Actually the screenshot you have at
> 
> >>
> https://design.xwiki.org/xwiki/bin/download/Proposal/Upgrade/DWNeverButton/WebHome/dw-welcomeStep.png
>  is not the only issue for me.
> 
>  The problem I had was on the flavor screen because you need to click
>  “install” at the top and not click “never” which is at the bottom
> right,
>  see
> 
> >>
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
> 
>  To be more precise I was expecting to select the flavor and click
>  “Install” at the bottom right.
> 
>  Does your proposal address this issue? Would be great to see the
> Flavor
>  screen with your new proposal.
> 
>  VAR1 and 2 look nice. VAR2 seems a bit more explicit.
> 
>  Thanks
>  -Vincent
> 
> > On 17 Oct 2018, at 16:49, Ecaterina Moraru (Valica) <
> vali...@gmail.com
> >>>
>  wrote:
> >
> > Hi devs,
> >
> > This is an improvements proposal for the "Never" button from
> >> Distribution
> > Wizard.
> >
> >> https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
> >
> > I've listed several variants of fixing the usability issue, let me
> know
> > which one is your favorite.
> >
> > My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
> > Location.
> >
> > Thanks,
> > Caty
>
>

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button




> On 17 Oct 2018, at 18:10, Ecaterina Moraru (Valica)  wrote:
> 
> On Wed, Oct 17, 2018 at 6:58 PM Vincent Massol  wrote:
> 
>> 
>> 
>>> On 17 Oct 2018, at 17:46, Ecaterina Moraru (Valica) 
>> wrote:
>>> 
>>> Hi Vincent,
>>> 
>>> This proposal doesn't apply to the "Install" button, since there are
>> steps
>>> where there is not just a single button.
>>> Each Wiki or Flavor could have their own "Install", so that's why the
>>> position is related to each type of entity, see an example of multiple
>>> "Upgrade" buttons
>>> 
>> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-wikisStep.png
>>> or in the case of extensions, see
>>> 
>> https://network.xwiki.com/xwiki/bin/download/DocXE981En/XEAttachments/InvalidExtensions.png
>> 
>> ok. We could have not button at all on the bottom right for the flavor
>> screen for ex.
>> 
>> It’s not clear how VAR1 & 2 would looklike for the flavor screen. No
>> button in the bottom right corner, right? (which is fine with me).
>> 
> 
> Well the VAR1 and VAR2 only refer to the Later/Never/Skip actions.

If you check the flavor screen at 
https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
 you’ll see it has "later" and “never” so I think it applies to this screen 
too, no?

Thanks
-Vincent

> 
> In theory the "Continue" would still stay there. I don't know how mandatory
> is the presence of the "Continue" button in this wikis case. I guess you
> could continue the wizard, without doing the upgrades now and continue in
> each wiki separately. So the "Continue" is still a valid action. The issue
> is that it has the same visual importance as "Upgrade" and it provide an
> easy get away for the user, so the user might click it and not complete the
> needed steps.
> 
> Another problem is that we don't have an "Upgrade all" button (
> https://jira.xwiki.org/browse/XWIKI-15633) so in case we would have more
> wikis / extensions, we would need to press on each of them. A solution
> would be to have the "Upgrade all" near the "Continue".
> 
> 
>> 
>> Thanks
>> -Vincent
>> 
>>> 
>>> Thanks,
>>> Caty
>>> 
>>> On Wed, Oct 17, 2018 at 6:16 PM Vincent Massol 
>> wrote:
>>> 
 Hi Caty,
 
 Thanks for this.
 
 Actually the screenshot you have at
 
>> https://design.xwiki.org/xwiki/bin/download/Proposal/Upgrade/DWNeverButton/WebHome/dw-welcomeStep.png
 is not the only issue for me.
 
 The problem I had was on the flavor screen because you need to click
 “install” at the top and not click “never” which is at the bottom right,
 see
 
>> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
 
 To be more precise I was expecting to select the flavor and click
 “Install” at the bottom right.
 
 Does your proposal address this issue? Would be great to see the Flavor
 screen with your new proposal.
 
 VAR1 and 2 look nice. VAR2 seems a bit more explicit.
 
 Thanks
 -Vincent
 
> On 17 Oct 2018, at 16:49, Ecaterina Moraru (Valica) >> 
 wrote:
> 
> Hi devs,
> 
> This is an improvements proposal for the "Never" button from
>> Distribution
> Wizard.
> 
>> https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
> 
> I've listed several variants of fixing the usability issue, let me know
> which one is your favorite.
> 
> My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
> Location.
> 
> Thanks,
> Caty

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button

On Wed, Oct 17, 2018 at 6:58 PM Vincent Massol  wrote:

>
>
> > On 17 Oct 2018, at 17:46, Ecaterina Moraru (Valica) 
> wrote:
> >
> > Hi Vincent,
> >
> > This proposal doesn't apply to the "Install" button, since there are
> steps
> > where there is not just a single button.
> > Each Wiki or Flavor could have their own "Install", so that's why the
> > position is related to each type of entity, see an example of multiple
> > "Upgrade" buttons
> >
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-wikisStep.png
> > or in the case of extensions, see
> >
> https://network.xwiki.com/xwiki/bin/download/DocXE981En/XEAttachments/InvalidExtensions.png
>
> ok. We could have not button at all on the bottom right for the flavor
> screen for ex.
>
> It’s not clear how VAR1 & 2 would looklike for the flavor screen. No
> button in the bottom right corner, right? (which is fine with me).
>

Well the VAR1 and VAR2 only refer to the Later/Never/Skip actions.

In theory the "Continue" would still stay there. I don't know how mandatory
is the presence of the "Continue" button in this wikis case. I guess you
could continue the wizard, without doing the upgrades now and continue in
each wiki separately. So the "Continue" is still a valid action. The issue
is that it has the same visual importance as "Upgrade" and it provide an
easy get away for the user, so the user might click it and not complete the
needed steps.

Another problem is that we don't have an "Upgrade all" button (
https://jira.xwiki.org/browse/XWIKI-15633) so in case we would have more
wikis / extensions, we would need to press on each of them. A solution
would be to have the "Upgrade all" near the "Continue".


>
> Thanks
> -Vincent
>
> >
> > Thanks,
> > Caty
> >
> > On Wed, Oct 17, 2018 at 6:16 PM Vincent Massol 
> wrote:
> >
> >> Hi Caty,
> >>
> >> Thanks for this.
> >>
> >> Actually the screenshot you have at
> >>
> https://design.xwiki.org/xwiki/bin/download/Proposal/Upgrade/DWNeverButton/WebHome/dw-welcomeStep.png
> >> is not the only issue for me.
> >>
> >> The problem I had was on the flavor screen because you need to click
> >> “install” at the top and not click “never” which is at the bottom right,
> >> see
> >>
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
> >>
> >> To be more precise I was expecting to select the flavor and click
> >> “Install” at the bottom right.
> >>
> >> Does your proposal address this issue? Would be great to see the Flavor
> >> screen with your new proposal.
> >>
> >> VAR1 and 2 look nice. VAR2 seems a bit more explicit.
> >>
> >> Thanks
> >> -Vincent
> >>
> >>> On 17 Oct 2018, at 16:49, Ecaterina Moraru (Valica)  >
> >> wrote:
> >>>
> >>> Hi devs,
> >>>
> >>> This is an improvements proposal for the "Never" button from
> Distribution
> >>> Wizard.
> >>>
> https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
> >>>
> >>> I've listed several variants of fixing the usability issue, let me know
> >>> which one is your favorite.
> >>>
> >>> My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
> >>> Location.
> >>>
> >>> Thanks,
> >>> Caty
> >>
> >>
>
>

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button




> On 17 Oct 2018, at 17:46, Ecaterina Moraru (Valica)  wrote:
> 
> Hi Vincent,
> 
> This proposal doesn't apply to the "Install" button, since there are steps
> where there is not just a single button.
> Each Wiki or Flavor could have their own "Install", so that's why the
> position is related to each type of entity, see an example of multiple
> "Upgrade" buttons
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-wikisStep.png
> or in the case of extensions, see
> https://network.xwiki.com/xwiki/bin/download/DocXE981En/XEAttachments/InvalidExtensions.png

ok. We could have not button at all on the bottom right for the flavor screen 
for ex. 

It’s not clear how VAR1 & 2 would looklike for the flavor screen. No button in 
the bottom right corner, right? (which is fine with me).

Thanks
-Vincent

> 
> Thanks,
> Caty
> 
> On Wed, Oct 17, 2018 at 6:16 PM Vincent Massol  wrote:
> 
>> Hi Caty,
>> 
>> Thanks for this.
>> 
>> Actually the screenshot you have at
>> https://design.xwiki.org/xwiki/bin/download/Proposal/Upgrade/DWNeverButton/WebHome/dw-welcomeStep.png
>> is not the only issue for me.
>> 
>> The problem I had was on the flavor screen because you need to click
>> “install” at the top and not click “never” which is at the bottom right,
>> see
>> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
>> 
>> To be more precise I was expecting to select the flavor and click
>> “Install” at the bottom right.
>> 
>> Does your proposal address this issue? Would be great to see the Flavor
>> screen with your new proposal.
>> 
>> VAR1 and 2 look nice. VAR2 seems a bit more explicit.
>> 
>> Thanks
>> -Vincent
>> 
>>> On 17 Oct 2018, at 16:49, Ecaterina Moraru (Valica) 
>> wrote:
>>> 
>>> Hi devs,
>>> 
>>> This is an improvements proposal for the "Never" button from Distribution
>>> Wizard.
>>> https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
>>> 
>>> I've listed several variants of fixing the usability issue, let me know
>>> which one is your favorite.
>>> 
>>> My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
>>> Location.
>>> 
>>> Thanks,
>>> Caty
>> 
>>

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button

Hi Vincent,

This proposal doesn't apply to the "Install" button, since there are steps
where there is not just a single button.
Each Wiki or Flavor could have their own "Install", so that's why the
position is related to each type of entity, see an example of multiple
"Upgrade" buttons
https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-wikisStep.png
or in the case of extensions, see
https://network.xwiki.com/xwiki/bin/download/DocXE981En/XEAttachments/InvalidExtensions.png

Thanks,
Caty

On Wed, Oct 17, 2018 at 6:16 PM Vincent Massol  wrote:

> Hi Caty,
>
> Thanks for this.
>
> Actually the screenshot you have at
> https://design.xwiki.org/xwiki/bin/download/Proposal/Upgrade/DWNeverButton/WebHome/dw-welcomeStep.png
> is not the only issue for me.
>
> The problem I had was on the flavor screen because you need to click
> “install” at the top and not click “never” which is at the bottom right,
> see
> https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png
>
> To be more precise I was expecting to select the flavor and click
> “Install” at the bottom right.
>
> Does your proposal address this issue? Would be great to see the Flavor
> screen with your new proposal.
>
> VAR1 and 2 look nice. VAR2 seems a bit more explicit.
>
> Thanks
> -Vincent
>
> > On 17 Oct 2018, at 16:49, Ecaterina Moraru (Valica) 
> wrote:
> >
> > Hi devs,
> >
> > This is an improvements proposal for the "Never" button from Distribution
> > Wizard.
> > https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
> >
> > I've listed several variants of fixing the usability issue, let me know
> > which one is your favorite.
> >
> > My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
> > Location.
> >
> > Thanks,
> > Caty
>
>

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button

Hi Caty,

Thanks for this.

Actually the screenshot you have at 
https://design.xwiki.org/xwiki/bin/download/Proposal/Upgrade/DWNeverButton/WebHome/dw-welcomeStep.png
 is not the only issue for me.

The problem I had was on the flavor screen because you need to click “install” 
at the top and not click “never” which is at the bottom right, see 
https://www.xwiki.org/xwiki/bin/download/Documentation/UserGuide/Features/DistributionWizard/dw-flavor-step1.png

To be more precise I was expecting to select the flavor and click “Install” at 
the bottom right.

Does your proposal address this issue? Would be great to see the Flavor screen 
with your new proposal.

VAR1 and 2 look nice. VAR2 seems a bit more explicit.

Thanks
-Vincent

> On 17 Oct 2018, at 16:49, Ecaterina Moraru (Valica)  wrote:
> 
> Hi devs,
> 
> This is an improvements proposal for the "Never" button from Distribution
> Wizard.
> https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
> 
> I've listed several variants of fixing the usability issue, let me know
> which one is your favorite.
> 
> My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
> Location.
> 
> Thanks,
> Caty

Re: [xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button

2018-10-17 Thread Marius Dumitru Florea

Both VAR1 and VAR2 look good to me.

Thanks,
Marius

On Wed, Oct 17, 2018 at 5:49 PM Ecaterina Moraru (Valica) 
wrote:

> Hi devs,
>
> This is an improvements proposal for the "Never" button from Distribution
> Wizard.
> https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/
>
> I've listed several variants of fixing the usability issue, let me know
> which one is your favorite.
>
> My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
> Location.
>
> Thanks,
> Caty
>

[xwiki-devs] [UX][Proposal] Distribution Wizard: Never Button

Hi devs,

This is an improvements proposal for the "Never" button from Distribution
Wizard.
https://design.xwiki.org/xwiki/bin/view/Proposal/Upgrade/DWNeverButton/

I've listed several variants of fixing the usability issue, let me know
which one is your favorite.

My +1 goes to VAR2: Confirmation; with minimum we can do being VAR1:
Location.

Thanks,
Caty

Re: [xwiki-devs] [STAMP/Test] Metrics we need to improve + strategy

Hi,

> On 17 Oct 2018, at 11:20, Vincent Massol  wrote:
> 
> Hi,
> 
> [snip]
> 
>> Process to run DSpot:
>> 1) Pick a module. Measure coverage and mutation score (or take the value 
>> there already if they’re in the pom.xml). Same as for Descartes testing.
>> 2) Run DSpot on the module, see 
>> https://massol.myxwiki.org/xwiki/bin/view/Blog/TestGenerationDspot for 
>> explanations
> 
> One important detail that I had missed. We need to run Dspot with 
> “—descartes” on the command line so that it uses Descartes for computing the 
> mutation score for mutations and only keep tests that increase the mutation 
> score as reported by Descartes.

So actually, after speaking with Benjamin, I’ve realized a few things:

* By default DSpot runs with the PIT selector (PitMutantScoreSelector) which is 
configured to use the default PIT mutations. This is why we need to run with 
the PIT selector but configured to use the Descartes mutation, and this is done 
by specifying --descartes.
* Now this will optimize the generation of new tests for their increased 
mutation score. Right now we got 0% all the time on our tests (see 
https://docs.google.com/spreadsheets/d/1LULpGpsJirmFyvHNstLGv-Gv5DVBdpLTM2hm0jgCKUw/edit#gid=2061481816)
 and it’s because we didn’t use --descartes. We need to try again or run on new 
modules with --descartes and see what it gives us. It’s possible it’ll generate 
even less tests…
* For the coverage part, there are 2 other selectors that can be used with 
DSpot to generate tests that all increase the coverage:
** "--test-criterion JacocoCoverageSelector": uses jacoco and keep tests that 
increase the instruction coverage
** "--test-criterion CloverCoverageSelector”: uses openclover and keep tests 
that increase the branch coverage

So we need to test with the various selectors and see what we get. 

If we want to get the best values, we should use --descartes for K03 and either 
jacoco or clover selector for K01. Now we need to see what tests we get.

Thanks
-Vincent

> 
>> 3) If DSpot has generated tests, add them to XWiki’s source code in 
>> src/test/dspot and add the following to the pom of that module:
>> 
>> 
>> 
>>   
>>   
>> org.codehaus.mojo
>> build-helper-maven-plugin
>>   
>> 
>> 
>> 
>> Example: 
>> https://github.com/xwiki/xwiki-commons/tree/244ee07976c691c335b7f54c48e6308004ba3d82/xwiki-commons-core/xwiki-commons-crypto/xwiki-commons-crypto-cipher
>> 
>> Note: The generated tests sometimes need to be modified a bit to pass. 
>> Personally I’ve only committed tests that were passing and I reported issues 
>> for those that were not passing.
>> 
>> 4) File the various reports:
>> a) https://github.com/STAMP-project/dspot-usecases-output/tree/master/xwiki 
>> both for success and failures
>> b) 
>> https://docs.google.com/spreadsheets/d/1LULpGpsJirmFyvHNstLGv-Gv5DVBdpLTM2hm0jgCKUw/edit#gid=2061481816
>> c) for failures, file a github issue at 
>> https://github.com/STAMP-project/dspot/issues and link to the place on 
>> https://github.com/STAMP-project/dspot-usecases-output/tree/master/xwiki 
>> where we put the failing result.
>> 
>> Note: The reason we need to report failures too is because DSpot fails a lot 
>> so we need to show what we have tested
>> 
>> Thanks
>> -Vincent
>> 
> 
> [snip]
> 
> Thanks
> -Vincent
> 
>

[xwiki-devs] [ANN] XWiki 9.11.8 released

2018-10-17 Thread Eduard Moraru

The XWiki development team is proud to announce the availability of XWiki
9.11.8.

This is a bugfix release that covers important issues that we have
discovered since 9.11.7 has been released.

You can download it here: http://www.xwiki.org/xwiki/bin/view/Main/Download

Make sure to review the release notes:
http://www.xwiki.org/xwiki/bin/view/ReleaseNotes/Data/XWiki/9.11.8

Thanks for your support
-The XWiki dev team

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents

2018-10-17 Thread Marius Dumitru Florea

An use case is to be able to log all the events that happen in XWiki. It
would be stupid to not be able to log an event because there's another
event listener that cancels the event before.

On Wed, Oct 17, 2018 at 3:21 PM Marius Dumitru Florea <
mariusdumitru.flo...@xwiki.com> wrote:

> -0. I would be OK to stop the event propagation if the list of event
> listeners were *ordered* but AFAIK there's no order between event
> listeners ATM. It should always be possible to catch an event, even if that
> event is going to be canceled by some other listener:
>
> * either by registering an event listener with higher priority than the
> one canceling the event
> * or by not stopping the event propagation and letting each listener
> decide what to do based on the canceled state of the event
>
> For instance, DOM events have this
> https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-flow-capture
>
> If the capturing EventListener
>> 
>> wishes to prevent further processing of the event from occurring it may
>> call the stopProgagation method of the Event
>> 
>> interface. This will prevent further dispatch of the event, although
>> additional EventListeners
>> 
>> registered at the same hierarchy level will still receive the event.
>>
>
> So you can always catch the DOM event by registering the event listener on
> the element that triggers the event (rather than on one of its parents).
>
> Thanks,
> Marius
>
>
> On Tue, Oct 16, 2018 at 7:07 PM Simon Urli  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.u...@xwiki.com
>> More about us at http://www.xwiki.com
>>
>

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents

2018-10-17 Thread Marius Dumitru Florea

-0. I would be OK to stop the event propagation if the list of event
listeners were *ordered* but AFAIK there's no order between event listeners
ATM. It should always be possible to catch an event, even if that event is
going to be canceled by some other listener:

* either by registering an event listener with higher priority than the one
canceling the event
* or by not stopping the event propagation and letting each listener decide
what to do based on the canceled state of the event

For instance, DOM events have this
https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-flow-capture

If the capturing EventListener
> 
> wishes to prevent further processing of the event from occurring it may
> call the stopProgagation method of the Event
> 
> interface. This will prevent further dispatch of the event, although
> additional EventListeners
> 
> registered at the same hierarchy level will still receive the event.
>

So you can always catch the DOM event by registering the event listener on
the element that triggers the event (rather than on one of its parents).

Thanks,
Marius


On Tue, Oct 16, 2018 at 7:07 PM Simon Urli  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.u...@xwiki.com
> More about us at http://www.xwiki.com
>

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition




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

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition

2018-10-17 Thread Thomas Mortagne

On Wed, Oct 17, 2018 at 11:29 AM Vincent Massol  wrote:
>
>
>
> > On 17 Oct 2018, at 11:08, Simon Urli  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

>
> All that’s important is that the user can cancel before the refactoring 
> starts and that’s great!
>
> >> Thanks
> >> -Vincent
> >
> >>>
>  2) this is after the fact. Imagine that you’re renaming a set of 
>  pages and among them there are several coming from an app. It’ll 
>  work fine on pages not having an XClass (like moving the page having 
>  an XObject of that XClass) and then failing down the line on the 
>  page having the XClass. That’s a problem because the xobject page 
>  will be wrongly moved, since it doesn’t make sense that it’s moved 
>  if the other pages of the app are not moved. Generally speaking 
>  you’ll have a bad state that is not easy to rollback.
>  This is why for me the check also has to be done in the move/rename 
>  UI and verify that among the list of pages there are none with 
>  XClass and if so prevent moving/renaming any page.
>  This is not in contradiction with the listener but the more 
>  important (from a usage POV) is the check in the move/rename UI and 
>  not the listener which is a more advanced use case.
> >>>
> >>> There might be a misunderstanding here: I use the listener to check 
> >>> the event that are fired during the rename/move. As you can see in my 
> >>> screenshot, I got the warning in the move/refactoring UI.
> >> This listener is registered only during rename/move?
> >> What happens if I write a script that moves/renames pages with XClass?
> >
> > Nop the listener is globally registered, so I assume it would be 
> > triggered when running a script too.
>  ok, so try this: 
>  http://playground.xwiki.org/xwiki/bin/delete/MyPage/?confirm=1
>  What happens?
> >
> > When doing:
> > localhost:8080/xwiki/bin/delete/Menu/?confirm=1
> >
> > I first get a warning because of the secret token, and if I ask to continue 
> > I get

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents





On 10/17/18 11:29 AM, Guillaume Delhumeau wrote:
Le mer. 17 oct. 2018 à 10:47, Simon Urli > a écrit :




On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:
 > I'm OK.
 >
 > 
 > 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.


I'll create an issue in that way.



 > 
 >


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
mailto:thomas.morta...@xwiki.com>> a
 > écrit :
 >
 >> +1 to stopping event propagation when it's cancelled
 >> On Tue, Oct 16, 2018 at 6:07 PM Simon Urli mailto:simon.u...@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.u...@xwiki.com 
 >>> More about us at http://www.xwiki.com
 >>
 >>
 >>
 >> --
 >> Thomas Mortagne
 >>
 >
 >

-- 
Simon Urli

Software Engineer at XWiki SAS
simon.u...@xwiki.com 
More about us at http://www.xwiki.com



--
Guillaume Delhumeau (guillaume.delhum...@xwiki.com 
)

Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project


--
Simon Urli
Software Engineer at XWiki SAS
simon.u...@xwiki.com
More about us at http://www.xwiki.com

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition




> On 17 Oct 2018, at 11:08, Simon Urli  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).

All that’s important is that the user can cancel before the refactoring starts 
and that’s great!

>> Thanks
>> -Vincent
> 
>>> 
 2) this is after the fact. Imagine that you’re renaming a set of pages 
 and among them there are several coming from an app. It’ll work fine 
 on pages not having an XClass (like moving the page having an XObject 
 of that XClass) and then failing down the line on the page having the 
 XClass. That’s a problem because the xobject page will be wrongly 
 moved, since it doesn’t make sense that it’s moved if the other pages 
 of the app are not moved. Generally speaking you’ll have a bad state 
 that is not easy to rollback.
 This is why for me the check also has to be done in the move/rename UI 
 and verify that among the list of pages there are none with XClass and 
 if so prevent moving/renaming any page.
 This is not in contradiction with the listener but the more important 
 (from a usage POV) is the check in the move/rename UI and not the 
 listener which is a more advanced use case.
>>> 
>>> There might be a misunderstanding here: I use the listener to check the 
>>> event that are fired during the rename/move. As you can see in my 
>>> screenshot, I got the warning in the move/refactoring UI.
>> This listener is registered only during rename/move?
>> What happens if I write a script that moves/renames pages with XClass?
> 
> Nop the listener is globally registered, so I assume it would be 
> triggered when running a script too.
 ok, so try this: 
 http://playground.xwiki.org/xwiki/bin/delete/MyPage/?confirm=1
 What happens?
> 
> When doing:
> localhost:8080/xwiki/bin/delete/Menu/?confirm=1
> 
> I first get a warning because of the secret token, and if I ask to continue I 
> get redirected to the delete page, so I got the warning as for 
> renaming/moving pages.

Ok, I was trying to find a UI way to not go through the rename/move warning but 
I don’t think that’s possible ;-)

What I wanted to show is that right now we don’t have a nice UI when an event 
is cancelled and it bubble up in the UI (which is what you can see on the 
Antispam app screensht).

But it doesn’t matter in this case since it always go through the job and thus 
we won’t have it and for scripts there’s no UI anyway so it’s normal to get an 
exception.

> Now concerning scripts, I was wrong in my previous answer: we check in the 
> listener if the job is interactive or not, and if it's not interactive we 
> skip the listener.
> Now we might

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents

2018-10-17 Thread Guillaume Delhumeau

Le mer. 17 oct. 2018 à 10:47, Simon Urli  a écrit :

>
>
> On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:
> > I'm OK.
> >
> > 
> > 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.


>
> > 
> >
> > Le mer. 17 oct. 2018 à 09:09, Thomas Mortagne 
> a
> > écrit :
> >
> >> +1 to stopping event propagation when it's cancelled
> >> On Tue, Oct 16, 2018 at 6:07 PM Simon Urli 
> 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.u...@xwiki.com
> >>> More about us at http://www.xwiki.com
> >>
> >>
> >>
> >> --
> >> Thomas Mortagne
> >>
> >
> >
>
> --
> Simon Urli
> Software Engineer at XWiki SAS
> simon.u...@xwiki.com
> More about us at http://www.xwiki.com
>


-- 
Guillaume Delhumeau (guillaume.delhum...@xwiki.com)
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project

Re: [xwiki-devs] [STAMP/Test] Metrics we need to improve + strategy

Hi,

[snip]

> Process to run DSpot:
> 1) Pick a module. Measure coverage and mutation score (or take the value 
> there already if they’re in the pom.xml). Same as for Descartes testing.
> 2) Run DSpot on the module, see 
> https://massol.myxwiki.org/xwiki/bin/view/Blog/TestGenerationDspot for 
> explanations

One important detail that I had missed. We need to run Dspot with “—descartes” 
on the command line so that it uses Descartes for computing the mutation score 
for mutations and only keep tests that increase the mutation score as reported 
by Descartes.

> 3) If DSpot has generated tests, add them to XWiki’s source code in 
> src/test/dspot and add the following to the pom of that module:
> 
> 
>  
>
>
>  org.codehaus.mojo
>  build-helper-maven-plugin
>
>  
> 
> 
> Example: 
> https://github.com/xwiki/xwiki-commons/tree/244ee07976c691c335b7f54c48e6308004ba3d82/xwiki-commons-core/xwiki-commons-crypto/xwiki-commons-crypto-cipher
> 
> Note: The generated tests sometimes need to be modified a bit to pass. 
> Personally I’ve only committed tests that were passing and I reported issues 
> for those that were not passing.
> 
> 4) File the various reports:
> a) https://github.com/STAMP-project/dspot-usecases-output/tree/master/xwiki 
> both for success and failures
> b) 
> https://docs.google.com/spreadsheets/d/1LULpGpsJirmFyvHNstLGv-Gv5DVBdpLTM2hm0jgCKUw/edit#gid=2061481816
> c) for failures, file a github issue at 
> https://github.com/STAMP-project/dspot/issues and link to the place on 
> https://github.com/STAMP-project/dspot-usecases-output/tree/master/xwiki 
> where we put the failing result.
> 
> Note: The reason we need to report failures too is because DSpot fails a lot 
> so we need to show what we have tested
> 
> Thanks
> -Vincent
> 

[snip]

Thanks
-Vincent

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition

On 10/17/18 10:43 AM, Vincent Massol wrote:

On 17 Oct 2018, at 10:41, Simon Urli wrote:

On 10/17/18 10:37 AM, Vincent Massol wrote:

On 17 Oct 2018, at 10:31, Simon Urli wrote:

On 10/17/18 10:22 AM, Vincent Massol wrote:

Hi Simon,

On 17 Oct 2018, at 10:12, Simon Urli wrote:

Hi Vincent and all,

On 10/17/18 9:41 AM, Vincent Massol wrote:

Hi Simon,

On 16 Oct 2018, at 17:43, Simon Urli wrote:

Hello everyone,

I'm coming back on this proposal as the work is going on, to basically propose
to dropping the warning on copy action.

I try to sum up why in the following.

When implementing the proposal, I was adviced to use an event listener,
observing the deleting event for informing the user if he were doing a
refactoring on a document containing an XClass.
This work is already implemented and working for Moving/Renaming pages (which
involve deleting the old page) and of course deleting.

The nice part about the listener is that it works for all use cases:
* rename/move from the UI
* rename/move from scripts
* etc

To ease the discussion I just created a design page with some screenshot of my
current work. Then you can see what it looks like for the user:
https://design.xwiki.org/xwiki/bin/view/Proposal/PreventUserFromXClassRefactoring

Ok cool so it seems you have it implemented at both the job level and the
listener level.

The bad parts are:
1) right now we don’t provide a nice UI when an event is cancelled. AFAIR we
just display a stack trace in the UI which is definitely not good enough. Are
you improving this part?

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.

Thanks
-Vincent

2) this is after the fact. Imagine that you’re renaming a set of pages and
among them there are several coming from an app. It’ll work fine on pages not
having an XClass (like moving the page having an XObject of that XClass) and
then failing down the line on the page having the XClass. That’s a problem
because the xobject page will be wrongly moved, since it doesn’t make sense
that it’s moved if the other pages of the app are not moved. Generally speaking
you’ll have a bad state that is not easy to rollback.
This is why for me the check also has to be done in the move/rename UI and
verify that among the list of pages there are none with XClass and if so
prevent moving/renaming any page.
This is not in contradiction with the listener but the more important (from a
usage POV) is the check in the move/rename UI and not the listener which is a
more advanced use case.

There might be a misunderstanding here: I use the listener to check the event
that are fired during the rename/move. As you can see in my screenshot, I got
the warning in the move/refactoring UI.

This listener is registered only during rename/move?
What happens if I write a script that moves/renames pages with XClass?

Nop the listener is globally registered, so I assume it would be triggered when
running a script too.

ok, so try this:

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents





On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:

I'm OK.


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?





Le mer. 17 oct. 2018 à 09:09, Thomas Mortagne  a
écrit :


+1 to stopping event propagation when it's cancelled
On Tue, Oct 16, 2018 at 6:07 PM Simon Urli  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.u...@xwiki.com
More about us at http://www.xwiki.com




--
Thomas Mortagne






--
Simon Urli
Software Engineer at XWiki SAS
simon.u...@xwiki.com
More about us at http://www.xwiki.com

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition




> On 17 Oct 2018, at 10:41, Simon Urli  wrote:
> 
> 
> 
> On 10/17/18 10:37 AM, Vincent Massol wrote:
>>> On 17 Oct 2018, at 10:31, Simon Urli  wrote:
>>> 
>>> 
>>> 
>>> On 10/17/18 10:22 AM, Vincent Massol wrote:
 Hi Simon,
> On 17 Oct 2018, at 10:12, Simon Urli  wrote:
> 
> Hi Vincent and all,
> 
> On 10/17/18 9:41 AM, Vincent Massol wrote:
>> Hi Simon,
>>> On 16 Oct 2018, at 17:43, Simon Urli  wrote:
>>> 
>>> Hello everyone,
>>> 
>>> I'm coming back on this proposal as the work is going on, to basically 
>>> propose to dropping the warning on copy action.
>>> 
>>> I try to sum up why in the following.
>>> 
>>> When implementing the proposal, I was adviced to use an event listener, 
>>> observing the deleting event for informing the user if he were doing a 
>>> refactoring on a document containing an XClass.
>>> This work is already implemented and working for Moving/Renaming pages 
>>> (which involve deleting the old page) and of course deleting.
>> The nice part about the listener is that it works for all use cases:
>> * rename/move from the UI
>> * rename/move from scripts
>> * etc
> 
> To ease the discussion I just created a design page with some screenshot 
> of my current work. Then you can see what it looks like for the user: 
> https://design.xwiki.org/xwiki/bin/view/Proposal/PreventUserFromXClassRefactoring
 Ok cool so it seems you have it implemented at both the job level and the 
 listener level.
> 
>> The bad parts are:
>> 1) right now we don’t provide a nice UI when an event is cancelled. 
>> AFAIR we just display a stack trace in the UI which is definitely not 
>> good enough. Are you improving this part?
> 
> 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?

Thanks
-Vincent

>>> 
> 
>> 2) this is after the fact. Imagine that you’re renaming a set of pages 
>> and among them there are several coming from an app. It’ll work fine on 
>> pages not having an XClass (like moving the page having an XObject of 
>> that XClass) and then failing down the line on the page having the 
>> XClass. That’s a problem because the xobject page will be wrongly moved, 
>> since it doesn’t make sense that it’s moved if the other pages of the 
>> app are not moved. Generally speaking you’ll have a bad state that is 
>> not easy to rollback.
>> This is why for me the check also has to be done in the move/rename UI 
>> and verify that among the list of pages there are none with XClass and 
>> if so prevent moving/renaming any page.
>> This is not in contradiction with the listener but the more important 
>> (from a usage POV) is the check in the move/rename UI and not the 
>> listener which is a more advanced use case.
> 
> There might be a misunderstanding here: I use the listener to check the 
> event that are fired during the rename/move. As you can see in my 
> screenshot, I got the warning in the move/refactoring UI.
 This listener is registered only during rename/move?
 What happens if I write a script that moves/renames pages with XClass?
>>> 
>>> Nop the listener is globally registered, so I assume it would be triggered 
>>> when running a script too.
>> ok, so try this: 
>>

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition

On 10/17/18 10:37 AM, Vincent Massol wrote:

On 17 Oct 2018, at 10:31, Simon Urli wrote:

On 10/17/18 10:22 AM, Vincent Massol wrote:

Hi Simon,

On 17 Oct 2018, at 10:12, Simon Urli wrote:

Hi Vincent and all,

On 10/17/18 9:41 AM, Vincent Massol wrote:

Hi Simon,

On 16 Oct 2018, at 17:43, Simon Urli wrote:

Hello everyone,

I'm coming back on this proposal as the work is going on, to basically propose
to dropping the warning on copy action.

I try to sum up why in the following.

The nice part about the listener is that it works for all use cases:
* rename/move from the UI
* rename/move from scripts
* etc

Ok cool so it seems you have it implemented at both the job level and the
listener level.

I reused the existing UI which does not look so bad IMO (see the screenshot in
the design page).

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.

There might be a misunderstanding here: I use the listener to check the event
that are fired during the rename/move. As you can see in my screenshot, I got
the warning in the move/refactoring UI.

This listener is registered only during rename/move?
What happens if I write a script that moves/renames pages with XClass?

Nop the listener is globally registered, so I assume it would be triggered when
running a script too.

ok, so try this: http://playground.xwiki.org/xwiki/bin/delete/MyPage/?confirm=1

What happens?

Coming back to you after I checked :)

Now going back on "Copy" the page, it's another job as I cannot rely on a
"Deleting" event. I checked quickly and I don't think I really could rely on other events
for this: basically copying is about creating a document and updating its content, and I don't
think we want to rely on those event for this mechanism.

So unless you have another proposal to handle this case, I propose to simply
drop it.

Do you agree?

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition




> On 17 Oct 2018, at 10:31, Simon Urli  wrote:
> 
> 
> 
> On 10/17/18 10:22 AM, Vincent Massol wrote:
>> Hi Simon,
>>> On 17 Oct 2018, at 10:12, Simon Urli  wrote:
>>> 
>>> Hi Vincent and all,
>>> 
>>> On 10/17/18 9:41 AM, Vincent Massol wrote:
 Hi Simon,
> On 16 Oct 2018, at 17:43, Simon Urli  wrote:
> 
> Hello everyone,
> 
> I'm coming back on this proposal as the work is going on, to basically 
> propose to dropping the warning on copy action.
> 
> I try to sum up why in the following.
> 
> When implementing the proposal, I was adviced to use an event listener, 
> observing the deleting event for informing the user if he were doing a 
> refactoring on a document containing an XClass.
> This work is already implemented and working for Moving/Renaming pages 
> (which involve deleting the old page) and of course deleting.
 The nice part about the listener is that it works for all use cases:
 * rename/move from the UI
 * rename/move from scripts
 * etc
>>> 
>>> To ease the discussion I just created a design page with some screenshot of 
>>> my current work. Then you can see what it looks like for the user: 
>>> https://design.xwiki.org/xwiki/bin/view/Proposal/PreventUserFromXClassRefactoring
>> Ok cool so it seems you have it implemented at both the job level and the 
>> listener level.
>>> 
 The bad parts are:
 1) right now we don’t provide a nice UI when an event is cancelled. AFAIR 
 we just display a stack trace in the UI which is definitely not good 
 enough. Are you improving this part?
>>> 
>>> 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.

> 
>>> 
 2) this is after the fact. Imagine that you’re renaming a set of pages and 
 among them there are several coming from an app. It’ll work fine on pages 
 not having an XClass (like moving the page having an XObject of that 
 XClass) and then failing down the line on the page having the XClass. 
 That’s a problem because the xobject page will be wrongly moved, since it 
 doesn’t make sense that it’s moved if the other pages of the app are not 
 moved. Generally speaking you’ll have a bad state that is not easy to 
 rollback.
 This is why for me the check also has to be done in the move/rename UI and 
 verify that among the list of pages there are none with XClass and if so 
 prevent moving/renaming any page.
 This is not in contradiction with the listener but the more important 
 (from a usage POV) is the check in the move/rename UI and not the listener 
 which is a more advanced use case.
>>> 
>>> There might be a misunderstanding here: I use the listener to check the 
>>> event that are fired during the rename/move. As you can see in my 
>>> screenshot, I got the warning in the move/refactoring UI.
>> This listener is registered only during rename/move?
>> What happens if I write a script that moves/renames pages with XClass?
> 
> Nop the listener is globally registered, so I assume it would be triggered 
> when running a script too.

ok, so try this: http://playground.xwiki.org/xwiki/bin/delete/MyPage/?confirm=1

What happens?

> 
> Now going back on "Copy" the page, it's another job as I cannot rely on a 
> "Deleting" event. I checked quickly and I don't think I really could rely 
> on other events for this: basically copying is about creating a document 
> and updating its content, and I don't think we want to rely on those 
> event for this mechanism.
> 
> So unless you have another proposal to handle this case, I propose to 
> simply drop it.
> 
> Do you agree?
 AFAIK if you copy a set of pages with pages having an XClass in it, then 
 the copied pages won’t work so we

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents

2018-10-17 Thread Guillaume Delhumeau

I'm OK.


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?


Le mer. 17 oct. 2018 à 09:09, Thomas Mortagne  a
écrit :

> +1 to stopping event propagation when it's cancelled
> On Tue, Oct 16, 2018 at 6:07 PM Simon Urli  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.u...@xwiki.com
> > More about us at http://www.xwiki.com
>
>
>
> --
> Thomas Mortagne
>


-- 
Guillaume Delhumeau (guillaume.delhum...@xwiki.com)
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition

On 10/17/18 10:22 AM, Vincent Massol wrote:

Hi Simon,

On 17 Oct 2018, at 10:12, Simon Urli wrote:

Hi Vincent and all,

On 10/17/18 9:41 AM, Vincent Massol wrote:

Hi Simon,

On 16 Oct 2018, at 17:43, Simon Urli wrote:

Hello everyone,

I'm coming back on this proposal as the work is going on, to basically propose
to dropping the warning on copy action.

I try to sum up why in the following.

The nice part about the listener is that it works for all use cases:
* rename/move from the UI
* rename/move from scripts
* etc

Ok cool so it seems you have it implemented at both the job level and the
listener level.

I reused the existing UI which does not look so bad IMO (see the screenshot in
the design page).

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.

There might be a misunderstanding here: I use the listener to check the event
that are fired during the rename/move. As you can see in my screenshot, I got
the warning in the move/refactoring UI.

This listener is registered only during rename/move?

What happens if I write a script that moves/renames pages with XClass?

Nop the listener is globally registered, so I assume it would be
triggered when running a script too.

So unless you have another proposal to handle this case, I propose to simply
drop it.

Do you agree?

AFAIK if you copy a set of pages with pages having an XClass in it, then the
copied pages won’t work so we shouldn’t drop this. We should just implement the
protection at the UI level (ie the copy action), same as for rename/move and
not implement the listener part (ie not support the script use case).

I don't agree: the pages would work, but if they contain XObject they won't use
the copied XClass, only the older one.
So for me the issue is not exactly the same: the problematic is not about
copying an XClass here, but a couple XClass + XObject. More difficult to detect
and to handle IMO.

I still think we should handle it with a warning to explain this. It’s easy for
me, we just need to check for XClass and warn that any copied pages having a
link to this XClass will no longer point to it but will keep pointing to the
original XClass. Easy to do.

I can handle it only on the UI side then.

Simon

Thanks
-Vincent

Simon

Thanks
-Vincent

Simon

On 9/26/18 10:27 AM, Simon Urli wrote:

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition

Hi Simon,

> On 17 Oct 2018, at 10:12, Simon Urli  wrote:
> 
> Hi Vincent and all,
> 
> On 10/17/18 9:41 AM, Vincent Massol wrote:
>> Hi Simon,
>>> On 16 Oct 2018, at 17:43, Simon Urli  wrote:
>>> 
>>> Hello everyone,
>>> 
>>> I'm coming back on this proposal as the work is going on, to basically 
>>> propose to dropping the warning on copy action.
>>> 
>>> I try to sum up why in the following.
>>> 
>>> When implementing the proposal, I was adviced to use an event listener, 
>>> observing the deleting event for informing the user if he were doing a 
>>> refactoring on a document containing an XClass.
>>> This work is already implemented and working for Moving/Renaming pages 
>>> (which involve deleting the old page) and of course deleting.
>> The nice part about the listener is that it works for all use cases:
>> * rename/move from the UI
>> * rename/move from scripts
>> * etc
> 
> To ease the discussion I just created a design page with some screenshot of 
> my current work. Then you can see what it looks like for the user: 
> https://design.xwiki.org/xwiki/bin/view/Proposal/PreventUserFromXClassRefactoring

Ok cool so it seems you have it implemented at both the job level and the 
listener level.

> 
>> The bad parts are:
>> 1) right now we don’t provide a nice UI when an event is cancelled. AFAIR we 
>> just display a stack trace in the UI which is definitely not good enough. 
>> Are you improving this part?
> 
> 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.

> 
>> 2) this is after the fact. Imagine that you’re renaming a set of pages and 
>> among them there are several coming from an app. It’ll work fine on pages 
>> not having an XClass (like moving the page having an XObject of that XClass) 
>> and then failing down the line on the page having the XClass. That’s a 
>> problem because the xobject page will be wrongly moved, since it doesn’t 
>> make sense that it’s moved if the other pages of the app are not moved. 
>> Generally speaking you’ll have a bad state that is not easy to rollback.
>> This is why for me the check also has to be done in the move/rename UI and 
>> verify that among the list of pages there are none with XClass and if so 
>> prevent moving/renaming any page.
>> This is not in contradiction with the listener but the more important (from 
>> a usage POV) is the check in the move/rename UI and not the listener which 
>> is a more advanced use case.
> 
> There might be a misunderstanding here: I use the listener to check the event 
> that are fired during the rename/move. As you can see in my screenshot, I got 
> the warning in the move/refactoring UI.

This listener is registered only during rename/move?

What happens if I write a script that moves/renames pages with XClass?

>>> Now going back on "Copy" the page, it's another job as I cannot rely on a 
>>> "Deleting" event. I checked quickly and I don't think I really could rely 
>>> on other events for this: basically copying is about creating a document 
>>> and updating its content, and I don't think we want to rely on those event 
>>> for this mechanism.
>>> 
>>> So unless you have another proposal to handle this case, I propose to 
>>> simply drop it.
>>> 
>>> Do you agree?
>> AFAIK if you copy a set of pages with pages having an XClass in it, then the 
>> copied pages won’t work so we shouldn’t drop this. We should just implement 
>> the protection at the UI level (ie the copy action), same as for rename/move 
>> and not implement the listener part (ie not support the script use case).
> 
> 
> I don't agree: the pages would work, but if they contain XObject they won't 
> use the copied XClass, only the older one.
> So for me the issue is not exactly the same: the problematic is not about 
> copying an XClass here, but a couple XClass + XObject. More difficult to 
> detect and to handle IMO.

I still think we should handle it with a warning to explain this. It’s easy for 
me, we just need to check for XClass and warn that any copied pages having a 
link to this XClass will no longer point to it but will keep pointing to the 
original XClass. Easy to do.

Thanks
-Vincent

> 
> Simon
>> Thanks
>> -Vincent
>>> 
>>> Simon
>>> 
>>> 
>>> On 9/26/18 10:27 AM, Simon Urli wrote:
 Hi everyone,
 ok trying to sum-up (I'm only talking about cases with XClass below, to 
 simplify):
   - according to Vincent, we should completely prevent simple users to 
 copy/move/rename and only allow

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition

Hi Vincent and all,

On 10/17/18 9:41 AM, Vincent Massol wrote:

Hi Simon,

On 16 Oct 2018, at 17:43, Simon Urli wrote:

Hello everyone,

I'm coming back on this proposal as the work is going on, to basically propose
to dropping the warning on copy action.

I try to sum up why in the following.

The nice part about the listener is that it works for all use cases:
* rename/move from the UI
* rename/move from scripts
* etc

To ease the discussion I just created a design page with some screenshot
of my current work. Then you can see what it looks like for the user:
https://design.xwiki.org/xwiki/bin/view/Proposal/PreventUserFromXClassRefactoring

I reused the existing UI which does not look so bad IMO (see the
screenshot in the design page).

This is why for me the check also has to be done in the move/rename UI and
verify that among the list of pages there are none with XClass and if so
prevent moving/renaming any page.

This is not in contradiction with the listener but the more important (from a
usage POV) is the check in the move/rename UI and not the listener which is a
more advanced use case.

There might be a misunderstanding here: I use the listener to check the
event that are fired during the rename/move. As you can see in my
screenshot, I got the warning in the move/refactoring UI.

So unless you have another proposal to handle this case, I propose to simply
drop it.

Do you agree?

I don't agree: the pages would work, but if they contain XObject they
won't use the copied XClass, only the older one.
So for me the issue is not exactly the same: the problematic is not
about copying an XClass here, but a couple XClass + XObject. More
difficult to detect and to handle IMO.

Simon

Thanks
-Vincent

Simon

On 9/26/18 10:27 AM, Simon Urli wrote:

Hi everyone,
ok trying to sum-up (I'm only talking about cases with XClass below, to
simplify):
- according to Vincent, we should completely prevent simple users to
copy/move/rename and only allow advanced users to do it after a warning
- according to Adel & Clément: preventing simple users will be useless as
they can easily switch the advanced feature in their account
- according to Marius copying a page/app is not necessarily harmful compared
to moving/renaming and we should manage it differently.
I really don't know the practice of users on the field, but it looks to me that
preventing simple users to do the action and telling them to ask an advanced
user is actually a good trade-off:
1. it will warn users that they might be doing something wrong
2. it's not something completely blocking: either they ask for the
admin/advanced user, or they know they can switch the advanced features by
themselves, at their own risks
Now maybe we can only do the warning for the "copy" action.
WDYT?
Simon
On 9/25/18 11:36 AM, Vincent Massol wrote:

Hi Marius,

On 25 Sep 2018, at 11:34, Marius Dumitru Florea
wrote:

On Sun, Sep 23, 2018 at 11:12 AM Vincent Massol wrote:

Hi Simon,

On 21 Sep 2018, at 16:58, Simon Urli wrote:

On 9/21/18 4:53 PM, Adel Atallah wrote:

+1 for the warning, but I would not forbid simple users from renaming
or moving pages but instead just hide the action (from the page menu).

OK I should have written it: by "forbid" I meant:

1. Hide the

Re: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents


Hi all,

I took the liberty to already create the PR, then you can see the impact 
on the code: https://github.com/xwiki/xwiki-commons/pull/49


Simon.

On 10/17/18 9:09 AM, Thomas Mortagne wrote:

+1 to stopping event propagation when it's cancelled
On Tue, Oct 16, 2018 at 6:07 PM Simon Urli  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.u...@xwiki.com
More about us at http://www.xwiki.com






--
Simon Urli
Software Engineer at XWiki SAS
simon.u...@xwiki.com
More about us at http://www.xwiki.com

Re: [xwiki-devs] [Proposal] Prevent users from renaming/move pages with XClass definition