Moodle
  1. Moodle
  2. MDL-33398

Cron fails when course completion is enabled

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.1.7, 2.2.4
    • Fix Version/s: 2.1.7, 2.2.4
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide

      Test Pre-requisites:

      • Completion must be ticked in Advanced Settings.
      • Create and enable completion for two courses
      • First course, on completion settings page enable "self completion"
      • Add the self completion block to course, run cron
      • Enrol a user in the course, as that user complete the course using the block
      • Second course, on Completion settings page, use the following settings:
      • "All" overall aggregation method
      • Add the first course as a prerequisite to this course
      • Enable the "date" criteria and set it to a date one month in the paste
      • Enable the "duration" criteria and set to 1 day after enrolment
      • Enable the "grade" criteria and set a grade of 50%
      • Enrol the user from the first course in the second
      • Edit their enrolment and change the start date to a week in the past
      • Via the gradebook, give them a grade of 100% in the course

      Test steps:

      • Run cron, no errors should occur
      Show
      Test Pre-requisites: Completion must be ticked in Advanced Settings. Create and enable completion for two courses First course, on completion settings page enable "self completion" Add the self completion block to course, run cron Enrol a user in the course, as that user complete the course using the block Second course, on Completion settings page, use the following settings: "All" overall aggregation method Add the first course as a prerequisite to this course Enable the "date" criteria and set it to a date one month in the paste Enable the "duration" criteria and set to 1 day after enrolment Enable the "grade" criteria and set a grade of 50% Enrol the user from the first course in the second Edit their enrolment and change the start date to a week in the past Via the gradebook, give them a grade of 100% in the course Test steps: Run cron, no errors should occur
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
    • Rank:
      41272

      Description

      /admin/cron.php fails when completion tracking is enabled. The following error is reported:

      Running completion_criteria_date->cron()
      Running completion_criteria_activity->cron()
      !!! Coding error detected, it must be fixed by a programmer: data_object params should be in the form of an array, not an object !!!
      !!
      Error code: codingerror !!
      !! Stack trace: * line 89 of /lib/completion/data_object.php: coding_exception thrown

      • line 238 of /lib/completion/completion_criteria_activity.php: call to data_object->__construct()
      • line 220 of /lib/completion/cron.php: call to completion_criteria_activity->cron()
      • line 40 of /lib/completion/cron.php: call to completion_cron_criteria()
      • line 323 of /lib/cronlib.php: call to completion_cron()
      • line 88 of /admin/cron.php: call to cron_run()
        !!

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment -

          Hi. You need to fix up the commit message to include the MDL number.

          Move the testing instructions out of the description and into the testing instructions field. Add to them how to check that course completion is enabled prior to running cron.

          If you haven't already, check that this doesn't need to go into 2.2 and 2.1.

          Casting a moodle_recordset to an array seems odd. I can see why you've done it and I can't think how else to fix this off the top of my head. It just looks odd.
          http://www.php.net/manual/en/language.types.array.php#language.types.array.casting
          "The keys are the member variable names, with a few notable exceptions: integer properties are unaccessible"
          I'm assuming that's not causing any problems here.

          Show
          Andrew Davis added a comment - Hi. You need to fix up the commit message to include the MDL number. Move the testing instructions out of the description and into the testing instructions field. Add to them how to check that course completion is enabled prior to running cron. If you haven't already, check that this doesn't need to go into 2.2 and 2.1. Casting a moodle_recordset to an array seems odd. I can see why you've done it and I can't think how else to fix this off the top of my head. It just looks odd. http://www.php.net/manual/en/language.types.array.php#language.types.array.casting "The keys are the member variable names, with a few notable exceptions: integer properties are unaccessible" I'm assuming that's not causing any problems here.
          Hide
          Aaron Barnes added a comment -

          Hi Andrew,

          This will need to go back into 2.1 and 2.2. It was caused by http://tracker.moodle.org/browse/MDL-32203 which now requires an input of an array rather than an object.

          Casting to an array will cause no data loss here.

          I'll update the patch now.

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Andrew, This will need to go back into 2.1 and 2.2. It was caused by http://tracker.moodle.org/browse/MDL-32203 which now requires an input of an array rather than an object. Casting to an array will cause no data loss here. I'll update the patch now. Cheers, Aaron
          Hide
          Aaron Barnes added a comment -

          Hi Andrew,

          Patch updated and applied to other branches.

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Andrew, Patch updated and applied to other branches. Cheers, Aaron
          Hide
          Andrew Davis added a comment -

          Cool. Fix up the testing instructions and submit for integration whenever you're ready

          Show
          Andrew Davis added a comment - Cool. Fix up the testing instructions and submit for integration whenever you're ready
          Hide
          Aaron Barnes added a comment -

          Done, thanks heaps Andrew

          Show
          Aaron Barnes added a comment - Done, thanks heaps Andrew
          Hide
          Dan Poltawski added a comment -

          Just a note that the moodle_recordset isn't casted to array, its the result of the recordset iterator (which is stdClass object).

          Show
          Dan Poltawski added a comment - Just a note that the moodle_recordset isn't casted to array, its the result of the recordset iterator (which is stdClass object).
          Hide
          Dan Poltawski added a comment -

          Thanks Aaron for the quick fix.

          I've integrated this now.

          Show
          Dan Poltawski added a comment - Thanks Aaron for the quick fix. I've integrated this now.
          Hide
          Aaron Barnes added a comment -

          Thank you guys for your quick responses/integration

          Show
          Aaron Barnes added a comment - Thank you guys for your quick responses/integration
          Hide
          Frédéric Massart added a comment -

          Thanks guys! Successful on 2.1, 2.2 and master.

          Show
          Frédéric Massart added a comment - Thanks guys! Successful on 2.1, 2.2 and master.
          Hide
          Steve-O added a comment -

          The fix on completion_criteria_date doesn't work

          Running completion_criteria_date->cron()
          PHP Fatal error: Cannot use object of type stdClass as array in moodle2/docs/lib/completion/completion_criteria_date.php on line 186 (where we added the cast to array)

          Show
          Steve-O added a comment - The fix on completion_criteria_date doesn't work Running completion_criteria_date->cron() PHP Fatal error: Cannot use object of type stdClass as array in moodle2/docs/lib/completion/completion_criteria_date.php on line 186 (where we added the cast to array)
          Hide
          Frédéric Massart added a comment -

          That's weird cause I had this issue but only on master, not 2.1 or 2.2. Then I made sure I had the last version of master and pulled from the repository. From that point I could not reproduce the error. Did you checkout the latest master branch from integration?

          Show
          Frédéric Massart added a comment - That's weird cause I had this issue but only on master, not 2.1 or 2.2. Then I made sure I had the last version of master and pulled from the repository. From that point I could not reproduce the error. Did you checkout the latest master branch from integration?
          Hide
          Dan Poltawski added a comment -

          Steve-O, it looks like you don't have the latest changes from integration, this change hasn't made its way into the production moodle repository yet.

          Show
          Dan Poltawski added a comment - Steve-O, it looks like you don't have the latest changes from integration, this change hasn't made its way into the production moodle repository yet.
          Hide
          Steve-O added a comment -

          Yesterday evening I updated from 2.2.2+ to 2.2.3+ and now I have the error on the completion.

          Unfortunately I don't use git... i'm still stuck with cvs
          I didn't download the latests master branch but manually modified the files as described in the diff URL, I don't know if I have to modify more files... where can i see the latest commit?

          in my installation I just added (array) in this line of each file:
          $completion = new completion_criteria_completion((array) $record, DATA_OBJECT_FETCH_BY_KEY);

          and if you have a course with "date" in the completion tracking it gives the error:Running completion_criteria_date->cron()
          PHP Fatal error: Cannot use object of type stdClass as array in moodle2/docs/lib/completion/completion_criteria_date.php on line 186 (where we added the cast to array)

          Show
          Steve-O added a comment - Yesterday evening I updated from 2.2.2+ to 2.2.3+ and now I have the error on the completion. Unfortunately I don't use git... i'm still stuck with cvs I didn't download the latests master branch but manually modified the files as described in the diff URL, I don't know if I have to modify more files... where can i see the latest commit? in my installation I just added (array) in this line of each file: $completion = new completion_criteria_completion((array) $record, DATA_OBJECT_FETCH_BY_KEY); and if you have a course with "date" in the completion tracking it gives the error:Running completion_criteria_date->cron() PHP Fatal error: Cannot use object of type stdClass as array in moodle2/docs/lib/completion/completion_criteria_date.php on line 186 (where we added the cast to array)
          Hide
          Aaron Barnes added a comment -

          Hi Steve-O,

          Could you point me to a copy of the line you are having the error on (and a few lines above and below it), a copy of the exact error message with that file, and the version of PHP you are running?

          Can you confirm you have the patch fixing MDL-32203 installed (should be able to find it in your CSV log?)

          The PHP error you are getting doesn't seem to add up.

          Thanks,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Steve-O, Could you point me to a copy of the line you are having the error on (and a few lines above and below it), a copy of the exact error message with that file, and the version of PHP you are running? Can you confirm you have the patch fixing MDL-32203 installed (should be able to find it in your CSV log?) The PHP error you are getting doesn't seem to add up. Thanks, Aaron
          Hide
          Frédéric Massart added a comment -

          Oh... I just tested again with the latest integration and I have an error...

          Marking users as started
          Running completion_criteria_date->cron()
          
          Fatal error: Cannot use object of type stdClass as array in /home/fred/www/repositories/testing_master/moodle/lib/completion/completion_criteria_date.php on line 183
          

          (You will notice that the line # is different than the one from Steve-O)

          I keep on investigating.

          Show
          Frédéric Massart added a comment - Oh... I just tested again with the latest integration and I have an error... Marking users as started Running completion_criteria_date->cron() Fatal error: Cannot use object of type stdClass as array in /home/fred/www/repositories/testing_master/moodle/lib/completion/completion_criteria_date.php on line 183 (You will notice that the line # is different than the one from Steve-O) I keep on investigating.
          Hide
          Frédéric Massart added a comment -

          Now I have the error on 2.2 too, but not 2.1. The error is triggered on criteria_date even if I haven't set a date criteria. And as far as I remember, when I had the error yesterday it was on 'activity' not on grade or date.

          Show
          Frédéric Massart added a comment - Now I have the error on 2.2 too, but not 2.1. The error is triggered on criteria_date even if I haven't set a date criteria. And as far as I remember, when I had the error yesterday it was on 'activity' not on grade or date.
          Hide
          Frédéric Massart added a comment -

          All right. I think I have found the bug. I did not see the error anymore because there was nothing to do in criteria... anyway, what happens is that $record is not converted to an array, only the copy passed to completion_criteria_completion() will be an array.

          The line just before completion_criteria_completion() should be

          $record = (array) $record;
          

          I recommend setting a criteria date as yesterday to trigger the cron execution.

          Show
          Frédéric Massart added a comment - All right. I think I have found the bug. I did not see the error anymore because there was nothing to do in criteria... anyway, what happens is that $record is not converted to an array, only the copy passed to completion_criteria_completion() will be an array. The line just before completion_criteria_completion() should be $record = (array) $record; I recommend setting a criteria date as yesterday to trigger the cron execution.
          Hide
          Dan Poltawski added a comment -

          Reopening this as it seems the problem has not been properly fixed

          Show
          Dan Poltawski added a comment - Reopening this as it seems the problem has not been properly fixed
          Hide
          Frédéric Massart added a comment -

          Here is a patch for it: https://github.com/FMCorz/moodle/commit/67ec19a21a06566090688b658e5c0c41a3009987
          Aaron, if you wish to integrate this in your branches it can probably be cherry-picked in all versions, but please note that this includes your changes.

          Looking at the other cron methods, I thought that converting the object to array was unnecessary as only completion_criteria_completion() really need it. Therefore I only updated the following lines which were waiting for an array.

          Show
          Frédéric Massart added a comment - Here is a patch for it: https://github.com/FMCorz/moodle/commit/67ec19a21a06566090688b658e5c0c41a3009987 Aaron, if you wish to integrate this in your branches it can probably be cherry-picked in all versions, but please note that this includes your changes. Looking at the other cron methods, I thought that converting the object to array was unnecessary as only completion_criteria_completion() really need it. Therefore I only updated the following lines which were waiting for an array.
          Hide
          Aaron Barnes added a comment -

          Thanks Frederic!

          That's perfect, cheers! Will update my branches with this fix, ammending it to my existing patch.

          Show
          Aaron Barnes added a comment - Thanks Frederic! That's perfect, cheers! Will update my branches with this fix, ammending it to my existing patch.
          Hide
          Frédéric Massart added a comment -

          No worries. I thought that maybe you could update the testing instructions so that we can make sure the 5 different methods behave as expected. I don't have enough knowledge in cron and completion criteria to do it properly...

          Thanks!

          Show
          Frédéric Massart added a comment - No worries. I thought that maybe you could update the testing instructions so that we can make sure the 5 different methods behave as expected. I don't have enough knowledge in cron and completion criteria to do it properly... Thanks!
          Hide
          Aaron Barnes added a comment -

          Done, and thanks again Frederic. Sorry for not crediting you in the patch - didn't think of it until after I had pushed Your help is greatly appreciated!

          Show
          Aaron Barnes added a comment - Done, and thanks again Frederic. Sorry for not crediting you in the patch - didn't think of it until after I had pushed Your help is greatly appreciated!
          Hide
          Dan Poltawski added a comment -

          Thanks both.

          I ended up cherry picking freds patch onto all the branches because you amended the patch Aaron. As we've already pushed the original we needed a commit on top (so freds commit worked for that).

          Be great if that can be tested again - thanks!

          Show
          Dan Poltawski added a comment - Thanks both. I ended up cherry picking freds patch onto all the branches because you amended the patch Aaron. As we've already pushed the original we needed a commit on top (so freds commit worked for that). Be great if that can be tested again - thanks!
          Hide
          Frédéric Massart added a comment -

          Great job guys! Successfully tested on master, 2.2 and 2.1!

          Show
          Frédéric Massart added a comment - Great job guys! Successfully tested on master, 2.2 and 2.1!
          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:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: