From 8680c082fee82b3495882c326931dbceca430205 Mon Sep 17 00:00:00 2001 From: Ryan Westfall Date: Mon, 10 Aug 2026 09:47:04 -0500 Subject: [PATCH] Improve campaign personalization and contact duplicate handling. Add merge tags for email/SMS, paginated removable recipients, PCM event panels for postcard campaigns, and a portal modal when phone/address matches an existing contact. --- site/contacts/services.py | 131 ++++++++++------ site/contacts/templates/contacts/create.html | 81 +++++++++- site/contacts/templates/contacts/detail.html | 20 ++- site/contacts/tests_merge.py | 67 ++++++++ site/contacts/views.py | 143 +++++++++++++++--- site/messaging/providers/email/smtp2go.py | 4 +- site/messaging/providers/sms/smtp2go.py | 3 + site/messaging/services.py | 54 +++++++ .../templates/messaging/campaign_detail.html | 87 +++++++++-- .../templates/messaging/campaign_list.html | 6 +- site/messaging/tests.py | 141 +++++++++++++++++ site/messaging/urls.py | 5 + site/messaging/views.py | 106 +++++++++++-- 13 files changed, 747 insertions(+), 101 deletions(-) diff --git a/site/contacts/services.py b/site/contacts/services.py index 0bf604c..8d290db 100644 --- a/site/contacts/services.py +++ b/site/contacts/services.py @@ -60,6 +60,9 @@ def find_matching_contact( email: str = "", phone: str = "", postal_address: dict | None = None, + match_email: bool = True, + match_phone: bool = True, + match_address: bool = True, ) -> tuple[Contact | None, str]: """ Resolve an existing contact. @@ -68,20 +71,21 @@ def find_matching_contact( Returns (contact, reason) where reason is email|phone|address|"". """ email_norm = (email or "").strip().lower() - if email_norm: + if match_email and email_norm: hit = Contact.objects.filter(email__iexact=email_norm).first() if hit: return hit, "email" - phone_digits = normalize_phone_digits(phone) - if len(phone_digits) >= _PHONE_MIN_DIGITS: - tail = phone_digits[-10:] - for row in Contact.objects.exclude(phone="").only("id", "phone").iterator(): - other = normalize_phone_digits(row.phone) - if len(other) >= _PHONE_MIN_DIGITS and other[-10:] == tail: - return Contact.objects.get(pk=row.pk), "phone" + if match_phone: + phone_digits = normalize_phone_digits(phone) + if len(phone_digits) >= _PHONE_MIN_DIGITS: + tail = phone_digits[-10:] + for row in Contact.objects.exclude(phone="").only("id", "phone").iterator(): + other = normalize_phone_digits(row.phone) + if len(other) >= _PHONE_MIN_DIGITS and other[-10:] == tail: + return Contact.objects.get(pk=row.pk), "phone" - if Contact.postal_address_has_content(postal_address): + if match_address and Contact.postal_address_has_content(postal_address): line1 = (postal_address.get("line1") or "").strip() zip_code = (postal_address.get("zip") or "").strip() candidates = Contact.objects.exclude(postal_address={}) @@ -110,45 +114,18 @@ def _merge_notes(existing: str, addition: str) -> str: return f"{existing}\n{addition}".strip() -def upsert_contact( +def _apply_contact_fields( + existing: Contact, *, - email: str, - first_name: str = "", - last_name: str = "", - phone: str = "", - postal_address: dict | None = None, - source: str = Contact.Source.CONTACT_FORM, - notes_append: str = "", -) -> tuple[Contact, bool, str]: - """ - Find or create a contact, merging on email / phone / address. - - Returns (contact, created, match_reason). - When merged onto a different email, the submitted email is noted in ``notes``. - """ - email_norm = (email or "").strip().lower() - phone = (phone or "").strip() - postal = postal_address if isinstance(postal_address, dict) else {} - has_postal = Contact.postal_address_has_content(postal) - - existing, reason = find_matching_contact( - email=email_norm, - phone=phone, - postal_address=postal if has_postal else None, - ) - - if existing is None: - contact = Contact.objects.create( - email=email_norm or None, - first_name=(first_name or "").strip(), - last_name=(last_name or "").strip(), - phone=phone, - postal_address=postal if has_postal else {}, - source=source, - notes=(notes_append or "").strip(), - ) - return contact, True, "" - + email_norm: str, + first_name: str, + last_name: str, + phone: str, + postal: dict, + has_postal: bool, + source: str, + notes_append: str, +) -> None: changed: list[str] = [] if first_name and first_name.strip(): existing.first_name = first_name.strip() @@ -199,4 +176,64 @@ def upsert_contact( fields = sorted(set(changed) | {"updated_at"}) existing.save(update_fields=fields) + +def upsert_contact( + *, + email: str, + first_name: str = "", + last_name: str = "", + phone: str = "", + postal_address: dict | None = None, + source: str = Contact.Source.CONTACT_FORM, + notes_append: str = "", + merge_phone_address: bool = True, + merge_into: Contact | None = None, +) -> tuple[Contact, bool, str]: + """ + Find or create a contact, merging on email / phone / address. + + Returns (contact, created, match_reason). + + ``merge_phone_address=False`` only merges on exact email. + ``merge_into`` forces merge into that contact row. + """ + email_norm = (email or "").strip().lower() + phone = (phone or "").strip() + postal = postal_address if isinstance(postal_address, dict) else {} + has_postal = Contact.postal_address_has_content(postal) + + if merge_into is not None: + existing, reason = merge_into, "manual" + else: + existing, reason = find_matching_contact( + email=email_norm, + phone=phone, + postal_address=postal if has_postal else None, + match_phone=merge_phone_address, + match_address=merge_phone_address, + ) + + if existing is None: + contact = Contact.objects.create( + email=email_norm or None, + first_name=(first_name or "").strip(), + last_name=(last_name or "").strip(), + phone=phone, + postal_address=postal if has_postal else {}, + source=source, + notes=(notes_append or "").strip(), + ) + return contact, True, "" + + _apply_contact_fields( + existing, + email_norm=email_norm, + first_name=first_name, + last_name=last_name, + phone=phone, + postal=postal, + has_postal=has_postal, + source=source, + notes_append=notes_append, + ) return existing, False, reason diff --git a/site/contacts/templates/contacts/create.html b/site/contacts/templates/contacts/create.html index 9130255..d95f161 100644 --- a/site/contacts/templates/contacts/create.html +++ b/site/contacts/templates/contacts/create.html @@ -4,10 +4,32 @@ {% block topbar_title %}New contact{% endblock %} {% block extra_head %} + {% endblock %} {% block portal_content %} -
+ {% csrf_token %} + +

Profile

@@ -87,14 +109,67 @@

- Matches existing contacts by email, phone, or address when possible. - Postcard defaults on when an address is saved. + Same email always updates that contact. Same phone or address asks whether to update or create new.

+ +{% if match_prompt %} + +{% endif %} {% endblock %} {% block extra_js %} +{% if match_prompt %} + +{% endif %} {% endblock %} diff --git a/site/contacts/templates/contacts/detail.html b/site/contacts/templates/contacts/detail.html index 91f4790..7039f20 100644 --- a/site/contacts/templates/contacts/detail.html +++ b/site/contacts/templates/contacts/detail.html @@ -13,12 +13,24 @@

Profile

-
-
+
+ + +
+
+ + +
-
-
+
+ + +
+
+ + +
diff --git a/site/contacts/tests_merge.py b/site/contacts/tests_merge.py index b112487..0e11313 100644 --- a/site/contacts/tests_merge.py +++ b/site/contacts/tests_merge.py @@ -1,3 +1,4 @@ +from django.contrib.auth import get_user_model from django.test import Client, TestCase from django.urls import reverse @@ -152,3 +153,69 @@ class ContactFormMergeTests(TestCase): self.assertEqual(lead.contact_id, self.existing.pk) self.assertIn("alt@example.com", lead.message) self.assertIn("merged by phone", lead.message) + + +class PortalCreateMatchPromptTests(TestCase): + def setUp(self): + User = get_user_model() + self.user = User.objects.create_user( + username="adder", password="test-pass-123" + ) + self.client = Client() + self.client.login(username="adder", password="test-pass-123") + self.existing = Contact.objects.create( + email="primary@example.com", + phone="6301112222", + first_name="Alex", + ) + + def test_phone_match_shows_prompt(self): + url = reverse("contacts:create") + response = self.client.post( + url, + { + "first_name": "Alex", + "email": "alt@example.com", + "phone": "(630) 111-2222", + "consent_email": "1", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Possible duplicate") + self.assertContains(response, "Update existing") + self.assertEqual(Contact.objects.count(), 1) + + def test_choose_create_new_keeps_both(self): + url = reverse("contacts:create") + response = self.client.post( + url, + { + "first_name": "Alex", + "email": "alt@example.com", + "phone": "(630) 111-2222", + "consent_email": "1", + "resolve_match": "create", + "match_id": str(self.existing.pk), + }, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(Contact.objects.count(), 2) + + def test_choose_update_merges(self): + url = reverse("contacts:create") + response = self.client.post( + url, + { + "first_name": "Alexander", + "email": "alt@example.com", + "phone": "(630) 111-2222", + "consent_email": "1", + "resolve_match": "update", + "match_id": str(self.existing.pk), + }, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(Contact.objects.count(), 1) + self.existing.refresh_from_db() + self.assertEqual(self.existing.first_name, "Alexander") + self.assertIn("alt@example.com", self.existing.notes) diff --git a/site/contacts/views.py b/site/contacts/views.py index 9dfd83a..01247ac 100644 --- a/site/contacts/views.py +++ b/site/contacts/views.py @@ -9,7 +9,7 @@ from django.views.decorators.http import require_GET, require_http_methods from contacts.models import Channel, ConsentRecord, Contact from contacts.nominatim import NominatimError, suggest_addresses -from contacts.services import upsert_contact +from contacts.services import find_matching_contact, upsert_contact from messaging.services import channel_preferences, set_channel_preferences @@ -70,6 +70,7 @@ def contact_create(request): "consent_sms": False, "consent_postcard": True, } + match_prompt = None if request.method == "POST": for key in list(form.keys()): if key.startswith("consent_"): @@ -77,6 +78,8 @@ def contact_create(request): else: form[key] = (request.POST.get(key) or "").strip() email = form["email"].lower() + resolve = (request.POST.get("resolve_match") or "").strip() + match_id = (request.POST.get("match_id") or "").strip() errors: list[str] = [] if not form["first_name"]: errors.append("First name is required.") @@ -92,34 +95,77 @@ def contact_create(request): if form["consent_sms"] and not form["phone"]: errors.append("Phone is required for SMS consent.") if form["consent_postcard"] and not has_postal: - # Soft: allow save but clear postcard consent if no address form["consent_postcard"] = False + if not errors: - contact, created, reason = upsert_contact( + existing, reason = find_matching_contact( email=email, - first_name=form["first_name"], - last_name=form["last_name"], phone=form["phone"], postal_address=postal if has_postal else None, - source=Contact.Source.MANUAL, - notes_append=form["notes"], ) - set_channel_preferences( - contact, - { - Channel.EMAIL: form["consent_email"], - Channel.SMS: form["consent_sms"], - Channel.POSTCARD: form["consent_postcard"], - }, - reason="portal_manual", - ) - verb = "Added" if created else f"Updated (matched by {reason or 'email'})" - messages.success(request, f"{verb} {contact}.") - return redirect("contacts:detail", pk=contact.pk) + # Phone/address collision (different email): ask user unless they chose. + if ( + existing + and reason in {"phone", "address"} + and resolve not in {"update", "create"} + ): + match_prompt = { + "contact": existing, + "reason": reason, + "reason_label": "phone number" + if reason == "phone" + else "mailing address", + } + else: + merge_into = None + merge_phone_address = True + if resolve == "update" and match_id: + merge_into = Contact.objects.filter(pk=match_id).first() + if merge_into is None: + errors.append("Matched contact no longer exists.") + elif resolve == "create": + merge_phone_address = False + elif reason == "email" and existing: + merge_into = existing + + if not errors: + contact, created, used_reason = upsert_contact( + email=email, + first_name=form["first_name"], + last_name=form["last_name"], + phone=form["phone"], + postal_address=postal if has_postal else None, + source=Contact.Source.MANUAL, + notes_append=form["notes"], + merge_phone_address=merge_phone_address, + merge_into=merge_into, + ) + set_channel_preferences( + contact, + { + Channel.EMAIL: form["consent_email"], + Channel.SMS: form["consent_sms"], + Channel.POSTCARD: form["consent_postcard"], + }, + reason="portal_manual", + ) + if created: + verb = "Added" + elif used_reason == "email": + verb = "Updated (same email)" + elif resolve == "update": + verb = f"Updated (matched by {reason or used_reason})" + else: + verb = f"Updated (matched by {used_reason or 'email'})" + messages.success(request, f"{verb} {contact}.") + return redirect("contacts:detail", pk=contact.pk) for err in errors: messages.error(request, err) - return render(request, "contacts/create.html", {"form": form}) - + return render( + request, + "contacts/create.html", + {"form": form, "match_prompt": match_prompt}, + ) @login_required @require_http_methods(["GET", "POST"]) @@ -128,9 +174,62 @@ def contact_detail(request, pk): Contact.objects.prefetch_related("consents"), pk=pk ) if request.method == "POST": + first_name = (request.POST.get("first_name") or "").strip() + last_name = (request.POST.get("last_name") or "").strip() + email = (request.POST.get("email") or "").strip().lower() + phone = (request.POST.get("phone") or "").strip() + errors: list[str] = [] + if not first_name: + errors.append("First name is required.") + if not email: + errors.append("Email is required.") + else: + try: + validate_email(email) + except ValidationError: + errors.append("Enter a valid email address.") + else: + taken = ( + Contact.objects.filter(email__iexact=email) + .exclude(pk=contact.pk) + .exists() + ) + if taken: + errors.append("Another contact already uses that email.") + if errors: + for err in errors: + messages.error(request, err) + prefs = _consent_flags(contact) + # Reflect submitted values so the user can fix them. + contact.first_name = first_name + contact.last_name = last_name + contact.email = email + contact.phone = phone + contact.postal_address = _postal_from_post(request.POST) + contact.notes = (request.POST.get("notes") or "").strip() + return render( + request, + "contacts/detail.html", + {"contact": contact, "prefs": prefs}, + ) + + contact.first_name = first_name + contact.last_name = last_name + contact.email = email + contact.phone = phone contact.postal_address = _postal_from_post(request.POST) contact.notes = (request.POST.get("notes") or "").strip() - contact.save(update_fields=["postal_address", "notes", "updated_at"]) + contact.save( + update_fields=[ + "first_name", + "last_name", + "email", + "phone", + "postal_address", + "notes", + "updated_at", + ] + ) set_channel_preferences( contact, { diff --git a/site/messaging/providers/email/smtp2go.py b/site/messaging/providers/email/smtp2go.py index 4f4f2ca..d7dc777 100644 --- a/site/messaging/providers/email/smtp2go.py +++ b/site/messaging/providers/email/smtp2go.py @@ -5,7 +5,7 @@ from django.core.mail import EmailMultiAlternatives from django.template.loader import get_template from contacts.models import Channel -from messaging.services import one_click_unsubscribe_url, preferences_url +from messaging.services import one_click_unsubscribe_url, preferences_url, render_merge_tags from public.email_branding import ( campaign_body_to_email_html, campaign_body_to_plain_text, @@ -28,6 +28,8 @@ def send_email(message) -> str: body = message.body_snapshot or campaign.body_override or ( campaign.template.body if campaign.template else "" ) + subject = render_merge_tags(subject, contact) + body = render_merge_tags(body, contact) site = (settings.PUBLIC_SITE_URL or "").rstrip("/") prefs_path = preferences_url(str(contact.pk), Channel.EMAIL) diff --git a/site/messaging/providers/sms/smtp2go.py b/site/messaging/providers/sms/smtp2go.py index 937f866..eab3d67 100644 --- a/site/messaging/providers/sms/smtp2go.py +++ b/site/messaging/providers/sms/smtp2go.py @@ -5,6 +5,8 @@ import logging import requests from django.conf import settings +from messaging.services import render_merge_tags + logger = logging.getLogger(__name__) @@ -21,6 +23,7 @@ def send_sms(message) -> str: body = message.body_snapshot or campaign.body_override or ( campaign.template.body if campaign.template else "" ) + body = render_merge_tags(body, contact) payload = { "api_key": api_key, diff --git a/site/messaging/services.py b/site/messaging/services.py index e12db32..f8a1d79 100644 --- a/site/messaging/services.py +++ b/site/messaging/services.py @@ -2,6 +2,7 @@ from __future__ import annotations +import re from datetime import datetime from typing import TYPE_CHECKING @@ -25,6 +26,49 @@ AUDIENCE_CHANNEL = { UNSUB_SALT = "monica-site-unsubscribe" UNSUB_MAX_AGE = 60 * 60 * 24 * 365 # 1 year +# {{first_name}} preferred; {first_name} also accepted (composer hint legacy). +_MERGE_TAG_RE = re.compile( + r"\{\{\s*(first_name|last_name|email|phone|full_name)\s*\}\}" + r"|\{\s*(first_name|last_name|email|phone|full_name)\s*\}", + re.IGNORECASE, +) + +REMOVABLE_MESSAGE_STATUSES = frozenset( + { + Message.Status.DRAFT, + Message.Status.SCHEDULED, + Message.Status.FAILED, + } +) + + +def render_merge_tags(text: str, contact: Contact | None) -> str: + """Replace personalization tags with contact field values.""" + if not text: + return text or "" + first = (getattr(contact, "first_name", None) or "").strip() if contact else "" + last = (getattr(contact, "last_name", None) or "").strip() if contact else "" + email = (getattr(contact, "email", None) or "").strip() if contact else "" + phone = (getattr(contact, "phone", None) or "").strip() if contact else "" + full = f"{first} {last}".strip() + values = { + "first_name": first, + "last_name": last, + "email": email, + "phone": phone, + "full_name": full, + } + + def _replace(match: re.Match[str]) -> str: + key = (match.group(1) or match.group(2) or "").lower() + return values.get(key, "") + + return _MERGE_TAG_RE.sub(_replace, text) + + +def message_is_removable(message: Message) -> bool: + return message.status in REMOVABLE_MESSAGE_STATUSES + def contact_may_receive(contact: Contact, channel: str) -> bool: if Suppression.objects.filter( @@ -443,6 +487,16 @@ def send_campaign_test_email(campaign: Campaign, to_email: str) -> None: if not body.strip(): raise ValueError("Campaign has no body.") + # Preview merge tags using first recipient when available. + sample = ( + campaign.messages.select_related("contact") + .order_by("created_at") + .first() + ) + sample_contact = sample.contact if sample else None + subject = render_merge_tags(subject, sample_contact) + body = render_merge_tags(body, sample_contact) + notice = ( "This is a test send from the Monica portal. " "Recipient list was not notified." diff --git a/site/messaging/templates/messaging/campaign_detail.html b/site/messaging/templates/messaging/campaign_detail.html index b7defb8..0a3a33a 100644 --- a/site/messaging/templates/messaging/campaign_detail.html +++ b/site/messaging/templates/messaging/campaign_detail.html @@ -39,7 +39,11 @@ {% csrf_token %}

+ {% if campaign.channel == "postcard" %} + Enqueues draft / scheduled / failed messages via PCM Integrations. + {% else %} Enqueues draft / scheduled / failed messages via SMTP2GO (dev ImmediateBackend runs inline). + {% endif %}

{% endif %} @@ -60,6 +64,7 @@
Delivered
{{ stats.delivered }}
+ {% if campaign.channel == "email" %}
Opens
{{ stats.opens }}
@@ -68,6 +73,7 @@
Clicks
{{ stats.clicks }}
+ {% endif %}
Bounced / failed
{{ stats.failed }}
@@ -82,21 +88,32 @@

Engagement

+ {% if campaign.channel == "email" %}

Unique recipients: {{ stats.opens }} opened · {{ stats.clicks }} clicked ({{ stats.open_events }} open events / {{ stats.click_events }} click events from SMTP2GO).

+ {% elif campaign.channel == "sms" %} +

+ Delivery status updates from SMTP2GO SMS webhooks. +

+ {% else %} +

+ Postcard status updates from PCM Integrations webhooks. +

+ {% endif %}
+
+
-

Recent SMTP2GO events

+

{{ events_title }}

@@ -108,7 +125,7 @@ {% empty %} - + {% endfor %}
WhenEventContact
{% if event.message %}{{ event.message.contact }}{% else %}—{% endif %}
No webhook events yet. SMTP2GO must POST opens/clicks to /portal/messaging/webhooks/email/ (see messaging README). SMTP2GO’s own “Clicked” feed does not fill this table by itself.
{{ events_empty|safe }}
@@ -116,15 +133,23 @@
-
-

Recipients

+
+
+

Recipients

+ + {{ page_obj.paginator.count }} total + {% if page_obj.paginator.num_pages > 1 %} + · page {{ page_obj.number }} of {{ page_obj.paginator.num_pages }} + {% endif %} + +
- + - {% for message in messages %} + {% for message in recipient_messages %} + {% empty %} - + {% endfor %}
ContactStatusProvider idError
ContactStatusProvider idError
{{ message.contact }}
@@ -135,24 +160,56 @@
{{ message.get_status_display }} {{ message.provider_message_id|default:"—" }} {{ message.error|truncatechars:60|default:"—" }} + {% if message.can_remove %} +
+ {% csrf_token %} + + +
+ {% else %} + + {% endif %} +
No messages on this campaign.
No messages on this campaign.
+ {% if page_obj.paginator.num_pages > 1 %} +
+ {% if page_obj.has_previous %} + ← Prev + {% endif %} + Page {{ page_obj.number }} / {{ page_obj.paginator.num_pages }} + {% if page_obj.has_next %} + Next → + {% endif %} +
+ {% endif %}
{% endblock %} {% block extra_js %}