diff --git a/api/src/shared/database/users_database.py b/api/src/shared/database/users_database.py index 397545457..b573bc2b4 100644 --- a/api/src/shared/database/users_database.py +++ b/api/src/shared/database/users_database.py @@ -119,11 +119,17 @@ def wrapper(*args, **kwargs): @event.listens_for(mapper, "mapper_configured") def _configure_users_passive_deletes(mapper_, class_): - """Enable ``passive_deletes`` on ``NotificationSubscription.notification_logs``. + """Enable ``delete-orphan`` + ``passive_deletes`` on ``NotificationSubscription`` children. - Deleting a subscription then relies on the database ``ON DELETE CASCADE`` to remove its - ``notification_log`` rows, instead of SQLAlchemy trying to NULL the NOT NULL - ``notification_log.subscription_id`` (which would raise a NotNullViolation). + Applies to both child collections: + * ``notification_logs`` — delivery history rows. + * ``notification_subscription_feeds`` — feed-scoped subscription targets (issue #1778). + + Deleting a subscription then relies on the database ``ON DELETE CASCADE`` to remove these + rows, instead of SQLAlchemy trying to NULL the NOT NULL child foreign keys + (``notification_log.subscription_id`` / ``notification_subscription_feed.subscription_id``), + which would raise a NotNullViolation. ``delete-orphan`` additionally lets a feed be dropped + from a subscription simply by removing it from the in-memory collection. This mirrors the ``set_cascade`` mechanism in ``shared.database.database`` but is scoped to the users models, so ``users_database`` stays independent of the feeds ``database_gen``. @@ -132,7 +138,9 @@ def _configure_users_passive_deletes(mapper_, class_): module does not require ``shared.users_database_gen`` to be present (functions that symlink ``shared`` without the users models must still be able to import ``users_database``). """ - if class_.__name__ == "NotificationSubscription" and "notification_logs" in mapper_.relationships: - rel = mapper_.relationships["notification_logs"] - rel.cascade = "all, delete-orphan" - rel.passive_deletes = True + if class_.__name__ == "NotificationSubscription": + for rel_name in ("notification_logs", "notification_subscription_feeds"): + if rel_name in mapper_.relationships: + rel = mapper_.relationships[rel_name] + rel.cascade = "all, delete-orphan" + rel.passive_deletes = True diff --git a/api/src/shared/db_models/notification_subscription_impl.py b/api/src/shared/db_models/notification_subscription_impl.py index cec6ae93d..4c61fe1c8 100644 --- a/api/src/shared/db_models/notification_subscription_impl.py +++ b/api/src/shared/db_models/notification_subscription_impl.py @@ -14,10 +14,15 @@ class Config: def from_orm(cls, sub: NotificationSubscriptionOrm | None) -> NotificationSubscription | None: if not sub: return None + # feed_ids is the set of feed stable IDs this subscription targets, taken from the + # notification_subscription_feed join table. Sorted for a stable response; None when + # the subscription targets no feeds (i.e. non feed-scoped types). + feed_ids = sorted(f.feed_stable_id for f in sub.notification_subscription_feeds) return cls( id=sub.id, user_id=sub.user_id, notification_id=sub.notification_type_id, active=sub.active, created_at=sub.created_at, + feed_ids=feed_ids or None, ) diff --git a/api/src/shared/db_models/notification_type_impl.py b/api/src/shared/db_models/notification_type_impl.py new file mode 100644 index 000000000..c39348ae5 --- /dev/null +++ b/api/src/shared/db_models/notification_type_impl.py @@ -0,0 +1,20 @@ +from shared.users_database_gen.sqlacodegen_models import NotificationType as NotificationTypeOrm +from user_service_gen.models.notification_type import NotificationType + + +class NotificationTypeImpl(NotificationType): + """Implementation of the NotificationType model. + Converts a SQLAlchemy NotificationType ORM object to a Pydantic NotificationType model. + """ + + class Config: + from_attributes = True + + @classmethod + def from_orm(cls, notification_type: NotificationTypeOrm | None) -> NotificationType | None: + if not notification_type: + return None + return cls( + id=notification_type.id, + description=notification_type.description, + ) diff --git a/api/src/user_service/impl/notifications_api_impl.py b/api/src/user_service/impl/notifications_api_impl.py index 4f6722053..4491e9b30 100644 --- a/api/src/user_service/impl/notifications_api_impl.py +++ b/api/src/user_service/impl/notifications_api_impl.py @@ -15,16 +15,33 @@ # from typing import List -from fastapi import HTTPException - +from middleware.request_context import get_request_context +from shared.database.users_database import with_users_db_session +from shared.db_models.notification_type_impl import NotificationTypeImpl +from shared.users_database_gen.sqlacodegen_models import NotificationType as NotificationTypeOrm +from user_service.impl.subscription_helpers import ( + ADMIN_EVENT_SUMMARY_NOTIFICATION_TYPE_ID, + ADMIN_SUMMARY_FEATURE_FLAG_ID, + feature_flag_enabled, +) from user_service_gen.apis.notifications_api_base import BaseNotificationsApi from user_service_gen.models.notification_type import NotificationType -_NOT_IMPLEMENTED = "Not yet implemented." - class NotificationsApiImpl(BaseNotificationsApi): - """Stub implementation — scheduled for a follow-up issue.""" + """Implementation of the User Service notifications API.""" + + @with_users_db_session + def get_notifications(self, db_session=None) -> List[NotificationType]: + """Returns the notification types the caller can subscribe to, ordered by id. + + ``admin.event_summary`` is only included for users with the + ``isAdminSummarySubscriptionEnabled`` flag, mirroring the create/update gate. + """ + types = db_session.query(NotificationTypeOrm).order_by(NotificationTypeOrm.id).all() + + user_id = get_request_context().get("user_id") + if not (user_id and feature_flag_enabled(db_session, user_id, ADMIN_SUMMARY_FEATURE_FLAG_ID)): + types = [t for t in types if t.id != ADMIN_EVENT_SUMMARY_NOTIFICATION_TYPE_ID] - def get_notifications(self) -> List[NotificationType]: - raise HTTPException(status_code=501, detail=_NOT_IMPLEMENTED) + return [NotificationTypeImpl.from_orm(t) for t in types] diff --git a/api/src/user_service/impl/subscription_helpers.py b/api/src/user_service/impl/subscription_helpers.py index decb419f9..212ff5579 100644 --- a/api/src/user_service/impl/subscription_helpers.py +++ b/api/src/user_service/impl/subscription_helpers.py @@ -28,16 +28,92 @@ get_announcements_list_id, remove_contact_from_list, ) -from shared.database.database import generate_unique_id +from shared.database.database import Database, generate_unique_id +from shared.database_gen.sqlacodegen_models import Feed from shared.users_database_gen.sqlacodegen_models import ( AppUser, + FeatureFlag, NotificationSubscription as NotificationSubscriptionOrm, + UserFeatureFlag, ) logger = logging.getLogger(__name__) +# Shared 403 detail returned when a user lacks the feature flag gating a subscription action. +ERROR_MESSAGE_USER_FEATURE_NOT_ENABLED = "This feature is not enabled for the user." + ANNOUNCEMENTS_NOTIFICATION_TYPE_ID = "api.announcements" +# Feature flag (feature_flag.id) that gates notification-subscription management: a user must +# have this boolean flag resolved to true to create, update or delete subscriptions. +NOTIFICATIONS_FEATURE_FLAG_ID = "isNotificationsEnabled" + +# The admin dispatch-summary notification type and the additional feature flag required to +# subscribe to (or manage a subscription for) it. This is layered on top of the general +# NOTIFICATIONS_FEATURE_FLAG_ID gate. +ADMIN_EVENT_SUMMARY_NOTIFICATION_TYPE_ID = "admin.event_summary" +ADMIN_SUMMARY_FEATURE_FLAG_ID = "isAdminSummarySubscriptionEnabled" + +# Notification types that are scoped to specific feeds. A subscription to any of +# these must carry a non-empty list of feed stable IDs (persisted in the +# notification_subscription_feed join table); other types must not carry feeds. +FEED_SCOPED_NOTIFICATION_TYPE_IDS = frozenset( + { + "feed.url_updated", + "feed.url_availability", + "feed.coverage", + } +) + + +def feature_flag_enabled(db_session, user_id: str, flag_id: str) -> bool: + """Resolve a boolean feature flag for a user. + + The user's override wins, falling back to the flag's default value; a missing flag denies + (nothing to resolve). The ``disabled`` column is deliberately NOT consulted here: it only + controls consumer-facing *exposure* (whether the flag is surfaced in the user profile's + ``features`` list) — it is a backend concern and must not block a user who has been granted + access from subscribing. Otherwise a disabled-but-granted flag would leave no way to subscribe. + """ + flag = db_session.get(FeatureFlag, flag_id) + if flag is None: + logger.debug("feature flag %r not found; denying user %s", flag_id, user_id) + return False + override = db_session.get(UserFeatureFlag, (user_id, flag_id)) + value = override.value if override is not None and override.value is not None else flag.default_value + enabled = value is True + logger.debug( + "feature flag %r for user %s: override=%r default=%r resolved=%r enabled=%s", + flag_id, + user_id, + getattr(override, "value", None), + flag.default_value, + value, + enabled, + ) + return enabled + + +def find_unknown_feed_ids(feed_ids, feeds_db_session=None) -> list: + """Return the supplied feed stable IDs that do not exist as a ``Feed`` in the feeds DB. + + Order is preserved and duplicates collapsed. Feeds live in a separate database from + subscriptions, so this opens its own feeds-DB session unless one is injected (tests). + """ + unique_ids = list(dict.fromkeys(feed_ids or [])) + if not unique_ids: + return [] + + def _missing(session) -> list: + rows = session.query(Feed.stable_id).filter(Feed.stable_id.in_(unique_ids)).all() + existing = {stable_id for (stable_id,) in rows} + return [feed_id for feed_id in unique_ids if feed_id not in existing] + + if feeds_db_session is not None: + return _missing(feeds_db_session) + with Database().start_db_session() as session: + return _missing(session) + def sync_announcements( email: str, diff --git a/api/src/user_service/impl/subscriptions_api_impl.py b/api/src/user_service/impl/subscriptions_api_impl.py index 83c010767..a0020ba55 100644 --- a/api/src/user_service/impl/subscriptions_api_impl.py +++ b/api/src/user_service/impl/subscriptions_api_impl.py @@ -14,6 +14,8 @@ # limitations under the License. # +import logging + from fastapi import HTTPException from shared.database.users_database import with_users_db_session @@ -24,11 +26,16 @@ ) from user_service.impl.subscription_helpers import ( ANNOUNCEMENTS_NOTIFICATION_TYPE_ID, + ERROR_MESSAGE_USER_FEATURE_NOT_ENABLED, + NOTIFICATIONS_FEATURE_FLAG_ID, + feature_flag_enabled, set_announcements_optin, ) from user_service_gen.apis.subscriptions_api_base import BaseSubscriptionsApi from user_service_gen.models.notification_subscription import NotificationSubscription +logger = logging.getLogger(__name__) + class SubscriptionsApiImpl(BaseSubscriptionsApi): """Public, unauthenticated subscription management. @@ -53,6 +60,18 @@ def delete_subscription(self, id: str, db_session=None) -> None: if sub is None: raise HTTPException(status_code=404, detail="Subscription not found.") + # Gated by the subscription owner's isNotificationsEnabled flag (this endpoint is + # unauthenticated; the subscription UUID is the capability, so we resolve the flag for + # the subscription's owner rather than a request user). + if not feature_flag_enabled(db_session, sub.user_id, NOTIFICATIONS_FEATURE_FLAG_ID): + logger.info( + "Public delete denied for subscription %s (owner %s): feature flag %r not enabled.", + id, + sub.user_id, + NOTIFICATIONS_FEATURE_FLAG_ID, + ) + raise HTTPException(status_code=403, detail=ERROR_MESSAGE_USER_FEATURE_NOT_ENABLED) + if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID: user = db_session.get(AppUser, sub.user_id) if user is not None: diff --git a/api/src/user_service/impl/users_api_impl.py b/api/src/user_service/impl/users_api_impl.py index eaa2b7394..4d7ceced3 100644 --- a/api/src/user_service/impl/users_api_impl.py +++ b/api/src/user_service/impl/users_api_impl.py @@ -32,10 +32,18 @@ FeatureFlag, UserFeatureFlag, NotificationSubscription as NotificationSubscriptionOrm, + NotificationSubscriptionFeed as NotificationSubscriptionFeedOrm, NotificationType, ) from user_service.impl.subscription_helpers import ( + ADMIN_EVENT_SUMMARY_NOTIFICATION_TYPE_ID, + ADMIN_SUMMARY_FEATURE_FLAG_ID, ANNOUNCEMENTS_NOTIFICATION_TYPE_ID, + ERROR_MESSAGE_USER_FEATURE_NOT_ENABLED, + FEED_SCOPED_NOTIFICATION_TYPE_IDS, + NOTIFICATIONS_FEATURE_FLAG_ID, + feature_flag_enabled, + find_unknown_feed_ids, set_announcements_optin, ) from user_service_gen.apis.users_api_base import BaseUsersApi @@ -149,6 +157,7 @@ def get_user_subscriptions(self, db_session=None) -> List[NotificationSubscripti user_id = self._require_user_id() subs = ( db_session.query(NotificationSubscriptionOrm) + .options(selectinload(NotificationSubscriptionOrm.notification_subscription_feeds)) .filter(NotificationSubscriptionOrm.user_id == user_id) .order_by(NotificationSubscriptionOrm.created_at) .all() @@ -159,13 +168,48 @@ def get_user_subscriptions(self, db_session=None) -> List[NotificationSubscripti def create_user_subscription( self, create_notification_subscription_request: CreateNotificationSubscriptionRequest, db_session=None ) -> NotificationSubscription: - """Subscribes the authenticated user to a notification type (idempotent).""" + """Subscribes the authenticated user to a notification type (idempotent). + + For feed-scoped notification types (``feed.url_updated``, ``feed.url_availability``, + ``feed.coverage``) a non-empty ``feed_ids`` list is required and is persisted in the + ``notification_subscription_feed`` join table; the single (user, type) subscription's + feed set is replaced with the supplied feeds. ``feed_ids`` must not be supplied for + other notification types. + """ user_id = self._require_user_id() + self._require_notifications_enabled(db_session, user_id) notification_id = create_notification_subscription_request.notification_id + # De-duplicate while preserving order; treat null as no feeds. + feed_ids = list(dict.fromkeys(create_notification_subscription_request.feed_ids or [])) if db_session.get(NotificationType, notification_id) is None: raise HTTPException(status_code=400, detail=f"Unknown notification type '{notification_id}'.") + # The admin dispatch-summary type is gated by an additional feature flag. + if notification_id == ADMIN_EVENT_SUMMARY_NOTIFICATION_TYPE_ID: + self._require_admin_summary_enabled(db_session, user_id) + + # Feed scoping is validated in code (the OpenAPI 3.0 schema keeps feed_ids optional). + is_feed_scoped = notification_id in FEED_SCOPED_NOTIFICATION_TYPE_IDS + if is_feed_scoped and not feed_ids: + raise HTTPException( + status_code=400, + detail=f"feed_ids is required for notification type '{notification_id}'.", + ) + if not is_feed_scoped and feed_ids: + raise HTTPException( + status_code=400, + detail=f"feed_ids is not supported for notification type '{notification_id}'.", + ) + + # Every supplied feed must exist (feeds live in a separate DB, referenced by stable_id). + unknown_feed_ids = find_unknown_feed_ids(feed_ids) + if unknown_feed_ids: + raise HTTPException( + status_code=400, + detail=f"Unknown feed stable IDs: {', '.join(unknown_feed_ids)}.", + ) + user = db_session.get(AppUser, user_id) if user is None: raise HTTPException(status_code=404, detail="User not found.") @@ -191,6 +235,9 @@ def create_user_subscription( created_at=datetime.now(timezone.utc), ) sub.active = True + # Replace the subscription's feed set with the supplied feeds (no-op for non feed-scoped + # types, whose feed_ids is always empty here). + self._sync_subscription_feeds(sub, feed_ids) if existing is None: db_session.add(sub) @@ -203,8 +250,13 @@ def update_user_subscription( ) -> NotificationSubscription: """Activates or deactivates a notification subscription by ID.""" user_id = self._require_user_id() + self._require_notifications_enabled(db_session, user_id) sub = self._get_owned_subscription(db_session, id, user_id) + # Managing an admin.event_summary subscription requires the additional admin flag. + if sub.notification_type_id == ADMIN_EVENT_SUMMARY_NOTIFICATION_TYPE_ID: + self._require_admin_summary_enabled(db_session, user_id) + active = update_notification_subscription_request.active if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID: user = db_session.get(AppUser, user_id) @@ -221,6 +273,7 @@ def delete_user_subscription(self, id: str, db_session=None) -> None: The announcements subscription cannot be deleted; it is disabled instead. """ user_id = self._require_user_id() + self._require_notifications_enabled(db_session, user_id) sub = self._get_owned_subscription(db_session, id, user_id) if sub.notification_type_id == ANNOUNCEMENTS_NOTIFICATION_TYPE_ID: @@ -251,6 +304,58 @@ def _require_user_id() -> str: raise HTTPException(status_code=403, detail="Guest users cannot perform this action.") return user_id + @classmethod + def _require_notifications_enabled(cls, db_session, user_id: str) -> None: + """Gate: only users with the ``isNotificationsEnabled`` feature flag may manage subscriptions. + + Raises 403 unless the flag resolves to true for this user. + """ + if not feature_flag_enabled(db_session, user_id, NOTIFICATIONS_FEATURE_FLAG_ID): + logger.info( + "Subscription action denied for user %s: feature flag %r not enabled.", + user_id, + NOTIFICATIONS_FEATURE_FLAG_ID, + ) + raise HTTPException(status_code=403, detail=ERROR_MESSAGE_USER_FEATURE_NOT_ENABLED) + + @classmethod + def _require_admin_summary_enabled(cls, db_session, user_id: str) -> None: + """Gate: the ``admin.event_summary`` type additionally requires ``isAdminSummarySubscriptionEnabled``. + + Layered on top of the general notifications gate. Raises 403 unless the flag resolves to + true for this user. + """ + if not feature_flag_enabled(db_session, user_id, ADMIN_SUMMARY_FEATURE_FLAG_ID): + logger.info( + "Subscription action denied for user %s: feature flag %r not enabled.", + user_id, + ADMIN_SUMMARY_FEATURE_FLAG_ID, + ) + raise HTTPException( + status_code=403, + detail=ERROR_MESSAGE_USER_FEATURE_NOT_ENABLED, + ) + + @staticmethod + def _sync_subscription_feeds(sub: NotificationSubscriptionOrm, feed_ids: List[str]) -> None: + """Make the subscription's feed set exactly ``feed_ids``. + + Relies on the ``delete-orphan`` cascade configured on + ``NotificationSubscription.notification_subscription_feeds`` (see + ``shared.database.users_database``): removing a row from the collection deletes it and + appending one inserts it on flush. + """ + desired = set(feed_ids) + current = {row.feed_stable_id: row for row in sub.notification_subscription_feeds} + + for stable_id, row in current.items(): + if stable_id not in desired: + sub.notification_subscription_feeds.remove(row) + + for stable_id in feed_ids: + if stable_id not in current: + sub.notification_subscription_feeds.append(NotificationSubscriptionFeedOrm(feed_stable_id=stable_id)) + @staticmethod def _get_owned_subscription(db_session, sub_id: str, user_id: str) -> NotificationSubscriptionOrm: sub = db_session.get(NotificationSubscriptionOrm, sub_id) diff --git a/api/tests/unittest/user_service/test_event_loop_not_blocked.py b/api/tests/unittest/user_service/test_event_loop_not_blocked.py index db4f3e4f0..7d82bcdcc 100644 --- a/api/tests/unittest/user_service/test_event_loop_not_blocked.py +++ b/api/tests/unittest/user_service/test_event_loop_not_blocked.py @@ -92,9 +92,11 @@ def _slow_remove(*_args, **_kwargs): app.dependency_overrides[get_token_Authentication] = lambda: "test-token" try: - with patch("shared.database.users_database.UsersDatabase", return_value=fake_db), patch.object( - helpers, "remove_contact_from_list", side_effect=_slow_remove - ), patch.object(helpers, "get_announcements_list_id", return_value=1): + with patch("shared.database.users_database.UsersDatabase", return_value=fake_db), patch( + "user_service.impl.subscriptions_api_impl.feature_flag_enabled", return_value=True + ), patch.object(helpers, "remove_contact_from_list", side_effect=_slow_remove), patch.object( + helpers, "get_announcements_list_id", return_value=1 + ): response, ticks_during = asyncio.run(_delete_with_heartbeat(app)) finally: app.dependency_overrides.pop(get_token_Authentication, None) diff --git a/api/tests/unittest/user_service/test_notifications_api_impl.py b/api/tests/unittest/user_service/test_notifications_api_impl.py index abfd21654..aa4cb5b6f 100644 --- a/api/tests/unittest/user_service/test_notifications_api_impl.py +++ b/api/tests/unittest/user_service/test_notifications_api_impl.py @@ -1,18 +1,140 @@ import unittest +import uuid +from unittest.mock import MagicMock -from fastapi import HTTPException - +from middleware.request_context import _request_context +from shared.database.users_database import UsersDatabase +from shared.users_database_gen.sqlacodegen_models import ( + FeatureFlag, + NotificationType as NotificationTypeOrm, + UserFeatureFlag, +) from user_service.impl.notifications_api_impl import NotificationsApiImpl +from user_service_gen.models.notification_type import NotificationType + +ADMIN_TYPE = "admin.event_summary" + + +def _set_context(user_id="uid-123"): + _request_context.set({"user_id": user_id, "user_email": "u@e.com", "is_guest": False}) -class TestNotificationsApiImpl(unittest.TestCase): +class TestGetNotifications(unittest.TestCase): def setUp(self): self.api = NotificationsApiImpl() + self.mock_session = MagicMock() + _set_context() + + def _set_types(self, types): + self.mock_session.query.return_value.order_by.return_value.all.return_value = types + + def _set_admin_flag(self, enabled): + flag = FeatureFlag( + id="isAdminSummarySubscriptionEnabled", value_type="boolean", default_value=enabled, disabled=False + ) + + def _get(model, key): + if model is FeatureFlag: + return flag + if model is UserFeatureFlag: + return None + return None + + self.mock_session.get.side_effect = _get + + def test_returns_mapped_types(self): + self._set_types( + [ + NotificationTypeOrm(id="api.announcements", description="Announcements"), + NotificationTypeOrm(id="feed.url_updated", description=None), + ] + ) + self._set_admin_flag(False) + + result = self.api.get_notifications(db_session=self.mock_session) + + self.mock_session.query.assert_called_once_with(NotificationTypeOrm) + self.assertTrue(all(isinstance(t, NotificationType) for t in result)) + self.assertEqual([t.id for t in result], ["api.announcements", "feed.url_updated"]) + self.assertEqual(result[0].description, "Announcements") + self.assertIsNone(result[1].description) + + def test_admin_type_hidden_when_flag_off(self): + self._set_types( + [ + NotificationTypeOrm(id="api.announcements", description=None), + NotificationTypeOrm(id=ADMIN_TYPE, description="admin"), + ] + ) + self._set_admin_flag(False) + + result = self.api.get_notifications(db_session=self.mock_session) + + self.assertEqual([t.id for t in result], ["api.announcements"]) + + def test_admin_type_visible_when_flag_on(self): + self._set_types( + [ + NotificationTypeOrm(id="api.announcements", description=None), + NotificationTypeOrm(id=ADMIN_TYPE, description="admin"), + ] + ) + self._set_admin_flag(True) + + result = self.api.get_notifications(db_session=self.mock_session) + + self.assertEqual([t.id for t in result], ["api.announcements", ADMIN_TYPE]) + + def test_admin_type_hidden_for_anonymous_without_flag_lookup(self): + _request_context.set({}) # no user_id + self._set_types( + [ + NotificationTypeOrm(id="api.announcements", description=None), + NotificationTypeOrm(id=ADMIN_TYPE, description="admin"), + ] + ) + + result = self.api.get_notifications(db_session=self.mock_session) + + self.assertEqual([t.id for t in result], ["api.announcements"]) + # Without a user there is no flag to resolve. + self.mock_session.get.assert_not_called() + + def test_empty(self): + self._set_types([]) + self._set_admin_flag(False) + self.assertEqual(self.api.get_notifications(db_session=self.mock_session), []) + + +def _reset_singleton(): + UsersDatabase.instance = None + UsersDatabase.initialized = False + + +def test_get_notifications_db_backed(users_test_database_url): + """End-to-end against the real users test DB: an inserted type appears; admin type is hidden.""" + _reset_singleton() + db = UsersDatabase() + type_id = f"test.type-{uuid.uuid4().hex}" + session = db.Session() + try: + session.add(NotificationTypeOrm(id=type_id, description="db-backed test type")) + session.flush() + _request_context.set({}) # anonymous → admin.event_summary filtered out + + result = NotificationsApiImpl().get_notifications(db_session=session) - def test_get_notifications_returns_501(self): - with self.assertRaises(HTTPException) as ctx: - self.api.get_notifications() - self.assertEqual(ctx.exception.status_code, 501) + by_id = {t.id: t for t in result} + assert type_id in by_id + assert by_id[type_id].description == "db-backed test type" + # admin.event_summary is seeded (feat_1723) but hidden without the admin flag. + assert ADMIN_TYPE not in by_id + assert [t.id for t in result] == sorted(t.id for t in result) + finally: + session.rollback() + session.close() + _reset_singleton() + _request_context.set({}) if __name__ == "__main__": diff --git a/api/tests/unittest/user_service/test_subscription_feeds.py b/api/tests/unittest/user_service/test_subscription_feeds.py new file mode 100644 index 000000000..f3a7c743b --- /dev/null +++ b/api/tests/unittest/user_service/test_subscription_feeds.py @@ -0,0 +1,154 @@ +# +# 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. +# +"""DB-backed tests for feed-scoped notification subscriptions (issue #1778). + +These exercise the full create path (validation, persistence into the +``notification_subscription_feed`` join table, idempotent feed-set replacement) and the +``ON DELETE CASCADE`` / ``delete-orphan`` behaviour against the real users test database. +""" + +import uuid + +import pytest +from fastapi import HTTPException +from sqlalchemy.orm import configure_mappers + +from middleware.request_context import _request_context + +# Importing the module registers the mapper_configured listener under test. +from shared.database.users_database import UsersDatabase +from shared.users_database_gen.sqlacodegen_models import ( + AppUser, + FeatureFlag, + NotificationSubscription, + NotificationSubscriptionFeed, + NotificationType, + UserFeatureFlag, +) +from user_service.impl.users_api_impl import UsersApiImpl +from user_service_gen.models.create_notification_subscription_request import ( + CreateNotificationSubscriptionRequest, +) + +FEED_SCOPED_TYPE = "feed.url_updated" + + +def _reset_singleton(): + UsersDatabase.instance = None + UsersDatabase.initialized = False + + +def test_feed_scoped_relationship_uses_delete_orphan_and_passive_deletes(): + """Regression guard: the listener configures the feed join collection for DB-driven cascade.""" + configure_mappers() + rel = NotificationSubscription.__mapper__.relationships["notification_subscription_feeds"] + assert rel.passive_deletes is True + assert "delete-orphan" in rel.cascade + + +@pytest.fixture +def api_session(users_test_database_url, monkeypatch): + """A real users-DB session plus seeded user/type, always rolled back.""" + # Feed existence is checked against the separate feeds DB; treat the test feed ids as valid. + monkeypatch.setattr("user_service.impl.users_api_impl.find_unknown_feed_ids", lambda *a, **k: []) + _reset_singleton() + db = UsersDatabase() + suffix = uuid.uuid4().hex + user_id = f"feed-user-{suffix}" + session = db.Session() + session.add(AppUser(id=user_id, email=f"{user_id}@test.org")) + # The feed-scoped type may already be seeded in the test DB; only insert if absent. + if session.get(NotificationType, FEED_SCOPED_TYPE) is None: + session.add(NotificationType(id=FEED_SCOPED_TYPE, description="url updated")) + # Subscription management is gated by the isNotificationsEnabled flag (default off); enable it + # for this user. The flag row is seeded by migration, but insert-if-absent keeps the test + # self-contained. + if session.get(FeatureFlag, "isNotificationsEnabled") is None: + session.add(FeatureFlag(id="isNotificationsEnabled", value_type="boolean", default_value=False)) + session.add(UserFeatureFlag(user_id=user_id, feature_flag_id="isNotificationsEnabled", value=True)) + session.flush() + _request_context.set({"user_id": user_id, "user_email": f"{user_id}@test.org", "is_guest": False}) + try: + yield UsersApiImpl(), session, user_id + finally: + session.rollback() + session.close() + _reset_singleton() + _request_context.set({}) + + +def _feed_rows(session, sub_id): + return { + r.feed_stable_id for r in session.query(NotificationSubscriptionFeed).filter_by(subscription_id=sub_id).all() + } + + +def test_create_persists_feed_ids(api_session): + api, session, _ = api_session + + result = api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id=FEED_SCOPED_TYPE, feed_ids=["mdb-2", "mdb-1", "mdb-2"]), + db_session=session, + ) + + assert result.notification_id == FEED_SCOPED_TYPE + assert result.feed_ids == ["mdb-1", "mdb-2"] # sorted + deduped + assert _feed_rows(session, result.id) == {"mdb-1", "mdb-2"} + + +def test_create_replaces_feed_set_idempotently(api_session): + api, session, _ = api_session + + first = api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id=FEED_SCOPED_TYPE, feed_ids=["mdb-1", "mdb-2"]), + db_session=session, + ) + second = api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id=FEED_SCOPED_TYPE, feed_ids=["mdb-3"]), + db_session=session, + ) + + # Same single subscription, feed set replaced. + assert second.id == first.id + assert second.feed_ids == ["mdb-3"] + assert _feed_rows(session, first.id) == {"mdb-3"} + + +def test_delete_subscription_cascades_to_feed_rows(api_session): + api, session, _ = api_session + + sub = api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id=FEED_SCOPED_TYPE, feed_ids=["mdb-1", "mdb-2"]), + db_session=session, + ) + assert _feed_rows(session, sub.id) == {"mdb-1", "mdb-2"} + + # Must not raise NotNullViolation; the DB ON DELETE CASCADE removes the feed rows. + session.delete(session.get(NotificationSubscription, sub.id)) + session.flush() + + assert _feed_rows(session, sub.id) == set() + + +def test_create_requires_feed_ids_for_feed_scoped_type(api_session): + api, session, _ = api_session + + with pytest.raises(HTTPException) as exc: + api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id=FEED_SCOPED_TYPE), + db_session=session, + ) + assert exc.value.status_code == 400 diff --git a/api/tests/unittest/user_service/test_subscription_helpers.py b/api/tests/unittest/user_service/test_subscription_helpers.py new file mode 100644 index 000000000..4e478ee53 --- /dev/null +++ b/api/tests/unittest/user_service/test_subscription_helpers.py @@ -0,0 +1,35 @@ +import unittest +from unittest.mock import MagicMock + +from user_service.impl.subscription_helpers import find_unknown_feed_ids + + +class TestFindUnknownFeedIds(unittest.TestCase): + def _session_with_existing(self, existing_ids): + session = MagicMock() + session.query.return_value.filter.return_value.all.return_value = [(fid,) for fid in existing_ids] + return session + + def test_empty_returns_empty_without_query(self): + session = MagicMock() + self.assertEqual(find_unknown_feed_ids([], feeds_db_session=session), []) + session.query.assert_not_called() + + def test_all_known(self): + session = self._session_with_existing(["mdb-1", "mdb-2"]) + self.assertEqual(find_unknown_feed_ids(["mdb-1", "mdb-2"], feeds_db_session=session), []) + + def test_returns_only_unknown_preserving_order(self): + session = self._session_with_existing(["mdb-1"]) + self.assertEqual( + find_unknown_feed_ids(["mdb-1", "mdba-100", "mdb-nope"], feeds_db_session=session), + ["mdba-100", "mdb-nope"], + ) + + def test_dedupes_before_checking(self): + session = self._session_with_existing([]) + self.assertEqual(find_unknown_feed_ids(["mdba-100", "mdba-100"], feeds_db_session=session), ["mdba-100"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/api/tests/unittest/user_service/test_subscriptions_api_impl.py b/api/tests/unittest/user_service/test_subscriptions_api_impl.py index a860a7df4..e5ccb7023 100644 --- a/api/tests/unittest/user_service/test_subscriptions_api_impl.py +++ b/api/tests/unittest/user_service/test_subscriptions_api_impl.py @@ -8,6 +8,7 @@ from shared.users_database_gen.sqlacodegen_models import ( AppUser, NotificationSubscription as NotificationSubscriptionOrm, + NotificationSubscriptionFeed as NotificationSubscriptionFeedOrm, ) from user_service.impl.subscriptions_api_impl import SubscriptionsApiImpl @@ -61,6 +62,17 @@ def test_get_does_not_touch_brevo(self): rem.assert_not_called() add.assert_not_called() + def test_returns_feed_ids_for_feed_scoped_subscription(self): + sub = _make_sub(notification_type_id="feed.url_updated") + sub.notification_subscription_feeds.append(NotificationSubscriptionFeedOrm(feed_stable_id="mdb-9")) + sub.notification_subscription_feeds.append(NotificationSubscriptionFeedOrm(feed_stable_id="mdb-2")) + self.mock_session.get.return_value = sub + + result = self.api.get_subscription("sub-1", db_session=self.mock_session) + + # feed_ids come from the join table, sorted. + self.assertEqual(result.feed_ids, ["mdb-2", "mdb-9"]) + def test_missing_returns_404(self): self.mock_session.get.return_value = None with self.assertRaises(HTTPException) as ctx: @@ -72,6 +84,21 @@ class TestPublicDeleteSubscription(unittest.TestCase): def setUp(self): self.api = SubscriptionsApiImpl() self.mock_session = MagicMock() + # Delete is gated by the owner's isNotificationsEnabled flag; enable it for these tests. + gate_patcher = patch("user_service.impl.subscriptions_api_impl.feature_flag_enabled", return_value=True) + gate_patcher.start() + self.addCleanup(gate_patcher.stop) + + def test_delete_denied_when_flag_off(self): + sub = _make_sub(notification_type_id="feed.published") + self.mock_session.get.return_value = sub + + with patch("user_service.impl.subscriptions_api_impl.feature_flag_enabled", return_value=False): + with self.assertRaises(HTTPException) as ctx: + self.api.delete_subscription("sub-1", db_session=self.mock_session) + + self.assertEqual(ctx.exception.status_code, 403) + self.mock_session.delete.assert_not_called() def test_delete_non_announcement_no_brevo(self): sub = _make_sub(notification_type_id="feed.published") 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 1ab26e8bd..3a45651e6 100644 --- a/api/tests/unittest/user_service/test_users_api_impl.py +++ b/api/tests/unittest/user_service/test_users_api_impl.py @@ -287,6 +287,14 @@ def setUp(self): self.api = UsersApiImpl() self.mock_session = MagicMock() _set_context() + # These tests exercise create/update logic, not the feature-flag gate; enable it. + gate_patcher = patch.object(UsersApiImpl, "_require_notifications_enabled", return_value=None) + gate_patcher.start() + self.addCleanup(gate_patcher.stop) + # Feed existence is checked against the (separate) feeds DB; treat all feeds as valid here. + feed_patcher = patch("user_service.impl.users_api_impl.find_unknown_feed_ids", return_value=[]) + feed_patcher.start() + self.addCleanup(feed_patcher.stop) def _make_sub(self, **kwargs): from shared.users_database_gen.sqlacodegen_models import NotificationSubscription as Orm @@ -305,13 +313,16 @@ def _make_sub(self, **kwargs): 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] + # The list query eager-loads feeds via .options(selectinload(...)). + query.options.return_value.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") + # A non feed-scoped subscription reports no feeds. + self.assertIsNone(result[0].feed_ids) def test_get_user_subscriptions_guest_403(self): _set_context(is_guest=True) @@ -383,6 +394,116 @@ def test_create_idempotent_reactivates_existing(self): self.mock_session.add.assert_not_called() self.assertEqual(result.id, "sub-1") + # ── create: feed-scoped (issue #1778) ── + def test_create_feed_scoped_persists_feed_ids(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.url_updated") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + result = self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.url_updated", feed_ids=["mdb-2", "mdb-1"]), + db_session=self.mock_session, + ) + + self.mock_session.add.assert_called_once() + added_sub = self.mock_session.add.call_args[0][0] + # Both feeds are attached to the new subscription's join collection. + self.assertEqual({f.feed_stable_id for f in added_sub.notification_subscription_feeds}, {"mdb-1", "mdb-2"}) + # Response feed_ids are sorted and deduped. + self.assertEqual(result.feed_ids, ["mdb-1", "mdb-2"]) + + def test_create_feed_scoped_dedupes_feed_ids(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.coverage") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + result = self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.coverage", feed_ids=["mdb-1", "mdb-1"]), + db_session=self.mock_session, + ) + + self.assertEqual(result.feed_ids, ["mdb-1"]) + + def test_create_feed_scoped_rejects_unknown_feed_ids(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.url_updated") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + with patch("user_service.impl.users_api_impl.find_unknown_feed_ids", return_value=["mdba-100"]): + with self.assertRaises(HTTPException) as ctx: + self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.url_updated", feed_ids=["mdba-100"]), + db_session=self.mock_session, + ) + + self.assertEqual(ctx.exception.status_code, 400) + self.assertIn("mdba-100", ctx.exception.detail) + self.mock_session.add.assert_not_called() + + def test_create_feed_scoped_requires_feed_ids(self): + from shared.users_database_gen.sqlacodegen_models import NotificationType + + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.url_availability") if model is NotificationType else _make_user() + ) + + for req in ( + CreateNotificationSubscriptionRequest(notification_id="feed.url_availability"), + CreateNotificationSubscriptionRequest(notification_id="feed.url_availability", feed_ids=[]), + ): + with self.assertRaises(HTTPException) as ctx: + self.api.create_user_subscription(req, db_session=self.mock_session) + self.assertEqual(ctx.exception.status_code, 400) + self.mock_session.add.assert_not_called() + + def test_create_non_scoped_rejects_feed_ids(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() + ) + + with self.assertRaises(HTTPException) as ctx: + self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.published", feed_ids=["mdb-1"]), + db_session=self.mock_session, + ) + self.assertEqual(ctx.exception.status_code, 400) + self.mock_session.add.assert_not_called() + + def test_create_feed_scoped_replaces_existing_feeds(self): + from shared.users_database_gen.sqlacodegen_models import ( + NotificationType, + NotificationSubscriptionFeed as FeedOrm, + ) + + existing = self._make_sub(notification_type_id="feed.url_updated", active=False) + existing.notification_subscription_feeds.append(FeedOrm(feed_stable_id="mdb-old")) + self.mock_session.get.side_effect = lambda model, key: ( + NotificationType(id="feed.url_updated") if model is NotificationType else _make_user() + ) + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = existing + + result = self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.url_updated", feed_ids=["mdb-new"]), + db_session=self.mock_session, + ) + + self.mock_session.add.assert_not_called() + self.assertTrue(existing.active) + # The old feed was dropped and the new one attached (delete-orphan handles the DB side). + self.assertEqual({f.feed_stable_id for f in existing.notification_subscription_feeds}, {"mdb-new"}) + self.assertEqual(result.feed_ids, ["mdb-new"]) + # ── update ── def test_update_deactivate_announcement_removes_brevo(self): sub = self._make_sub(notification_type_id="api.announcements", active=True) @@ -480,6 +601,195 @@ def test_brevo_failure_raises_502(self): self.assertEqual(ctx.exception.status_code, 502) +class TestSubscriptionGate(unittest.TestCase): + """The isNotificationsEnabled feature flag gates POST/PATCH subscriptions.""" + + def setUp(self): + self.api = UsersApiImpl() + self.mock_session = MagicMock() + _set_context() + + def _configure_gate(self, *, default=False, disabled=False, has_override=False, override_value=None): + from shared.users_database_gen.sqlacodegen_models import ( + FeatureFlag, + NotificationType, + UserFeatureFlag, + ) + + flag = FeatureFlag(id="isNotificationsEnabled", value_type="boolean", default_value=default, disabled=disabled) + override = ( + UserFeatureFlag(user_id="uid-123", feature_flag_id="isNotificationsEnabled", value=override_value) + if has_override + else None + ) + + def _get(model, key): + if model is FeatureFlag: + return flag + if model is UserFeatureFlag: + return override + if model is NotificationType: + return NotificationType(id="feed.published") + return _make_user() + + self.mock_session.get.side_effect = _get + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + def _create(self): + return self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id="feed.published"), db_session=self.mock_session + ) + + def test_create_denied_when_default_false(self): + self._configure_gate(default=False) + with self.assertRaises(HTTPException) as ctx: + self._create() + self.assertEqual(ctx.exception.status_code, 403) + self.mock_session.add.assert_not_called() + + def test_create_allowed_when_default_true(self): + self._configure_gate(default=True) + result = self._create() + self.assertEqual(result.notification_id, "feed.published") + self.mock_session.add.assert_called_once() + + def test_create_allowed_with_user_override_true(self): + self._configure_gate(default=False, has_override=True, override_value=True) + self.assertEqual(self._create().notification_id, "feed.published") + + def test_create_denied_with_user_override_false(self): + self._configure_gate(default=True, has_override=True, override_value=False) + with self.assertRaises(HTTPException) as ctx: + self._create() + self.assertEqual(ctx.exception.status_code, 403) + + def test_create_allowed_when_flag_disabled_but_granted(self): + # `disabled` only hides the flag from the consumer profile; it must NOT block a granted + # user from subscribing (backend-only concern). + self._configure_gate(default=False, disabled=True, has_override=True, override_value=True) + self.assertEqual(self._create().notification_id, "feed.published") + self.mock_session.add.assert_called_once() + + def test_create_denied_when_flag_missing(self): + from shared.users_database_gen.sqlacodegen_models import FeatureFlag, NotificationType + + def _get(model, key): + if model is FeatureFlag: + return None + if model is NotificationType: + return NotificationType(id="feed.published") + return _make_user() + + self.mock_session.get.side_effect = _get + with self.assertRaises(HTTPException) as ctx: + self._create() + self.assertEqual(ctx.exception.status_code, 403) + + def test_update_denied_when_default_false(self): + self._configure_gate(default=False) + with self.assertRaises(HTTPException) as ctx: + self.api.update_user_subscription( + "sub-1", UpdateNotificationSubscriptionRequest(active=False), db_session=self.mock_session + ) + self.assertEqual(ctx.exception.status_code, 403) + self.mock_session.delete.assert_not_called() + + def test_delete_denied_when_default_false(self): + self._configure_gate(default=False) + with self.assertRaises(HTTPException) as ctx: + self.api.delete_user_subscription("sub-1", db_session=self.mock_session) + self.assertEqual(ctx.exception.status_code, 403) + self.mock_session.delete.assert_not_called() + + +class TestAdminSummaryGate(unittest.TestCase): + """admin.event_summary additionally requires the isAdminSummarySubscriptionEnabled flag.""" + + ADMIN_TYPE = "admin.event_summary" + + def setUp(self): + self.api = UsersApiImpl() + self.mock_session = MagicMock() + _set_context() + + def _configure(self, *, notifications=True, admin=True, sub=None): + """Wire get() to resolve both gate flags (enabled per args) and, optionally, a subscription.""" + from shared.users_database_gen.sqlacodegen_models import ( + FeatureFlag, + NotificationSubscription as Orm, + NotificationType, + UserFeatureFlag, + ) + + flags = { + "isNotificationsEnabled": FeatureFlag( + id="isNotificationsEnabled", value_type="boolean", default_value=notifications, disabled=False + ), + "isAdminSummarySubscriptionEnabled": FeatureFlag( + id="isAdminSummarySubscriptionEnabled", value_type="boolean", default_value=admin, disabled=False + ), + } + + def _get(model, key): + if model is FeatureFlag: + return flags.get(key) + if model is UserFeatureFlag: + return None + if model is NotificationType: + return NotificationType(id=key) + if model is Orm: + return sub + return _make_user() + + self.mock_session.get.side_effect = _get + self.mock_session.query.return_value.filter.return_value.one_or_none.return_value = None + + def _create(self, notification_id): + return self.api.create_user_subscription( + CreateNotificationSubscriptionRequest(notification_id=notification_id), db_session=self.mock_session + ) + + def test_create_admin_allowed_when_admin_flag_true(self): + self._configure(notifications=True, admin=True) + self.assertEqual(self._create(self.ADMIN_TYPE).notification_id, self.ADMIN_TYPE) + + def test_create_admin_denied_when_admin_flag_false(self): + self._configure(notifications=True, admin=False) + with self.assertRaises(HTTPException) as ctx: + self._create(self.ADMIN_TYPE) + self.assertEqual(ctx.exception.status_code, 403) + self.mock_session.add.assert_not_called() + + def test_create_admin_denied_when_notifications_flag_false(self): + # The general gate runs first, so it fails even with the admin flag on. + self._configure(notifications=False, admin=True) + with self.assertRaises(HTTPException) as ctx: + self._create(self.ADMIN_TYPE) + self.assertEqual(ctx.exception.status_code, 403) + + def test_create_non_admin_type_ignores_admin_flag(self): + # A non-admin type is unaffected by the admin flag being off. + self._configure(notifications=True, admin=False) + self.assertEqual(self._create("feed.published").notification_id, "feed.published") + + def test_update_admin_denied_when_admin_flag_false(self): + from shared.users_database_gen.sqlacodegen_models import NotificationSubscription as Orm + + sub = Orm( + id="sub-1", + user_id="uid-123", + notification_type_id=self.ADMIN_TYPE, + active=True, + created_at=FIXED_NOW, + ) + self._configure(notifications=True, admin=False, sub=sub) + with self.assertRaises(HTTPException) as ctx: + self.api.update_user_subscription( + "sub-1", UpdateNotificationSubscriptionRequest(active=False), db_session=self.mock_session + ) + self.assertEqual(ctx.exception.status_code, 403) + + if __name__ == "__main__": unittest.main() diff --git a/docs/UserServiceAPI.yaml b/docs/UserServiceAPI.yaml index 3a60e9691..f7735085e 100644 --- a/docs/UserServiceAPI.yaml +++ b/docs/UserServiceAPI.yaml @@ -111,13 +111,11 @@ paths: $ref: "#/components/schemas/NotificationType" "401": description: Unauthorized. - "501": - description: Not yet implemented. /v1/user/subscriptions: get: summary: List the current user's notification subscriptions - description: Returns all notification subscriptions for the authenticated user. + description: Returns all notification subscriptions for the authenticated user. This feature is available to selected users. To request access or learn more, contact us at api@mobilitydata.org. operationId: getUserSubscriptions tags: - "users" @@ -134,11 +132,9 @@ paths: $ref: "#/components/schemas/NotificationSubscription" "401": description: Unauthorized. - "501": - description: Not yet implemented. post: summary: Create a notification subscription - description: Subscribes the authenticated user to a notification type. + description: Subscribes the authenticated user to a notification type. This feature is available to selected users. To request access or learn more, contact us at api@mobilitydata.org. operationId: createUserSubscription tags: - "users" @@ -161,13 +157,11 @@ paths: description: Invalid request. "401": description: Unauthorized. - "501": - description: Not yet implemented. /v1/user/subscriptions/{id}: patch: summary: Toggle a notification subscription - description: Activates or deactivates a notification subscription by ID. + description: Activates or deactivates a notification subscription by ID. This feature is available to selected users. To request access or learn more, contact us at api@mobilitydata.org. operationId: updateUserSubscription tags: - "users" @@ -197,14 +191,12 @@ paths: description: Unauthorized. "404": description: Subscription not found. - "501": - description: Not yet implemented. delete: summary: Delete or disable a notification subscription description: >- Removes a notification subscription by ID. The announcements subscription (`api.announcements`) cannot be deleted; calling this endpoint for it - disables the subscription (sets it inactive) instead of removing it. + disables the subscription (sets it inactive) instead of removing it. This feature is available to selected users. To request access or learn more, contact us at api@mobilitydata.org. operationId: deleteUserSubscription tags: - "users" @@ -224,8 +216,6 @@ paths: description: Unauthorized. "404": description: Subscription not found. - "501": - description: Not yet implemented. /v1/subscriptions/{id}: get: @@ -257,7 +247,7 @@ paths: Removes a notification subscription identified by its ID. The announcements subscription (`api.announcements`) cannot be deleted; calling this endpoint for it disables the subscription (sets it inactive) - instead of removing it. + instead of removing it. This feature is available to selected users. To request access or learn more, contact us at api@mobilitydata.org. operationId: deleteSubscription tags: - "subscriptions" @@ -390,6 +380,16 @@ components: type: string format: date-time description: Timestamp when the subscription was created. + feed_ids: + type: array + items: + type: string + nullable: true + description: > + Feed stable IDs this subscription targets. Populated for the + feed-scoped notification types (feed.url_updated, + feed.url_availability, feed.coverage); empty for other types. + example: ["mdb-1", "mdb-42"] CreateNotificationSubscriptionRequest: type: object @@ -400,6 +400,16 @@ components: type: string description: The notification type to subscribe to. example: "feed.published" + feed_ids: + type: array + items: + type: string + description: > + Feed stable IDs to subscribe to. Required (non-empty) for the + feed-scoped notification types feed.url_updated, + feed.url_availability and feed.coverage; must be omitted or empty + for other types. Validation is enforced in code, not the schema. + example: ["mdb-1", "mdb-42"] UpdateNotificationSubscriptionRequest: type: object diff --git a/liquibase/changelog_user.xml b/liquibase/changelog_user.xml index dadd1ae01..604ad53ad 100644 --- a/liquibase/changelog_user.xml +++ b/liquibase/changelog_user.xml @@ -27,6 +27,10 @@ + + diff --git a/liquibase/changes_user/feat_1778.sql b/liquibase/changes_user/feat_1778.sql new file mode 100644 index 000000000..be80371ca --- /dev/null +++ b/liquibase/changes_user/feat_1778.sql @@ -0,0 +1,26 @@ +-- liquibase formatted sql + +-- changeset MobilityData:feat_1778_subscription_feed +-- comment: Add notification_subscription_feed join table linking a subscription to the feeds it targets (feed.url_updated, feed.url_availability, feed.coverage). Issue #1778. + +-- A notification_subscription can target one-or-more feeds for the feed-scoped +-- notification types. This mirrors the existing notification_event_feed table: +-- feeds are referenced by their public stable_id and there is deliberately NO +-- foreign key to the feeds table, because feeds live in a separate database +-- (see docs/notifications.md). +CREATE TABLE IF NOT EXISTS notification_subscription_feed ( + subscription_id TEXT NOT NULL + REFERENCES notification_subscription(id) ON DELETE CASCADE, + -- Feed referenced by its public stable_id, e.g. 'mdb-1' (matches + -- notification_event_feed.feed_stable_id). No FK: feeds are in another DB. + feed_stable_id TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + -- One row per (subscription, feed); the PK also serves the + -- "which feeds does this subscription target?" lookup. + PRIMARY KEY (subscription_id, feed_stable_id) +); + +-- Reverse fan-out: "which subscriptions target this feed?" when matching a +-- feed.* event to its subscribers. +CREATE INDEX IF NOT EXISTS idx_notification_subscription_feed_feed_stable_id + ON notification_subscription_feed (feed_stable_id); diff --git a/liquibase/test_data/users_test_data.sql b/liquibase/test_data/users_test_data.sql index ae42158cc..4e7d6d607 100644 --- a/liquibase/test_data/users_test_data.sql +++ b/liquibase/test_data/users_test_data.sql @@ -7,7 +7,8 @@ INSERT INTO notification_type (id, description) VALUES ('feed.published', 'A new feed has been published'), ('feed.updated', 'An existing feed has new data available'), ('feed.deprecated', 'A feed has been marked deprecated'), - ('validation.failed', 'Validation failed for a tracked feed') + ('validation.failed', 'Validation failed for a tracked feed'), + ('feed.url_updated', 'A tracked feed''s URL changed (feed-scoped)') ON CONFLICT (id) DO NOTHING; -- App users ------------------------------------------------------------------ @@ -25,9 +26,17 @@ INSERT INTO notification_subscription (id, user_id, notification_type_id, filter ('sub_0000000000000000000000000002', 'test_user_alice_000000000001', 'feed.updated', '{"feed_ids": ["mdb-1", "mdb-42"]}'::jsonb, true), ('sub_0000000000000000000000000003', 'test_user_bob_00000000000002', 'feed.published', '{"country": "CA"}'::jsonb, true), ('sub_0000000000000000000000000004', 'test_user_bob_00000000000002', 'validation.failed', '{"feed_ids": ["mdb-7"]}'::jsonb, false), - ('sub_0000000000000000000000000005', 'test_user_dan_00000000000004', 'feed.deprecated', NULL, true) + ('sub_0000000000000000000000000005', 'test_user_dan_00000000000004', 'feed.deprecated', NULL, true), + ('sub_0000000000000000000000000006', 'test_user_alice_000000000001', 'feed.url_updated', NULL, true) ON CONFLICT (id) DO NOTHING; +-- Feed-scoped subscription targets (issue #1778) ---------------------------- +-- Alice's feed.url_updated subscription targets two feeds by their stable_id. +INSERT INTO notification_subscription_feed (subscription_id, feed_stable_id) VALUES + ('sub_0000000000000000000000000006', 'mdb-1'), + ('sub_0000000000000000000000000006', 'mdb-42') +ON CONFLICT (subscription_id, feed_stable_id) DO NOTHING; + -- Notification log ---------------------------------------------------------- INSERT INTO notification_log (id, subscription_id, sent_at, channel, status, error_message) VALUES ('log_0000000000000000000000000001', 'sub_0000000000000000000000000001', now() - interval '5 days', 'email', 'sent', NULL),