Skip to content

Story 2436: Mailing List Subscription Post Auth #2475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
herzog0 wants to merge 8 commits into
base: develop
Choose a base branch
from
+444 −55
Open

Story 2436: Mailing List Subscription Post Auth #2475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3061f4d
feat: set post-auth modal session flag on first login
herzog0 May 28, 2026
c5df2c7
fix: enable ESC to close post-auth mailing list modal
herzog0 Jun 29, 2026
90b84b1
fix: validate email before post-auth modal subscription
herzog0 Jun 29, 2026
95b37a3
feat: support checkbox-toggled dialog modals in dialog.js/css
herzog0 Jun 29, 2026
15c69ba
feat: open and submit post-auth mailing list modal without JS
herzog0 Jun 29, 2026
1246da3
feat: redirect modal subscribe for non-HTMX (no-JS) requests
herzog0 Jun 29, 2026
6fcd20d
feat: open and submit manage subscriptions modal without JS
herzog0 Jun 29, 2026
bf23387
fix: clear scroll lock when post-auth modal closes after submit
herzog0 Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ak/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from core.templatetags.custom_static import large_static
from libraries.constants import LATEST_RELEASE_URL_PATH_STR
from libraries.mixins import ContributorMixin
from mailing_list.constants import MAILING_LIST_LABELS
from news.models import Entry
from testimonials.models import Testimonial
from core.mock_data import SharedResources
Expand Down Expand Up @@ -191,6 +192,22 @@ def get_v3_context_data(self, **kwargs):
ctx["hero_image_url"] = SharedResources.hero_image_url
ctx["hero_image_url_light"] = SharedResources.hero_image_url_light
ctx["hero_image_url_dark"] = SharedResources.hero_image_url_dark

user = self.request.user
if user.is_authenticated and self.request.session.pop(
"show_ml_post_auth_modal", False
):
user.data["ml_post_auth_seen"] = True
user.save(update_fields=["data"])
ctx["show_ml_post_auth_modal"] = True
ctx["post_auth_modal_subscribe_url"] = reverse(
"mailing-list-post-auth-subscribe"
)
ctx["post_auth_modal_mailing_lists"] = [
{**v} for v in MAILING_LIST_LABELS.values()
]
ctx["post_auth_modal_user_email"] = user.email

return ctx


Expand Down
6 changes: 6 additions & 0 deletions mailing_list/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from mailing_list.views import ConfirmSubscriptionView
from mailing_list.views import ModalSubscribeView
from mailing_list.views import PostAuthSubscribeView
from mailing_list.views import QuickSubscribeView
from mailing_list.views import SubscribeView

Expand All @@ -17,6 +18,11 @@
ModalSubscribeView.as_view(),
name="mailing-list-modal-subscribe",
),
path(
"post-auth-subscribe/",
PostAuthSubscribeView.as_view(),
name="mailing-list-post-auth-subscribe",
),
path(
"confirm/<str:token>/",
ConfirmSubscriptionView.as_view(),
Expand Down
121 changes: 94 additions & 27 deletions mailing_list/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
from django.contrib.auth.mixins import LoginRequiredMixin
from django.core import signing
from django.core.cache import cache
from django.db import IntegrityError, transaction
from django.core.mail import send_mail
from django.http import HttpResponseRedirect
from django.db import IntegrityError, transaction
from django.http import HttpResponse, HttpResponseRedirect
from django.shortcuts import render
from django.template.loader import render_to_string
from django.urls import reverse
Expand Down Expand Up @@ -83,6 +83,43 @@ def _is_rate_limited(request) -> bool:
return cache.incr(key) > _SUBSCRIBE_RATE_LIMIT


def _subscribe_pending(
request, user, email: str, list_ids: list[str]
) -> tuple[list[str], str | None]:
"""Create PENDING subscription records and send a confirmation email.

Returns (succeeded, error_message). On email failure the records are
rolled back and error_message is set; on partial IntegrityError the
affected list is silently skipped.
"""
succeeded = []
for lid in list_ids:
try:
with transaction.atomic():
UserMailingListSubscription.objects.update_or_create(
user=user,
list_id=lid,
defaults={"email": email, "status": SubscriptionStatus.PENDING},
)
succeeded.append(lid)
except IntegrityError:
pass

if not succeeded:
return [], None

try:
_send_confirmation_email(request, email, user.pk, succeeded)
except Exception as exc:
logger.error("Failed to send confirmation email to %s: %s", email, exc)
UserMailingListSubscription.objects.filter(
user=user, list_id__in=succeeded
).delete()
Comment on lines +95 to +117

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Roll back only rows created by this request.

The rollback path deletes every UserMailingListSubscription matching user/list_id__in=succeeded, but update_or_create() does not guarantee those rows were created here. A duplicate submit race can hit created=False, and an email failure in this request would then delete the other request’s pending row. Track the created PKs (or the created flag) and only delete those on rollback.

Suggested fix 
 def _subscribe_pending(
     request, user, email: str, list_ids: list[str]
 ) -> tuple[list[str], str | None]:
@@
-    succeeded = []
+    succeeded = []
+    created_pks = []
     for lid in list_ids:
         try:
             with transaction.atomic():
-                UserMailingListSubscription.objects.update_or_create(
+                sub, created = UserMailingListSubscription.objects.update_or_create(
                     user=user,
                     list_id=lid,
                     defaults={"email": email, "status": SubscriptionStatus.PENDING},
                 )
+            if created:
+                created_pks.append(sub.pk)
             succeeded.append(lid)
         except IntegrityError:
             pass
@@
     try:
         _send_confirmation_email(request, email, user.pk, succeeded)
     except Exception as exc:
         logger.error("Failed to send confirmation email to %s: %s", email, exc)
-        UserMailingListSubscription.objects.filter(
-            user=user, list_id__in=succeeded
-        ).delete()
+        UserMailingListSubscription.objects.filter(pk__in=created_pks).delete()
         return [], "Could not send confirmation email. Please try again."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
succeeded = []
for lid in list_ids:
try:
with transaction.atomic():
UserMailingListSubscription.objects.update_or_create(
user=user,
list_id=lid,
defaults={"email": email, "status": SubscriptionStatus.PENDING},
)
succeeded.append(lid)
except IntegrityError:
pass
if not succeeded:
return [], None
try:
_send_confirmation_email(request, email, user.pk, succeeded)
except Exception as exc:
logger.error("Failed to send confirmation email to %s: %s", email, exc)
UserMailingListSubscription.objects.filter(
user=user, list_id__in=succeeded
).delete()
succeeded = []
created_pks = []
for lid in list_ids:
try:
with transaction.atomic():
sub, created = UserMailingListSubscription.objects.update_or_create(
user=user,
list_id=lid,
defaults={"email": email, "status": SubscriptionStatus.PENDING},
)
if created:
created_pks.append(sub.pk)
succeeded.append(lid)
except IntegrityError:
pass
if not succeeded:
return [], None
try:
_send_confirmation_email(request, email, user.pk, succeeded)
except Exception as exc:
logger.error("Failed to send confirmation email to %s: %s", email, exc)
UserMailingListSubscription.objects.filter(pk__in=created_pks).delete()
return [], "Could not send confirmation email. Please try again."
🧰 Tools
🪛 Ruff (0.15.18)

[warning] 113-113: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mailing_list/views.py` around lines 95 - 117, The rollback in the
confirmation-email failure path is deleting all subscriptions in succeeded, but
some of those rows may have already existed before this request. In the
UserMailingListSubscription update_or_create flow, capture the created flag or
primary key for each list_id inside the transaction, store only rows actually
created by this request, and use that tracked set in the exception handler
instead of filtering by user and list_id__in=succeeded. Keep the fix localized
to the subscription creation loop and the _send_confirmation_email rollback
block in mailing_list/views.py.

return [], "Could not send confirmation email. Please try again."

return succeeded, None


def _send_confirmation_email(
request, email: str, user_id: int | None, list_ids: list[str]
) -> None:
Expand Down Expand Up @@ -520,9 +557,27 @@ class ModalSubscribeView(View):
any currently tracked lists that were unchecked.
Anonymous flow: subscribe-only - sends a single confirmation email for all
checked lists. Unsubscribe not supported for anonymous users.

With JS the form posts via HTMX and the card partial is swapped in place.
Without JS the form posts natively; we redirect (PRG) back to the page so
the card re-renders from DB state on the next GET.
"""

def _card(self, request, **ctx):
if not _is_htmx(request):
state = ctx.get("state")
if state == "error":
return _prg_redirect(
request,
ml_state="error",
ml_error=ctx.get("error_message", ""),
ml_email=ctx.get("user_email", ""),
)
if state == "pending" and not request.user.is_authenticated:
return _prg_redirect(
request, ml_state="pending", ml_email=ctx.get("user_email", "")
)
return _prg_redirect(request)
return render(
request,
"v3/includes/_mailing_list_card.html",
Expand Down Expand Up @@ -594,38 +649,20 @@ def _handle_authenticated(self, request, email, list_ids, managed_lists):
manage_url=manage_url,
)

succeeded = []
for lid in to_subscribe:
try:
with transaction.atomic():
UserMailingListSubscription.objects.update_or_create(
user=request.user,
list_id=lid,
defaults={"email": email, "status": SubscriptionStatus.PENDING},
)
succeeded.append(lid)
except IntegrityError:
pass
succeeded, error = _subscribe_pending(
request, request.user, email, to_subscribe
)

if not succeeded:
if error:
return self._card(
request,
state="error",
error_message="Could not subscribe. Please try again.",
user_email=email,
request, state="error", error_message=error, user_email=email
)

try:
_send_confirmation_email(request, email, request.user.pk, succeeded)
except Exception as exc:
logger.error("Failed to send confirmation email to %s: %s", email, exc)
UserMailingListSubscription.objects.filter(
user=request.user, list_id__in=succeeded
).delete()
if not succeeded:
return self._card(
request,
state="error",
error_message="Could not send confirmation email. Please try again.",
error_message="Could not subscribe. Please try again.",
user_email=email,
)

Expand Down Expand Up @@ -657,3 +694,33 @@ def _handle_anonymous(self, request, email, list_ids):
)

return self._card(request, state="pending", user_email=email)


class PostAuthSubscribeView(LoginRequiredMixin, View):
"""Subscribe to one or more lists from the post-login homepage modal.

Only for authenticated users. Returns an empty fragment so HTMX removes
the modal from the DOM. Falls back to a homepage redirect for non-HTMX.
"""

def post(self, request):
email = (request.POST.get("email") or "").strip() or request.user.email
managed_lists = set(constants.MAILMAN_LISTS)
list_ids = [
lid for lid in request.POST.getlist("list_id") if lid in managed_lists
]

if list_ids and not _is_rate_limited(request):
current = {
sub.list_id
for sub in UserMailingListSubscription.objects.filter(
user=request.user, list_id__in=managed_lists
)
}
to_subscribe = [lid for lid in list_ids if lid not in current]

_subscribe_pending(request, request.user, email, to_subscribe)

if _is_htmx(request):
return HttpResponse("")
Comment on lines +713 to +725

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Don’t close the modal on failed subscriptions.

This branch swallows both rate-limit failures and _subscribe_pending() errors, then still returns an empty HTMX response. Since HomepageView.get_v3_context_data() already persists user.data["ml_post_auth_seen"] = True, any transient mail/backend failure becomes a silent one-shot no-op for the user. Keep the modal open and surface an error state before dismissing it.

🧰 Tools
🪛 ast-grep (0.44.0)

[error] 706-706: Lack of sanitization of user data
Context: HttpResponse("")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(http-response-from-request)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mailing_list/views.py` around lines 695 - 707, The HTMX branch in the
subscription flow is returning an empty response even when
`_is_rate_limited(request)` blocks the action or `_subscribe_pending()` fails,
which causes the modal to close on a silent failure. Update the subscription
handler around the `list_ids`, `_is_rate_limited`, and `_subscribe_pending`
logic so it detects these failure cases, keeps the modal open, and returns an
error state/message instead of `HttpResponse("")` when the subscription did not
succeed. Ensure the `_is_htmx(request)` response path only dismisses the modal
after a successful subscribe.

return _prg_redirect(request)
13 changes: 12 additions & 1 deletion static/css/v3/dialog.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,19 @@
display: flex !important;
}

/* Checkbox-driven open (no-JS auto-open / toggle): a checked .dialog-modal__toggle
shows the adjacent .dialog-modal, mirroring the :target behaviour. */
.dialog-modal__toggle {
display: none;
}

.dialog-modal__toggle:checked ~ .dialog-modal {
display: flex !important;
}

/* Lock background page scroll while any dialog is open. */
html:has(.dialog-modal:target) {
html:has(.dialog-modal:target),
html:has(.dialog-modal__toggle:checked) {
overflow: hidden;
}

Expand Down
121 changes: 121 additions & 0 deletions static/css/v3/mailing-list-card.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,124 @@ input.mailing-list-modal__checkbox:focus-visible + .mailing-list-modal__checkbox
line-height: var(--line-height-default);
color: var(--color-text-secondary);
}

/* ============================================
POST-AUTH MODAL — first-login subscription prompt
============================================ */

/* Header contains only the title (no close X per design) */
.ml-post-auth-modal__header {
padding: var(--space-large) var(--space-large) 0 var(--space-large);
width: 100%;
box-sizing: border-box;
}

.ml-post-auth-modal__header .dialog-modal__title {
margin: 0;
}

/* Email input row */
.ml-post-auth-modal__email-row {
padding: 0 var(--space-large);
width: 100%;
box-sizing: border-box;
}

.ml-post-auth-modal__email-field {
display: block;
width: 100%;
height: 40px;
padding: 0 var(--space-large);
box-sizing: border-box;
background-color: var(--color-surface-weak) !important;
border: 1px solid var(--color-stroke-weak) !important;
border-radius: var(--border-radius-xl) !important;
font-family: var(--font-sans);
font-size: var(--font-size-base);
font-weight: var(--font-weight-regular);
line-height: var(--line-height-default);
letter-spacing: var(--letter-spacing-tight);
color: var(--color-text-secondary);
outline: none;
box-shadow: none;
-webkit-appearance: none;
appearance: none;
}

.ml-post-auth-modal__email-field::placeholder {
color: var(--color-text-secondary);
}

.ml-post-auth-modal__email-field:focus-visible {
background-color: var(--color-surface-mid);
border-color: var(--color-stroke-strong);
}

.ml-post-auth-modal__email-field--error {
background-color: var(--color-surface-error-weak) !important;
border-color: var(--color-stroke-error) !important;
color: var(--color-text-error);
}
Comment on lines +254 to +263

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Invalid email state hides the keyboard focus indicator.

outline: none removes the native ring, and the --error rule’s !important border/background override the :focus-visible styling. Once validation fails, the focused field no longer has a clear focus cue.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@static/css/v3/mailing-list-card.css` around lines 254 - 263, The email
field’s invalid state is overriding the keyboard focus cue, so fix the
interaction between `.ml-post-auth-modal__email-field:focus-visible` and
`.ml-post-auth-modal__email-field--error`. Keep a visible focus indicator when
the field is both focused and in error by adding a more specific focused-error
state or adjusting the error styles so they don’t suppress the focus ring.
Update the focus styling in this CSS module to ensure the focus-visible cue
remains clear even when validation errors are present.


.ml-post-auth-modal__email-row .field__error {
margin-top: var(--space-s);
}

.ml-post-auth-modal__container .dialog-modal__buttons {
padding-top: var(--space-large);
}

/* Description + list card section */
.ml-post-auth-modal__body {
display: flex;
flex-direction: column;
gap: var(--space-default);
width: 100%;
box-sizing: border-box;
}

.ml-post-auth-modal__description {
padding: 0 var(--space-large);
margin: 0;
font-family: var(--font-sans);
font-size: var(--font-size-base);
font-weight: var(--font-weight-regular);
line-height: var(--line-height-default);
letter-spacing: var(--letter-spacing-tight);
color: var(--color-text-secondary);
}

.ml-post-auth-modal__list-section {
display: flex;
flex-direction: column;
gap: var(--space-large);
padding: 0 var(--space-large) var(--space-large);
width: 100%;
box-sizing: border-box;
}

/* Override the bottom margin that .mailing-list-modal__list-card applies
since margin is handled by the parent flex gap here */
.ml-post-auth-modal__list-card {
margin: 0;
}

/* Unselect All link + "X of Y selected" counter */
.ml-post-auth-modal__footer-row {
display: flex;
flex-direction: row;
align-items: center;
justify-content: space-between;
width: 100%;
box-sizing: border-box;
font-size: var(--font-size-small);
line-height: var(--line-height-relaxed);
letter-spacing: var(--letter-spacing-tight);
white-space: nowrap;
}

.ml-post-auth-modal__count {
font-family: var(--font-sans);
font-weight: var(--font-weight-regular);
color: var(--color-text-secondary);
}
Loading
Loading