feat: Add Brevo list update as part of migrate_firebase_users & consistent update brevo list

.github/workflows/api-dev.yml

@@ -26,7 +26,7 @@ jobs:
       OPERATIONS_OAUTH2_CLIENT_ID_1PASSWORD: "op://rbiv7rvkkrsdlpcrz3bmv7nmcu/GCP_RETOOL_OAUTH2_CREDS/username"
       WEB_APP_REVALIDATE_URL: "https://staging.mobilitydatabase.org/api/revalidate"
       WEB_APP_REVALIDATE_SECRET_1PASSWORD: "op://rbiv7rvkkrsdlpcrz3bmv7nmcu/MobilityDatabase Vercel Deployment/REVALIDATE_SECRET_QA"
-      BREVO_API_ANNOUNCEMENTS_LIST_ID: ${{ vars.BREVO_API_ANNOUNCEMENTS_LIST_ID }}
+      BREVO_API_ANNOUNCEMENTS_LIST_ID: ${{ vars.DEV_BREVO_API_ANNOUNCEMENTS_LIST_ID }}
     secrets:
       GCP_MOBILITY_FEEDS_SA_KEY: ${{ secrets.DEV_GCP_MOBILITY_FEEDS_SA_KEY }}
       OAUTH2_CLIENT_ID: ${{ secrets.DEV_MOBILITY_FEEDS_OAUTH2_CLIENT_ID}}

.github/workflows/api-prod.yml

@@ -22,7 +22,7 @@ jobs:
       OPERATIONS_OAUTH2_CLIENT_ID_1PASSWORD: "op://rbiv7rvkkrsdlpcrz3bmv7nmcu/GCP_RETOOL_OAUTH2_CREDS/username"
       WEB_APP_REVALIDATE_URL: "https://mobilitydatabase.org/api/revalidate"
       WEB_APP_REVALIDATE_SECRET_1PASSWORD: "op://rbiv7rvkkrsdlpcrz3bmv7nmcu/MobilityDatabase Vercel Deployment/REVALIDATE_SECRET"
-      BREVO_API_ANNOUNCEMENTS_LIST_ID: ${{ vars.BREVO_API_ANNOUNCEMENTS_LIST_ID }}
+      BREVO_API_ANNOUNCEMENTS_LIST_ID: ${{ vars.PROD_BREVO_API_ANNOUNCEMENTS_LIST_ID }}
     secrets:
       GCP_MOBILITY_FEEDS_SA_KEY: ${{ secrets.PROD_GCP_MOBILITY_FEEDS_SA_KEY }}
       OAUTH2_CLIENT_ID: ${{ secrets.PROD_MOBILITY_FEEDS_OAUTH2_CLIENT_ID}}

.github/workflows/api-qa.yml

@@ -22,7 +22,7 @@ jobs:
       OPERATIONS_OAUTH2_CLIENT_ID_1PASSWORD: "op://rbiv7rvkkrsdlpcrz3bmv7nmcu/GCP_RETOOL_OAUTH2_CREDS/username"
       WEB_APP_REVALIDATE_URL: "https://staging.mobilitydatabase.org/api/revalidate"
       WEB_APP_REVALIDATE_SECRET_1PASSWORD: "op://rbiv7rvkkrsdlpcrz3bmv7nmcu/MobilityDatabase Vercel Deployment/REVALIDATE_SECRET_QA"
-      BREVO_API_ANNOUNCEMENTS_LIST_ID: ${{ vars.BREVO_API_ANNOUNCEMENTS_LIST_ID }}
+      BREVO_API_ANNOUNCEMENTS_LIST_ID: ${{ vars.QA_BREVO_API_ANNOUNCEMENTS_LIST_ID }}
     secrets:
       GCP_MOBILITY_FEEDS_SA_KEY: ${{ secrets.QA_GCP_MOBILITY_FEEDS_SA_KEY }}
       OAUTH2_CLIENT_ID: ${{ secrets.DEV_MOBILITY_FEEDS_OAUTH2_CLIENT_ID}}

api/src/shared/common/brevo.py

@@ -73,17 +73,34 @@ def get_announcements_list_id() -> int:
     return int(raw)
 
 
-def add_contact_to_list(email: str, list_id: int, subscription_id: str) -> None:
-    """Create/update a Brevo contact, add it to the list, and set MDB_SUBSCRIPTION_ID.
+def add_contact_to_list(
+    email: str,
+    list_id: int,
+    subscription_id: str,
+    *,
+    first_name: str | None = None,
+    organization: str | None = None,
+) -> None:
+    """Create/update a Brevo contact, add it to the list, and set its attributes.
+
+    Always sets MDB_SUBSCRIPTION_ID. When provided, also sets the standard
+    FIRSTNAME attribute (from the user's full name) and the ORGANIZATION attribute
+    (from the user's legacy organisation name). Attributes whose value is None are
+    omitted so we never overwrite an existing Brevo value with a blank.
 
     Uses create_contact with update_enabled so it works whether or not the
     contact already exists.
     """
+    attributes = {"MDB_SUBSCRIPTION_ID": subscription_id}
+    if first_name is not None:
+        attributes["FIRSTNAME"] = first_name
+    if organization is not None:
+        attributes["ORGANIZATION"] = organization
     api = _get_contacts_api()
     api.create_contact(
         sib_api_v3_sdk.CreateContact(
             email=email,
-            attributes={"MDB_SUBSCRIPTION_ID": subscription_id},
+            attributes=attributes,
             list_ids=[list_id],
             update_enabled=True,
         ),

api/src/user_service/impl/subscription_helpers.py

@@ -16,27 +16,131 @@
 """Helpers shared between the authenticated (users) and public (subscriptions) APIs."""
 
 import logging
+from datetime import datetime, timezone
 
 from fastapi import HTTPException
 
 import sib_api_v3_sdk
 import urllib3
-from shared.common.brevo import add_contact_to_list, get_announcements_list_id, remove_contact_from_list
+from sqlalchemy.orm import Session
+from shared.common.brevo import (
+    add_contact_to_list,
+    get_announcements_list_id,
+    remove_contact_from_list,
+)
+from shared.database.database import generate_unique_id
+from shared.users_database_gen.sqlacodegen_models import (
+    AppUser,
+    NotificationSubscription as NotificationSubscriptionOrm,
+)
 
 logger = logging.getLogger(__name__)
 
 ANNOUNCEMENTS_NOTIFICATION_TYPE_ID = "api.announcements"
 
 
-def sync_announcements(email: str, subscribe: bool, subscription_id: str | None = None) -> None:
+def sync_announcements(
+    email: str,
+    subscribe: bool,
+    subscription_id: str | None = None,
+    *,
+    first_name: str | None = None,
+    organization: str | None = None,
+) -> None:
     """Sync an api.announcements subscription with Brevo, mapping provider errors to 502."""
     try:
         if subscribe:
-            add_contact_to_list(email, get_announcements_list_id(), subscription_id)
+            add_contact_to_list(
+                email,
+                get_announcements_list_id(),
+                subscription_id,
+                first_name=first_name,
+                organization=organization,
+            )
         else:
             remove_contact_from_list(email, get_announcements_list_id())
-    except (RuntimeError, sib_api_v3_sdk.rest.ApiException, urllib3.exceptions.HTTPError, OSError) as exc:
+    except (
+        RuntimeError,
+        sib_api_v3_sdk.rest.ApiException,
+        urllib3.exceptions.HTTPError,
+        OSError,
+    ) as exc:
         # urllib3.exceptions.HTTPError / OSError cover connection failures and timeouts (e.g. Brevo
         # unreachable), so the request fails fast with a 502 instead of hanging on retries.
         logger.error("Brevo sync failed for %s: %s", email, exc)
         raise HTTPException(status_code=502, detail="Failed to sync subscription with email provider.")
+
+
+def set_announcements_optin(
+    db_session: Session,
+    user: AppUser,
+    subscribe: bool,
+    *,
+    subscription: NotificationSubscriptionOrm | None = None,
+    release_connection_before_brevo: bool = False,
+) -> NotificationSubscriptionOrm | None:
+    """Reconcile a user's api.announcements opt-in across all three representations
+    that must agree: the notification_subscription row, Brevo list membership, and
+    app_user.is_registered_to_receive_api_announcements. Idempotent.
+
+    subscribe=True  → create/reactivate the subscription, add the contact to the
+                      Brevo list with its MDB_SUBSCRIPTION_ID, set the flag True.
+    subscribe=False → deactivate the subscription (api.announcements is never
+                      deleted, only disabled), remove the contact from the Brevo
+                      list, set the flag False.
+    Returns the subscription row (None only when unsubscribing a user that has no
+    subscription row).
+
+    ``subscription``: the caller's already-loaded announcements subscription, if it
+    has one (endpoints that address a subscription by id). When omitted the row is
+    looked up by user + type (there is at most one announcements subscription per
+    user).
+
+    ``release_connection_before_brevo`` commits the pending transaction before the
+    Brevo call so a slow/unreachable provider does not hold a pooled DB connection.
+    Only pass True when there are no pending writes you would want rolled back if
+    the Brevo call fails (e.g. the delete endpoints, which only read beforehand).
+    On the subscribe path the connection is always held so the whole change stays
+    atomic (a Brevo failure rolls the new subscription + flag back).
+    """
+    existing = subscription
+    if existing is None:
+        existing = (
+            db_session.query(NotificationSubscriptionOrm)
+            .filter(
+                NotificationSubscriptionOrm.user_id == user.id,
+                NotificationSubscriptionOrm.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID,
+            )
+            .one_or_none()
+        )
+
+    if subscribe:
+        sub = existing or NotificationSubscriptionOrm(
+            id=generate_unique_id(),
+            user_id=user.id,
+            notification_type_id=ANNOUNCEMENTS_NOTIFICATION_TYPE_ID,
+            created_at=datetime.now(timezone.utc),
+        )
+        sub.active = True
+        if existing is None:
+            db_session.add(sub)
+        sync_announcements(
+            user.email,
+            subscribe=True,
+            subscription_id=sub.id,
+            first_name=user.full_name,
+            organization=user.legacy_org_name,
+        )
+        user.is_registered_to_receive_api_announcements = True
+        return sub
+
+    # Unsubscribe. Capture the email before any commit so we never trigger a
+    # reload of the (possibly expired) user just to read it during the Brevo call.
+    email = user.email
+    if release_connection_before_brevo:
+        db_session.commit()
+    sync_announcements(email, subscribe=False)
+    if existing is not None:
+        existing.active = False
+    user.is_registered_to_receive_api_announcements = False
+    return existing

api/src/user_service/impl/subscriptions_api_impl.py

@@ -24,7 +24,7 @@
 )
 from user_service.impl.subscription_helpers import (
     ANNOUNCEMENTS_NOTIFICATION_TYPE_ID,
-    sync_announcements,
+    set_announcements_optin,
 )
 from user_service_gen.apis.subscriptions_api_base import BaseSubscriptionsApi
 from user_service_gen.models.notification_subscription import NotificationSubscription
@@ -55,14 +55,20 @@ def delete_subscription(self, id: str, db_session=None) -> None:
 
         if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID:
             user = db_session.get(AppUser, sub.user_id)
-            email = user.email if user is not None else None
-            # Release the pooled DB connection before the (potentially slow) Brevo call so a slow
-            # or unreachable provider never holds a connection while we talk to it. The read above
-            # took no row locks, so committing here only returns the connection to the pool.
-            db_session.commit()
-            if email is not None:
-                sync_announcements(email, subscribe=False)
-            sub.active = False
+            if user is not None:
+                # release_connection_before_brevo=True: only reads happened above,
+                # so committing just returns the pooled connection before the
+                # (possibly slow) Brevo call. Also clears the app_user opt-in flag.
+                set_announcements_optin(
+                    db_session,
+                    user,
+                    subscribe=False,
+                    subscription=sub,
+                    release_connection_before_brevo=True,
+                )
+            else:
+                # Orphaned subscription (no owning user): just disable it.
+                sub.active = False
         else:
             db_session.delete(sub)
         db_session.flush()

api/src/user_service/impl/users_api_impl.py

@@ -36,7 +36,7 @@
 )
 from user_service.impl.subscription_helpers import (
     ANNOUNCEMENTS_NOTIFICATION_TYPE_ID,
-    sync_announcements,
+    set_announcements_optin,
 )
 from user_service_gen.apis.users_api_base import BaseUsersApi
 from user_service_gen.models.create_notification_subscription_request import (
@@ -114,9 +114,28 @@ def update_user(self, update_user_request: UpdateUserRequest, db_session=None) -
             raise HTTPException(status_code=404, detail="User not found.")
 
         update_data = update_user_request.model_dump(exclude_unset=True)
+
+        # Detect a real change to the announcements opt-in so we can reconcile the
+        # subscription + Brevo membership below. Writing the boolean alone would
+        # leave the DB flag, the notification_subscription row, and Brevo out of
+        # sync (the historical bug this closes). Only act on an actual change so
+        # unrelated profile edits never make a (slow, failable) Brevo call.
+        optin_field = "is_registered_to_receive_api_announcements"
+        optin_target = None
+        if optin_field in update_data:
+            new_value = bool(update_data[optin_field])
+            if new_value != bool(user.is_registered_to_receive_api_announcements):
+                optin_target = new_value
+
         for field, value in update_data.items():
             setattr(user, field, value)
         user.updated_at = datetime.now(timezone.utc)
+
+        if optin_target is not None:
+            # Holds the DB connection through the Brevo call so the profile write
+            # and the opt-in change commit (or roll back on a 502) atomically.
+            set_announcements_optin(db_session, user, subscribe=optin_target)
+
         db_session.flush()
 
         all_flags = db_session.query(FeatureFlag).filter(FeatureFlag.disabled.is_(False)).order_by(FeatureFlag.id).all()
@@ -151,28 +170,30 @@ def create_user_subscription(
         if user is None:
             raise HTTPException(status_code=404, detail="User not found.")
 
-        # Idempotent: reuse an existing subscription, reactivating if needed.
-        existing = (
-            db_session.query(NotificationSubscriptionOrm)
-            .filter(
-                NotificationSubscriptionOrm.user_id == user_id,
-                NotificationSubscriptionOrm.notification_type_id == notification_id,
-            )
-            .one_or_none()
-        )
-        sub = existing or NotificationSubscriptionOrm(
-            id=generate_unique_id(),
-            user_id=user_id,
-            notification_type_id=notification_id,
-            created_at=datetime.now(timezone.utc),
-        )
-        sub.active = True
-
         if notification_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID:
-            sync_announcements(user.email, subscribe=True, subscription_id=sub.id)
+            # Reconcile the subscription row, Brevo membership and the app_user
+            # opt-in flag together so all three stay consistent.
+            sub = set_announcements_optin(db_session, user, subscribe=True)
+        else:
+            # Idempotent: reuse an existing subscription, reactivating if needed.
+            existing = (
+                db_session.query(NotificationSubscriptionOrm)
+                .filter(
+                    NotificationSubscriptionOrm.user_id == user_id,
+                    NotificationSubscriptionOrm.notification_type_id == notification_id,
+                )
+                .one_or_none()
+            )
+            sub = existing or NotificationSubscriptionOrm(
+                id=generate_unique_id(),
+                user_id=user_id,
+                notification_type_id=notification_id,
+                created_at=datetime.now(timezone.utc),
+            )
+            sub.active = True
+            if existing is None:
+                db_session.add(sub)
 
-        if existing is None:
-            db_session.add(sub)
         db_session.flush()
         return NotificationSubscriptionImpl.from_orm(sub)
 
@@ -187,9 +208,9 @@ def update_user_subscription(
         active = update_notification_subscription_request.active
         if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID:
             user = db_session.get(AppUser, user_id)
-            sync_announcements(user.email, subscribe=active, subscription_id=sub.id)
-
-        sub.active = active
+            set_announcements_optin(db_session, user, subscribe=active, subscription=sub)
+        else:
+            sub.active = active
         db_session.flush()
         return NotificationSubscriptionImpl.from_orm(sub)
 
@@ -203,13 +224,17 @@ def delete_user_subscription(self, id: str, db_session=None) -> None:
         sub = self._get_owned_subscription(db_session, id, user_id)
 
         if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID:
-            email = db_session.get(AppUser, user_id).email
-            # Release the pooled DB connection before the (potentially slow) Brevo call so a slow
-            # or unreachable provider never holds a connection while we talk to it. The reads above
-            # took no row locks, so committing here only returns the connection to the pool.
-            db_session.commit()
-            sync_announcements(email, subscribe=False)
-            sub.active = False
+            user = db_session.get(AppUser, user_id)
+            # release_connection_before_brevo=True: only reads happened above, so
+            # committing just returns the pooled connection before the (possibly
+            # slow) Brevo call. Also clears the app_user opt-in flag.
+            set_announcements_optin(
+                db_session,
+                user,
+                subscribe=False,
+                subscription=sub,
+                release_connection_before_brevo=True,
+            )
         else:
             db_session.delete(sub)
         db_session.flush()