Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-31202

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore 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
            salvetore 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
            salvetore 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
            salvetore 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
            mfedorko 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
            mfedorko 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            samhemelryk 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
            samhemelryk 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_frenz 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_frenz Ankit Agarwal added a comment - This is working as expected. I got the error msg on cli as expected. Great Fix. Passing! Thanks
            Hide
            mfedorko Matt Fedorko added a comment -

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

            Show
            mfedorko Matt Fedorko added a comment - Thanks, everyone! It's nice to see this process work so well.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  12/Mar/12