Moodle
  1. Moodle
  2. MDL-36131

Conditional activities don't behave same in ajax and non-ajax mode.

    Details

    • Testing Instructions:
      Hide
      1. Turn on conditional activities and enable AJAX mode.
      2. Visit a course and create an activity that is set 'visible to students' but has a date restriction in future
      3. Visit the course page and check activity, is should be grey and availability information should be grey as well.
      4. Click hide button and activity is greyed and available information is hidden
      5. Click show button and activity is still greyed and available information is shown
      6. Visit a course and create an activity that is set 'visible to students' but has a date restriction in past
      7. Visit the course page and check activity, is should be active link, but availability information should be grey.
      8. Click hide button and activity is greyed and available information is hidden
      9. Click show button and activity is active and available information is shown
      Show
      Turn on conditional activities and enable AJAX mode. Visit a course and create an activity that is set 'visible to students' but has a date restriction in future Visit the course page and check activity, is should be grey and availability information should be grey as well. Click hide button and activity is greyed and available information is hidden Click show button and activity is still greyed and available information is shown Visit a course and create an activity that is set 'visible to students' but has a date restriction in past Visit the course page and check activity, is should be active link, but availability information should be grey. Click hide button and activity is greyed and available information is hidden Click show button and activity is active and available information is shown
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-36131
    • Rank:
      44902

      Description

      In AJAX mode with conditional activities on, click on show/hide button and nothing happens at first click.
      On second click activity hide icon is not in sync with dimmed text. Also, availability information is not hidden when activity is hidden.

      NON-AJAX Mode, which should be same in AJAX mode.

      1. Turn on conditional activities and disable AJAX mode.
      2. Visit a course and create an activity that is set 'visible to students' but has a date restriction in future
      3. Visit the course page and check activity, is should be grey and availability information should be grey as well.
      4. Visit a course and create an activity that is set 'visible to students' but has a date restriction in past
      5. Visit the course page and check activity, is should be active link, but availability information should be grey.
      6. Click hide button and activity is greyed and available information is hidden

        Issue Links

          Activity

          Hide
          Mark Nelson added a comment -

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

          Hi Raj, just a few points.

          1) Maybe add spacing for the $actionurl, just looks nicer. Even though it looks like you did not touch this line at all. Trivial point, can be ignored.
          $actionurl = $CFG->wwwroot . make_log_url($log->module, $log->url);

          2) When generating the div with the class availabilityinfo, I was wondering if you should you be using html_writer::tag('div'). However, I did look at the rest of the code contained in that file and noticed that it did not use the html_writer for the rest of the HTML that was rendered, and since you are only adding a class I feel what you have done is reasonable.

          3) A couple of the comments are missing full-stops.

          I also ran this while using JS console and did not see any errors. Good job.

          Show
          Mark Nelson added a comment - [?] Syntax [?] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Hi Raj, just a few points. 1) Maybe add spacing for the $actionurl, just looks nicer. Even though it looks like you did not touch this line at all. Trivial point, can be ignored. $actionurl = $CFG->wwwroot . make_log_url($log->module, $log->url); 2) When generating the div with the class availabilityinfo, I was wondering if you should you be using html_writer::tag('div'). However, I did look at the rest of the code contained in that file and noticed that it did not use the html_writer for the rest of the HTML that was rendered, and since you are only adding a class I feel what you have done is reasonable. 3) A couple of the comments are missing full-stops. I also ran this while using JS console and did not see any errors. Good job.
          Hide
          Rajesh Taneja added a comment -

          Thanks Mark,

          1. I managed to get the spacing issue resolved.
          2. Not sure about using html_writer as existing code is not using it, as discussed in previous bugs it's nice to use html_writer if existing code around is using it.
          3. Added full-stops on comments.

          Pushing it for integration.

          Not sure if it's worth fixing for 22, as code is different in 22 and is on verge of getting in security fixes mode. Leaving it for integrators to decide.

          Show
          Rajesh Taneja added a comment - Thanks Mark, I managed to get the spacing issue resolved. Not sure about using html_writer as existing code is not using it, as discussed in previous bugs it's nice to use html_writer if existing code around is using it. Added full-stops on comments. Pushing it for integration. Not sure if it's worth fixing for 22, as code is different in 22 and is on verge of getting in security fixes mode. Leaving it for integrators to decide.
          Hide
          Sam Hemelryk added a comment -

          Thanks Raj, this has been integrated now.
          In regards to 22 given that the code has changed quite a bit, and that the functionality on the backend is fine and its just an out of sync UI issue that not backporting to 22 is fine.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Raj, this has been integrated now. In regards to 22 given that the code has changed quite a bit, and that the functionality on the backend is fine and its just an out of sync UI issue that not backporting to 22 is fine. Many thanks Sam
          Hide
          Jason Fowler added a comment -

          works as described Raj, thanks for the fix

          Show
          Jason Fowler added a comment - works as described Raj, thanks for the fix
          Hide
          Dan Poltawski added a comment -

          Hurray!

          You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

          Show
          Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

            People

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

              Dates

              • Created:
                Updated:
                Resolved: