Moodle
  1. Moodle
  2. MDL-33212

Activity Completion and Restrict Access fails to hide

    Details

    • Testing Instructions:
      Hide

      NOTE: When describing this test I avoided using the eye icons. That's because these have never worked quite correctly in AJAX mode, and I didn't want to confuse this issue with that.

      The second half of this test checks the previous (not changed) behaviour to make sure it still works when they aren't hidden.

      1. You must have the conditional availability options enabled in server settings.

      2. In a course, create a Label with these settings:

      • Name = 'Test'
      • Visible = Hide
      • Available from = Enabled; select future date

      3. While still logged in as a manager/admin/teacher, turn editing off, and look at the label on the course page.

      • It should display as:

      Test <-- greyed-out

      4. Using another browser, log in as a student. The label should not display at all.

      5. Back as manager, turn editing on, edit the label settings and change 'Visible' to 'Show', then turn editing off again.

      • Label should now look like

      Test <-- still greyed-out

      Available from 23 May 2016.

      6. As student, reload the page.

      • Label should now look the same as it does for manager. (Note: setting labels to be unavailable but visible is kind of silly, but that doesn't matter from the purpose of this test.)
      Show
      NOTE: When describing this test I avoided using the eye icons. That's because these have never worked quite correctly in AJAX mode, and I didn't want to confuse this issue with that. The second half of this test checks the previous (not changed) behaviour to make sure it still works when they aren't hidden. 1. You must have the conditional availability options enabled in server settings. 2. In a course, create a Label with these settings: Name = 'Test' Visible = Hide Available from = Enabled; select future date 3. While still logged in as a manager/admin/teacher, turn editing off, and look at the label on the course page. It should display as: Test <-- greyed-out 4. Using another browser, log in as a student. The label should not display at all. 5. Back as manager, turn editing on, edit the label settings and change 'Visible' to 'Show', then turn editing off again. Label should now look like Test <-- still greyed-out Available from 23 May 2016. 6. As student, reload the page. Label should now look the same as it does for manager. (Note: setting labels to be unavailable but visible is kind of silly, but that doesn't matter from the purpose of this test.)
    • Workaround:
      Hide

      Set 'Before activity can be accessed' option to 'Hide completely' at the same time as setting the visible option to 'Hide'.

      Show
      Set 'Before activity can be accessed' option to 'Hide completely' at the same time as setting the visible option to 'Hide'.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-33212-master
    • Rank:
      41126

      Description

      Working with activity completion and restrict access functions. Created resources that have access dependent on previous resources being viewed, as well as an activity dependent on all resources being viewed. First resource is set as shown, subsequent resources and activity hidden, but are still visible to student.

      RECREATION STEPS:

      1)Add a Resource - URL: test1
      A)Set visibility to "Show"
      B)Activity completion - "Show activity as complete when conditions are met" and "Student must view this activity to complete it" activated
      2)Add a Resource - URL: test 2
      A)Set visibility to "Hide"
      B) Activity Completion - "Show activity as complete when conditions are met" and "Student must view this activity to complete it" activated
      3)Add an activity - Quiz: test 3
      A) set visibility to hide

      At this point everything is at it should be. The first resource is available, and the subsequent resource and activity are hidden as per settings.

      4)Edit settings test 2
      A) Restrict Access - Set Activity completion condition to "Test 1" and "Must be marked as complete"
      5)Edit settings test 3
      A) Restrict Access - Set Activity completion condition to "Test 2" and "Must be marked as complete"

      Now the items "test 2" and "test 3", which were previously hidden from students are visible to students with the message "Not available until the activity ' ' is marked complete." Student is unable to access, view, even click on the items, but are nonetheless able to see that the item is there, and what access to that item is contingent upon.

      If the entire week/topic is hidden then the items do not appear as that week/section is unavailable.

      Need to have the functionality to hide these items from students if that is what the teacher wants.

        Activity

        Hide
        Sam Marshall added a comment -

        Confirmed also on current moodle (qa.moodle.net).

        I agree this is a bug. The eye icon (hide/show) should take precedence over the 'before activity can be accessed = show greyed out' option. I.e. if the activity is hidden then it should never display to students. Looking at it now.

        Show
        Sam Marshall added a comment - Confirmed also on current moodle (qa.moodle.net). I agree this is a bug. The eye icon (hide/show) should take precedence over the 'before activity can be accessed = show greyed out' option. I.e. if the activity is hidden then it should never display to students. Looking at it now.
        Hide
        Sam Marshall added a comment -

        I have written a new test script (and done the code, will send for review shortly).

        I was initially a bit uncertain about what to do for people who have the permission to see hidden activities - should it show the restriction details then, or not? In the end I decided not because although the restrictions are still present, they have no effect if you've hidden it with the eye icon. For example, if you intend to have an activity become available in future, but actually you've also left it hidden, it is a bit misleading to say that it's restricted until x date, when in fact, it's just hidden forever (unless you unhide it).

        Show
        Sam Marshall added a comment - I have written a new test script (and done the code, will send for review shortly). I was initially a bit uncertain about what to do for people who have the permission to see hidden activities - should it show the restriction details then, or not? In the end I decided not because although the restrictions are still present, they have no effect if you've hidden it with the eye icon. For example, if you intend to have an activity become available in future, but actually you've also left it hidden, it is a bit misleading to say that it's restricted until x date, when in fact, it's just hidden forever (unless you unhide it).
        Hide
        Sam Marshall added a comment -

        Ready for review. Note, due to changes in conditionlib, the 2.2 patch is marginally different from the master version (variable name). 2.1 is a straight cherry-pick.

        Show
        Sam Marshall added a comment - Ready for review. Note, due to changes in conditionlib, the 2.2 patch is marginally different from the master version (variable name). 2.1 is a straight cherry-pick.
        Hide
        Sam Hemelryk added a comment -

        Looks spot on thanks Sam, putting this up for integration review now.

        Show
        Sam Hemelryk added a comment - Looks spot on thanks Sam, putting this up for integration review now.
        Hide
        Adam Sanders added a comment -

        Thanks for the prompt job. will use the workaround for now until new update. Keep up the good work!

        Show
        Adam Sanders added a comment - Thanks for the prompt job. will use the workaround for now until new update. Keep up the good work!
        Hide
        Dan Poltawski added a comment -

        Thanks Sam, i've integrated this now.

        Show
        Dan Poltawski added a comment - Thanks Sam, i've integrated this now.
        Hide
        Dan Poltawski added a comment -

        Hi Sam,

        Failing this as its causing notices during unit tests on all branches:

        conditionlib_testcase::test_section_is_available
        Undefined property: stdClass::$visible

        /Users/Shared/Jenkins/Home/git_repositories/master/lib/conditionlib.php:855
        /Users/Shared/Jenkins/Home/git_repositories/master/lib/conditionlib.php:245
        /Users/Shared/Jenkins/Home/git_repositories/master/lib/tests/conditionlib_test.php:587
        /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/advanced_testcase.php:76

        Show
        Dan Poltawski added a comment - Hi Sam, Failing this as its causing notices during unit tests on all branches: conditionlib_testcase::test_section_is_available Undefined property: stdClass::$visible /Users/Shared/Jenkins/Home/git_repositories/master/lib/conditionlib.php:855 /Users/Shared/Jenkins/Home/git_repositories/master/lib/conditionlib.php:245 /Users/Shared/Jenkins/Home/git_repositories/master/lib/tests/conditionlib_test.php:587 /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/advanced_testcase.php:76
        Hide
        Dan Poltawski added a comment -

        I'm reopening this as i've reverted it whilst the tests are breaking.

        Show
        Dan Poltawski added a comment - I'm reopening this as i've reverted it whilst the tests are breaking.
        Hide
        Sam Marshall added a comment -

        Sorry, running unit tests now, will fix and resubmit.

        Show
        Sam Marshall added a comment - Sorry, running unit tests now, will fix and resubmit.
        Hide
        Sam Marshall added a comment -

        Resubmit for integration - I have pushed new versions of the commit to each branch. The master one was rebased on top of current.

        Bit of a result from unit testing here - in addition to things which needed changing in the unit test, I think this was an actual bug (basically, I had added code that uses the 'visible' field of the cm/section object, but forgotten to add it to the list of required fields). Unlikely to cause a problem but still.

        Note: I also fixed and ran the simpletest unit test on 2.2 (just for this). On master I ran phpunit only on this test script so far, although I am now starting a full run and will look at anything else that comes up if it appears related.

        Show
        Sam Marshall added a comment - Resubmit for integration - I have pushed new versions of the commit to each branch. The master one was rebased on top of current. Bit of a result from unit testing here - in addition to things which needed changing in the unit test, I think this was an actual bug (basically, I had added code that uses the 'visible' field of the cm/section object, but forgotten to add it to the list of required fields). Unlikely to cause a problem but still. Note: I also fixed and ran the simpletest unit test on 2.2 (just for this). On master I ran phpunit only on this test script so far, although I am now starting a full run and will look at anything else that comes up if it appears related.
        Hide
        Sam Marshall added a comment -

        update: full phpunit ran with no (0) (!!!!!!) failures/errors anywhere.

        Show
        Sam Marshall added a comment - update: full phpunit ran with no (0) (!!!!!!) failures/errors anywhere.
        Hide
        Dan Poltawski added a comment -

        update: full phpunit ran with no (0) (!!!!!!) failures/errors anywhere.

        Sam, this is normal now, thats why I reverted your change

        Show
        Dan Poltawski added a comment - update: full phpunit ran with no (0) (!!!!!!) failures/errors anywhere. Sam, this is normal now, thats why I reverted your change
        Hide
        Dan Poltawski added a comment -

        Thanks Sam, looks good and integrated now!

        Show
        Dan Poltawski added a comment - Thanks Sam, looks good and integrated now!
        Hide
        Rossiani Wijaya added a comment -

        This is working great.

        Test passed.

        Show
        Rossiani Wijaya added a comment - This is working great. Test passed.
        Hide
        Dan Poltawski added a comment -

        Congratulations!

        Your work has made into the latest Moodle release!

        You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

        Show
        Dan Poltawski added a comment - Congratulations! Your work has made into the latest Moodle release! You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: