Notifications: Don't send mail if recipient has no email address. #680
Conversation
| @@ -103,6 +103,15 @@ def index(self): | |||
| ) | |||
| ) | |||
|
|
|||
| def send_email(self): | |||
| if not self.account.email: | |||
There was a problem hiding this comment.
This won't work for DTMB because the email address is not in the account there. On the other hand in plain euphorie this is unnecessary because the user ID is an email address, so every account has an email.
You probably need to use webhelpers.get_user_email().
There was a problem hiding this comment.
IMO you should not even reach this point.
Results should be filter out earlier.
There was a problem hiding this comment.
@reinhardt tnx for the pointer. fixed.
@ale-rt that would involve two things:
a) Implementing this logic and the logging message in all of the different notification classes.
b) or restructuring the code so that the notification base class does that.
I think we can't or shouldn't filter out on the SQL query level - the ws_user_cache reponse JSON field wouldn't allow that easily (not considering Postgres' JSON capabilities, which I don't have experience with) and we would loose the logging feature.
I'm 👎 for a) and :+0: for b) - we're over budget and we would need a full test circle again.
There was a problem hiding this comment.
Yes, Alex is going to be furious if we do more than this quick fix.
In other projects the user management can be implemented differently, so that account.email doesn't return the actual email address and some more magic is necessary. This fix uses webhelpers.get_user_email which can be customized in depending projects.
|
Merging this. |
Ref: scrum-1572