Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-33937

Student can see completion information for a section even if settings are set to hide the section

          Details

          • Testing Instructions:
            Hide

            1. Set up a course with 'one section per page' option.
            2. Edit one section option and set it to have a date in the future and to "hide section entirely".
            3. Log in as student and access the hidden section by editing the url from a normal section.

            Expected: Should give error message because section is not available.

            Show
            1. Set up a course with 'one section per page' option. 2. Edit one section option and set it to have a date in the future and to "hide section entirely". 3. Log in as student and access the hidden section by editing the url from a normal section. Expected: Should give error message because section is not available.
          • Affected Branches:
            MOODLE_23_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            MDL-33937-master
          • Pull Master Diff URL:

            Description

            Replication steps:-

            1. Set restriction on a section based on some activity completion. Make sure you set it to "hide section entirely"
            2. Login as student and visit any section page.
            3. Try to access the hidden section by editing the url
            4. You can see the restriction information, which should not be visible.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  caused a regression

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-34444 Topic links active with restricted conditional topics

                  • Critical - Crashes server, loss of data, severe memory leak
                  • Closed
                  Discovered while testing

                  Release Test - A set of tests for a single release MDLQA-3913 Moodle 2.3 QA Cycle 2 Test Session 4

                  • Blocker - Blocks development and/or testing, prevents Moodle from running (applicable to bugs only)
                  • Open

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    This is activity completion, assigning to Sam

                    Show
                    poltawski Dan Poltawski added a comment - This is activity completion, assigning to Sam
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    (maybe we need a new component)

                    Show
                    poltawski Dan Poltawski added a comment - (maybe we need a new component)
                    Hide
                    Permalink
                    quen Sam Marshall added a comment -

                    I'd argue this bug is actually with multipage course formats...!

                    Here is code that fixes it by checking that sections are visible to current user before it lets you see them. This code also improves performance by replacing one unnecessary database query with data from modinfo.

                    I suspect this change is not sufficient to make the multipage course format work correctly with hidden sections, for example there are still links to the hidden section which doesn't seem ideal.

                    Show
                    quen Sam Marshall added a comment - I'd argue this bug is actually with multipage course formats...! Here is code that fixes it by checking that sections are visible to current user before it lets you see them. This code also improves performance by replacing one unnecessary database query with data from modinfo. I suspect this change is not sufficient to make the multipage course format work correctly with hidden sections, for example there are still links to the hidden section which doesn't seem ideal.
                    Hide
                    Permalink
                    nebgor Aparup Banerjee added a comment -

                    this was up or peer-review, so looking..

                    Show
                    nebgor Aparup Banerjee added a comment - this was up or peer-review, so looking..
                    Hide
                    Permalink
                    nebgor Aparup Banerjee added a comment -

                    this looks good to me.

                    just wondering if maybe there should be a case for IGNORE_MULTIPLE in get_section_info() for an exception?

                    Show
                    nebgor Aparup Banerjee added a comment - this looks good to me. just wondering if maybe there should be a case for IGNORE_MULTIPLE in get_section_info() for an exception?
                    Hide
                    Permalink
                    quen Sam Marshall added a comment -

                    Thanks for review!

                    Because I wasn't sure, I now checked what IGNORE_MULTIPLE does if there are no results; it is the same as IGNORE_MISSING (i.e. returns false), so I think this behaviour is consistent.

                    (Arguably it would be pretty silly for developers to specify IGNORE_MULTIPLE for $sttrictness, as it isn't a database query directly and there is no possibility of multiple results, but I don't think it's worth actively checking for that; they could also specify 78 or 'sausage' for that matter

                    Submitting for integration review.

                    Show
                    quen Sam Marshall added a comment - Thanks for review! Because I wasn't sure, I now checked what IGNORE_MULTIPLE does if there are no results; it is the same as IGNORE_MISSING (i.e. returns false), so I think this behaviour is consistent. (Arguably it would be pretty silly for developers to specify IGNORE_MULTIPLE for $sttrictness, as it isn't a database query directly and there is no possibility of multiple results, but I don't think it's worth actively checking for that; they could also specify 78 or 'sausage' for that matter Submitting for integration review.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Ha, I agree this was my bug (multi-page course format). Ooops!

                    Show
                    poltawski Dan Poltawski added a comment - Ha, I agree this was my bug (multi-page course format). Ooops!
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Integrated to 23_STABLE and master, thanks Sam.

                    (actually, didn't conditional sections land afterwards )

                    Show
                    poltawski Dan Poltawski added a comment - Integrated to 23_STABLE and master, thanks Sam. (actually, didn't conditional sections land afterwards )
                    Hide
                    Permalink
                    quen Sam Marshall added a comment -

                    dan - yes, conditional sections landed afterwards even though they were ready months earlier, and apparently I am still sore about that

                    thanks.

                    Show
                    quen Sam Marshall added a comment - dan - yes, conditional sections landed afterwards even though they were ready months earlier, and apparently I am still sore about that thanks.
                    Hide
                    Permalink
                    ankit_frenz Ankit Agarwal added a comment -

                    Works as expected!
                    Thanks

                    Show
                    ankit_frenz Ankit Agarwal added a comment - Works as expected! Thanks
                    Hide
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

                    Congratulations your code is upstream - gold star for you!

                    This issue + 79 others made it in in time for the minor releases.
                    Thank you everyone involved for your exuberant efforts.

                    Show
                    samhemelryk Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

                      People

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

                        Dates

                        • Created:
                          Updated:
                          Resolved:
                          Fix Release Date:
                          9/Jul/12