Skip to content

Commit 843289f

Browse files
committed
feat: warn about duplicate recipients in CSV uploads (#3319)
Surfaces a non-blocking warning on the bulk-send preview page when the uploaded CSV contains the same recipient more than once. The warning is case-insensitive, ignores leading/trailing whitespace, and (for SMS) treats phone numbers in different formats as equivalent. Letters are excluded because multiple recipients can legitimately share an address. Changes: - new get_warnings_for_csv helper in app/utils.py - new download_duplicate_recipients route serves a CSV of duplicate rows so the sender can review them before sending; the response is built server-side from the original upload, so duplicates are visible only to the authenticated sender (privacy) - ok.html renders the warning in a banner inside an aria-live=polite region, including a link to download the duplicates list - EN/FR translation strings added to fr.csv (en.csv and messages.po files are regenerated by 'make babel') - unit tests for get_warnings_for_csv Depends on notifications-utils with duplicate-recipient detection (see corresponding PR in notification-utils). Refs: cds-snc/notification-planning#3319
1 parent 3e359d8 commit 843289f

5 files changed

Lines changed: 197 additions & 1 deletion

File tree

app/main/views/send.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
get_help_argument,
7171
get_limit_reset_time_et,
7272
get_template,
73+
get_warnings_for_csv,
7374
should_skip_template_page,
7475
unicode_truncate,
7576
user_has_permissions,
@@ -771,8 +772,11 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
771772
template=template,
772773
errors=recipients.has_errors,
773774
row_errors=get_errors_for_csv(recipients, template.template_type),
775+
row_warnings=get_warnings_for_csv(recipients, template.template_type),
774776
count_of_recipients=len(recipients),
775777
count_of_displayed_recipients=len(list(recipients.displayed_rows)),
778+
count_of_duplicate_recipients=recipients.count_of_unique_duplicate_recipients,
779+
count_of_duplicate_recipient_rows=recipients.count_of_duplicate_recipient_rows,
776780
original_file_name=request.args.get("original_file_name", ""),
777781
upload_id=upload_id,
778782
form=CsvUploadForm(),
@@ -919,6 +923,48 @@ def check_messages_preview(service_id, template_id, upload_id, filetype, row_ind
919923
return TemplatePreview.from_utils_template(template, filetype, page=page)
920924

921925

926+
@main.route(
927+
"/services/<service_id>/<uuid:template_id>/check/<upload_id>/duplicates.csv",
928+
methods=["GET"],
929+
)
930+
@user_has_permissions("send_messages", restrict_admin_usage=True)
931+
def download_duplicate_recipients(service_id, template_id, upload_id):
932+
"""Return a CSV of the rows whose recipient is a duplicate of an earlier
933+
row in the same upload. The list is rendered server-side from the original
934+
upload (which is scoped to this authenticated user) and is not persisted,
935+
so duplicates are visible only to the authenticated sender (issue #3319).
936+
"""
937+
contents = s3download(service_id, upload_id)
938+
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
939+
template = get_template(db_template, current_service)
940+
recipients = RecipientCSV(
941+
contents,
942+
template=template,
943+
template_type=template.template_type,
944+
placeholders=template.placeholders,
945+
max_rows=get_csv_max_rows(service_id),
946+
international_sms=current_service.has_permission("international_sms"),
947+
user_language=get_current_locale(current_app),
948+
)
949+
950+
column_headers = recipients.column_headers
951+
duplicate_rows = list(recipients.rows_with_duplicate_recipients)
952+
953+
rows = [["Row number", *column_headers]]
954+
for row in duplicate_rows:
955+
rows.append([str(row.index + 2), *(row[column].data or "" for column in column_headers)])
956+
957+
safe_filename = SanitiseASCII.encode(request.args.get("original_file_name", "duplicates.csv"))
958+
return (
959+
Spreadsheet.from_rows(rows).as_csv_data,
960+
200,
961+
{
962+
"Content-Type": "text/csv; charset=utf-8",
963+
"Content-Disposition": 'attachment; filename="duplicates-{}"'.format(safe_filename),
964+
},
965+
)
966+
967+
922968
@main.route(
923969
"/services/<service_id>/<uuid:template_id>/check.<filetype>",
924970
methods=["GET"],

app/templates/views/check/ok.html

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
{% from "components/page-footer.html" import page_footer %}
77
{% from "components/message-count-label.html" import message_count_label %}
88
{% from "components/banner.html" import banner %}
9+
{% from "components/banner.html" import banner_wrapper %}
910
{% from "components/remaining-messages-summary.html" import remaining_messages_summary with context %}
1011
{% set page_title = _('Review before sending') %}
1112
{% block service_page_title %}
@@ -23,6 +24,28 @@
2324
with_tick=True
2425
) }}
2526
</section>
27+
28+
{# Issue #3319: non-blocking warning when the CSV contains duplicate recipients. #}
29+
{% if row_warnings %}
30+
<div class="mb-12 clear-both contain-floats" aria-live="polite" data-testid="duplicate-recipients-warning">
31+
{% call banner_wrapper(type='warning') %}
32+
<h2 class='banner-title' data-module="track-error" data-error-type="Duplicate recipients" data-error-label="{{ upload_id }}">
33+
{{ row_warnings[0] }}
34+
</h2>
35+
{% for warning in row_warnings[1:] %}
36+
<p class="mb-0">{{ warning }}</p>
37+
{% endfor %}
38+
{% if count_of_duplicate_recipients %}
39+
<p class="mb-0">
40+
<a href="{{ url_for('.download_duplicate_recipients', service_id=current_service.id, template_id=template.id, upload_id=upload_id, original_file_name=original_file_name) }}" download>
41+
{{ _('Download a list of duplicate recipients') }}
42+
</a>
43+
</p>
44+
{% endif %}
45+
{% endcall %}
46+
</div>
47+
{% endif %}
48+
2649
{% if not request.args.from_test %}
2750
<section class="clear-both contain-floats" id="ok-file">
2851
<h2 class="heading-medium">{{ _('Recipients') }}</h2>

app/translations/csv/fr.csv

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2453,4 +2453,8 @@
24532453
"Use ((unsub_link)) for one-click unsubscribe","Utilisez ((unsub_link)) pour le désabonnement en un clic"
24542454
"Allow this key to manage templates (create, update, or delete)","Permettre à cette clé de gérer les gabarits (créer, mettre à jour ou supprimer)"
24552455
"API key permissions","Permissions de la clé API"
2456-
"Manage templates","Gestion des gabarits"
2456+
"Manage templates","Gestion des gabarits"
2457+
"1 recipient appears in your list more than once. They will receive the message multiple times.","1 destinataire figure plusieurs fois dans votre liste. Cette personne recevra le message plusieurs fois."
2458+
"{} recipients appear in your list more than once. They will receive the message multiple times.","{} destinataires figurent plusieurs fois dans votre liste. Ces personnes recevront le message plusieurs fois."
2459+
"{} rows are duplicates of an earlier row.","{} lignes sont des doublons d’une ligne précédente."
2460+
"Download a list of duplicate recipients","Télécharger la liste des destinataires en double"

app/utils.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,42 @@ def get_errors_for_csv(recipients, template_type):
231231
return errors
232232

233233

234+
def get_warnings_for_csv(recipients, template_type):
235+
"""
236+
Return a list of non-blocking warning messages for a CSV upload.
237+
238+
Currently this surfaces duplicate-recipient warnings for issue #3319:
239+
duplicate detection is case-insensitive, ignores leading/trailing
240+
whitespace, and (for SMS) treats phone numbers in different formats as
241+
equivalent. Letters are intentionally excluded because multiple recipients
242+
can legitimately share an address.
243+
"""
244+
warnings = []
245+
246+
if template_type == TemplateType.LETTER.value:
247+
return warnings
248+
249+
if recipients.has_duplicate_recipients:
250+
unique_duplicates = recipients.count_of_unique_duplicate_recipients
251+
duplicate_rows = recipients.count_of_duplicate_recipient_rows
252+
if unique_duplicates == 1:
253+
warnings.append(
254+
_("1 recipient appears in your list more than once. " "They will receive the message multiple times.")
255+
)
256+
else:
257+
warnings.append(
258+
_("{} recipients appear in your list more than once. " "They will receive the message multiple times.").format(
259+
unique_duplicates
260+
)
261+
)
262+
# Provide a secondary line so that the count of *rows* affected is also visible
263+
# (useful when one recipient appears many times).
264+
if duplicate_rows != unique_duplicates:
265+
warnings.append(_("{} rows are duplicates of an earlier row.").format(duplicate_rows))
266+
267+
return warnings
268+
269+
234270
def localize_and_format_csv_headers(column_headers: list) -> list:
235271
from app import get_current_locale
236272

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
from collections import namedtuple
2+
3+
import pytest
4+
5+
from app.utils import get_warnings_for_csv
6+
7+
# A minimal stand-in for ``RecipientCSV`` covering only the attributes consumed
8+
# by ``get_warnings_for_csv``. Keeps these tests independent of the real
9+
# notifications-utils CSV parser.
10+
MockRecipients = namedtuple(
11+
"MockRecipients",
12+
[
13+
"has_duplicate_recipients",
14+
"count_of_unique_duplicate_recipients",
15+
"count_of_duplicate_recipient_rows",
16+
"template_type",
17+
],
18+
)
19+
20+
21+
@pytest.mark.parametrize(
22+
"template_type",
23+
["sms", "email"],
24+
)
25+
def test_returns_empty_list_when_no_duplicates(app_, template_type):
26+
recipients = MockRecipients(
27+
has_duplicate_recipients=False,
28+
count_of_unique_duplicate_recipients=0,
29+
count_of_duplicate_recipient_rows=0,
30+
template_type=template_type,
31+
)
32+
with app_.test_request_context():
33+
assert get_warnings_for_csv(recipients, template_type) == []
34+
35+
36+
def test_returns_singular_warning_for_one_duplicate_recipient(app_):
37+
recipients = MockRecipients(
38+
has_duplicate_recipients=True,
39+
count_of_unique_duplicate_recipients=1,
40+
count_of_duplicate_recipient_rows=1,
41+
template_type="email",
42+
)
43+
with app_.test_request_context():
44+
warnings = get_warnings_for_csv(recipients, "email")
45+
assert len(warnings) == 1
46+
assert "1 recipient appears in your list more than once" in warnings[0]
47+
48+
49+
def test_returns_plural_warning_for_multiple_duplicate_recipients(app_):
50+
recipients = MockRecipients(
51+
has_duplicate_recipients=True,
52+
count_of_unique_duplicate_recipients=3,
53+
count_of_duplicate_recipient_rows=3,
54+
template_type="email",
55+
)
56+
with app_.test_request_context():
57+
warnings = get_warnings_for_csv(recipients, "email")
58+
assert len(warnings) == 1
59+
assert "3 recipients appear in your list more than once" in warnings[0]
60+
61+
62+
def test_includes_row_count_when_it_differs_from_unique_count(app_):
63+
# one recipient appears 4 times → 1 unique duplicate, 3 duplicate rows
64+
recipients = MockRecipients(
65+
has_duplicate_recipients=True,
66+
count_of_unique_duplicate_recipients=1,
67+
count_of_duplicate_recipient_rows=3,
68+
template_type="sms",
69+
)
70+
with app_.test_request_context():
71+
warnings = get_warnings_for_csv(recipients, "sms")
72+
assert len(warnings) == 2
73+
assert "1 recipient appears in your list more than once" in warnings[0]
74+
assert "3 rows are duplicates of an earlier row" in warnings[1]
75+
76+
77+
def test_letter_template_never_warns_about_duplicates(app_):
78+
# Letters can legitimately share an address (e.g. roommates), so the
79+
# warning would be misleading and is therefore suppressed.
80+
recipients = MockRecipients(
81+
has_duplicate_recipients=True,
82+
count_of_unique_duplicate_recipients=2,
83+
count_of_duplicate_recipient_rows=2,
84+
template_type="letter",
85+
)
86+
with app_.test_request_context():
87+
assert get_warnings_for_csv(recipients, "letter") == []

0 commit comments

Comments
 (0)