-
Notifications
You must be signed in to change notification settings - Fork 85
WIP enable mw api emailuser backend #1498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
f685cf3 to
0201ab9
Compare
065941a to
c086792
Compare
c086792 to
e6fd993
Compare
e6fd993 to
1608291
Compare
1608291 to
1bc7bb9
Compare
- mediawiki mail backend for django
- delay, retry, and maxlag handling adapted from pywikiapi
- test_email
- management command
- email task and templates
- task-specific mail backend switching
- additional config
- optional .env # @todo: document
Bug: T409420
Bug: T412427
1bc7bb9 to
a61018e
Compare
Bug: T409420
Bug: T409420
937980a to
66d0f18
Compare
- new email message model
- proxy djmail messages and add model manager with helper methods
- cleanup_email cmd deletes draft emails with subject and optionally
clears user flag
- requires email subject
- optional argument for userprofile flag field to clear sent status
Bug: T409420
Bug: T412427
b0b158c to
289f1a1
Compare
- limit emails sent per run to 1000 by default - naively creates one session + login per email email mediawiki backend: - skip accounts that share an email address Bug: T409420 Bug: T412427
Bug: T409420 Bug: T412427
289f1a1 to
6ffba2d
Compare
email tasks: create and save djmail message objects mediawiki backend: raise exceptions when mail can't be sent so status is tracked in djmail objects survey_active_users: clean up email sending Bug: T409420 Bug: T412427
| from TWLight.emails.models import Message | ||
|
|
||
|
|
||
| class Command(BaseCommand): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that this is simple db lookups, so the command would have to be run multiple times with each translated subject line to catch all the variations, eg.
python manage.py cleanup_email --subject "The Wikipedia Library needs your help!" --userprofile_flag_field "survey_email_sent"
would just get the english emails
| from django.contrib.auth.models import User | ||
| from django.db import models | ||
|
|
||
| from djmail.models import Message as DjMail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is creating a dependency issue. I just ran out of time before I could look at it. There are usually methods for using a string representation of the class to resolve this kind of issue, sometimes in a model, sometimes in app config, or maybe this needs an app label and a local migration added to define the dependency.
| Any time the related managment command is run, this sends a survey | ||
| invitation to qualifying editors. | ||
| """ | ||
| connection = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a connection isn't passed in, this will create 1 connection for each email. Bad for bulk mail, but fine for transactional mail.
| model_instance.save() | ||
|
|
||
| # send email and update state in djmail | ||
| _safe_send_message(model_instance, connection) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously, this is an internal method. Forking djmail would be cleaner, but this works for now.
| # have an non-wikimedia.org email address | ||
| Q(email__isnull=False) & ~Q(email__endswith="@wikimedia.org"), | ||
| Q(email__isnull=False) | ||
| & ~Q(email="") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized we were catching folks with blank email address fields
| connection.close() | ||
|
|
||
| # Record that we sent the email so that we only send one. | ||
| # user.userprofile.survey_email_sent = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented this out for repeated local testing, but it's kind of bad anyway. If the djmail message created for this has a status of "draft", then the user wasn't actually sent the email. We're having to go check that manually for the cleanup command anyway.
I was thinking of just dropping the flag altogether and just switching to doing a lookup based on email address & subject, or adding a m2m user:message relationship, etc. I might recommend starting with a lookup and then only switching to a relationship if we really fork the middleware. The proxy model has some helper methods on it that you could use as inspiration for making this better.
| services: | ||
| twlight: | ||
| extra_hosts: | ||
| - "host.docker.internal:host-gateway" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to be able to talk to a separate mediawiki container
| - ./conf/local.twlight.env | ||
| - path: ./conf/local.twlight.env | ||
| required: true | ||
| - path: .env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where you can stick your bot credential env vars
| DJANGO_DB_HOST=db | ||
| DJANGO_EMAIL_BACKEND=django_dkim.backends.console.EmailBackend | ||
| #DJANGO_EMAIL_BACKEND=TWLight.emails.backends.mediawiki.EmailBackend | ||
| DJANGO_EMAIL_BACKEND=django.core.mail.backends.console.EmailBackend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can swap this out to push everything through the mediawiki emailuser api.
| ) | ||
| emailable_data = _json_maxlag(emailable_response) | ||
| emailable = "emailable" in emailable_data["query"]["users"][0] | ||
| if not emailable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in mediawiki you'll need to add and confirm email addresses for users with wp_usernames that match what's in your TWLight. It's kind of a pain.
provides features required for mass emailing via WMF Cloud VPS.
Bug: T412427
Description
Describe your changes in detail following the commit message guidelines
Rationale
Phabricator Ticket
How Has This Been Tested?
Screenshots of your changes (if appropriate):
Types of changes
What types of changes does your code introduce? Add an
xin all the boxes that apply: