Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ref: Tweaks to send email task #833

Merged
merged 2 commits into from
Oct 29, 2024
Merged

ref: Tweaks to send email task #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ce6f33
Send email task tweaks
spalmurray-codecov Oct 29, 2024
a161b7b
Run lint
spalmurray-codecov Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 8 additions & 19 deletions tasks/send_email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import logging

from shared.celery_config import send_email_task_name
from shared.config import get_config

import services.smtp
from app import celery_app
from database.models import Owner
from helpers.email import Email
from helpers.metrics import metrics
from services.template import TemplateService
Expand All @@ -15,37 +15,26 @@

class SendEmailTask(BaseCodecovTask, name=send_email_task_name):
def run_impl(
self, db_session, ownerid, template_name, from_addr, subject, **kwargs
self, db_session, to_addr, subject, template_name, from_addr=None, **kwargs
):
with metrics.timer("worker.tasks.send_email"):
if from_addr is None:
from_addr = get_config(
"services", "smtp", "from_address", default="[email protected]"
)

log_extra_dict = {
"to_addr": to_addr,
"from_addr": from_addr,
"template_name": template_name,
"template_kwargs": kwargs,
"ownerid": ownerid,
}

log.info(
"Received send email task",
extra=log_extra_dict,
)

owner = db_session.query(Owner).filter_by(ownerid=ownerid).first()
if not owner:
log.warning(
"Unable to find owner",
extra=log_extra_dict,
)
return {"email_successful": False, "err_msg": "Unable to find owner"}

to_addr = owner.email
if not owner.email:
log.warning("Owner does not have email", extra=log_extra_dict)
return {
"email_successful": False,
"err_msg": "Owner does not have email",
}

smtp_service = services.smtp.SMTPService()

if not smtp_service.active():
Expand Down
4 changes: 2 additions & 2 deletions tasks/tests/integration/test_send_email_task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ def test_send_email_integration(

result = task.run_impl(
dbsession,
owner.ownerid,
owner.email,
"TestSubject",
"test",
"[email protected]",
"TestSubject",
username=owner.username,
)

Expand Down
91 changes: 35 additions & 56 deletions tasks/tests/unit/test_send_email_task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,51 +58,49 @@ def test_send_email(
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
from_addr="[email protected]",
username="test_username",
)

assert result == {"email_successful": True, "err_msg": None}

def test_send_email_non_existent_template(
def test_send_email_uses_default_from_addr(
self, mocker, mock_configuration, dbsession, mock_smtp
):
mock_smtp.configure_mock(**{"send.return_value": None})
owner = OwnerFactory.create(email=to_addr)
dbsession.add(owner)
dbsession.flush()
with pytest.raises(TemplateNotFound):
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="non_existent",
subject="Test",
username="test_username",
)

def test_send_email_no_owner(
self, mocker, mock_configuration, dbsession, mock_smtp
):
owner = OwnerFactory.create(email=None)
dbsession.add(owner)
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
username="test_username",
)

assert result == {
"email_successful": False,
"err_msg": "Owner does not have email",
"email_successful": True,
"err_msg": None,
}

def test_send_email_non_existent_template(
self, mocker, mock_configuration, dbsession, mock_smtp
):
owner = OwnerFactory.create(email=to_addr)
dbsession.add(owner)
dbsession.flush()
with pytest.raises(TemplateNotFound):
_ = SendEmailTask().run_impl(
db_session=dbsession,
to_addr=owner.email,
subject="Test",
template_name="non_existent",
username="test_username",
)

def test_send_email_missing_kwargs(
self, mocker, mock_configuration, dbsession, mock_smtp
):
Expand All @@ -112,25 +110,11 @@ def test_send_email_missing_kwargs(
with pytest.raises(UndefinedError):
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
to_addr=owner.email,
subject="Test",
template_name="test",
)

def test_send_email_invalid_owner_no_list_type(
self, mocker, mock_configuration, dbsession, mock_smtp
):
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=99999999,
from_addr="[email protected]",
template_name="test",
subject="Test",
username="test_username",
)
assert result == {"email_successful": False, "err_msg": "Unable to find owner"}

def test_send_email_recipients_refused(
self, mocker, mock_configuration, dbsession, mock_smtp
):
Expand All @@ -143,10 +127,9 @@ def test_send_email_recipients_refused(
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
username="test_username",
)

Expand All @@ -164,10 +147,9 @@ def test_send_email_sender_refused(
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
username="test_username",
)
assert result["email_successful"] == False
Expand All @@ -188,10 +170,9 @@ def test_send_email_data_error(
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
username="test_username",
)
assert result["email_successful"] == False
Expand All @@ -208,10 +189,9 @@ def test_send_email_sends_errs(
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
username="test_username",
)
assert result["email_successful"] == False
Expand All @@ -226,10 +206,9 @@ def test_send_email_no_smtp_config(
dbsession.flush()
result = SendEmailTask().run_impl(
db_session=dbsession,
ownerid=owner.ownerid,
from_addr="[email protected]",
template_name="test",
to_addr=owner.email,
subject="Test",
template_name="test",
username="test_username",
)
assert result == {
Expand Down
Loading