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
    • Rank:
      42846

      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

        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: