Closed Bug 854751 Opened 12 years ago Closed 12 years ago

apps/announcements/tasks.py send_group_emails could be better

Categories

(support.mozilla.org :: Code Quality, task)

Product:
support.mozilla.org
Component:
Code Quality
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: willkg, Assigned: boerni)

Details

(Whiteboard: u=contributor c=announcements p=[mentor=r1cky])

"could be better" is kind of vague, but here goes. The function sends an announcement email to a group of users. If it stumbles (i.e. throws an error) on one of those users, it stops sending mail and that's it. So if the group has 20 people in it and it throws an error on person 4, persons 5 through 20 get nothing. This was written in 2011. It's entirely possible this accounts for some/many of the "I didn't get the email" issues we've had. This should get reworked so that the try/except is in the for loop and it covers generate the EmailMessage instances. Then after all the looping, it sends them all in one send_messages call.
Whiteboard: u=contributor c=announcements p= s=2013.backlog
Whiteboard: u=contributor c=announcements p= s=2013.backlog → u=contributor c=announcements p= s=2013.backlog [mentor=r1cky]
Assignee: nobody → boerni
send_group_email uses sumo.email_utils.send_messages (fail_silently=True) what should not lead to the error described? Does EmailMultiAlternatives (_make_mail) raise some exceptions we need to take care of?
Flags: needinfo?
It can raise the sorts of errors we keep seeing with poorly localized text. But we should be safer than that and try as hard as we can to send as many emails as possible.
Flags: needinfo?
Oops--I forgot we had safe_translation decorators. I'm not sure there's anything we need to do here anymore.
Do you log celery exceptions somewhere?
If settings.WIKI_DEFAULT_LANGUAGE also contains poorly localized text the problem still exists. Do you think this is sth. we should take care about?
settings.WIKI_DEFAULT_LANGUAGE is the source of all localizations so it will always be correct assuming our test coverage is good.
I'm going to close this on as WFM. based on Comment 3
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
Cleaning out the backlog.
Whiteboard: u=contributor c=announcements p= s=2013.backlog [mentor=r1cky] → u=contributor c=announcements p=[mentor=r1cky]
You need to log in before you can comment on or make changes to this bug.