Re-add SMTP email provider #1047
Conversation
sea-kelp
force-pushed
the
add-email-providers
branch
from
c4f9ecc
to
d302699
Compare
sea-kelp
force-pushed
the
add-email-providers
branch
from
d302699
to
4a27d7e
Compare
|
@michplunkett - I just tested these changes using SMTP. Would you mind testing with a Google Workspace account?
|
| setattr(self, KEY_MAIL_SERVER, os.environ.get(KEY_MAIL_SERVER)) | ||
| setattr(self, KEY_MAIL_PORT, os.environ.get(KEY_MAIL_PORT)) | ||
| setattr(self, KEY_MAIL_USE_TLS, str_is_true(os.environ.get(KEY_MAIL_USE_TLS))) | ||
| setattr(self, KEY_MAIL_USERNAME, os.environ.get(KEY_MAIL_USERNAME)) | ||
| setattr(self, KEY_MAIL_PASSWORD, os.environ.get(KEY_MAIL_PASSWORD)) |
There was a problem hiding this comment.
What's the reasoning for using setattr instead of self.____?
There was a problem hiding this comment.
Yea, I was going back and forth about using setattr here but this lets us use the value of the constant as the config variable name, rather than hardcoding that name (for example self.MAIL_SERVER = ...) because otherwise we would need to update config.py along with constants.py if we wanted to change the config variable name.
I don't have a strong preference either way so let me know if you want me to change it
There was a problem hiding this comment.
The more I think about it, the more I like it. Keep it, imo. I may change the syntax for the rest of them in another PR.
| @@ -13,17 +15,23 @@ | |||
| class Email: | |||
| """Base class for all emails.""" | |||
|
|
|||
| def __init__(self, body: str, subject: str, receiver: str): | |||
| def __init__(self, body: str, html: str, subject: str, receiver: str): | |||
| def str_is_true(str_): | ||
| if str_ is None: | ||
| return False |
| ENCODING_UTF_8 = "utf-8" | ||
| FILE_TYPE_HTML = "html" | ||
| FILE_TYPE_PLAIN = "plain" |
|
Some small nits here and there, but it otherwise looks great! I'll do some testing on it Monday afternoon and stamp this bb with an 'Approved' afterward! |
| @classmethod | ||
| def auto_detect(cls): | ||
| """Auto-detect the configured email provider to use for email sending.""" | ||
| if current_app.testing: |
There was a problem hiding this comment.
DEBUG is on in dev mode, so keeping DEBUG would prevent local Gmail testing.
This reverts commit d2092fb.
CONTRIB.md
Outdated
| ## Gmail Requirements | ||
| **NOTE:** If you are running on dev and do not currently have a `service_account_key.json` file, create one and leave it empty. The email client will then default to an empty object and simulate emails in the logs. | ||
| ## Setting Up Email | ||
| **NOTE:** If you are running on dev and do not currently have a `service_account_key.json` file, create one and leave it empty. This file is required by docker compose. |
There was a problem hiding this comment.
This SHOULD be done automatically. You can remove this line because of this change: https://github.com/lucyparsons/OpenOversight/pull/1047/files#r1314475688
| @@ -33,7 +33,7 @@ assets: | |||
| docker-compose run --rm web yarn build | |||
|
|
|||
| .PHONY: dev | |||
| dev: build start create_db populate | |||
| dev: create_empty_secret build start create_db populate | |||
|
Tested with GSuite and made the appropriate changes. Provided you're happy with it, @sea-kelp, it's good to merge! |
|
@michplunkett Thanks for the changes! Please do merge this PR when you get a chance |
Description of Changes
The current
EmailClientclass only supports sending emails from a GSuite email account.This change allows OpenOversight to auto-detect which email implementation to use (Google, SMTP, or Simulated) based on which of the following is configured:
service_account_key.jsonfile exists and is non-emptyMAIL_SERVERandMAIL_PORTare setNotes for Deployment
Screenshots (if appropriate)
Tests and linting
developbranch.pytestpasses on my local development environment.pre-commitpasses on my local development environment.