Moodle
  1. Moodle
  2. MDL-41144

flatfile enrolment should ignore deleted users

    Details

    • Testing Instructions:
      Hide

      set up flatfile enrolment.
      Find the idnumber of a deleted user.
      create an entry in the flatfile enrolment that enrols that deleted user in a course.
      run cron and make sure it doesn't cause a fatal error.

      Show
      set up flatfile enrolment. Find the idnumber of a deleted user. create an entry in the flatfile enrolment that enrols that deleted user in a course. run cron and make sure it doesn't cause a fatal error.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      master_MDL-41144

      Description

      we're seeing this error in cron on one of our sites causing cron to completely fail:

      !!! Coding error detected, it must be fixed by a programmer: User ID does not exist or is deleted! !!!
      !! userid:2043
      Error code: codingerror !!
      !! Stack trace: * line 1623 of /lib/accesslib.php: coding_exception thrown

      • line 1311 of /lib/enrollib.php: call to role_assign()
      • line 243 of /enrol/flatfile/lib.php: call to enrol_plugin->enrol_user()
      • line 144 of /enrol/flatfile/lib.php: call to enrol_flatfile_plugin->process_records()
      • line 54 of /enrol/flatfile/lib.php: call to enrol_flatfile_plugin->process_file()
      • line 243 of /lib/cronlib.php: call to enrol_flatfile_plugin->cron()
      • line 88 of /admin/cron.php: call to cron_run()

      flatfile shouldn't try to handle users that have been deleted.

        Gliffy Diagrams

          Activity

          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Dan,

          Patch looks good, I think you should update trace output stating unknown idnumber or deleted user.

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

          Feel free to push it for integration when ready.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Dan, Patch looks good, I think you should update trace output stating unknown idnumber or deleted user. [y] Syntax [y] Whitespace [y] Output [-] Language [y] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check Feel free to push it for integration when ready.
          Hide
          Dan Marsden added a comment -

          thanks Rajesh - makes sense, have updated trace output and bouncing through for integration.

          Show
          Dan Marsden added a comment - thanks Rajesh - makes sense, have updated trace output and bouncing through 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 -

          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 Marsden added a comment -

          rebased

          Show
          Dan Marsden added a comment - rebased
          Hide
          Sam Hemelryk added a comment -

          Thanks Dan - this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Dan - this has been integrated now
          Hide
          Rossiani Wijaya added a comment -

          Hi DanM,

          The patch is working as expected. However I got an error for phpunit_util right after the flatfile enrolments. I'm pretty sure this is not related to your fix, but I will wait passing this issue until MDL-41196 get tested and received a comment from Andrew.

          Thanks.

          Show
          Rossiani Wijaya added a comment - Hi DanM, The patch is working as expected. However I got an error for phpunit_util right after the flatfile enrolments. I'm pretty sure this is not related to your fix, but I will wait passing this issue until MDL-41196 get tested and received a comment from Andrew. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Although surely the error is unrelated to this issue, my ill mind can see some association between enrolments (this) and mails (MDL-41196).

          So I think it's perfect to hold this testing until MDL-41196 is fixed/passed.

          Thanks Rossie!

          Show
          Eloy Lafuente (stronk7) added a comment - Although surely the error is unrelated to this issue, my ill mind can see some association between enrolments (this) and mails ( MDL-41196 ). So I think it's perfect to hold this testing until MDL-41196 is fixed/passed. Thanks Rossie!
          Hide
          Rossiani Wijaya added a comment -

          MDL-41196 has been fixed and tested.

          This is working as expected.

          Tested for 2.4, 2.5, and master

          Test passed.

          Show
          Rossiani Wijaya added a comment - MDL-41196 has been fixed and tested. This is working as expected. Tested for 2.4, 2.5, and master Test passed.
          Hide
          Damyon Wiese added a comment -

          Thanks for your efforts. This issue is one of the outstanding issues that passed all our testing and was accepted into Moodle this week.

          Hurray!

          Show
          Damyon Wiese added a comment - Thanks for your efforts. This issue is one of the outstanding issues that passed all our testing and was accepted into Moodle this week. Hurray!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: