Problem/Motivation

The email field is not mandatory for users so a user account may exist with no email address associated. If email notifications are enabled, there is no check that a user has an email address before attempting to send an email. This causes an error to be logged.

Proposed resolution

Prior to sending an email, a check should be added to ensure that an email address is present.

CommentFileSizeAuthor
#3 3026004-3-check-notification-user-has-email.patch1.7 KBalanhdev
#2 3026004-2-notification-user-has-email-test.patch1.04 KBalanhdev

Comments

Alan-H created an issue. See original summary.

alanhdev’s picture

alanhdev commented
Issue summary: View changes
StatusFileSize
new 3026004-2-notification-user-has-email-test.patch1.04 KB

The patch supplied adds a test to check that no email message will be attempted for a user who has no email address set.

alanhdev’s picture

alanhdev commented
StatusFileSize
new 3026004-3-check-notification-user-has-email.patch1.7 KB

Attached patch adds a check to the shouldSend method to return FALSE if there is no email address for the recipient.

alanhdev’s picture

alanhdev commented
Issue summary: View changes
Status: Active » Needs review
opdavies’s picture

opdavies
He/him
Primary language English
Location Wales
commented
Status: Needs review » Reviewed & tested by the community

The patches work as expected for me. One adds the just the failing test to demonstrate the failure, and the second also includes the fix which passes the test, so I'm happy to RTBC it.

phjou’s picture

phjou
he/him
Location Vancouver 🇨🇦 🇪🇺
commented

Is it also needed for 8.x-2.x-dev?

  • anmolgoyal74 committed 356f243 on 8.x-1.x authored by AlanHDev 
    Issue #3026004 by AlanHDev: Email notification attempted where user has...
anmolgoyal74’s picture

anmolgoyal74 commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.