From 443f8849eb8bc40090e77d9019f85d31b45ae68b Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 15 Jun 2026 14:24:03 -0400 Subject: [PATCH 1/3] feat: user subscriptions + subscription endpoints impl --- api/.openapi-generator/FILES | 2 + api/src/main.py | 2 + api/src/shared/common/brevo.py | 56 ++++- .../notification_subscription_impl.py | 24 +++ .../user_service/impl/subscription_helpers.py | 39 ++++ .../impl/subscriptions_api_impl.py | 55 +++++ api/src/user_service/impl/users_api_impl.py | 123 +++++++++-- .../test_subscriptions_api_impl.py | 139 +++++++++++++ .../user_service/test_users_api_impl.py | 194 +++++++++++++++--- docs/UserServiceAPI.yaml | 52 +++++ infra/functions-python/vars.tf | 2 +- 11 files changed, 637 insertions(+), 51 deletions(-) create mode 100644 api/src/shared/db_models/notification_subscription_impl.py create mode 100644 api/src/user_service/impl/subscription_helpers.py create mode 100644 api/src/user_service/impl/subscriptions_api_impl.py create mode 100644 api/tests/unittest/user_service/test_subscriptions_api_impl.py diff --git a/api/.openapi-generator/FILES b/api/.openapi-generator/FILES index 654dfb1ca..f1652d643 100644 --- a/api/.openapi-generator/FILES +++ b/api/.openapi-generator/FILES @@ -59,3 +59,5 @@ src/user_service_gen/models/update_notification_subscription_request.py src/user_service_gen/models/update_user_request.py src/user_service_gen/models/user_profile.py src/user_service_gen/security_api.py +src/user_service_gen/apis/subscriptions_api.py +src/user_service_gen/apis/subscriptions_api_base.py diff --git a/api/src/main.py b/api/src/main.py index 251ce72a7..15572ca67 100644 --- a/api/src/main.py +++ b/api/src/main.py @@ -27,6 +27,7 @@ from feeds_gen.apis.licenses_api import router as LicensesApiRouter from user_service_gen.apis.users_api import router as UsersApiRouter from user_service_gen.apis.notifications_api import router as NotificationsApiRouter +from user_service_gen.apis.subscriptions_api import router as SubscriptionsApiRouter # Using the starlettte implementaiton as fastapi implementation generates errors with CORS in certain situations and # returns 200 in the method response. More info, https://github.com/tiangolo/fastapi/issues/1663#issuecomment-730362611 @@ -59,6 +60,7 @@ app.include_router(LicensesApiRouter) app.include_router(UsersApiRouter) app.include_router(NotificationsApiRouter) +app.include_router(SubscriptionsApiRouter) @app.on_event("startup") diff --git a/api/src/shared/common/brevo.py b/api/src/shared/common/brevo.py index c6562b09b..7eb520aaf 100644 --- a/api/src/shared/common/brevo.py +++ b/api/src/shared/common/brevo.py @@ -40,6 +40,54 @@ class BrevoSubscriptionStatus(Enum): NOT_FOUND = "not_found" +def _get_contacts_api() -> "sib_api_v3_sdk.ContactsApi": + """Build a Brevo ContactsApi client. Raises RuntimeError if BREVO_API_KEY is unset.""" + api_key = os.getenv("BREVO_API_KEY") + if not api_key: + raise RuntimeError("BREVO_API_KEY environment variable is not set") + configuration = sib_api_v3_sdk.Configuration() + configuration.api_key["api-key"] = api_key + return sib_api_v3_sdk.ContactsApi(sib_api_v3_sdk.ApiClient(configuration)) + + +def get_announcements_list_id() -> int: + """Return the Brevo API-announcements list id from BREVO_API_ANNOUNCEMENTS_LIST_ID.""" + raw = os.getenv("BREVO_API_ANNOUNCEMENTS_LIST_ID") + if not raw: + raise RuntimeError("BREVO_API_ANNOUNCEMENTS_LIST_ID environment variable is not set") + 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. + + Uses create_contact with update_enabled so it works whether or not the + contact already exists. + """ + api = _get_contacts_api() + api.create_contact( + sib_api_v3_sdk.CreateContact( + email=email, + attributes={"MDB_SUBSCRIPTION_ID": subscription_id}, + list_ids=[list_id], + update_enabled=True, + ) + ) + + +def remove_contact_from_list(email: str, list_id: int) -> None: + """Remove a Brevo contact from the list. No-op if the contact is not on the list.""" + api = _get_contacts_api() + try: + api.remove_contact_from_list(list_id, sib_api_v3_sdk.RemoveContactFromList(emails=[email])) + except sib_api_v3_sdk.rest.ApiException as exc: + # 400 "Contact already removed from list" / 404 contact-not-found are idempotent no-ops. + if exc.status in (400, 404): + logger.info("Contact %s not on list %s, nothing to remove", email, list_id) + return + raise + + def get_contact_subscription_status( email: str, list_id: int | None = None, @@ -56,13 +104,7 @@ def get_contact_subscription_status( Raises RuntimeError if BREVO_API_KEY is not set. Raises sib_api_v3_sdk.rest.ApiException on unexpected API errors. """ - api_key = os.getenv("BREVO_API_KEY") - if not api_key: - raise RuntimeError("BREVO_API_KEY environment variable is not set") - - configuration = sib_api_v3_sdk.Configuration() - configuration.api_key["api-key"] = api_key - api = sib_api_v3_sdk.ContactsApi(sib_api_v3_sdk.ApiClient(configuration)) + api = _get_contacts_api() try: contact = api.get_contact_info(email) diff --git a/api/src/shared/db_models/notification_subscription_impl.py b/api/src/shared/db_models/notification_subscription_impl.py new file mode 100644 index 000000000..99f1908eb --- /dev/null +++ b/api/src/shared/db_models/notification_subscription_impl.py @@ -0,0 +1,24 @@ +from shared.users_database_gen.sqlacodegen_models import NotificationSubscription as NotificationSubscriptionOrm +from user_service_gen.models.notification_subscription import NotificationSubscription + + +class NotificationSubscriptionImpl(NotificationSubscription): + """Implementation of the NotificationSubscription model. + Converts a SQLAlchemy NotificationSubscription ORM object to a Pydantic NotificationSubscription model. + """ + + class Config: + from_attributes = True + + @classmethod + def from_orm(cls, sub: NotificationSubscriptionOrm | None) -> NotificationSubscription | None: + if not sub: + return None + return cls( + id=sub.id, + user_id=sub.user_id, + notification_id=sub.notification_type_id, + active=sub.active, + last_notified_at=sub.last_notified_at, + created_at=sub.created_at, + ) diff --git a/api/src/user_service/impl/subscription_helpers.py b/api/src/user_service/impl/subscription_helpers.py new file mode 100644 index 000000000..26690b9ca --- /dev/null +++ b/api/src/user_service/impl/subscription_helpers.py @@ -0,0 +1,39 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Helpers shared between the authenticated (users) and public (subscriptions) APIs.""" + +import logging + +from fastapi import HTTPException + +import sib_api_v3_sdk +from shared.common.brevo import add_contact_to_list, get_announcements_list_id, remove_contact_from_list + +logger = logging.getLogger(__name__) + +ANNOUNCEMENTS_NOTIFICATION_TYPE_ID = "api.announcements" + + +def sync_announcements(email: str, subscribe: bool, subscription_id: 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) + else: + remove_contact_from_list(email, get_announcements_list_id()) + except (RuntimeError, sib_api_v3_sdk.rest.ApiException) as exc: + logger.error("Brevo sync failed for %s: %s", email, exc) + raise HTTPException(status_code=502, detail="Failed to sync subscription with email provider.") diff --git a/api/src/user_service/impl/subscriptions_api_impl.py b/api/src/user_service/impl/subscriptions_api_impl.py new file mode 100644 index 000000000..2da017bd6 --- /dev/null +++ b/api/src/user_service/impl/subscriptions_api_impl.py @@ -0,0 +1,55 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +from fastapi import HTTPException + +from shared.database.users_database import with_users_db_session +from shared.db_models.notification_subscription_impl import NotificationSubscriptionImpl +from shared.users_database_gen.sqlacodegen_models import ( + AppUser, + NotificationSubscription as NotificationSubscriptionOrm, +) +from user_service.impl.subscription_helpers import ANNOUNCEMENTS_NOTIFICATION_TYPE_ID, sync_announcements +from user_service_gen.apis.subscriptions_api_base import BaseSubscriptionsApi +from user_service_gen.models.notification_subscription import NotificationSubscription + + +class SubscriptionsApiImpl(BaseSubscriptionsApi): + """Public, unauthenticated subscription management. + + The subscription UUID is the access capability + """ + + @with_users_db_session + def get_subscription(self, id: str, db_session=None) -> NotificationSubscription: + sub = db_session.get(NotificationSubscriptionOrm, id) + if sub is None: + raise HTTPException(status_code=404, detail="Subscription not found.") + return NotificationSubscriptionImpl.from_orm(sub) + + @with_users_db_session + def delete_subscription(self, id: str, db_session=None) -> None: + sub = db_session.get(NotificationSubscriptionOrm, id) + if sub is None: + raise HTTPException(status_code=404, detail="Subscription not found.") + + if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID: + user = db_session.get(AppUser, sub.user_id) + if user is not None: + sync_announcements(user.email, subscribe=False) + + db_session.delete(sub) + db_session.flush() diff --git a/api/src/user_service/impl/users_api_impl.py b/api/src/user_service/impl/users_api_impl.py index 7a1366bfa..c416cfee1 100644 --- a/api/src/user_service/impl/users_api_impl.py +++ b/api/src/user_service/impl/users_api_impl.py @@ -21,9 +21,16 @@ from fastapi import HTTPException from middleware.request_context import get_request_context +from shared.database.database import generate_unique_id from shared.database.users_database import with_users_db_session from shared.db_models.app_user_impl import AppUserImpl -from shared.users_database_gen.sqlacodegen_models import AppUser +from shared.db_models.notification_subscription_impl import NotificationSubscriptionImpl +from shared.users_database_gen.sqlacodegen_models import ( + AppUser, + NotificationSubscription as NotificationSubscriptionOrm, + NotificationType, +) +from user_service.impl.subscription_helpers import ANNOUNCEMENTS_NOTIFICATION_TYPE_ID, sync_announcements from user_service_gen.apis.users_api_base import BaseUsersApi from user_service_gen.models.create_notification_subscription_request import CreateNotificationSubscriptionRequest from user_service_gen.models.notification_subscription import NotificationSubscription @@ -33,8 +40,6 @@ logger = logging.getLogger(__name__) -_NOT_IMPLEMENTED = "Not yet implemented." - class UsersApiImpl(BaseUsersApi): """Implementation of the User Service users API.""" @@ -77,14 +82,7 @@ def update_user(self, update_user_request: UpdateUserRequest, db_session=None) - Email is intentionally excluded (requires re-verification). Guest users cannot update their profile. """ - context = get_request_context() - user_id: str | None = context.get("user_id") - - if not user_id: - raise HTTPException(status_code=401, detail="Unable to determine user identity from token.") - - if context.get("is_guest"): - raise HTTPException(status_code=403, detail="Guest users cannot update a profile.") + user_id = self._require_user_id() user = db_session.get(AppUser, user_id) if user is None: @@ -98,20 +96,105 @@ def update_user(self, update_user_request: UpdateUserRequest, db_session=None) - return AppUserImpl.from_orm(user) - # ── Subscription stubs — implemented in a follow-up issue ──────────────── + # ── Subscriptions ──────────────────────────────────────────────────────── - def get_user_subscriptions(self) -> List[NotificationSubscription]: - raise HTTPException(status_code=501, detail=_NOT_IMPLEMENTED) + @with_users_db_session + def get_user_subscriptions(self, db_session=None) -> List[NotificationSubscription]: + """Returns all notification subscriptions for the authenticated user.""" + user_id = self._require_user_id() + subs = ( + db_session.query(NotificationSubscriptionOrm) + .filter(NotificationSubscriptionOrm.user_id == user_id) + .order_by(NotificationSubscriptionOrm.created_at) + .all() + ) + return [NotificationSubscriptionImpl.from_orm(s) for s in subs] + @with_users_db_session def create_user_subscription( - self, create_notification_subscription_request: CreateNotificationSubscriptionRequest + self, create_notification_subscription_request: CreateNotificationSubscriptionRequest, db_session=None ) -> NotificationSubscription: - raise HTTPException(status_code=501, detail=_NOT_IMPLEMENTED) + """Subscribes the authenticated user to a notification type (idempotent).""" + user_id = self._require_user_id() + notification_id = create_notification_subscription_request.notification_id + + if db_session.get(NotificationType, notification_id) is None: + raise HTTPException(status_code=400, detail=f"Unknown notification type '{notification_id}'.") + + user = db_session.get(AppUser, user_id) + 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) + + if existing is None: + db_session.add(sub) + db_session.flush() + return NotificationSubscriptionImpl.from_orm(sub) + @with_users_db_session def update_user_subscription( - self, id: str, update_notification_subscription_request: UpdateNotificationSubscriptionRequest + self, id: str, update_notification_subscription_request: UpdateNotificationSubscriptionRequest, db_session=None ) -> NotificationSubscription: - raise HTTPException(status_code=501, detail=_NOT_IMPLEMENTED) + """Activates or deactivates a notification subscription by ID.""" + user_id = self._require_user_id() + sub = self._get_owned_subscription(db_session, id, user_id) + + 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 + db_session.flush() + return NotificationSubscriptionImpl.from_orm(sub) + + @with_users_db_session + def delete_user_subscription(self, id: str, db_session=None) -> None: + """Removes a notification subscription by ID.""" + user_id = self._require_user_id() + sub = self._get_owned_subscription(db_session, id, user_id) - def delete_user_subscription(self, id: str) -> None: - raise HTTPException(status_code=501, detail=_NOT_IMPLEMENTED) + if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID: + user = db_session.get(AppUser, user_id) + sync_announcements(user.email, subscribe=False) + + db_session.delete(sub) + db_session.flush() + + # ── Helpers ────────────────────────────────────────────────────────────── + + @staticmethod + def _require_user_id() -> str: + context = get_request_context() + user_id = context.get("user_id") + if not user_id: + raise HTTPException(status_code=401, detail="Unable to determine user identity from token.") + if context.get("is_guest"): + raise HTTPException(status_code=403, detail="Guest users cannot perform this action.") + return user_id + + @staticmethod + def _get_owned_subscription(db_session, sub_id: str, user_id: str) -> NotificationSubscriptionOrm: + sub = db_session.get(NotificationSubscriptionOrm, sub_id) + if sub is None or sub.user_id != user_id: + raise HTTPException(status_code=404, detail="Subscription not found.") + return sub diff --git a/api/tests/unittest/user_service/test_subscriptions_api_impl.py b/api/tests/unittest/user_service/test_subscriptions_api_impl.py new file mode 100644 index 000000000..e2fa5f3bc --- /dev/null +++ b/api/tests/unittest/user_service/test_subscriptions_api_impl.py @@ -0,0 +1,139 @@ +import unittest +from datetime import datetime, timezone +from unittest.mock import MagicMock, patch + +from fastapi import HTTPException + +import user_service.impl.subscription_helpers as helpers +from shared.users_database_gen.sqlacodegen_models import ( + AppUser, + NotificationSubscription as NotificationSubscriptionOrm, +) +from user_service.impl.subscriptions_api_impl import SubscriptionsApiImpl + +FIXED_NOW = datetime(2024, 1, 15, 12, 0, 0, tzinfo=timezone.utc) + + +def _make_sub(**kwargs): + defaults = dict( + id="sub-1", + user_id="uid-123", + notification_type_id="feed.published", + active=True, + created_at=FIXED_NOW, + last_notified_at=None, + ) + defaults.update(kwargs) + return NotificationSubscriptionOrm(**defaults) + + +def _make_user(email="user@example.com"): + return AppUser(id="uid-123", email=email, created_at=FIXED_NOW, updated_at=FIXED_NOW) + + +class TestPublicGetSubscription(unittest.TestCase): + def setUp(self): + self.api = SubscriptionsApiImpl() + self.mock_session = MagicMock() + + def test_returns_subscription(self): + self.mock_session.get.return_value = _make_sub( + notification_type_id="api.announcements", + active=False, + last_notified_at=FIXED_NOW, + ) + + result = self.api.get_subscription("sub-1", db_session=self.mock_session) + + self.mock_session.get.assert_called_once_with(NotificationSubscriptionOrm, "sub-1") + self.assertEqual(result.id, "sub-1") + self.assertEqual(result.user_id, "uid-123") + self.assertEqual(result.notification_id, "api.announcements") + self.assertFalse(result.active) + self.assertEqual(result.last_notified_at, FIXED_NOW) + self.assertEqual(result.created_at, FIXED_NOW) + + def test_get_does_not_touch_brevo(self): + self.mock_session.get.return_value = _make_sub(notification_type_id="api.announcements") + + with patch.object(helpers, "remove_contact_from_list") as rem, patch.object( + helpers, "add_contact_to_list" + ) as add: + self.api.get_subscription("sub-1", db_session=self.mock_session) + + rem.assert_not_called() + add.assert_not_called() + + def test_missing_returns_404(self): + self.mock_session.get.return_value = None + with self.assertRaises(HTTPException) as ctx: + self.api.get_subscription("missing", db_session=self.mock_session) + self.assertEqual(ctx.exception.status_code, 404) + + +class TestPublicDeleteSubscription(unittest.TestCase): + def setUp(self): + self.api = SubscriptionsApiImpl() + self.mock_session = MagicMock() + + def test_delete_non_announcement_no_brevo(self): + sub = _make_sub(notification_type_id="feed.published") + self.mock_session.get.return_value = sub + + with patch.object(helpers, "remove_contact_from_list") as rem: + self.api.delete_subscription("sub-1", db_session=self.mock_session) + + rem.assert_not_called() + self.mock_session.delete.assert_called_once_with(sub) + + def test_delete_announcement_removes_brevo(self): + sub = _make_sub(notification_type_id="api.announcements") + self.mock_session.get.side_effect = lambda model, key: ( + sub if model is NotificationSubscriptionOrm else _make_user() + ) + + with patch.object(helpers, "remove_contact_from_list") as rem, patch.object( + helpers, "get_announcements_list_id", return_value=42 + ): + self.api.delete_subscription("sub-1", db_session=self.mock_session) + + rem.assert_called_once_with("user@example.com", 42) + self.mock_session.delete.assert_called_once_with(sub) + + def test_delete_announcement_missing_user_skips_brevo(self): + sub = _make_sub(notification_type_id="api.announcements") + self.mock_session.get.side_effect = lambda model, key: (sub if model is NotificationSubscriptionOrm else None) + + with patch.object(helpers, "remove_contact_from_list") as rem: + self.api.delete_subscription("sub-1", db_session=self.mock_session) + + rem.assert_not_called() + self.mock_session.delete.assert_called_once_with(sub) + + def test_delete_missing_returns_404(self): + self.mock_session.get.return_value = None + with self.assertRaises(HTTPException) as ctx: + self.api.delete_subscription("missing", db_session=self.mock_session) + self.assertEqual(ctx.exception.status_code, 404) + self.mock_session.delete.assert_not_called() + + def test_delete_announcement_brevo_failure_502(self): + import sib_api_v3_sdk + + sub = _make_sub(notification_type_id="api.announcements") + self.mock_session.get.side_effect = lambda model, key: ( + sub if model is NotificationSubscriptionOrm else _make_user() + ) + + with patch.object( + helpers, "remove_contact_from_list", side_effect=sib_api_v3_sdk.rest.ApiException(status=500) + ), patch.object(helpers, "get_announcements_list_id", return_value=42): + with self.assertRaises(HTTPException) as ctx: + self.api.delete_subscription("sub-1", db_session=self.mock_session) + + self.assertEqual(ctx.exception.status_code, 502) + self.mock_session.delete.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/api/tests/unittest/user_service/test_users_api_impl.py b/api/tests/unittest/user_service/test_users_api_impl.py index 499890757..6c9091b65 100644 --- a/api/tests/unittest/user_service/test_users_api_impl.py +++ b/api/tests/unittest/user_service/test_users_api_impl.py @@ -1,13 +1,16 @@ import unittest from datetime import datetime, timezone -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from fastapi import HTTPException from middleware.request_context import _request_context from shared.db_models.app_user_impl import AppUserImpl from shared.users_database_gen.sqlacodegen_models import AppUser +import user_service.impl.subscription_helpers as helpers from user_service.impl.users_api_impl import UsersApiImpl +from user_service_gen.models.create_notification_subscription_request import CreateNotificationSubscriptionRequest +from user_service_gen.models.update_notification_subscription_request import UpdateNotificationSubscriptionRequest FIXED_NOW = datetime(2024, 1, 15, 12, 0, 0, tzinfo=timezone.utc) @@ -160,42 +163,187 @@ def test_guest_raises_403_on_update(self): self.mock_session.get.assert_not_called() -if __name__ == "__main__": - unittest.main() - - -class TestSubscriptionStubs(unittest.TestCase): +class TestSubscriptions(unittest.TestCase): def setUp(self): self.api = UsersApiImpl() + self.mock_session = MagicMock() _set_context() - def test_get_user_subscriptions_returns_501(self): - with self.assertRaises(HTTPException) as ctx: - self.api.get_user_subscriptions() - self.assertEqual(ctx.exception.status_code, 501) + def _make_sub(self, **kwargs): + from shared.users_database_gen.sqlacodegen_models import NotificationSubscription as Orm - def test_create_user_subscription_returns_501(self): - from user_service_gen.models.create_notification_subscription_request import ( - CreateNotificationSubscriptionRequest, + defaults = dict( + id="sub-1", + user_id="uid-123", + notification_type_id="feed.published", + active=True, + created_at=FIXED_NOW, + last_notified_at=None, ) + defaults.update(kwargs) + return Orm(**defaults) + + # ── list ── + def test_get_user_subscriptions_returns_user_subs(self): + sub = self._make_sub() + query = self.mock_session.query.return_value + query.filter.return_value.order_by.return_value.all.return_value = [sub] + + result = self.api.get_user_subscriptions(db_session=self.mock_session) + + self.assertEqual(len(result), 1) + self.assertEqual(result[0].id, "sub-1") + self.assertEqual(result[0].notification_id, "feed.published") + + def test_get_user_subscriptions_guest_403(self): + _set_context(is_guest=True) + with self.assertRaises(HTTPException) as ctx: + self.api.get_user_subscriptions(db_session=self.mock_session) + self.assertEqual(ctx.exception.status_code, 403) + # ── create ── + def test_create_unknown_type_400(self): + self.mock_session.get.return_value = None # NotificationType lookup with self.assertRaises(HTTPException) as ctx: - self.api.create_user_subscription(CreateNotificationSubscriptionRequest(notification_id="type-1")) - self.assertEqual(ctx.exception.status_code, 501) + self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="nope"), db_session=self.mock_session + ) + self.assertEqual(ctx.exception.status_code, 400) + + def test_create_non_announcement_no_brevo(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.published") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + with patch.object(helpers, "add_contact_to_list") as add: + result = self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.published"), db_session=self.mock_session + ) + + add.assert_not_called() + self.mock_session.add.assert_called_once() + self.assertTrue(result.active) + self.assertEqual(result.notification_id, "feed.published") + + def test_create_announcement_syncs_brevo(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="api.announcements") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + with patch.object(helpers, "add_contact_to_list") as add, patch.object( + helpers, "get_announcements_list_id", return_value=42 + ): + result = self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="api.announcements"), db_session=self.mock_session + ) + + add.assert_called_once() + self.assertEqual(add.call_args[0][0], "user@example.com") + self.assertEqual(add.call_args[0][1], 42) + self.assertEqual(result.notification_id, "api.announcements") + + def test_create_idempotent_reactivates_existing(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + existing = self._make_sub(notification_type_id="feed.published", active=False) + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.published") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = existing - def test_update_user_subscription_returns_501(self): - from user_service_gen.models.update_notification_subscription_request import ( - UpdateNotificationSubscriptionRequest, + result = self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.published"), db_session=self.mock_session ) + self.assertTrue(existing.active) + self.mock_session.add.assert_not_called() + self.assertEqual(result.id, "sub-1") + + # ── update ── + def test_update_deactivate_announcement_removes_brevo(self): + sub = self._make_sub(notification_type_id="api.announcements", active=True) + self.mock_session.get.side_effect = lambda model, key: ( + sub if "Subscription" in model.__name__ else _make_user() + ) + + with patch.object(helpers, "remove_contact_from_list") as rem, patch.object( + helpers, "get_announcements_list_id", return_value=42 + ): + result = self.api.update_user_subscription( + "sub-1", UpdateNotificationSubscriptionRequest(active=False), db_session=self.mock_session + ) + + rem.assert_called_once_with("user@example.com", 42) + self.assertFalse(result.active) + + def test_update_not_owned_404(self): + other = self._make_sub(user_id="someone-else") + self.mock_session.get.return_value = other with self.assertRaises(HTTPException) as ctx: - self.api.update_user_subscription("sub-id", UpdateNotificationSubscriptionRequest(active=True)) - self.assertEqual(ctx.exception.status_code, 501) + self.api.update_user_subscription( + "sub-1", UpdateNotificationSubscriptionRequest(active=False), db_session=self.mock_session + ) + self.assertEqual(ctx.exception.status_code, 404) + + # ── delete ── + def test_delete_announcement_removes_brevo(self): + sub = self._make_sub(notification_type_id="api.announcements") + self.mock_session.get.side_effect = lambda model, key: ( + sub if "Subscription" in model.__name__ else _make_user() + ) + + with patch.object(helpers, "remove_contact_from_list") as rem, patch.object( + helpers, "get_announcements_list_id", return_value=42 + ): + self.api.delete_user_subscription("sub-1", db_session=self.mock_session) + + rem.assert_called_once_with("user@example.com", 42) + self.mock_session.delete.assert_called_once_with(sub) + + def test_delete_non_announcement_no_brevo(self): + sub = self._make_sub(notification_type_id="feed.published") + self.mock_session.get.return_value = sub + + with patch.object(helpers, "remove_contact_from_list") as rem: + self.api.delete_user_subscription("sub-1", db_session=self.mock_session) - def test_delete_user_subscription_returns_501(self): + rem.assert_not_called() + self.mock_session.delete.assert_called_once_with(sub) + + def test_delete_not_found_404(self): + self.mock_session.get.return_value = None with self.assertRaises(HTTPException) as ctx: - self.api.delete_user_subscription("sub-id") - self.assertEqual(ctx.exception.status_code, 501) + self.api.delete_user_subscription("missing", db_session=self.mock_session) + self.assertEqual(ctx.exception.status_code, 404) + + def test_brevo_failure_raises_502(self): + import sib_api_v3_sdk + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="api.announcements") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + with patch.object( + helpers, "add_contact_to_list", side_effect=sib_api_v3_sdk.rest.ApiException(status=500) + ), patch.object(helpers, "get_announcements_list_id", return_value=42): + with self.assertRaises(HTTPException) as ctx: + self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="api.announcements"), + db_session=self.mock_session, + ) + self.assertEqual(ctx.exception.status_code, 502) + + +if __name__ == "__main__": + unittest.main() class TestAppUserImpl(unittest.TestCase): diff --git a/docs/UserServiceAPI.yaml b/docs/UserServiceAPI.yaml index d5dc980bb..5e195d9d2 100644 --- a/docs/UserServiceAPI.yaml +++ b/docs/UserServiceAPI.yaml @@ -31,6 +31,8 @@ tags: description: "User profile management" - name: "notifications" description: "Notification subscriptions" + - name: "subscriptions" + description: "Public subscription management via unguessable subscription ID (e.g. email unsubscribe links)" paths: /v1/user: @@ -222,6 +224,56 @@ paths: "501": description: Not yet implemented. + /v1/subscriptions/{id}: + get: + summary: Get a subscription by ID (unauthenticated) + description: | + Returns a single notification subscription identified by its unguessable ID. + Intended for use from email links (e.g. a "manage your subscription" page) where the + user is not authenticated — the subscription ID itself acts as the access capability. + operationId: getSubscription + tags: + - "subscriptions" + security: [] + parameters: + - name: id + in: path + required: true + schema: + type: string + description: Subscription ID. + responses: + "200": + description: Subscription retrieved. + content: + application/json: + schema: + $ref: "#/components/schemas/NotificationSubscription" + "404": + description: Subscription not found. + delete: + summary: Delete a subscription by ID (unauthenticated) + description: | + Removes a notification subscription identified by its unguessable ID. Intended for + one-click unsubscribe links in emails where the user is not authenticated — the + subscription ID itself acts as the access capability. + operationId: deleteSubscription + tags: + - "subscriptions" + security: [] + parameters: + - name: id + in: path + required: true + schema: + type: string + description: Subscription ID. + responses: + "204": + description: Subscription deleted. + "404": + description: Subscription not found. + components: schemas: UserProfile: diff --git a/infra/functions-python/vars.tf b/infra/functions-python/vars.tf index f70df30c7..8700f7cfe 100644 --- a/infra/functions-python/vars.tf +++ b/infra/functions-python/vars.tf @@ -133,5 +133,5 @@ variable "update_feed_status_schedule" { variable "brevo_api_announcements_list_id" { type = string description = "Brevo list ID for API announcements" - default = "" + default = "3" } \ No newline at end of file From 85fcfed696364a0130410c725fd0800db6b55e53 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 15 Jun 2026 14:29:23 -0400 Subject: [PATCH 2/3] fix: doc --- docs/UserServiceAPI.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/UserServiceAPI.yaml b/docs/UserServiceAPI.yaml index 5e195d9d2..2eda52e3c 100644 --- a/docs/UserServiceAPI.yaml +++ b/docs/UserServiceAPI.yaml @@ -32,7 +32,7 @@ tags: - name: "notifications" description: "Notification subscriptions" - name: "subscriptions" - description: "Public subscription management via unguessable subscription ID (e.g. email unsubscribe links)" + description: "Public subscription management via subscription ID (e.g. email unsubscribe links)" paths: /v1/user: @@ -228,7 +228,7 @@ paths: get: summary: Get a subscription by ID (unauthenticated) description: | - Returns a single notification subscription identified by its unguessable ID. + Returns a single notification subscription identified by its ID. Intended for use from email links (e.g. a "manage your subscription" page) where the user is not authenticated — the subscription ID itself acts as the access capability. operationId: getSubscription @@ -254,7 +254,7 @@ paths: delete: summary: Delete a subscription by ID (unauthenticated) description: | - Removes a notification subscription identified by its unguessable ID. Intended for + Removes a notification subscription identified by its ID. Intended for one-click unsubscribe links in emails where the user is not authenticated — the subscription ID itself acts as the access capability. operationId: deleteSubscription From c73dc06b34384f77d3f077c3795c3db02bf0d8f7 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 15 Jun 2026 14:59:53 -0400 Subject: [PATCH 3/3] fix: tf --- infra/feed-api/main.tf | 16 ++++++++++++++++ infra/feed-api/vars.tf | 5 +++++ infra/functions-python/vars.tf | 2 +- infra/main.tf | 2 ++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/infra/feed-api/main.tf b/infra/feed-api/main.tf index 0ff88e4e8..e3ff7f1bd 100644 --- a/infra/feed-api/main.tf +++ b/infra/feed-api/main.tf @@ -31,6 +31,9 @@ locals { "USERS_DATABASE_URL" = { secret_id = "${var.environment}_USERS_DATABASE_URL" } + "BREVO_API_KEY" = { + secret_id = "${var.environment}_BREVO_API_KEY" + } } # DEV and QA use the vpc connector vpc_connector_name = lower(var.environment) == "dev" ? "vpc-connector-qa" : "vpc-connector-${lower(var.environment)}" @@ -116,6 +119,19 @@ resource "google_cloud_run_v2_service" "mobility-feed-api" { } } } + env { + name = "BREVO_API_KEY" + value_source { + secret_key_ref { + secret = "${upper(var.environment)}_BREVO_API_KEY" + version = "latest" + } + } + } + env { + name = "BREVO_API_ANNOUNCEMENTS_LIST_ID" + value = var.brevo_api_announcements_list_id + } resources { limits = { cpu = "1" diff --git a/infra/feed-api/vars.tf b/infra/feed-api/vars.tf index eefc333a0..d19989ab2 100644 --- a/infra/feed-api/vars.tf +++ b/infra/feed-api/vars.tf @@ -42,4 +42,9 @@ variable "feed_api_service" { variable "feed_api_image_version" { type = string description = "Docker image version" +} + +variable "brevo_api_announcements_list_id" { + type = string + description = "Brevo list ID for API announcements" } \ No newline at end of file diff --git a/infra/functions-python/vars.tf b/infra/functions-python/vars.tf index 8700f7cfe..f70df30c7 100644 --- a/infra/functions-python/vars.tf +++ b/infra/functions-python/vars.tf @@ -133,5 +133,5 @@ variable "update_feed_status_schedule" { variable "brevo_api_announcements_list_id" { type = string description = "Brevo list ID for API announcements" - default = "3" + default = "" } \ No newline at end of file diff --git a/infra/main.tf b/infra/main.tf index 2aa37888a..888a3796b 100644 --- a/infra/main.tf +++ b/infra/main.tf @@ -96,6 +96,8 @@ module "feed-api" { feed_api_service = "feed-api" feed_api_image_version = var.feed_api_image_version + brevo_api_announcements_list_id = var.brevo_api_announcements_list_id + source = "./feed-api" }