[#2577] Fix strategy: expect membership tasks to exist

amy/emails/actions/new_membership_onboarding.py

@@ -27,6 +27,8 @@
 
 logger = logging.getLogger("amy")
 
+MEMBERSHIP_TASK_ROLES_EXPECTED = ["billing_contact", "programmatic_contact"]
+
 
 def new_membership_onboarding_strategy(membership: Membership) -> StrategyEnum:
     logger.info(f"Running NewMembershipOnboarding strategy for {membership}")
@@ -39,9 +41,14 @@ def new_membership_onboarding_strategy(membership: Membership) -> StrategyEnum:
         state=ScheduledEmailStatus.SCHEDULED,
     ).exists()
 
-    # TODO: check how to handle removing membership
+    # Membership can't be removed without removing the tasks first. This is when the
+    # email would be de-scheduled.
     email_should_exist = (
-        membership.pk and getattr(membership, "rolled_from_membership", None) is None
+        membership.pk
+        and getattr(membership, "rolled_from_membership", None) is None
+        and MembershipTask.objects.filter(
+            membership=membership, role__name__in=MEMBERSHIP_TASK_ROLES_EXPECTED
+        ).count()
     )
 
     if not email_scheduled and email_should_exist:
@@ -115,7 +122,7 @@ def get_recipients(
     membership = context["membership"]
     tasks = MembershipTask.objects.filter(
         membership=membership,
-        role__name__in=["billing_contact", "programmatic_contact"],
+        role__name__in=MEMBERSHIP_TASK_ROLES_EXPECTED,
     ).select_related("person", "role")
     return [task.person.email for task in tasks if task.person.email]
 

amy/emails/tests/actions/test_new_membership_onboarding_integration.py

@@ -247,6 +247,90 @@ def test_integration(self) -> None:
         assert latest_log
         self.assertEqual(latest_log.details, "Email was cancelled")
 
+    @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]})
+    def test_integration_when_removing_membership_task(self) -> None:
+        # Arrange
+        self._setUpUsersAndLogin()
+
+        membership = Membership.objects.create(
+            name="Test Membership",
+            variant="partner",
+            registration_code="test-beta-code-test",
+            agreement_start=date.today(),
+            agreement_end=date.today() + timedelta(days=365),
+            contribution_type="financial",
+            workshops_without_admin_fee_per_agreement=10,
+            public_instructor_training_seats=25,
+            additional_public_instructor_training_seats=3,
+        )
+        Member.objects.create(
+            membership=membership,
+            organization=self.org_beta,
+            role=MemberRole.objects.first(),
+        )
+        billing_contact_role = MembershipPersonRole.objects.create(
+            name="billing_contact"
+        )
+        task = MembershipTask.objects.create(
+            person=self.hermione,
+            membership=membership,
+            role=billing_contact_role,
+        )
+
+        signal = NEW_MEMBERSHIP_ONBOARDING_SIGNAL_NAME
+        template = EmailTemplate.objects.create(
+            name="Test Email Template",
+            signal=signal,
+            from_header="[email protected]",
+            cc_header=["[email protected]"],
+            bcc_header=[],
+            subject="Greetings",
+            body="Hello! Nice to meet **you**.",
+        )
+        request = RequestFactory().get("/")
+
+        with patch(
+            "emails.actions.base_action.messages_action_scheduled"
+        ) as mock_action_scheduled:
+            run_new_membership_onboarding_strategy(
+                new_membership_onboarding_strategy(membership),
+                request=request,
+                membership=membership,
+            )
+        scheduled_email = ScheduledEmail.objects.get(template=template)
+
+        url = reverse("membership_tasks", args=[membership.pk])
+
+        # Act
+        data = {
+            "form-TOTAL_FORMS": "1",
+            "form-INITIAL_FORMS": "1",
+            "form-MIN_NUM_FORMS": "0",
+            "form-MAX_NUM_FORMS": "1000",
+            "form-0-membership": membership.pk,
+            "form-0-person": task.person.pk,
+            "form-0-role": task.role.pk,
+            "form-0-id": task.pk,
+            "form-0-DELETE": "on",
+        }
+        rv = self.client.post(url, data=data)
+
+        # Arrange
+        mock_action_scheduled.assert_called_once()
+        self.assertEqual(rv.status_code, 302)
+        self.assertEqual(
+            MembershipTask.objects.filter(membership=membership).count(), 0
+        )
+        scheduled_email.refresh_from_db()
+        self.assertEqual(scheduled_email.state, ScheduledEmailStatus.CANCELLED)
+        latest_log = (
+            ScheduledEmailLog.objects.filter(scheduled_email=scheduled_email)
+            .order_by("-created_at")
+            .first()
+        )
+        assert latest_log
+        self.assertEqual(latest_log.details, "Email was cancelled")
+
     @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]})
     def test_integration__not_removed_because_of_related_objects(self) -> None:
         # Arrange

amy/emails/tests/actions/test_new_membership_onboarding_strategy.py

@@ -11,7 +11,8 @@
 from emails.models import EmailTemplate, ScheduledEmail, ScheduledEmailStatus
 from emails.signals import NEW_MEMBERSHIP_ONBOARDING_SIGNAL_NAME
 from emails.types import StrategyEnum
-from workshops.models import Membership
+from fiscal.models import MembershipPersonRole, MembershipTask
+from workshops.models import Membership, Person
 
 
 class TestNewMembershipOnboardingStrategy(TestCase):
@@ -23,6 +24,15 @@ def setUp(self) -> None:
             agreement_end=date(2023, 1, 1),
             contribution_type="financial",
         )
+        billing_contact_role, _ = MembershipPersonRole.objects.get_or_create(
+            name="billing_contact"
+        )
+        person = Person.objects.create()
+        MembershipTask.objects.create(
+            membership=self.membership,
+            person=person,
+            role=billing_contact_role,
+        )
 
     def setUpScheduledEmail(
         self,
@@ -79,7 +89,7 @@ def test_strategy_update(self) -> None:
         # Assert
         self.assertEqual(result, StrategyEnum.UPDATE)
 
-    def test_strategy_remove(self) -> None:
+    def test_strategy_remove_because_rolled_over(self) -> None:
         # Arrange
         new_membership = self.rollOverMembership(self.membership)
         self.setUpScheduledEmail(new_membership)
@@ -88,6 +98,15 @@ def test_strategy_remove(self) -> None:
         # Assert
         self.assertEqual(result, StrategyEnum.REMOVE)
 
+    def test_strategy_remove_because_no_tasks(self) -> None:
+        # Arrange
+        self.setUpScheduledEmail(self.membership)
+        MembershipTask.objects.all().delete()
+        # Act
+        result = new_membership_onboarding_strategy(self.membership)
+        # Assert
+        self.assertEqual(result, StrategyEnum.REMOVE)
+
     def test_strategy_noop__membership_not_saved(self) -> None:
         # Act
         membership = Membership()