Moodle
  1. Moodle
  2. MDL-38101

Error occurs when teacher/admin manually marks an item as complete

    Details

    • Testing Instructions:
      Hide

      (Do this test as admin)

      1. Ensure that activity completion is enabled at server level under advanced features.
      2. For a test course, ensure that completion is enabled in the course settings.
      3. Add a new label using the default completion settings (manual completion).
      4. Turn editing off.
      5. Click the completion tickbox next to the label.

      EXPECTED:

      The tickbox is ticked.

      BEFORE FIX:

      An alert appears to tell you that there was an error saving the tick mark.

      Show
      (Do this test as admin) 1. Ensure that activity completion is enabled at server level under advanced features. 2. For a test course, ensure that completion is enabled in the course settings. 3. Add a new label using the default completion settings (manual completion). 4. Turn editing off. 5. Click the completion tickbox next to the label. EXPECTED: The tickbox is ticked. BEFORE FIX: An alert appears to tell you that there was an error saving the tick mark.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-38101-m24
    • Pull Master Branch:
      MDL-38101-master
    • Rank:
      47912

      Description

      When logged in as a teacher or admin, if the user clicks on an a box to manually mark an activity as completed, an error occurs in the Ajax and is displayed in a modular window on-screen.

      An error occurred when attempting to save your tick mark.
      
      (<!DOCTYPE html>
      <html  dir="ltr" lang="en" xml:lang="en">
      <head>
          <title>Error</title>
          <link rel="shortcut icon" href="http://michael.moodle.local/master_dev/theme/image.php/standard/theme/1360572777/favicon" />
          <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
      <meta name="keywords" content="moodle, Error" />
      <link rel="stylesheet" type="text/css" href="http://michael.moodle.local/master_dev/theme/yui_combo.php?3.8.0/build/cssreset/reset.css&amp;3.8.0/build/cssfonts/fonts.css&amp;3.8.0/build/cssgrids/grids.css&amp;3.8.0/build/cssbase/base.css" /><script type="text/javascript" src="http://michael.moodle.local/master_dev/theme/yui_combo.php?3.8.0/build/simpleyui/simpleyui.js&amp;3.8.0/build/loader/loader.js"></script><script id="firstthemesheet" type="text/css">/** Required in order to fix style inclusion problems in IE with YUI **/</script><link rel="stylesheet" type="text/css" href="http://michael.moodle.local/master_dev/theme/styles.php/standard/1360572777/all" />
      <script type="text/javascript">
      //<![CDATA[
      var M = {}; M.yui = {};
      ...
      The page content is repeated.
      

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Adrian was kind enough to replicate this for me.

          Show
          Michael de Raadt added a comment - Adrian was kind enough to replicate this for me.
          Hide
          Sam Marshall added a comment -

          I can reproduce this one on current master also. Will look at it now.

          Show
          Sam Marshall added a comment - I can reproduce this one on current master also. Will look at it now.
          Hide
          Sam Marshall added a comment -

          This is a regression caused by MDL-37473. Basically it was checking that they are included in the tracked-user reports, finding out they weren't, and giving an error. But you are supposed to be able to mark stuff complete if you like, even if you don't appear in the reports.

          Show
          Sam Marshall added a comment - This is a regression caused by MDL-37473 . Basically it was checking that they are included in the tracked-user reports, finding out they weren't, and giving an error. But you are supposed to be able to mark stuff complete if you like, even if you don't appear in the reports.
          Hide
          Sam Marshall added a comment -

          Please could somebody review this change - it's pretty trivial, I have just removed the part where there it gave an error if you aren't tracked, and put a comment explaining why it shouldn't check that.

          I also tested (as admin, which was previously failing) in both JavaScript and non-JavaScript modes.

          Show
          Sam Marshall added a comment - Please could somebody review this change - it's pretty trivial, I have just removed the part where there it gave an error if you aren't tracked, and put a comment explaining why it shouldn't check that. I also tested (as admin, which was previously failing) in both JavaScript and non-JavaScript modes.
          Hide
          Sam Marshall added a comment -

          (By the way - not blaming anyone, because I actually reviewed the issue I mentioned and I didn't spot it...)

          Show
          Sam Marshall added a comment - (By the way - not blaming anyone, because I actually reviewed the issue I mentioned and I didn't spot it...)
          Hide
          Mónica Sánchez added a comment -

          Hi Sam, I checked your change and everything works fine (on M2.4.1+). Admin and teachers can now check/uncheck manually any activity. Thanks!

          Show
          Mónica Sánchez added a comment - Hi Sam, I checked your change and everything works fine (on M2.4.1+). Admin and teachers can now check/uncheck manually any activity. Thanks!
          Hide
          Dan Poltawski added a comment -

          Hi Sam,

          This looks good to me. But we need testing instructions.

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

          Show
          Dan Poltawski added a comment - Hi Sam, This looks good to me. But we need testing instructions. [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [N] Testing [Y] Security [-] Documentation [Y] Git [Y] Sanity check
          Hide
          Sam Marshall added a comment -

          Thanks Dan. Ooops, I forgot the test instructions! Added now.

          Show
          Sam Marshall added a comment - Thanks Dan. Ooops, I forgot the test instructions! Added now.
          Hide
          Sam Hemelryk added a comment -

          Hi Sam,

          That change was introduced recently by MDL-37473.
          Looking at the changes made on MDL-37473 I see there was another code block identical to the one you've modified several lines above.
          Does that need to be addressed as well? it looks to me as though it does but perhaps I've missed something.

          Leaving this in integration review in progress.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Sam, That change was introduced recently by MDL-37473 . Looking at the changes made on MDL-37473 I see there was another code block identical to the one you've modified several lines above. Does that need to be addressed as well? it looks to me as though it does but perhaps I've missed something. Leaving this in integration review in progress. Many thanks Sam
          Hide
          Sam Marshall added a comment -

          Sam - Well spotted. However I think this change is correct. I've investigated and the upper code branch is used in two places:

          1) The 'self completion' block
          2) If you mark somebody else complete from the report

          In case 1, the block displays informational text instead of giving a link to togglecompletion.php if you are not a tracked user. In case 2, obviously only tracked users are shown in the report, that's what the option does. So in both of these, the error is probably correct.

          Show
          Sam Marshall added a comment - Sam - Well spotted. However I think this change is correct. I've investigated and the upper code branch is used in two places: 1) The 'self completion' block 2) If you mark somebody else complete from the report In case 1, the block displays informational text instead of giving a link to togglecompletion.php if you are not a tracked user. In case 2, obviously only tracked users are shown in the report, that's what the option does. So in both of these, the error is probably correct.
          Hide
          Sam Hemelryk added a comment -

          Thanks for checking that out and clarifying - this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks for checking that out and clarifying - this has been integrated now
          Hide
          Rossiani Wijaya added a comment -

          This is working as expected.

          However I found an undefined property error while trying to add label to a section. The bug is not related to this patch. I created MDL-38342 to address the issue.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working as expected. However I found an undefined property error while trying to add label to a section. The bug is not related to this patch. I created MDL-38342 to address the issue. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities.

          Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied).

          Thanks, closing as fixed!

          Show
          Eloy Lafuente (stronk7) added a comment - This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities. Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied). Thanks, closing as fixed!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: