[#2530] Move the rest of complex actions to BaseAction*

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)

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
+)

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,