Moodle
  1. Moodle
  2. MDL-34444

Topic links active with restricted conditional topics

    Details

    • Workaround:
      Hide
      1. Log in as admin
      2. Navigate to Site admin > Advanced features
      3. Check "Enable conditional" (accessenableavailability)
      4. Navigate to a course
      5. Navigate to Course admin > Edit settings
      6. Set Format = 'Topics format' and Course layout = 'Show one section per page'
      7. Save settings
      8. Edit a section (topic) somewhere in the middle of the course [SECTION A]
      9. Under "Restrict access" set the section to be dependant on another activity to be completed (add a 'Activity completion condition')
      10. Still under "Restrict access" set "Before section can be accessed" to "Show section greyed out..."
      11. Edit another section [SECTION B] and set another restriction (maybe a date in future)
      12. Set "Before section can be accessed" to "Hide section entirely" for this section
      13. Log in as student
      14. If viewing a single section, click "Return to main course page"
      15. Note that the Navigation shows the correct list of sections (with restricted SECTION A and not SECTION B)
      16. Note that the SECTION A is shown and the topic heading is an active link
      17. Click the link - verify that the restriction information is shown, and that the section is not displayed
      18. Verify SECTION B is not displayed in the listing
      19. Go to a visible section and try to get to SECTION B using the navigation on the section (< > arrows)
      20. Verify that you can not get to the section
      21. Try to get to restricted SECTION B by manually changing the url
      22. Verify the section throws error message
      Show
      Log in as admin Navigate to Site admin > Advanced features Check "Enable conditional" (accessenableavailability) Navigate to a course Navigate to Course admin > Edit settings Set Format = 'Topics format' and Course layout = 'Show one section per page' Save settings Edit a section (topic) somewhere in the middle of the course [SECTION A] Under "Restrict access" set the section to be dependant on another activity to be completed (add a 'Activity completion condition') Still under "Restrict access" set "Before section can be accessed" to "Show section greyed out..." Edit another section [SECTION B] and set another restriction (maybe a date in future) Set "Before section can be accessed" to "Hide section entirely" for this section Log in as student If viewing a single section, click "Return to main course page" Note that the Navigation shows the correct list of sections (with restricted SECTION A and not SECTION B) Note that the SECTION A is shown and the topic heading is an active link Click the link - verify that the restriction information is shown, and that the section is not displayed Verify SECTION B is not displayed in the listing Go to a visible section and try to get to SECTION B using the navigation on the section (< > arrows) Verify that you can not get to the section Try to get to restricted SECTION B by manually changing the url Verify the section throws error message
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE

      Description

      I am using Moodle 2.3.1(Build: 20120706)

      In separate page per topic format:

      When I set a topic to be greyed out until XX activity in another topic is complete, the topic heading link is still active (not greyed out) and if clicked returns a message: Sorry, but you do not currently have permissions to do that (View hidden sections)

      Shouldn't the topic heading link be grey and inactive when it is set to be unavailable until XX activity is complete.

      This is confusing for students and I have subsequently withdrawn use of this feature until fixed. The scroll of death remains...

      Also, in single page per topic format:

      When I set a topic to be greyed out until XX activity in another topic is complete, the topic heading remains but all activities disappear (not greyed out).

      Shouldn't all of the activities be shown in the topic but simply greyed out if that topic is set to be unavailable until...

      Thanks

      Replication steps:

      1. Log in as admin
      2. Navigate to Site admin > Advanced features
      3. Check "Enable conditional" (accessenableavailability)
      4. Navigate to a course
      5. Navigate to Course admin > Edit settings
      6. Set Format = 'Topics format' and Course layout = 'Show one section per page'
      7. Save settings
      8. Edit a section (topic) somewhere in the middle of the course
      9. Under "Restrict access" set it to be dependant on another activity and set "Before section can be accessed" to "Show section greyed out..."
      10. Log in as student
      11. If viewing a single section, click "Return to main course page"
      12. Note that the Navigation shows the correct list of sections (without restricted section)
      13. Note that the restricted section is shown and the topic heading is an active link
      14. Click the link - an error is shown
      15. Click back
      16. Click on the section before the grouping restricted section
      17. Note that you can see a link to the grouping restricted section
      18. Click on the link - an error is shown

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Michael de Raadt added a comment -

            Thanks for reporting that. I was able to reproduce the problem. I have added some replication steps to your description.

            I've put that on the backlog.

            In the meantime feel free to help us work on this issue. 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 that. I was able to reproduce the problem. I have added some replication steps to your description. I've put that on the backlog. In the meantime feel free to help us work on this issue. 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
            Dan Poltawski added a comment -

            Looks like there are two issues here,

            1. Sams fix for MDL-33937 didn't take account of showing availability info when available
            2. The 'section navigation' was showing section links when not available.
            Show
            Dan Poltawski added a comment - Looks like there are two issues here, Sams fix for MDL-33937 didn't take account of showing availability info when available The 'section navigation' was showing section links when not available.
            Hide
            Dan Poltawski added a comment -

            Ok, patch up for peer review. Note that I have for this issue copied the navigation rules to be the same as in the actual navigation. The unavailable sections are not shown in navigation.

            Ideally Sam M would be able to review this.

            Show
            Dan Poltawski added a comment - Ok, patch up for peer review. Note that I have for this issue copied the navigation rules to be the same as in the actual navigation. The unavailable sections are not shown in navigation. Ideally Sam M would be able to review this.
            Hide
            Jason Fowler added a comment -

            Looks good Dan

            Show
            Jason Fowler added a comment - Looks good Dan
            Hide
            Dan Poltawski added a comment -

            Sam M - if you are able to look, it'd be great

            Show
            Dan Poltawski added a comment - Sam M - if you are able to look, it'd be great
            Hide
            Mark Langdale added a comment -

            Does anyone know if it will be possible to address this part of Glenn's issue? I have been looking for the same thing.

            "Also, in single page per topic format:

            When I set a topic to be greyed out until XX activity in another topic is complete, the topic heading remains but all activities disappear (not greyed out).

            Shouldn't all of the activities be shown in the topic but simply greyed out if that topic is set to be unavailable until..."

            Many thanks,

            Mark

            Show
            Mark Langdale added a comment - Does anyone know if it will be possible to address this part of Glenn's issue? I have been looking for the same thing. "Also, in single page per topic format: When I set a topic to be greyed out until XX activity in another topic is complete, the topic heading remains but all activities disappear (not greyed out). Shouldn't all of the activities be shown in the topic but simply greyed out if that topic is set to be unavailable until..." Many thanks, Mark
            Hide
            Sam Marshall added a comment -

            Reviewing this now (just commenting so you know I finally spotted it!)

            Show
            Sam Marshall added a comment - Reviewing this now (just commenting so you know I finally spotted it!)
            Hide
            Sam Marshall added a comment -

            +1 from me - as far as I can see this change achieves two things:

            1) Next/Previous links will not include unavailable sections (these will be skipped over so the link points to the section after that, instead).

            2) When viewing the page for a single section this now gives an error if you try to look at a section page you can't see.

            Basically both these look correct so I would be in favour of integrating them. I am not sure if they entirely address all the reported problems or not.

            Show
            Sam Marshall added a comment - +1 from me - as far as I can see this change achieves two things: 1) Next/Previous links will not include unavailable sections (these will be skipped over so the link points to the section after that, instead). 2) When viewing the page for a single section this now gives an error if you try to look at a section page you can't see. Basically both these look correct so I would be in favour of integrating them. I am not sure if they entirely address all the reported problems or not.
            Hide
            Glenn Sisson added a comment -

            On re-reading the issue I find my description of the problem to be poor. Apologies.

            It seems you have a handle on the issue concerning topics viewed when course format is "show one section per page." I am not in a position to be able to test new code but from the comments seems to address the issue.

            The second half of the problem is when the course format is set to "show all sections on one page." In this format, when I set a topic to be greyed out until XX activity in another topic is complete, the topic heading remains (full view, not greyed out) but all activities disappear (not greyed out).

            In my opinion, the whole topic should be greyed out (of course only when set to be greyed out).

            Hope this clarifies the issue.

            I look forward to the resolution.

            Show
            Glenn Sisson added a comment - On re-reading the issue I find my description of the problem to be poor. Apologies. It seems you have a handle on the issue concerning topics viewed when course format is "show one section per page." I am not in a position to be able to test new code but from the comments seems to address the issue. The second half of the problem is when the course format is set to "show all sections on one page." In this format, when I set a topic to be greyed out until XX activity in another topic is complete, the topic heading remains (full view, not greyed out) but all activities disappear (not greyed out). In my opinion, the whole topic should be greyed out (of course only when set to be greyed out). Hope this clarifies the issue. I look forward to the resolution.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

            This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

            This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

            Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
            Hide
            Dan Poltawski added a comment -

            Hi Glenn,

            Regarding your comment, I think that it is intentional that we display the title of the section even if greyed out. Although really, this is a question for Sam I think.

            I suggest we address this as a new issue, because its not clear to me and my fixes are one step in the right direction (I think!)

            Show
            Dan Poltawski added a comment - Hi Glenn, Regarding your comment, I think that it is intentional that we display the title of the section even if greyed out. Although really, this is a question for Sam I think. I suggest we address this as a new issue, because its not clear to me and my fixes are one step in the right direction (I think!)
            Hide
            Glenn Sisson added a comment -

            Created new issue MDL-35005

            Show
            Glenn Sisson added a comment - Created new issue MDL-35005
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Integrated (23 & master), thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Integrated (23 & master), thanks!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Note: Created MDL-35010 while testing this

            Show
            Eloy Lafuente (stronk7) added a comment - Note: Created MDL-35010 while testing this
            Hide
            Dan Poltawski added a comment -

            Ok, I just pushed two commits,

            1/ Reverting 87a31bb5d8fedd4447cf99cef13c5dd367c3ea3e. Actually I think what was happening there was the wrong approach. We should never let people get into the unavailable section in single section mode. Now the navigation mathces that too.
            2/ Don't link/show the title, actually I think this fixes Glenn's issue (MDL-35005).

            Sorry Glen for asking you to create a new issue, because I think you were right on.

            Also, Eloy - i'm sorry I didn't realise I didn't make a master branch for this and there was already two fixes on the branch.

            Show
            Dan Poltawski added a comment - Ok, I just pushed two commits, 1/ Reverting 87a31bb5d8fedd4447cf99cef13c5dd367c3ea3e. Actually I think what was happening there was the wrong approach. We should never let people get into the unavailable section in single section mode. Now the navigation mathces that too. 2/ Don't link/show the title, actually I think this fixes Glenn's issue ( MDL-35005 ). Sorry Glen for asking you to create a new issue, because I think you were right on. Also, Eloy - i'm sorry I didn't realise I didn't make a master branch for this and there was already two fixes on the branch.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Two new commits integrated (23) and cherry-picked to master, np Dan. Retesting...

            Show
            Eloy Lafuente (stronk7) added a comment - Two new commits integrated (23) and cherry-picked to master, np Dan. Retesting...
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Oki, I think now everything is is order. Summary:

            A) For one user not passing the section conditions/restrictions:

            0) navigation block does not show any grayed or completely hidden section ever.
            1) in course body, grayed sections are shown grayed (title, description and conditions) without links.
            2) hidden sections are not shown at all.
            3) manually linking to any "not passed" section (grayed or hidden) => error, no perms.
            4) next/prev skips all the not available sections.

            B) For one user passing the section conditions/restrictions:

            0) navigation block shows the available (conditions met) section.
            1) in course body, the section title is a link to the section and it works.
            2) next/prev show the available section.

            So, I'm passing this now. Plz, consider, given the above behavior, if MDL-35005 work is needed or can be closed.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Oki, I think now everything is is order. Summary: A) For one user not passing the section conditions/restrictions: 0) navigation block does not show any grayed or completely hidden section ever. 1) in course body, grayed sections are shown grayed (title, description and conditions) without links. 2) hidden sections are not shown at all. 3) manually linking to any "not passed" section (grayed or hidden) => error, no perms. 4) next/prev skips all the not available sections. B) For one user passing the section conditions/restrictions: 0) navigation block shows the available (conditions met) section. 1) in course body, the section title is a link to the section and it works. 2) next/prev show the available section. So, I'm passing this now. Plz, consider, given the above behavior, if MDL-35005 work is needed or can be closed. Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            YEAR!*

            CAF*, TOT!*

            • Your effort amazingly resulted. (unbelievable :-P)
            • Closing as fixed.
            • Tons of thanks.
            Show
            Eloy Lafuente (stronk7) added a comment - YEAR!* CAF*, TOT!* Your effort amazingly resulted. (unbelievable :-P) Closing as fixed. Tons of thanks.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: