Moodle
  1. Moodle
  2. MDL-31202

Forum emails not being sent, fixed by removing broken email address

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Forum, Messages
    • Labels:
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      For developers:
      1/ create a sample script that tries to send email to invalid email address via email_to_user()
      2/ test it both via CLI and web interface
      3/ expected "false" result and error message in CLI

      Show
      For developers: 1/ create a sample script that tries to send email to invalid email address via email_to_user() 2/ test it both via CLI and web interface 3/ expected "false" result and error message in CLI
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w04_MDL-31202_m23_emailvalidation
    • Rank:
      37651

      Description

      Forum emails were not being sent, or being sent very intermittently. We checked and re-checked cron, even running it manually, but emails from all kinds of forums with all types of subscriptions were not being sent regularly and correctly.

      While running some tests in a test course with a regular, forced subscription forum, we discovered that some of the emails had shown up in the default bounce-back email address inbox because they were being sent to a fake student account that had an inoperable email address listed (having been created by the admin, the account never went through an email address verification process). After fixing this email address, forum posts began working in every class that fake student was enrolled in.

      Obviously, one bad email address should not cause a choke point for an entire forum/class.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put that on the backlog.

          In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
          Hide
          Michael de Raadt added a comment -

          A recent change made with MDL-13572 has introduced validation before emails are sent. I suspect this should resolve the problem you have raised.

          Show
          Michael de Raadt added a comment - A recent change made with MDL-13572 has introduced validation before emails are sent. I suspect this should resolve the problem you have raised.
          Hide
          Matt Fedorko added a comment -

          Thanks, Michael. I am unable to find MDL-13572. Is there somewhere we can look at the change you're referring to?

          Thanks again.

          Show
          Matt Fedorko added a comment - Thanks, Michael. I am unable to find MDL-13572. Is there somewhere we can look at the change you're referring to? Thanks again.
          Hide
          Petr Škoda added a comment -

          Michael: not, the fix in MDL-13572 change only validation of messaging email setting, it does not try to validate emails from user->email

          I am going to tweak the sending of emails to fail if the email address does not have valid format and print a warning in main debug mode.

          Show
          Petr Škoda added a comment - Michael: not, the fix in MDL-13572 change only validation of messaging email setting, it does not try to validate emails from user->email I am going to tweak the sending of emails to fail if the email address does not have valid format and print a warning in main debug mode.
          Hide
          Petr Škoda added a comment -

          To integrators:

          • please cherry pick to supported 2.x branches
          • the use of mtrace() in email_to_users() seems incorrect, we should clean it up in a separate issue most probably; I have used a hack involving CLI_SCRIPT to prevent error messages from breaking normal HTML output and disclosing sensitive information from the error message

          Thanks for the report!!

          Show
          Petr Škoda added a comment - To integrators: please cherry pick to supported 2.x branches the use of mtrace() in email_to_users() seems incorrect, we should clean it up in a separate issue most probably; I have used a hack involving CLI_SCRIPT to prevent error messages from breaking normal HTML output and disclosing sensitive information from the error message Thanks for the report!!
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr - this has been integrated now. I'll also create an issue now to clean up the use of mtrace within email_to_user.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Petr - this has been integrated now. I'll also create an issue now to clean up the use of mtrace within email_to_user. Cheers Sam
          Hide
          Ankit Agarwal added a comment -

          This is working as expected.
          I got the error msg on cli as expected.
          Great Fix.
          Passing!
          Thanks

          Show
          Ankit Agarwal added a comment - This is working as expected. I got the error msg on cli as expected. Great Fix. Passing! Thanks
          Hide
          Matt Fedorko added a comment -

          Thanks, everyone! It's nice to see this process work so well.

          Show
          Matt Fedorko added a comment - Thanks, everyone! It's nice to see this process work so well.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many.

          Nah, joking, many thanks! Closing this a fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many. Nah, joking, many thanks! Closing this a fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: