Uploaded image for project: '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

      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)"

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            mspall Michael Spall added a comment -

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

            Show
            mspall Michael Spall added a comment - I tested this on Moodle 2.3.1+ (Build: 20120907)
            Hide
            marycooch 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
            marycooch 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
            salvetore 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
            salvetore 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
            mitjap 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
            mitjap 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
            maherne Michael Aherne added a comment -

            I've added a simple patch that fixes this.

            Show
            maherne Michael Aherne added a comment - I've added a simple patch that fixes this.
            Hide
            dobedobedoh 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
            dobedobedoh 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
            maherne 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
            maherne 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
            dobedobedoh 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
            dobedobedoh 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
            maherne Michael Aherne added a comment -

            2.3 branch added.

            Show
            maherne Michael Aherne added a comment - 2.3 branch added.
            Hide
            maherne 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
            maherne 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
            quen Sam Marshall added a comment -

            sorry, I hit the assign button by mistake assigning back

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

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

            Show
            poltawski Dan Poltawski added a comment - Thanks Michael, i've integrated this to 23 and master.
            Hide
            dmonllao 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
            dmonllao 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
            poltawski 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
            poltawski 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
            maherne Michael Aherne added a comment -

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

            Show
            maherne 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:
                  Fix Release Date:
                  12/Nov/12