Moodle
  1. Moodle
  2. MDL-43252

Group menu in wrong order when groupings are used

    Details

    • Testing Instructions:
      Hide

      1. Create 2 groups on a course, Group A and Group B. (The test activity course on qa.moodle.net already has these groups.)
      2. Create a grouping called 'Wrong order grouping'.
      3. Add Group B to the grouping, then add Group A.
      4. Create a new forum, using separate groups and selecting the grouping that was created.
      5. View the forum and look at the group dropdown.

      EXPECTED: The group dropdown should show 'Group A' and then 'Group B' (alphabetical order), just as if you hadn't selected a grouping.
      BEFORE FIX: Before fix for this problem it shows Group B first.

      Show
      1. Create 2 groups on a course, Group A and Group B. (The test activity course on qa.moodle.net already has these groups.) 2. Create a grouping called 'Wrong order grouping'. 3. Add Group B to the grouping, then add Group A. 4. Create a new forum, using separate groups and selecting the grouping that was created. 5. View the forum and look at the group dropdown. EXPECTED: The group dropdown should show 'Group A' and then 'Group B' (alphabetical order), just as if you hadn't selected a grouping. BEFORE FIX: Before fix for this problem it shows Group B first.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull 2.6 Branch:
      MDL-43252-m26
    • Pull Master Branch:
      MDL-43252-master

      Description

      (This is a regression introduced in Moodle 2.5 when the group caching was introduced. The problem still occurs in Moodle 2.6 as tested on qa.moodle.net.)

      If you set an activity to use e.g. separate groups mode, with a grouping selected, then teachers see a dropdown they can use to select a group. However, in this situation, the groups do not appear in alphabetical order.

      Groups do correctly appear in alphabetical order when no grouping is in use.

        Gliffy Diagrams

          Activity

          Hide
          Sam Marshall added a comment -

          Completed change. This includes a new unit test for the functionality. The commit is technically different for 2.5 because other changes in the groups unit test code made it conflict, but there is no actual difference between branches.

          Show
          Sam Marshall added a comment - Completed change. This includes a new unit test for the functionality. The commit is technically different for 2.5 because other changes in the groups unit test code made it conflict, but there is no actual difference between branches.
          Hide
          Sam Marshall added a comment -

          I've run the unit tests on master and 2.5, and also done the manual functionality test on master, so I think this works and am submitting for peer review.

          Note: I'm not stating that this is necessarily the best way to sort group lists in future (e.g. somebody might want to change it to do language-specific sorts), but I think this is the correct and simple fix for this regression.

          Show
          Sam Marshall added a comment - I've run the unit tests on master and 2.5, and also done the manual functionality test on master, so I think this works and am submitting for peer review. Note: I'm not stating that this is necessarily the best way to sort group lists in future (e.g. somebody might want to change it to do language-specific sorts), but I think this is the correct and simple fix for this regression.
          Hide
          Tim Hunt added a comment -

          You have not formatted the SQL in the Moode-standard way. Apart from that looks good, and well done for the unit tests.

          Show
          Tim Hunt added a comment - You have not formatted the SQL in the Moode-standard way. Apart from that looks good, and well done for the unit tests.
          Hide
          Sam Marshall added a comment -

          Thanks Tim. I think the Moodle standard was added quite recently as I didn't see that before... It is also a bit horrid and doesn't properly define what happens when you put it in the middle of a function call. Ah well, will change anyhow and then submit for integration.

          Show
          Sam Marshall added a comment - Thanks Tim. I think the Moodle standard was added quite recently as I didn't see that before... It is also a bit horrid and doesn't properly define what happens when you put it in the middle of a function call. Ah well, will change anyhow and then submit for integration.
          Hide
          Sam Marshall added a comment -

          Submitting for integration, thanks Tim for review. (After actually changing it - new SQL style could be worse, and I guess not using too many lines is kind of an advantage. Still hate the centre-align thing though!)

          Show
          Sam Marshall added a comment - Submitting for integration, thanks Tim for review. (After actually changing it - new SQL style could be worse, and I guess not using too many lines is kind of an advantage. Still hate the centre-align thing though!)
          Hide
          Sam Hemelryk added a comment -

          Thanks Sam - this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Sam - this has been integrated now.
          Hide
          Jérôme Mouneyrac added a comment - - edited

          All good, thanks guys.

          Show
          Jérôme Mouneyrac added a comment - - edited All good, thanks guys.
          Hide
          Sam Hemelryk added a comment -

          Thanks for the code, its now upstream!

          Heres a fun trick to try in the spirit of Friday the 13th.
          I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

          Show
          Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: