Merge pull request #2680 from carpentries/feature/2678-failed-emails-…

…circuit-breaker

[Emails] Failed emails circuit breaker

amy/emails/controller.py

@@ -1,5 +1,6 @@
 from datetime import datetime
 
+from django.conf import settings
 from django.db.models import Model
 import jinja2
 
@@ -203,10 +204,35 @@ def lock_email(
     def fail_email(
         scheduled_email: ScheduledEmail, details: str, author: Person | None = None
     ) -> ScheduledEmail:
-        return EmailController.change_state_with_log(
+        email = EmailController.change_state_with_log(
             scheduled_email, ScheduledEmailStatus.FAILED, details, author
         )
 
+        # Count the number of failures. If it's >= MAX_FAILED_ATTEMPTS, then cancel
+        # the email.
+        latest_status_changes = ScheduledEmailLog.objects.filter(
+            scheduled_email=email
+        ).order_by("-created_at")[: 2 * settings.EMAIL_MAX_FAILED_ATTEMPTS]
+        # 2* because the worker first locks the email, then it fails it.
+        # We don't want to cancel an email which was cancelled, and then the user
+        # decided to reschedule it.
+
+        failed_attempts_count = len(
+            [
+                log
+                for log in latest_status_changes
+                if log.state_after == ScheduledEmailStatus.FAILED
+            ]
+        )
+        if failed_attempts_count >= settings.EMAIL_MAX_FAILED_ATTEMPTS:
+            email = EmailController.cancel_email(
+                email,
+                f"Email failed {failed_attempts_count} times, cancelling.",
+                author,
+            )
+
+        return email
+
     @staticmethod
     def succeed_email(
         scheduled_email: ScheduledEmail, details: str, author: Person | None = None

amy/emails/tests/test_controller.py

@@ -1,6 +1,7 @@
 from datetime import UTC, datetime, timedelta
 import random
 
+from django.conf import settings
 from django.db.models import Model
 from django.template.exceptions import TemplateSyntaxError as DjangoTemplateSyntaxError
 from django.test import TestCase
@@ -54,8 +55,8 @@ def create_scheduled_email(
                     {
                         "api_uri": api_model_url("person", self.harry.pk),
                         "property": "email",
-                    }
-                ]  # type: ignore
+                    }  # type: ignore
+                ]
             ),
             generic_relation_obj=generic_relation_obj,
             author=author,
@@ -123,8 +124,8 @@ def test_schedule_email__no_template(self) -> None:
                         {
                             "api_uri": api_model_url("person", self.harry.pk),
                             "property": "email",
-                        }
-                    ]  # type: ignore
+                        }  # type: ignore
+                    ]
                 ),
             )
 
@@ -146,8 +147,8 @@ def test_schedule_email__inactive_template(self) -> None:
                         {
                             "api_uri": api_model_url("person", self.harry.pk),
                             "property": "email",
-                        }
-                    ]  # type: ignore
+                        }  # type: ignore
+                    ]
                 ),
             )
 
@@ -169,8 +170,8 @@ def test_schedule_email__invalid_template(self) -> None:
                         {
                             "api_uri": api_model_url("person", self.harry.pk),
                             "property": "email",
-                        }
-                    ]  # type: ignore
+                        }  # type: ignore
+                    ]
                 ),
             )
 
@@ -265,8 +266,8 @@ def test_update_scheduled_email(self) -> None:
                     {
                         "api_uri": api_model_url("person", self.harry.pk),
                         "property": "secondary_email",
-                    }
-                ]  # type: ignore
+                    }  # type: ignore
+                ]
             ),
             generic_relation_obj=None,
             author=self.harry,
@@ -342,8 +343,8 @@ def test_update_scheduled_email__missing_template(self) -> None:
                         {
                             "api_uri": api_model_url("person", self.harry.pk),
                             "property": "secondary_email",
-                        }
-                    ]  # type: ignore
+                        }  # type: ignore
+                    ]
                 ),
                 generic_relation_obj=None,
                 author=self.harry,
@@ -466,6 +467,59 @@ def test_fail_email(self) -> None:
         self.assertEqual(latest_log.details, "Email was failed 123")
         self.assertEqual(latest_log.author, self.harry)
 
+    def test_fail_email_so_many_times_it_gets_cancelled(self) -> None:
+        # Arrange
+        now = timezone.now()
+        scheduled_email = self.create_scheduled_email(now)
+
+        # Act
+        for _ in range(settings.EMAIL_MAX_FAILED_ATTEMPTS):
+            scheduled_email = EmailController.fail_email(
+                scheduled_email,
+                details="Email was failed",
+                author=self.harry,
+            )
+
+        # Assert
+        self.assertEqual(scheduled_email.state, ScheduledEmailStatus.CANCELLED)
+        self.assertEqual(
+            ScheduledEmailLog.objects.filter(scheduled_email=scheduled_email).count(),
+            settings.EMAIL_MAX_FAILED_ATTEMPTS + 2,
+            # +2 because of the initial log and the last log when the email
+            # gets cancelled.
+        )
+
+    def test_lock_and_fail_email_so_many_times_it_gets_cancelled(self) -> None:
+        """Similar test to `test_fail_email_so_many_times_it_gets_cancelled`,
+        but here the email is locked before failing. This is similar behavior as in
+        the actual email worker."""
+
+        # Arrange
+        now = timezone.now()
+        scheduled_email = self.create_scheduled_email(now)
+
+        # Act
+        for _ in range(settings.EMAIL_MAX_FAILED_ATTEMPTS):
+            scheduled_email = EmailController.lock_email(
+                scheduled_email,
+                details="Email was locked for sending",
+                author=self.harry,
+            )
+            scheduled_email = EmailController.fail_email(
+                scheduled_email,
+                details="Email was failed",
+                author=self.harry,
+            )
+
+        # Assert
+        self.assertEqual(scheduled_email.state, ScheduledEmailStatus.CANCELLED)
+        self.assertEqual(
+            ScheduledEmailLog.objects.filter(scheduled_email=scheduled_email).count(),
+            2 * settings.EMAIL_MAX_FAILED_ATTEMPTS + 2,
+            # +2 because of the initial log and the last log when the email
+            # gets cancelled.
+        )
+
     def test_succeed_email(self) -> None:
         # Arrange
         now = timezone.now()

config/settings.py

@@ -651,6 +651,7 @@
 # -----------------------------------------------------------------------------
 # This module is the next version of Autoemails.
 EMAIL_TEMPLATE_ENGINE_BACKEND = "email_jinja2_backend"
+EMAIL_MAX_FAILED_ATTEMPTS = 10  # value controls the circuit breaker for failed attempts
 
 # Reports
 # -----------------------------------------------------------------------------