diff --git a/tasks/send_email.py b/tasks/send_email.py index 9990b35cf..6c63f4b7e 100644 --- a/tasks/send_email.py +++ b/tasks/send_email.py @@ -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 @@ -15,14 +15,19 @@ 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="noreply@codecov.io" + ) + log_extra_dict = { + "to_addr": to_addr, "from_addr": from_addr, "template_name": template_name, "template_kwargs": kwargs, - "ownerid": ownerid, } log.info( @@ -30,22 +35,6 @@ def run_impl( 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(): diff --git a/tasks/tests/integration/test_send_email_task.py b/tasks/tests/integration/test_send_email_task.py index 591794f35..7eb7daec4 100644 --- a/tasks/tests/integration/test_send_email_task.py +++ b/tasks/tests/integration/test_send_email_task.py @@ -39,10 +39,10 @@ def test_send_email_integration( result = task.run_impl( dbsession, - owner.ownerid, + owner.email, + "TestSubject", "test", "test@codecov.io", - "TestSubject", username=owner.username, ) diff --git a/tasks/tests/unit/test_send_email_task.py b/tasks/tests/unit/test_send_email_task.py index 1a8bb0182..6044b2c8e 100644 --- a/tasks/tests/unit/test_send_email_task.py +++ b/tasks/tests/unit/test_send_email_task.py @@ -58,51 +58,49 @@ def test_send_email( dbsession.flush() result = SendEmailTask().run_impl( db_session=dbsession, - ownerid=owner.ownerid, - from_addr="test_from@codecov.io", - template_name="test", + to_addr=owner.email, subject="Test", + template_name="test", + from_addr="test_from@codecov.io", 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="test_from@codecov.io", - 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="test_from@codecov.io", - 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 ): @@ -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="test_from@codecov.io", + 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="test_from@codecov.io", - 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 ): @@ -143,10 +127,9 @@ def test_send_email_recipients_refused( dbsession.flush() result = SendEmailTask().run_impl( db_session=dbsession, - ownerid=owner.ownerid, - from_addr="test_from@codecov.io", - template_name="test", + to_addr=owner.email, subject="Test", + template_name="test", username="test_username", ) @@ -164,10 +147,9 @@ def test_send_email_sender_refused( dbsession.flush() result = SendEmailTask().run_impl( db_session=dbsession, - ownerid=owner.ownerid, - from_addr="test_from@codecov.io", - template_name="test", + to_addr=owner.email, subject="Test", + template_name="test", username="test_username", ) assert result["email_successful"] == False @@ -188,10 +170,9 @@ def test_send_email_data_error( dbsession.flush() result = SendEmailTask().run_impl( db_session=dbsession, - ownerid=owner.ownerid, - from_addr="test_from@codecov.io", - template_name="test", + to_addr=owner.email, subject="Test", + template_name="test", username="test_username", ) assert result["email_successful"] == False @@ -208,10 +189,9 @@ def test_send_email_sends_errs( dbsession.flush() result = SendEmailTask().run_impl( db_session=dbsession, - ownerid=owner.ownerid, - from_addr="test_from@codecov.io", - template_name="test", + to_addr=owner.email, subject="Test", + template_name="test", username="test_username", ) assert result["email_successful"] == False @@ -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="test_from@codecov.io", - template_name="test", + to_addr=owner.email, subject="Test", + template_name="test", username="test_username", ) assert result == {