Note: This is not really blocked by MDL-44890 but I made it based on that commit (otherwise they conflict - a trivial conflict but still, why make headaches). If MDL-44890 were rejected and this approved, I can remake the commit so it isn't based on the other one.

Results for MDL-44891

• Remote repository: https://github.com/sammarshallou/moodle.git

• Remote branch MDL-44891-m25 to be integrated into upstream MOODLE_25_STABLE
  • Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2657
  • Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2657/artifact/work/smurf.html

• Remote branch MDL-44891-m26 to be integrated into upstream MOODLE_26_STABLE
  • Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2658
  • Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2658/artifact/work/smurf.html

• Remote branch MDL-44891-master to be integrated into upstream master
  • Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2659
  • Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2659/artifact/work/smurf.html

While probably this could be achieved by some other rudimentary available steps, I like this exact one a lot:

• it's simple.
• it's generic.
• it's powerful.

So my +1 to send it to integration (all branches).

Note that, if landing "alone" it will need some specific .feature to verify it's working ok. If landing with MDL-44070, testing can be delegated to it, although I find it less optimal as far as this is defined as blocker, hence has to land earlier.

So I'd suggest to do add some trivial .feature to the issue (does not need to be added to core) and fill testing instructions to run it.

Ciao

Thanks very much for the review Eloy. I've rebased the branches and added a temporary feature file in the testing instructions, as suggested. Will submit to integration.

I've rebased again to latest code. There was also a slight change (thanks Marina) to correct the format of the regular expression, so that it shows up correctly on the documentation page with the dropdown thingy.

Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

Thanks Sam, this looks like a good step.

I tested on existing behat feature by making the change:

diff --git a/course/tests/behat/activities_group_icons.feature b/course/tests/behat/activities_group_icons.feature

index 35dbee9..88a9575 100644

--- a/course/tests/behat/activities_group_icons.feature

+++ b/course/tests/behat/activities_group_icons.feature

@@ -27,7 +27,7 @@ Feature: Toggle activities groups mode from the course page

       | Force group mode | No |

     When I press "Save changes"

     Then "No groups (Click to change)" "link" should exist

-    And "//a/child::img[contains(@src, 'groupn')]" "xpath_element" should exist

+    And the "src" attribute of "a.cm-edit-action img" "css_element" should contain "groupn"

     And I click on "No groups (Click to change)" "link" in the "Test forum name" activity

     And "Separate groups (Click to change)" "link" should exist

     And "//a/child::img[contains(@src, 'groups')]" "xpath_element" should exist

We usually create pairs of steps i.e. "should contain" and "should not contain". Will it be too much to ask for an opposite step?

Since it's blocking the conditional availability, I'm integrating it as is. I'll create another issue for opposite step.

Thanks, integrated in master, 2.6 and 2.5

Created an issue MDL-44991 for opposite step

Thanks for fixing this Sam,

Works great, passing...

FYI: I used "pagelayout-frontpage" css name in feature file to test on 26 and 25 branches, as default theme is standard on them.

Thanks! Sorry for bad example, I hadn't realised that was different between themes.

Thanks for your efforts, this change is now part of Moodle!