Moodle
  1. Moodle
  2. MDL-28166

Quiz tries to send email messages while still in the process_attempt transaction -> exception

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.1.1
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      1. Create a quiz in a course with students and teachers.
      2. Override the permission so that some combination of confirmation emails for students and notification emails for teachers is enabled.
      3. Complete the quiz as a student. Ensure there are no errors.
      4. Wait for cron to run.
      5. Verify that the expected messages have been sent.

      Show
      1. Create a quiz in a course with students and teachers. 2. Override the permission so that some combination of confirmation emails for students and notification emails for teachers is enabled. 3. Complete the quiz as a student. Ensure there are no errors. 4. Wait for cron to run. 5. Verify that the expected messages have been sent.
    • Workaround:
      Hide

      If you need to keep your quizzes open, turn off notification emails temporarily.

      Show
      If you need to keep your quizzes open, turn off notification emails temporarily.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      17868

      Description

      The messaging system does not allow sending message in a transaction. The code needs to be reordered.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          I am afraid this is quite a big change for a stable branch, but I decided it was worth fixing this properly, rather than doing a quick hack.

          I discussed the quiz version number in developer chat. Since the version number was 2011051250 - which does not leave much space for stable branch increments - and since I hope exactly the same change will go onto Master and 2.1 stable, Petr and I agreed it was better to increase the quiz version number to the release date.

          Show
          Tim Hunt added a comment - I am afraid this is quite a big change for a stable branch, but I decided it was worth fixing this properly, rather than doing a quick hack. I discussed the quiz version number in developer chat. Since the version number was 2011051250 - which does not leave much space for stable branch increments - and since I hope exactly the same change will go onto Master and 2.1 stable, Petr and I agreed it was better to increase the quiz version number to the release date.
          Hide
          Dan Marsden added a comment -

          my +1 to remove old event completely instead of deprecating it.

          Show
          Dan Marsden added a comment - my +1 to remove old event completely instead of deprecating it.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ho Tim,

          • I saw some references to "/[e]?mail/i" here and there. All the stuff is using messaging and observing its configuration, correct? If so, I think it would help to take them out. Not critical at all.
          • quiz_attempt_submitted_hander or quiz_attempt_submitted_handler (not sure but I always assumed it's "handler".
          • +1 to remove the quiz_attempt_processed, not useful and dupes functionality. I think it's ok if we detail that change in the 2.1.1 release notes. (also that will clean one "becuase" occurrence, lol!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Ho Tim, I saw some references to "/ [e] ?mail/i" here and there. All the stuff is using messaging and observing its configuration, correct? If so, I think it would help to take them out. Not critical at all. quiz_attempt_submitted_hander or quiz_attempt_submitted_handler (not sure but I always assumed it's "handler". +1 to remove the quiz_attempt_processed, not useful and dupes functionality. I think it's ok if we detail that change in the 2.1.1 release notes. (also that will clean one "becuase" occurrence, lol! Ciao
          Hide
          Tim Hunt added a comment -

          OK. I will do a cleaned up commit today. Thanks for reviewing this carefully.

          Show
          Tim Hunt added a comment - OK. I will do a cleaned up commit today. Thanks for reviewing this carefully.
          Hide
          Tim Hunt added a comment -
          Show
          Tim Hunt added a comment - Amended branch pushed: https://github.com/timhunt/moodle/compare/master...MDL-28166
          Show
          Tim Hunt added a comment - Note about the API change added to http://docs.moodle.org/dev/Moodle_2.1_release_notes#For_developers:_API_changes and http://docs.moodle.org/dev/Events_API#Other
          Hide
          Eloy Lafuente (stronk7) added a comment -

          thanks, under inspection...!

          Show
          Eloy Lafuente (stronk7) added a comment - thanks, under inspection...!
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Integrated, thanks! Docs look perfect!
          (master and cherry-pick backported to 21_STABLE - 2 commits)

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Integrated, thanks! Docs look perfect! (master and cherry-pick backported to 21_STABLE - 2 commits)
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim, tested - all working as described

          Show
          Sam Hemelryk added a comment - Thanks Tim, tested - all working as described
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Moodle's git/cvs repositories have been updated with this piece of art! Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Moodle's git/cvs repositories have been updated with this piece of art! Thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: