Moodle
  1. Moodle
  2. MDL-35766

Restricted activity does not change from hidden.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.5
    • Fix Version/s: 2.2.6
    • Component/s: AJAX and 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
    • Rank:
      44517

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          Rajesh Taneja added a comment -

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

          Show
          Rajesh Taneja added a comment - JS added to find class, is in-line with the existing coding style.
          Hide
          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
          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
          Rajesh Taneja added a comment -

          Thanks Mark, pushing for integration.

          Show
          Rajesh Taneja added a comment - Thanks Mark, pushing for integration.
          Hide
          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
          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
          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
          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
          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
          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
          Rossiani Wijaya added a comment -

          This looks great.

          Thanks for fixing this Raj.

          Test passed.

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

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

          (not really)

          Closing, thanks!

          Show
          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: