feat: detect duplicate recipients in CSV uploads (#3319) (#416)

* feat: detect duplicate recipients in CSV uploads (#3319)

Adds duplicate-recipient detection to RecipientCSV so that admin can warn
senders before a bulk send when their CSV contains the same recipient
more than once. Detection is case-insensitive, ignores leading/trailing
whitespace, and (for SMS) treats phone numbers in different formats as
equivalent.

The new properties (has_duplicate_recipients,
count_of_unique_duplicate_recipients, count_of_duplicate_recipient_rows,
rows_with_duplicate_recipients) are non-blocking: they do *not* affect
has_errors. Admin can use them to render a warning banner and a
download-duplicates link without preventing the user from sending.

Refs: cds-snc/notification-planning#3319

* improve perf, exclude simulator email addresses

* add excluded phone numbers

Author: smcmurtry

.github/actions/waffles/requirements.txt

@@ -2,4 +2,4 @@ docopt==0.6.2
 Flask==2.3.3
 markupsafe==2.1.5
 setuptools==78.1.1 # required for distutils in Python 3.12
-git+https://github.com/cds-snc/notifier-utils.git@53.2.25#egg=notifications-utils
+git+https://github.com/cds-snc/notifier-utils.git@53.2.26#egg=notifications-utils

notifications_utils/recipients.py

@@ -78,6 +78,30 @@
     "address line 6",
 }
 
+# Email domains used for testing-only / synthetic recipients. Addresses on
+# these domains are excluded from duplicate-recipient detection because they
+# are intentionally re-used (e.g. AWS SES has a fixed set of mailboxes at
+# ``simulator.amazonses.com`` that always deliver, bounce, complain, etc.).
+DEDUPE_EXCLUDED_EMAIL_DOMAINS = frozenset(
+    {
+        "simulator.amazonses.com",
+    }
+)
+
+# Phone numbers used for testing-only / synthetic recipients. These are the
+# simulator numbers configured in ``notification-api`` (``SIMULATED_SMS_NUMBERS``):
+# the API short-circuits delivery for these so that they are intentionally
+# re-used in load and smoke tests. They should not be flagged as duplicates
+# when senders include them in a bulk-send CSV. Values are stored in E.164 so
+# they match what ``validate_phone_number`` returns.
+DEDUPE_EXCLUDED_PHONE_NUMBERS = frozenset(
+    {
+        "+16132532222",
+        "+16132532223",
+        "+16132532224",
+    }
+)
+
 
 class RecipientCSV:
     def __init__(
@@ -311,6 +335,117 @@ def rows_with_combined_variable_content_too_long(self):
                     if total_length > SMS_CHAR_COUNT_LIMIT:
                         yield row
 
+    def _normalise_recipient_for_dedupe(self, recipient):
+        """
+        Normalise a recipient value for case-insensitive, whitespace-insensitive
+        duplicate detection. Phone numbers are normalised to E.164 when possible
+        so that "+1 555-123-4567" and "5551234567" are treated as the same recipient.
+        Returns ``None`` if the value cannot meaningfully be normalised (e.g. empty)
+        or if the value is a known testing-only address (see
+        ``DEDUPE_EXCLUDED_EMAIL_DOMAINS`` / ``DEDUPE_EXCLUDED_PHONE_NUMBERS``)
+        that should not be flagged as a duplicate.
+        """
+        if recipient is None:
+            return None
+        normalised = strip_and_remove_obscure_whitespace(str(recipient)).strip().lower()
+        if not normalised:
+            return None
+        if self.template_type == "email":
+            # ``user@DOMAIN`` -> domain part is everything after the last ``@``.
+            # We only need to skip dedupe for synthetic test mailboxes, so a
+            # cheap suffix check on the lowercased value is sufficient.
+            domain = normalised.rpartition("@")[2]
+            if domain in DEDUPE_EXCLUDED_EMAIL_DOMAINS:
+                return None
+        if self.template_type == "sms":
+            try:
+                normalised_phone = validate_phone_number(recipient, international=self.international_sms)
+            except InvalidPhoneError:
+                return normalised
+            if normalised_phone in DEDUPE_EXCLUDED_PHONE_NUMBERS:
+                return None
+            return normalised_phone
+        return normalised
+
+    def _compute_duplicate_recipient_summary(self):
+        """
+        Single-pass computation of everything we need to report duplicate
+        recipients. Iterating ``self.rows`` and (for SMS) calling
+        ``validate_phone_number`` per row is expensive on large uploads, so all
+        of the public ``*_duplicate_*`` properties read from this cached result
+        rather than recomputing.
+        """
+        DuplicateSummary = namedtuple("DuplicateSummary", ("row_indices", "unique_count"))
+        if self.template_type == "letter":
+            return DuplicateSummary(row_indices=frozenset(), unique_count=0)
+
+        seen: Dict[str, int] = {}
+        duplicate_indices = set()
+        for row in self.rows:
+            if row is None:
+                continue
+            if row.has_bad_recipient or row.recipient is None:
+                continue
+            normalised = self._normalise_recipient_for_dedupe(row.recipient)
+            if normalised is None:
+                continue
+            previous_count = seen.get(normalised, 0)
+            seen[normalised] = previous_count + 1
+            if previous_count:
+                duplicate_indices.add(row.index)
+
+        unique_count = sum(1 for count in seen.values() if count > 1)
+        return DuplicateSummary(row_indices=frozenset(duplicate_indices), unique_count=unique_count)
+
+    @property
+    def _duplicate_recipient_summary(self):
+        # Cached on the instance so all of the public duplicate-recipient
+        # properties cost only one full pass over the rows in total.
+        if not hasattr(self, "_duplicate_recipient_summary_cache"):
+            self._duplicate_recipient_summary_cache = self._compute_duplicate_recipient_summary()
+        return self._duplicate_recipient_summary_cache
+
+    @property
+    def _duplicate_recipient_row_indices(self):
+        """
+        Returns a set of row indices for rows whose recipient value has already
+        appeared in an earlier row. The first occurrence of each recipient is
+        not flagged. Rows with bad or missing recipients are skipped, and
+        duplicate detection is disabled for letter templates (where multiple
+        recipients can legitimately share an address).
+        """
+        return self._duplicate_recipient_summary.row_indices
+
+    @property
+    def rows_with_duplicate_recipients(self):
+        """
+        Yields rows whose recipient is a duplicate of an earlier row. The first
+        occurrence of each recipient is *not* yielded; only the subsequent
+        copies are yielded so callers can highlight or export them.
+        """
+        duplicate_indices = self._duplicate_recipient_row_indices
+        if not duplicate_indices:
+            return
+        for row in self.rows:
+            if row is None:
+                continue
+            if row.index in duplicate_indices:
+                yield row
+
+    @property
+    def has_duplicate_recipients(self):
+        return bool(self._duplicate_recipient_summary.row_indices)
+
+    @property
+    def count_of_duplicate_recipient_rows(self):
+        """Total number of duplicate rows (i.e. extra copies beyond the first)."""
+        return len(self._duplicate_recipient_summary.row_indices)
+
+    @property
+    def count_of_unique_duplicate_recipients(self):
+        """Number of distinct recipients that appear more than once."""
+        return self._duplicate_recipient_summary.unique_count
+
     @property
     def initial_rows_with_errors(self):
         return islice(self.rows_with_errors, self.max_errors_shown)

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "notifications-utils"
-version = "53.2.25"
+version = "53.2.26"
 description = "Shared python code for Notification - Provides logging utils etc."
 authors = ["Canadian Digital Service"]
 license = "MIT license"

tests/test_recipient_csv.py

@@ -1152,3 +1152,195 @@ def test_multi_line_placeholders_work():
     )
 
     assert recipients.rows[0].personalisation["data"] == "a\nb\n\nc"
+
+
+class TestDuplicateRecipients:
+    """Duplicate-recipient detection (issue #3319).
+
+    The detection should be case-insensitive, ignore leading/trailing
+    whitespace, and treat phone numbers as equivalent when they normalise to
+    the same E.164 form. It should be a non-blocking warning -- ``has_errors``
+    must remain ``False`` when the only issue is duplicate recipients.
+    """
+
+    def test_no_duplicates_when_all_emails_unique(self):
+        recipients = RecipientCSV(
+            """
+                email address
+                alice@example.com
+                bob@example.com
+                carol@example.com
+            """,
+            template_type="email",
+        )
+        assert recipients.has_duplicate_recipients is False
+        assert recipients.count_of_duplicate_recipient_rows == 0
+        assert recipients.count_of_unique_duplicate_recipients == 0
+        assert list(recipients.rows_with_duplicate_recipients) == []
+        assert recipients.has_errors is False
+
+    def test_detects_exact_duplicate_emails(self):
+        recipients = RecipientCSV(
+            """
+                email address
+                alice@example.com
+                bob@example.com
+                alice@example.com
+            """,
+            template_type="email",
+        )
+        assert recipients.has_duplicate_recipients is True
+        assert recipients.count_of_duplicate_recipient_rows == 1
+        assert recipients.count_of_unique_duplicate_recipients == 1
+        duplicates = list(recipients.rows_with_duplicate_recipients)
+        # Only the *second* occurrence is flagged; the first is kept.
+        assert [row.index for row in duplicates] == [2]
+        # Duplicates are a warning, not a hard error.
+        assert recipients.has_errors is False
+
+    def test_email_dedupe_is_case_insensitive_and_trims_whitespace(self):
+        # Build the CSV explicitly so leading/trailing whitespace and case
+        # differences are preserved without tripping the linter.
+        file_contents = "email address\n" "Alice@Example.com\n" "  alice@example.COM  \n" "ALICE@EXAMPLE.COM\n"
+        recipients = RecipientCSV(file_contents, template_type="email")
+        assert recipients.count_of_duplicate_recipient_rows == 2
+        assert recipients.count_of_unique_duplicate_recipients == 1
+
+    def test_counts_unique_duplicate_recipients(self):
+        recipients = RecipientCSV(
+            """
+                email address
+                alice@example.com
+                bob@example.com
+                alice@example.com
+                bob@example.com
+                carol@example.com
+                bob@example.com
+            """,
+            template_type="email",
+        )
+        # alice appears twice (1 extra), bob appears 3 times (2 extra) -> 3 duplicate rows
+        assert recipients.count_of_duplicate_recipient_rows == 3
+        # Two distinct recipients have duplicates.
+        assert recipients.count_of_unique_duplicate_recipients == 2
+
+    def test_detects_duplicate_phone_numbers_in_different_formats(self):
+        recipients = RecipientCSV(
+            """
+                phone number
+                6502532222
+                +1 650-253-2222
+                650 253 2222
+                6502532223
+            """,
+            template_type="sms",
+            international_sms=True,
+        )
+        assert recipients.count_of_duplicate_recipient_rows == 2
+        assert recipients.count_of_unique_duplicate_recipients == 1
+
+    def test_skips_rows_with_bad_or_missing_recipients(self):
+        file_contents = "email address\n" "alice@example.com\n" "not-an-email\n" "\n" "alice@example.com\n"
+        recipients = RecipientCSV(file_contents, template_type="email")
+        # The blank row and bad-email row should not be considered for dedupe.
+        assert recipients.count_of_duplicate_recipient_rows == 1
+
+    def test_duplicate_detection_disabled_for_letters(self):
+        recipients = RecipientCSV(
+            """
+                address line 1, address line 2, address line 3, address line 4, address line 5, address line 6, postcode
+                A, B, C, , , , X1A0A1
+                A, B, C, , , , X1A0A1
+            """,
+            template_type="letter",
+        )
+        # Letters can legitimately share an address, so we don't flag duplicates.
+        assert recipients.has_duplicate_recipients is False
+        assert recipients.count_of_duplicate_recipient_rows == 0
+        assert recipients.count_of_unique_duplicate_recipients == 0
+
+    def test_duplicates_do_not_make_has_errors_true(self):
+        recipients = RecipientCSV(
+            """
+                email address
+                alice@example.com
+                alice@example.com
+            """,
+            template_type="email",
+        )
+        assert recipients.has_duplicate_recipients is True
+        # Critically: duplicates are a non-blocking warning, not an error.
+        assert recipients.has_errors is False
+        assert list(recipients.rows_with_errors) == []
+
+    def test_ses_simulator_addresses_are_not_flagged_as_duplicates(self):
+        # The ``simulator.amazonses.com`` mailboxes (success@, bounce@,
+        # complaint@, etc.) are deliberately re-used for load/smoke testing,
+        # so a CSV full of them should not produce a "duplicate recipients"
+        # warning.
+        recipients = RecipientCSV(
+            """
+                email address
+                success@simulator.amazonses.com
+                SUCCESS@simulator.amazonses.com
+                bounce@simulator.amazonses.com
+                bounce@simulator.amazonses.com
+                alice@example.com
+                alice@example.com
+            """,
+            template_type="email",
+        )
+        # Only the real duplicate (alice) is flagged; the simulator addresses
+        # are excluded even though they appear multiple times.
+        assert recipients.count_of_unique_duplicate_recipients == 1
+        assert recipients.count_of_duplicate_recipient_rows == 1
+        duplicate_rows = list(recipients.rows_with_duplicate_recipients)
+        assert len(duplicate_rows) == 1
+        assert duplicate_rows[0].recipient == "alice@example.com"
+
+    def test_simulator_phone_numbers_are_not_flagged_as_duplicates(self):
+        # The simulator numbers configured in ``notification-api``
+        # (``SIMULATED_SMS_NUMBERS``) are short-circuited and never actually
+        # delivered, so they are intentionally re-used in load/smoke tests
+        # and should not produce a duplicate warning. The exclusion is on the
+        # E.164 form, so different input formats of the same simulator number
+        # must also be excluded.
+        recipients = RecipientCSV(
+            """
+                phone number
+                +16132532222
+                6132532222
+                (613) 253-2222
+                +16132532223
+                +16132532223
+                +16135551234
+                +16135551234
+            """,
+            template_type="sms",
+        )
+        # Only the real duplicate (+16135551234) is flagged.
+        assert recipients.count_of_unique_duplicate_recipients == 1
+        assert recipients.count_of_duplicate_recipient_rows == 1
+        duplicate_rows = list(recipients.rows_with_duplicate_recipients)
+        assert len(duplicate_rows) == 1
+
+    def test_duplicate_summary_is_cached(self):
+        # Re-reading any of the duplicate properties on a large upload should
+        # be cheap: the underlying single-pass computation must only run once.
+        recipients = RecipientCSV(
+            """
+                email address
+                alice@example.com
+                alice@example.com
+            """,
+            template_type="email",
+        )
+        # Prime the cache.
+        first_indices = recipients._duplicate_recipient_row_indices
+        # Subsequent accesses (incl. via different public properties) should
+        # all return the exact same cached object.
+        assert recipients._duplicate_recipient_row_indices is first_indices
+        assert recipients._duplicate_recipient_summary.row_indices is first_indices
+        assert recipients.count_of_duplicate_recipient_rows == 1
+        assert recipients.count_of_unique_duplicate_recipients == 1
+        assert recipients.has_duplicate_recipients is True