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

Restricted activity does not change from hidden.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.5
    • Fix Version/s: 2.2.6
    • Component/s: JavaScript
    • Labels:
    • Testing Instructions:
      Hide

      1. Turn on conditional activities and use AJAX mode.
      2. Visit a course and create an activity that is set 'visible to students' but has a date restriction.
      3. Visit the course page and look at the eye icon next to the activity.

      EXPECTED:

      The activity is greyed out to indicate that it is restricted, but the eye icon should be shown as 'open' eye because the activity is not hidden from students via the eye/visibility option.

      BUG:

      When using AJAX mode the eye icon is shown as 'closed' eye which is incorrect.

      Show
      1. Turn on conditional activities and use AJAX mode. 2. Visit a course and create an activity that is set 'visible to students' but has a date restriction. 3. Visit the course page and look at the eye icon next to the activity. EXPECTED: The activity is greyed out to indicate that it is restricted, but the eye icon should be shown as 'open' eye because the activity is not hidden from students via the eye/visibility option. BUG: When using AJAX mode the eye icon is shown as 'closed' eye which is incorrect.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE

      Description

      If you restrict an activity by some condition, then click on the eye icon in the course list to 'show' the activity, then refresh the page it will be set back to hidden.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            quen Sam Marshall added a comment -

            I think this is an AJAX bug. I just tested and it appears to work fine when using a course format that doesn't support AJAX. So, I guess this is because of all the new AJAX stuff in 2.3 probably...

            The problem is that in AJAX mode the eye icon shows as the 'hidden' eye when the activity is hidden from students e.g. due to a date restriction (i.e. whenever it is greyed out). This is incorrect. The eye icon should always be based on the 'visible' option in the database and not if the activity is hidden for another reason.

            To put it another way, the eye icon is a completely independent control for the 'visible' option for the activity and is not related to any of the conditional visibility options. The eye icon is not supposed to indicate whether students can view the activity, but is a control specifically for the visible flag.

            The actual behaviour when you click on the icon is actually correct(ish) because it is toggling the visible settting (from visible to not visible, in this case), the bug is the display of the icon in the first place.

            I've updated the testing instructions to reflect this and am reassigning to AJAX component. (If I got this wrong and there somehow is something wrong with conditional activities, assign back to me?)

            Show
            quen Sam Marshall added a comment - I think this is an AJAX bug. I just tested and it appears to work fine when using a course format that doesn't support AJAX. So, I guess this is because of all the new AJAX stuff in 2.3 probably... The problem is that in AJAX mode the eye icon shows as the 'hidden' eye when the activity is hidden from students e.g. due to a date restriction (i.e. whenever it is greyed out). This is incorrect. The eye icon should always be based on the 'visible' option in the database and not if the activity is hidden for another reason. To put it another way, the eye icon is a completely independent control for the 'visible' option for the activity and is not related to any of the conditional visibility options. The eye icon is not supposed to indicate whether students can view the activity, but is a control specifically for the visible flag. The actual behaviour when you click on the icon is actually correct(ish) because it is toggling the visible settting (from visible to not visible, in this case), the bug is the display of the icon in the first place. I've updated the testing instructions to reflect this and am reassigning to AJAX component. (If I got this wrong and there somehow is something wrong with conditional activities, assign back to me?)
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Sorry Guys, Can't reproduce this issue.
            Seems to be fixed by linked issue. Let me know if you are still observing this problem.

            FYI:
            Found another AJAX bug with conditional hidden. Conditional activities don't behave same in ajax and non-ajax mode.
            Will open another bug for that.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Sorry Guys, Can't reproduce this issue. Seems to be fixed by linked issue. Let me know if you are still observing this problem. FYI: Found another AJAX bug with conditional hidden. Conditional activities don't behave same in ajax and non-ajax mode. Will open another bug for that.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Aha, just checked on 22 and it's there.
            Seems it has been fixed on Master. Sorry for the noise.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Aha, just checked on 22 and it's there. Seems it has been fixed on Master. Sorry for the noise.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            JS added to find class, is in-line with the existing coding style.

            Show
            rajeshtaneja Rajesh Taneja added a comment - JS added to find class, is in-line with the existing coding style.
            Hide
            markn Mark Nelson added a comment -

            [Y] Syntax
            [-] Output
            [Y] Whitespace
            [-] Language
            [-] Databases
            [Y] Testing
            [-] Security
            [-] Documentation
            [Y] Git
            [Y] Sanity check

            Hi Raj, good job, pretty self explanatory patch. Passing.

            Show
            markn Mark Nelson added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Hi Raj, good job, pretty self explanatory patch. Passing.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Mark, pushing for integration.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Mark, pushing for integration.
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            poltawski Dan Poltawski added a comment -

            I've integrated this, thanks Raj.

            Although I would say that I think it would've been fine to let this stay as it in in 2.2, considering that its been fixed in 2.3 and above.

            Show
            poltawski Dan Poltawski added a comment - I've integrated this, thanks Raj. Although I would say that I think it would've been fine to let this stay as it in in 2.2, considering that its been fixed in 2.3 and above.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Leaving up-to-you Dan,

            If you feel this is not required then, I am file with It.
            Hope Mark will be fine with your decision too

            Show
            rajeshtaneja Rajesh Taneja added a comment - Leaving up-to-you Dan, If you feel this is not required then, I am file with It. Hope Mark will be fine with your decision too
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This looks great.

            Thanks for fixing this Raj.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This looks great. Thanks for fixing this Raj. Test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

            (not really)

            Closing, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Nov/12