diff --git a/apps/submissions/management/commands/process_submissions.py b/apps/submissions/management/commands/process_submissions.py index de3344c..0e6937b 100644 --- a/apps/submissions/management/commands/process_submissions.py +++ b/apps/submissions/management/commands/process_submissions.py @@ -18,7 +18,7 @@ from django.core.management.base import BaseCommand from django.db import transaction from django.utils import timezone -from apps.submissions.emails import send_rejection_email, send_verifying_email +from apps.submissions.emails import send_verifying_email from apps.submissions.models import Submission from apps.submissions.validation import ( ValidationError, @@ -70,8 +70,10 @@ class Command(BaseCommand): sub.closed_at = timezone.now() # closed_by stays NULL -- the validator did the rejecting, # not an operator (plan.md §5 / §7.3). + # `Submission.save()` detects the `processing -> rejected` + # transition and queues `send_rejection_email()` via + # transaction.on_commit -- no explicit email call needed. sub.save() - send_rejection_email(sub, previous_status="processing") self.stdout.write(f"rejected {sub.slug}: {exc}") else: sub.status = Submission.Status.VERIFYING diff --git a/apps/submissions/models.py b/apps/submissions/models.py index 19eaeeb..5200355 100644 --- a/apps/submissions/models.py +++ b/apps/submissions/models.py @@ -276,13 +276,30 @@ class Submission(models.Model): # dashboard index so post-submit redirects always land somewhere real. return reverse("dashboard:index") + @classmethod + def from_db(cls, db, field_names, values): + """Capture the `status` value the row had when it was loaded, so + `save()` can detect status transitions later. Stored on the instance + as `_original_status`; refreshed at the end of every `save()` so + successive saves compare against the freshly-persisted state.""" + instance = super().from_db(db, field_names, values) + instance._original_status = instance.status + return instance + def save(self, *args, **kwargs): """Auto-generate `slug` on first save so any creation path -- admin, `SubmitView`, fixtures, `objects.create()` -- gets a Docker-style codename without callers having to remember to set one. Also keeps `canonical_email` in sync with whichever side (OAuth user / guest) currently owns the row, so the per-email cap and trust list don't - depend on the caller remembering to set it.""" + depend on the caller remembering to set it. + + Additionally: when an UPDATE flips `status` to `rejected` from any + other state, this method queues `send_rejection_email()` via + `transaction.on_commit`. Centralising the email here means **every** + save path -- admin, the validation worker, ad-hoc shell, any future + view -- fires the email through a single hook. Plan.md §7.3. + """ if not self.slug: self.slug = self._generate_unique_slug() # Re-derive canonical_email every save: cheap, and survives an @@ -293,8 +310,40 @@ class Submission(models.Model): elif self.guest_email: owner_email = self.guest_email self.canonical_email = normalize_email(owner_email) + + # Snapshot for the transition check. `_state.adding` is the canonical + # Django way to distinguish "first save" from "subsequent update". + is_new = self._state.adding + new_status = self.status + old_status = getattr(self, "_original_status", None) + super().save(*args, **kwargs) + # Fire on TRANSITIONS only: an UPDATE that flips status to rejected. + # Don't fire on inserts that start out as rejected -- those should + # be impossible by design (plan.md §7.3 doesn't define a (none) -> + # rejected edge), and even if some weird path creates one we'd + # rather stay silent than spam a fresh victim. + if ( + not is_new + and old_status != new_status + and new_status == self.Status.REJECTED + ): + # Local imports keep this module out of the apps/submissions + # import-cycle (emails.py imports from here). + from django.db import transaction + from .emails import send_rejection_email + + transaction.on_commit( + lambda sub=self, prev=old_status: send_rejection_email( + sub, previous_status=prev + ) + ) + + # Refresh the snapshot so a follow-up save on the same instance + # compares against the just-persisted state, not the original load. + self._original_status = new_status + @classmethod def active_count_for_email(cls, email: str) -> int: """Return how many of this email's submissions count against the