From 40e16a13a5c197fff88897ebfa689441d28d181f Mon Sep 17 00:00:00 2001 From: Piotr Banaszkiewicz Date: Sat, 16 Dec 2023 16:58:00 +0100 Subject: [PATCH] [#2577] Fix strategy: expect membership tasks to exist --- .../actions/new_membership_onboarding.py | 13 ++- ...t_new_membership_onboarding_integration.py | 84 +++++++++++++++++++ ...test_new_membership_onboarding_strategy.py | 23 ++++- 3 files changed, 115 insertions(+), 5 deletions(-) diff --git a/amy/emails/actions/new_membership_onboarding.py b/amy/emails/actions/new_membership_onboarding.py index 1107a609f..bac9616ca 100644 --- a/amy/emails/actions/new_membership_onboarding.py +++ b/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] diff --git a/amy/emails/tests/actions/test_new_membership_onboarding_integration.py b/amy/emails/tests/actions/test_new_membership_onboarding_integration.py index 5923f4d3c..e49bf3929 100644 --- a/amy/emails/tests/actions/test_new_membership_onboarding_integration.py +++ b/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="workshops@carpentries.org", + cc_header=["team@carpentries.org"], + 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 diff --git a/amy/emails/tests/actions/test_new_membership_onboarding_strategy.py b/amy/emails/tests/actions/test_new_membership_onboarding_strategy.py index f4faf78f6..2d703f4a5 100644 --- a/amy/emails/tests/actions/test_new_membership_onboarding_strategy.py +++ b/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()