diff --git a/amy/emails/actions/base_action.py b/amy/emails/actions/base_action.py index fde7d71b5..f57937dba 100644 --- a/amy/emails/actions/base_action.py +++ b/amy/emails/actions/base_action.py @@ -3,15 +3,23 @@ import logging from typing import Any +from django.contrib.contenttypes.models import ContentType from flags.state import flag_enabled -from emails.controller import EmailController, EmailControllerMissingRecipientsException -from emails.models import EmailTemplate +from emails.controller import ( + EmailController, + EmailControllerMissingRecipientsException, + EmailControllerMissingTemplateException, +) +from emails.models import EmailTemplate, ScheduledEmail from emails.signals import SignalNameEnum from emails.utils import ( + messages_action_cancelled, messages_action_scheduled, + messages_action_updated, messages_missing_recipients, messages_missing_template, + messages_missing_template_link, person_from_request, ) @@ -79,3 +87,95 @@ def __call__(self, sender: Any, **kwargs) -> None: messages_missing_template(request, self.signal) else: messages_action_scheduled(request, self.signal, scheduled_email) + + +class BaseActionUpdate(BaseAction): + def __call__(self, sender: Any, **kwargs) -> None: + if not feature_flag_enabled("EMAIL_MODULE", f"{self.signal}_update", **kwargs): + return + + request = kwargs.pop("request") + + context = self.get_context(**kwargs) + scheduled_at = self.get_scheduled_at(**kwargs) + generic_relation_obj = self.get_generic_relation_object(context, **kwargs) + signal_name = self.signal + + ct = ContentType.objects.get_for_model(generic_relation_obj) + try: + scheduled_email = ( + ScheduledEmail.objects.select_for_update() + .select_related("template") + .get( + generic_relation_content_type=ct, + generic_relation_pk=generic_relation_obj.pk, + template__signal=signal_name, + state="scheduled", + ) + ) + + except ScheduledEmail.DoesNotExist: + logger.warning( + f"Scheduled email for signal {signal_name} and {generic_relation_obj=} " + "does not exist." + ) + return + + except ScheduledEmail.MultipleObjectsReturned: + logger.warning( + f"Too many scheduled emails for signal {signal_name} and " + f"{generic_relation_obj=}. Can't update them." + ) + return + + try: + scheduled_email = EmailController.update_scheduled_email( + scheduled_email=scheduled_email, + context=context, + scheduled_at=scheduled_at, + to_header=self.get_recipients(context, **kwargs), + generic_relation_obj=generic_relation_obj, + author=person_from_request(request), + ) + except EmailControllerMissingRecipientsException: + messages_missing_recipients(request, signal_name) + except EmailControllerMissingTemplateException: + # Note: this is not realistically possible because the scheduled email + # is looked up using a specific template signal. + messages_missing_template_link(request, scheduled_email) + else: + messages_action_updated(request, signal_name, scheduled_email) + + +class BaseActionCancel(BaseAction): + # Method is not needed in this action. + def get_recipients(self, context: dict[str, Any], **kwargs) -> list[str]: + raise NotImplementedError() + + # Method is not needed in this action. + def get_scheduled_at(self, **kwargs) -> datetime: + raise NotImplementedError() + + def __call__(self, sender: Any, **kwargs) -> None: + if not feature_flag_enabled("EMAIL_MODULE", f"{self.signal}_remove", **kwargs): + return + + request = kwargs["request"] + context = self.get_context(**kwargs) + generic_relation_obj = self.get_generic_relation_object(context, **kwargs) + signal_name = self.signal + + ct = ContentType.objects.get_for_model(generic_relation_obj) + scheduled_emails = ScheduledEmail.objects.filter( + generic_relation_content_type=ct, + generic_relation_pk=generic_relation_obj.pk, + template__signal=signal_name, + state="scheduled", + ).select_for_update() + + for scheduled_email in scheduled_emails: + scheduled_email = EmailController.cancel_email( + scheduled_email=scheduled_email, + author=person_from_request(request), + ) + messages_action_cancelled(request, signal_name, scheduled_email) diff --git a/amy/emails/actions/instructor_training_approaching.py b/amy/emails/actions/instructor_training_approaching.py index 3d2ea0b12..9a14461f9 100644 --- a/amy/emails/actions/instructor_training_approaching.py +++ b/amy/emails/actions/instructor_training_approaching.py @@ -1,19 +1,12 @@ from datetime import datetime import logging -from typing import Any from django.contrib.contenttypes.models import ContentType -from django.dispatch import receiver from django.http import HttpRequest from django.utils import timezone from typing_extensions import Unpack -from emails.actions.base_action import BaseAction -from emails.controller import ( - EmailController, - EmailControllerMissingRecipientsException, - EmailControllerMissingTemplateException, -) +from emails.actions.base_action import BaseAction, BaseActionCancel, BaseActionUpdate from emails.models import ScheduledEmail from emails.signals import ( INSTRUCTOR_TRAINING_APPROACHING_SIGNAL_NAME, @@ -27,16 +20,8 @@ InstructorTrainingApproachingKwargs, StrategyEnum, ) -from emails.utils import ( - messages_action_cancelled, - messages_action_updated, - messages_missing_recipients, - messages_missing_template_link, - one_month_before, - person_from_request, -) +from emails.utils import one_month_before from workshops.models import Event, Task -from workshops.utils.feature_flags import feature_flag_enabled logger = logging.getLogger("amy") @@ -139,99 +124,78 @@ def get_recipients( return [instructor.email for instructor in instructors if instructor.email] -instructor_training_approaching_receiver = InstructorTrainingApproachingReceiver() -instructor_training_approaching_signal.connect(instructor_training_approaching_receiver) +class InstructorTrainingApproachingUpdateReceiver(BaseActionUpdate): + signal = instructor_training_approaching_update_signal.signal_name + def get_scheduled_at(self, **kwargs) -> datetime: + event_start_date = kwargs["event_start_date"] + return one_month_before(event_start_date) -@receiver(instructor_training_approaching_update_signal) -@feature_flag_enabled("EMAIL_MODULE") -def instructor_training_approaching_update_receiver( - sender: Any, **kwargs: Unpack[InstructorTrainingApproachingKwargs] -) -> None: - request = kwargs["request"] - event = kwargs["event"] - event_start_date = kwargs["event_start_date"] - instructors = [ - task.person - for task in Task.objects.filter(event=event, role__name="instructor") - ] - instructor_emails = [ - instructor.email for instructor in instructors if instructor.email - ] - - scheduled_at = one_month_before(event_start_date) - context: InstructorTrainingApproachingContext = { - "event": event, - "instructors": instructors, - } - signal_name = INSTRUCTOR_TRAINING_APPROACHING_SIGNAL_NAME + def get_context( + self, **kwargs: Unpack[InstructorTrainingApproachingKwargs] + ) -> InstructorTrainingApproachingContext: + event = kwargs["event"] + instructors = [ + task.person + for task in Task.objects.filter(event=event, role__name="instructor") + ] + return { + "event": event, + "instructors": instructors, + } - ct = ContentType.objects.get_for_model(event) # type: ignore - try: - scheduled_email = ( - ScheduledEmail.objects.select_for_update() - .select_related("template") - .get( - generic_relation_content_type=ct, - generic_relation_pk=event.pk, - template__signal=signal_name, - state="scheduled", - ) - ) - - except ScheduledEmail.DoesNotExist: - logger.warning( - f"Scheduled email for signal {signal_name} and event {event} " - "does not exist." - ) - return + def get_generic_relation_object( + self, context: InstructorTrainingApproachingContext, **kwargs + ) -> Event: + return context["event"] - except ScheduledEmail.MultipleObjectsReturned: - logger.warning( - f"Too many scheduled emails for signal {signal_name} and event {event}." - " Can't update them." - ) - return + def get_recipients( + self, context: InstructorTrainingApproachingContext, **kwargs + ) -> list[str]: + instructors = context["instructors"] + return [instructor.email for instructor in instructors if instructor.email] - try: - scheduled_email = EmailController.update_scheduled_email( - scheduled_email=scheduled_email, - context=context, - scheduled_at=scheduled_at, - to_header=instructor_emails, - generic_relation_obj=event, - author=person_from_request(request), - ) - except EmailControllerMissingRecipientsException: - messages_missing_recipients(request, signal_name) - except EmailControllerMissingTemplateException: - # Note: this is not realistically possible because the scheduled email - # is looked up using a specific template signal. - messages_missing_template_link(request, scheduled_email) - else: - messages_action_updated(request, signal_name, scheduled_email) +class InstructorTrainingApproachingCancelReceiver(BaseActionCancel): + signal = instructor_training_approaching_remove_signal.signal_name -@receiver(instructor_training_approaching_remove_signal) -@feature_flag_enabled("EMAIL_MODULE") -def instructor_training_approaching_remove_receiver( - sender: Any, **kwargs: Unpack[InstructorTrainingApproachingKwargs] -) -> None: - request = kwargs["request"] - event = kwargs["event"] - signal_name = INSTRUCTOR_TRAINING_APPROACHING_SIGNAL_NAME + def get_context( + self, **kwargs: Unpack[InstructorTrainingApproachingKwargs] + ) -> InstructorTrainingApproachingContext: + event = kwargs["event"] + instructors = [ + task.person + for task in Task.objects.filter(event=event, role__name="instructor") + ] + return { + "event": event, + "instructors": instructors, + } - ct = ContentType.objects.get_for_model(event) # type: ignore - scheduled_emails = ScheduledEmail.objects.filter( - generic_relation_content_type=ct, - generic_relation_pk=event.pk, - template__signal=signal_name, - state="scheduled", - ).select_for_update() - - for scheduled_email in scheduled_emails: - scheduled_email = EmailController.cancel_email( - scheduled_email=scheduled_email, - author=person_from_request(request), - ) - messages_action_cancelled(request, signal_name, scheduled_email) + def get_generic_relation_object( + self, context: InstructorTrainingApproachingContext, **kwargs + ) -> Event: + return context["event"] + + +# ----------------------------------------------------------------------------- +# Receivers + +instructor_training_approaching_receiver = InstructorTrainingApproachingReceiver() +instructor_training_approaching_signal.connect(instructor_training_approaching_receiver) + + +instructor_training_approaching_update_receiver = ( + InstructorTrainingApproachingUpdateReceiver() +) +instructor_training_approaching_update_signal.connect( + instructor_training_approaching_update_receiver +) + + +instructor_training_approaching_remove_receiver = ( + InstructorTrainingApproachingCancelReceiver() +) +instructor_training_approaching_remove_signal.connect( + instructor_training_approaching_remove_receiver +) diff --git a/amy/emails/tests/actions/test_instructor_training_approaching_remove_receiver.py b/amy/emails/tests/actions/test_instructor_training_approaching_remove_receiver.py index 589dabb8f..65d5b6840 100644 --- a/amy/emails/tests/actions/test_instructor_training_approaching_remove_receiver.py +++ b/amy/emails/tests/actions/test_instructor_training_approaching_remove_receiver.py @@ -56,7 +56,7 @@ def setUpEmailTemplate(self) -> EmailTemplate: body="Hello, {{ name }}! Nice to meet **you**.", ) - @patch("workshops.utils.feature_flags.logger") + @patch("emails.actions.base_action.logger") def test_disabled_when_no_feature_flag(self, mock_logger) -> None: # Arrange request = RequestFactory().get("/") @@ -66,7 +66,7 @@ def test_disabled_when_no_feature_flag(self, mock_logger) -> None: # Assert mock_logger.debug.assert_called_once_with( "EMAIL_MODULE feature flag not set, skipping " - "instructor_training_approaching_remove_receiver" + "instructor_training_approaching_remove" ) def test_receiver_connected_to_signal(self) -> None: @@ -102,7 +102,7 @@ def test_action_triggered(self) -> None: # Act with patch( - "emails.actions.instructor_training_approaching.messages_action_cancelled" + "emails.actions.base_action.messages_action_cancelled" ) as mock_messages_action_cancelled: instructor_training_approaching_remove_signal.send( sender=self.event, @@ -120,7 +120,7 @@ def test_action_triggered(self) -> None: ) @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]}) - @patch("emails.actions.instructor_training_approaching.messages_action_cancelled") + @patch("emails.actions.base_action.messages_action_cancelled") def test_email_cancelled( self, mock_messages_action_cancelled: MagicMock, @@ -140,8 +140,7 @@ def test_email_cancelled( # Act with patch( - "emails.actions.instructor_training_approaching" - ".EmailController.cancel_email" + "emails.actions.base_action.EmailController.cancel_email" ) as mock_cancel_email: instructor_training_approaching_remove_signal.send( sender=self.event, @@ -157,7 +156,7 @@ def test_email_cancelled( ) @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]}) - @patch("emails.actions.instructor_training_approaching.messages_action_cancelled") + @patch("emails.actions.base_action.messages_action_cancelled") def test_multiple_emails_cancelled( self, mock_messages_action_cancelled: MagicMock, @@ -186,8 +185,7 @@ def test_multiple_emails_cancelled( # Act with patch( - "emails.actions.instructor_training_approaching" - ".EmailController.cancel_email" + "emails.actions.base_action.EmailController.cancel_email" ) as mock_cancel_email: instructor_training_approaching_remove_signal.send( sender=self.event, diff --git a/amy/emails/tests/actions/test_instructor_training_approaching_update_receiver.py b/amy/emails/tests/actions/test_instructor_training_approaching_update_receiver.py index 60eee88ff..c7cc40c6f 100644 --- a/amy/emails/tests/actions/test_instructor_training_approaching_update_receiver.py +++ b/amy/emails/tests/actions/test_instructor_training_approaching_update_receiver.py @@ -56,7 +56,7 @@ def setUpEmailTemplate(self) -> EmailTemplate: body="Hello, {{ name }}! Nice to meet **you**.", ) - @patch("workshops.utils.feature_flags.logger") + @patch("emails.actions.base_action.logger") def test_disabled_when_no_feature_flag(self, mock_logger) -> None: # Arrange request = RequestFactory().get("/") @@ -66,7 +66,7 @@ def test_disabled_when_no_feature_flag(self, mock_logger) -> None: # Assert mock_logger.debug.assert_called_once_with( "EMAIL_MODULE feature flag not set, skipping " - "instructor_training_approaching_update_receiver" + "instructor_training_approaching_update" ) def test_receiver_connected_to_signal(self) -> None: @@ -102,7 +102,7 @@ def test_action_triggered(self) -> None: # Act with patch( - "emails.actions.instructor_training_approaching.messages_action_updated" + "emails.actions.base_action.messages_action_updated" ) as mock_messages_action_updated: instructor_training_approaching_update_signal.send( sender=self.event, @@ -120,7 +120,7 @@ def test_action_triggered(self) -> None: ) @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]}) - @patch("emails.actions.instructor_training_approaching.messages_action_updated") + @patch("emails.actions.base_action.messages_action_updated") @patch("emails.actions.instructor_training_approaching.one_month_before") def test_email_updated( self, @@ -148,8 +148,7 @@ def test_email_updated( # Act with patch( - "emails.actions.instructor_training_approaching" - ".EmailController.update_scheduled_email" + "emails.actions.base_action.EmailController.update_scheduled_email" ) as mock_update_scheduled_email: instructor_training_approaching_update_signal.send( sender=self.event, @@ -169,8 +168,8 @@ def test_email_updated( ) @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]}) - @patch("emails.actions.instructor_training_approaching.logger") - @patch("emails.actions.instructor_training_approaching.EmailController") + @patch("emails.actions.base_action.logger") + @patch("emails.actions.base_action.EmailController") def test_previously_scheduled_email_not_existing( self, mock_email_controller: MagicMock, mock_logger: MagicMock ) -> None: @@ -190,12 +189,13 @@ def test_previously_scheduled_email_not_existing( # Assert mock_email_controller.update_scheduled_email.assert_not_called() mock_logger.warning.assert_called_once_with( - f"Scheduled email for signal {signal} and event {event} does not exist." + f"Scheduled email for signal {signal} and generic_relation_obj={event!r} " + "does not exist." ) @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]}) - @patch("emails.actions.instructor_training_approaching.logger") - @patch("emails.actions.instructor_training_approaching.EmailController") + @patch("emails.actions.base_action.logger") + @patch("emails.actions.base_action.EmailController") def test_multiple_previously_scheduled_emails( self, mock_email_controller: MagicMock, mock_logger: MagicMock ) -> None: @@ -234,12 +234,12 @@ def test_multiple_previously_scheduled_emails( # Assert mock_email_controller.update_scheduled_email.assert_not_called() mock_logger.warning.assert_called_once_with( - f"Too many scheduled emails for signal {signal} and event {event}." - " Can't update them." + f"Too many scheduled emails for signal {signal} and " + f"generic_relation_obj={event!r}. Can't update them." ) @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]}) - @patch("emails.actions.instructor_training_approaching.messages_missing_recipients") + @patch("emails.actions.base_action.messages_missing_recipients") def test_missing_recipients( self, mock_messages_missing_recipients: MagicMock ) -> None: