Add ability to send emails and notifications from the Messaging Center #3259
Conversation
mathjazz
force-pushed
the
3243-messaging-center-send
branch
from
89b0be7
to
efd150b
Compare
Note that until we implement recipient filters, the recipient of the messages is the sender. Also included: - Factor out check-box CSS - Make .notification selectors in style.css and main.js more specific
mathjazz
force-pushed
the
3243-messaging-center-send
branch
from
efd150b
to
50e6db5
Compare
| import html2text | ||
|
|
||
|
|
||
| def html_to_plain_text_with_links(html): |
There was a problem hiding this comment.
I would just inline this and consider DRY much later, but that's mostly just me.
There was a problem hiding this comment.
I'm planning to use this function in the email script, at least for the time being.
pontoon/messaging/views.py
Outdated
| return JsonResponse(dict(form.errors.items())) | ||
|
|
||
| # TODO: implement recipient filters | ||
| from django.contrib.auth.models import User |
There was a problem hiding this comment.
Why is this not a top-level import?
| body = form.cleaned_data.get("body") | ||
|
|
||
| if is_notification: | ||
| identifier = uuid.uuid4().hex |
There was a problem hiding this comment.
Is this actually used for anything?
There was a problem hiding this comment.
Yes, that allows us to find all instances of the same notification (one instance is created for each user as per django-notifications-hq).
To implement #3246, we'll need a new data model and than we can drop this.
pontoon/messaging/views.py
Outdated
| msg = EmailMultiAlternatives( | ||
| subject=subject, | ||
| body=text, | ||
| from_email="Mozilla L10n Team <team@pontoon.mozilla.com>", |
There was a problem hiding this comment.
This probably should not be hard-coded?
|
@eemeli Please let me know if the CSS changes look good. |
|
It looks like we're hardcoding the unsubscribe footer for all emails here. In the spec the plan is to have the ability to flag a message as transactional or not, then only attach the footer when an email is not transactional. I also realize that this was not covered by the spec, but do we also plan to adjust how messages sent as notifications appear in a users Notifications window? Testing this out it seems like a project is required and is defaulting to |
I was planning to add support for transactional email in a separate PR, which would also include special handling of the footer. I'll see if I can find some time today to amend this PR with this change.
I assume you tested on stage (which uses an old version of this patch). I'll deploy the current state of the PR. Should be up in 5 minutes. |
|
I'm okay with adding it to a later PR, but I didn't see an issue tracking this, so wanted to make sure this would be covered.
Thanks, yes that was what I was doing. Looks good now. 👍 |
|
Filed #3268. |
Fix #3243.
Fix #2072.
This is the first part of the implementation of the Messaging Center. It allows for sending emails and in-app notifications from
/messaging/. Locally emails are sent to stdout.Note that until we implement recipient filters (#3244), the recipient of the messages is the sender.
While the development is in progress, the page is not linked from the UI.
Also included: