Moodle
  1. Moodle
  2. MDL-35321

When editing a course section, if "Restrict access"->"Grouping access" is set to a grouping, it can't be set back to None

    Details

    • Testing Instructions:
      Hide

      In a course with multiple groupings (on a server with enablegroupmembersonly and enableavailability set):

      1. Turn Editing on
      2. Edit a section
        • Set a topic to be visible only to a specific grouping
        • Save changes
      3. Confirm that the topic shows the restricted to that grouping (the grouping name will appear in brackets) on the course/view page
      4. Edit the section again
        • Confirm that the topic is restricted to that grouping still
        • Set the grouping to "None"
        • Save changes
      5. Confirm that the topic no longer shows the restriction on the course/view page
      6. Edit the section again
        • Confirm that the topic is not restricted to any grouping
      Show
      In a course with multiple groupings (on a server with enablegroupmembersonly and enableavailability set): Turn Editing on Edit a section Set a topic to be visible only to a specific grouping Save changes Confirm that the topic shows the restricted to that grouping (the grouping name will appear in brackets) on the course/view page Edit the section again Confirm that the topic is restricted to that grouping still Set the grouping to "None" Save changes Confirm that the topic no longer shows the restriction on the course/view page Edit the section again Confirm that the topic is not restricted to any grouping
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-35321-master
    • Rank:
      43990

      Description

      When editing a course section, if "Restrict access"->"Grouping access" is set to a grouping, it can't be set back to None. It can be changed to a different grouping.

      To reproduce:
      1. In admin settings set "Enable conditional"/"accessenableavailability" to checked / yes
      2. In admin settings set "Enable group members"/"onlyenablegroupmembersonly" to checked / yes
      3. In a course create two groupings "A" and "B".
      4. Edit course section.
      5. In restrcit access section set "Grouping access" to grouping "A" and save changes.
      6. Notice the section restriction is "(A)"
      7. Edit course section and set "Grouping access" to grouping "None" and save changes.
      8. Notice the section restriction is still "(A)"
      9. Edit course section and set "Grouping access" to grouping "B" and save changes.
      10. Notice the section restriction is "(B)"

        Issue Links

          Activity

          Hide
          Michael Spall added a comment -

          I tested this on Moodle 2.3.1+ (Build: 20120907)

          Show
          Michael Spall added a comment - I tested this on Moodle 2.3.1+ (Build: 20120907)
          Hide
          Mary Cooch added a comment -

          I tested this on Moodle 2.3.1+ (Build: 20120907) as well and I have found exactly the same. It looks as if it will save because you can select "None" in the drop down and press the save changes button - but when you are returned to the course page, the previous grouping is still showing. You are able to change groupings as Michael says.

          Show
          Mary Cooch added a comment - I tested this on Moodle 2.3.1+ (Build: 20120907) as well and I have found exactly the same. It looks as if it will save because you can select "None" in the drop down and press the save changes button - but when you are returned to the course page, the previous grouping is still showing. You are able to change groupings as Michael says.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put it on our backlog.

          In the meantime adding more information, such as screenshots, fix test instructions, a workaround or even a code solution, will help us and other users. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put it on our backlog. In the meantime adding more information, such as screenshots, fix test instructions, a workaround or even a code solution, will help us and other users. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
          Hide
          Mitja Podreka added a comment -

          Workaround with phpMyAdmin (or similar editor):
          1. To find the right settings run the following MySQL query (replace 1234 with your course ID):
          SELECT * FROM `mdl_course_sections` where course = 1234
          2. In the appropriate section change the field groupingid to 0
          3. On the course's home page go to the section's edit form and without changing anything click the "Save changes" button.

          Show
          Mitja Podreka added a comment - Workaround with phpMyAdmin (or similar editor): 1. To find the right settings run the following MySQL query (replace 1234 with your course ID): SELECT * FROM `mdl_course_sections` where course = 1234 2. In the appropriate section change the field groupingid to 0 3. On the course's home page go to the section's edit form and without changing anything click the "Save changes" button.
          Hide
          Michael Aherne added a comment -

          I've added a simple patch that fixes this.

          Show
          Michael Aherne added a comment - I've added a simple patch that fixes this.
          Hide
          Andrew Nicols added a comment -

          Thanks Michael – do you want this peer reviewed? We've just hit the same issue and I had the same patch in mind.

          Can you create a 2.3 branch for this too? It shouldn't need backporting to 2.2.

          Show
          Andrew Nicols added a comment - Thanks Michael – do you want this peer reviewed? We've just hit the same issue and I had the same patch in mind. Can you create a 2.3 branch for this too? It shouldn't need backporting to 2.2.
          Hide
          Michael Aherne added a comment -

          Hi Andrew, yes I would like it peer reviewed! I'll create a 2.3 branch too if you think it seems like a reasonable patch.

          Show
          Michael Aherne added a comment - Hi Andrew, yes I would like it peer reviewed! I'll create a 2.3 branch too if you think it seems like a reasonable patch.
          Hide
          Andrew Nicols added a comment -

          Looks good to me. Tested and WFM too.

          If you can provide appropriate testing instructions, feel free to submit for Integration.

          Thanks

          Show
          Andrew Nicols added a comment - Looks good to me. Tested and WFM too. If you can provide appropriate testing instructions, feel free to submit for Integration. Thanks
          Hide
          Michael Aherne added a comment -

          2.3 branch added.

          Show
          Michael Aherne added a comment - 2.3 branch added.
          Hide
          Michael Aherne added a comment -

          Thanks, Andrew. I've added testing instructions for the patch, but I still don't have permissions to submit code for integration. Would you mind doing that for me please? Cheers.

          Show
          Michael Aherne added a comment - Thanks, Andrew. I've added testing instructions for the patch, but I still don't have permissions to submit code for integration. Would you mind doing that for me please? Cheers.
          Hide
          Sam Marshall added a comment -

          sorry, I hit the assign button by mistake assigning back

          Show
          Sam Marshall added a comment - sorry, I hit the assign button by mistake assigning back
          Hide
          Dan Poltawski added a comment -

          Thanks Michael, i've integrated this to 23 and master.

          Show
          Dan Poltawski added a comment - Thanks Michael, i've integrated this to 23 and master.
          Hide
          David Monllaó added a comment -

          It passes, tested in master; it works as expected, I've been able to reproduce the problem reverting the patch

          Show
          David Monllaó added a comment - It passes, tested in master; it works as expected, I've been able to reproduce the problem reverting the patch
          Hide
          Dan Poltawski added a comment -

          Congratulations, you've done it!

          Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc

          Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

          Show
          Dan Poltawski added a comment - Congratulations, you've done it! Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.
          Hide
          Michael Aherne added a comment -

          Thanks for the heads up! I'll be sure to use them whenever I can.

          Show
          Michael Aherne added a comment - Thanks for the heads up! I'll be sure to use them whenever I can.

            People

            • Votes:
              9 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: