silverstripe-contentreview icon indicating copy to clipboard operation
silverstripe-contentreview copied to clipboard

Published 20 hours ago • verified publisher icon silverstripe

Notification job marked as 'Broken' with invalid email

Open brynwhyman opened this issue 4 years ago • 3 comments

Overview

The Content Review Notification job will be marked as 'Broken' in the Jobs admin panel if one of the expected recipients has an invalid email address.

Error output message

[2020-07-21 14:24:20][INFO] Job caused exception Address in mailbox given [test.email.email] does not comply with RFC 2822, 3.6.2. in /sites/cmstest/releases/20200707013338/vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php at line 345 [2020-07-21 14:24:20][ERROR] Address in mailbox given [test.email.email] does not comply with RFC 2822, 3.6.2. [2020-07-21 14:24:20][INFO] [2020-07-21 14:24:20][INFO] Job paused at 2020-07-21 14:24:20

To recreate

  1. Create a user with an invalid email address
  2. Create a user with a valid email address
  3. Assign both users as a content reviewer in the site settings of a page
  4. Set the page's 'next review date' to yesterday
  5. run the Content Review Notification job
  6. confirm email has been received to the valid email address
  7. Visit Jobs admin panel to confirm successful run of the job Expected: I should see the job marked as 'Complete' Actual result: The job is marked as 'Broken' while emails have been successfully sent to valid emails

Acceptance criteria

  • [ ] All users with valid emails receives notifications even if some users don't have valid email addresses
  • [ ] A warning is log to the job, but the job is not broken.
  • [ ] Create a separate card for identifying what to do when the CMS tries to send an email to someone without a valid email address.

brynwhyman avatar Jul 21 '20 02:07 brynwhyman

I originally raised this as it appeared no emails would be sent if one recipient contained an invalid email address, however that's not true.

brynwhyman avatar Jul 21 '20 02:07 brynwhyman

This isn't really incorrect behaviour, but it's not helpful to users as it doesn't bubble up anywhere useful. The simplest improvement here would probably be to add validation to the field for entering the email address.

Cheddam avatar Jul 22 '20 04:07 Cheddam

Bumping this to high because the "ERROR" means that it's causing the entire job to stop, so other any other pages with valid addresses are not having emails sent

emteknetnz avatar Dec 02 '21 23:12 emteknetnz

Related to https://github.com/silverstripe/silverstripe-contentreview/issues/188

sabina-talipova avatar Mar 30 '23 23:03 sabina-talipova

@sabina-talipova have you made the follow-up card mentioned in the acceptance criteria?

GuySartorelli avatar Apr 12 '23 04:04 GuySartorelli

@GuySartorelli , I've created this issue, but it's probably more general problem. Do you think we need another ticket with more narrow description?

sabina-talipova avatar Apr 12 '23 22:04 sabina-talipova

identifying what to do when the CMS tries to send an email to someone without a valid email address.

This sounds to me like a different problem than the issue you have currently created which sounds more like

decide what validation is required for Member, and implement that validation

(in other words yes please I think a new issue about how to handle the email scenario specifically is warranted)

GuySartorelli avatar Apr 12 '23 22:04 GuySartorelli

Merged

GuySartorelli avatar Apr 14 '23 00:04 GuySartorelli