add feat delete and feature defaults to all users

Author: davidgamez

api/src/shared/db_models/app_user_impl.py

@@ -1,4 +1,4 @@
-from shared.users_database_gen.sqlacodegen_models import AppUser
+from shared.users_database_gen.sqlacodegen_models import AppUser, FeatureFlag
 from user_service_gen.models.feature_flag import FeatureFlag as FeatureFlagModel
 from user_service_gen.models.user_profile import UserProfile
 
@@ -12,27 +12,36 @@ class Config:
         from_attributes = True
 
     @classmethod
-    def from_orm(cls, user: AppUser | None) -> UserProfile | None:
+    def from_orm(
+        cls,
+        user: AppUser | None,
+        all_flags: list[FeatureFlag] | None = None,
+    ) -> UserProfile | None:
         if not user:
             return None
 
+        # Always return every feature flag with its resolved value: the user's
+        # override when set, otherwise the flag's default. A user need not be
+        # explicitly linked to a flag to receive its default.
+        overrides = {uff.feature_flag_id: uff.value for uff in (user.user_feature_flags or [])}
+        features = [
+            FeatureFlagModel(
+                id=flag.id,
+                name=flag.name,
+                value_type=flag.value_type,
+                value=overrides[flag.id] if overrides.get(flag.id) is not None else flag.default_value,
+            )
+            for flag in (all_flags or [])
+        ]
+
         return cls(
             id=user.id,
             email=user.email,
             full_name=user.full_name,
             legacy_org_name=user.legacy_org_name,
             email_verified=user.email_verified,
             is_registered_to_receive_api_announcements=user.is_registered_to_receive_api_announcements or False,
-            features=[
-                FeatureFlagModel(
-                    id=uff.feature_flag.id,
-                    name=uff.feature_flag.name,
-                    value_type=uff.feature_flag.value_type,
-                    value=uff.value if uff.value is not None else uff.feature_flag.default_value,
-                )
-                for uff in (user.user_feature_flags or [])
-                if uff.feature_flag.default_value is not None or uff.value is not None
-            ],
+            features=features,
             created_at=user.created_at,
             updated_at=user.updated_at,
         )

api/src/user_service/impl/users_api_impl.py

@@ -25,7 +25,7 @@
 from middleware.request_context import get_request_context
 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, UserFeatureFlag
+from shared.users_database_gen.sqlacodegen_models import AppUser, FeatureFlag, UserFeatureFlag
 from user_service_gen.apis.users_api_base import BaseUsersApi
 from user_service_gen.models.create_notification_subscription_request import (
     CreateNotificationSubscriptionRequest,
@@ -82,7 +82,8 @@ def get_user(self, db_session=None) -> UserProfile:
             db_session.add(user)
             db_session.flush()
 
-        return AppUserImpl.from_orm(user)
+        all_flags = db_session.query(FeatureFlag).filter(FeatureFlag.disabled.is_(False)).order_by(FeatureFlag.id).all()
+        return AppUserImpl.from_orm(user, all_flags)
 
     @with_users_db_session
     def update_user(self, update_user_request: UpdateUserRequest, db_session=None) -> UserProfile:
@@ -115,7 +116,8 @@ def update_user(self, update_user_request: UpdateUserRequest, db_session=None) -
         user.updated_at = datetime.now(timezone.utc)
         db_session.flush()
 
-        return AppUserImpl.from_orm(user)
+        all_flags = db_session.query(FeatureFlag).filter(FeatureFlag.disabled.is_(False)).order_by(FeatureFlag.id).all()
+        return AppUserImpl.from_orm(user, all_flags)
 
     # ── Subscription stubs — implemented in a follow-up issue ────────────────
 

api/tests/unittest/user_service/test_users_api_impl.py

@@ -31,14 +31,25 @@ def _make_user(**kwargs) -> AppUser:
     return user
 
 
-def _mock_query_first(session, return_value):
-    """Make session.query().options().filter_by().first() return return_value."""
-    mock_q = MagicMock()
-    mock_q.options.return_value = mock_q
-    mock_q.filter_by.return_value = mock_q
-    mock_q.first.return_value = return_value
-    session.query.return_value = mock_q
-    return mock_q
+def _mock_query_first(session, return_value, flags=None):
+    """Dispatch session.query per model.
+
+    AppUser query → .options().filter_by().first() returns `return_value`.
+    FeatureFlag query → .order_by().all() returns `flags` (default []).
+    """
+    flags = flags if flags is not None else []
+    user_q = MagicMock()
+    user_q.options.return_value = user_q
+    user_q.filter_by.return_value = user_q
+    user_q.first.return_value = return_value
+
+    flags_q = MagicMock()
+    flags_q.filter.return_value = flags_q
+    flags_q.order_by.return_value = flags_q
+    flags_q.all.return_value = flags
+
+    session.query.side_effect = lambda model: (flags_q if model is FeatureFlag else user_q)
+    return user_q
 
 
 def _set_context(user_id="uid-123", user_email="user@example.com", is_guest=False):
@@ -101,6 +112,35 @@ def test_guest(self):
         self.mock_session.add.assert_not_called()
         self.assertEqual(result.id, "uid-123")
 
+    def test_returns_active_flags_with_resolved_values(self):
+        flag = FeatureFlag(
+            id="beta_editor",
+            name="Beta Editor",
+            value_type="boolean",
+            default_value=False,
+            disabled=False,
+            created_at=FIXED_NOW,
+        )
+        user = _make_user()
+        _mock_query_first(self.mock_session, user, flags=[flag])
+        _set_context()
+
+        result = self.api.get_user(db_session=self.mock_session)
+
+        self.assertEqual([f.id for f in result.features], ["beta_editor"])
+        self.assertEqual(result.features[0].value, False)
+
+    def test_disabled_flags_filtered_out_of_query(self):
+        user = _make_user()
+        _mock_query_first(self.mock_session, user, flags=[])
+        _set_context()
+
+        self.api.get_user(db_session=self.mock_session)
+
+        # The user-facing query must exclude disabled flags.
+        flags_q = self.mock_session.query(FeatureFlag)
+        flags_q.filter.assert_called()
+
 
 class TestUpdateUserMe(unittest.TestCase):
     def setUp(self):
@@ -239,7 +279,7 @@ def test_from_orm_maps_all_fields(self):
         self.assertEqual(profile.created_at, now)
         self.assertEqual(profile.updated_at, now)
 
-    def test_from_orm_includes_feature_flags(self):
+    def test_from_orm_resolves_user_override_and_default(self):
         now = datetime(2024, 1, 15, 12, 0, 0, tzinfo=timezone.utc)
         flag = FeatureFlag(
             id="beta_editor",
@@ -249,34 +289,44 @@ def test_from_orm_includes_feature_flags(self):
             default_value=False,
             created_at=now,
         )
-        user_flag = UserFeatureFlag(user_id="uid-2", feature_flag_id=flag.id, assigned_at=now)
+        user_flag = UserFeatureFlag(user_id="uid-2", feature_flag_id=flag.id, value=True, assigned_at=now)
         user_flag.feature_flag = flag
         user = AppUser(id="uid-2", email="b@b.com", created_at=now, updated_at=now)
         user.user_feature_flags = [user_flag]
 
-        profile = AppUserImpl.from_orm(user)
+        profile = AppUserImpl.from_orm(user, [flag])
 
         self.assertEqual(len(profile.features), 1)
         self.assertEqual(profile.features[0].id, "beta_editor")
         self.assertEqual(profile.features[0].name, "Beta Editor")
         self.assertEqual(profile.features[0].value_type, "boolean")
-        self.assertEqual(profile.features[0].value, False)
+        # User override (True) wins over the default (False)
+        self.assertEqual(profile.features[0].value, True)
 
-    def test_from_orm_empty_feature_flags(self):
+    def test_from_orm_returns_all_flags_with_defaults_when_user_has_none(self):
         now = datetime(2024, 1, 15, 12, 0, 0, tzinfo=timezone.utc)
+        flag = FeatureFlag(
+            id="beta_editor",
+            name="Beta Editor",
+            value_type="boolean",
+            default_value=False,
+            created_at=now,
+        )
         user = AppUser(id="uid-3", email="c@b.com", created_at=now, updated_at=now)
         user.user_feature_flags = []
 
-        profile = AppUserImpl.from_orm(user)
+        profile = AppUserImpl.from_orm(user, [flag])
 
-        self.assertEqual(profile.features, [])
+        # Flag is returned even though the user has no override; value is the default
+        self.assertEqual(len(profile.features), 1)
+        self.assertEqual(profile.features[0].id, "beta_editor")
+        self.assertEqual(profile.features[0].value, False)
 
-    def test_from_orm_none_feature_flags_treated_as_empty(self):
+    def test_from_orm_empty_when_no_flags_exist(self):
         now = datetime(2024, 1, 15, 12, 0, 0, tzinfo=timezone.utc)
         user = AppUser(id="uid-4", email="d@b.com", created_at=now, updated_at=now)
-        # Simulate a user where feature_flags hasn't been loaded (set to empty list as selectinload would return)
         user.user_feature_flags = []
 
-        profile = AppUserImpl.from_orm(user)
+        profile = AppUserImpl.from_orm(user, [])
 
         self.assertEqual(profile.features, [])

docs/OperationsAPI.yaml

@@ -568,6 +568,8 @@ paths:
           description: Unauthorized.
         "409":
           description: A feature flag with this ID already exists.
+        "422":
+          description: default_value does not match the declared value_type.
   /v1/operations/feature-flags/{id}:
     put:
       summary: Update a feature flag
@@ -601,6 +603,32 @@ paths:
           description: Unauthorized.
         "404":
           description: Feature flag not found.
+        "422":
+          description: default_value does not match the declared value_type.
+    delete:
+      summary: Delete a feature flag
+      description: >
+        Deletes a feature flag definition. All user assignments for this flag are removed as well (cascading delete).
+
+      operationId: deleteFeatureFlag
+      tags:
+        - "users"
+      security:
+        - ApiKeyAuth: []
+      parameters:
+        - name: id
+          in: path
+          required: true
+          schema:
+            type: string
+          description: The feature flag slug ID.
+      responses:
+        "204":
+          description: Feature flag deleted.
+        "401":
+          description: Unauthorized.
+        "404":
+          description: Feature flag not found.
 components:
   schemas:
     Redirect:
@@ -1905,6 +1933,7 @@ components:
             example: vp
             description: >
               The type of realtime entry:
+
                 * vp - vehicle positions
                 * tu - trip updates
                 * sa - service alerts
@@ -2029,6 +2058,7 @@ components:
             example: vp
             description: >
               The type of realtime entry:
+
                 * vp - vehicle positions
                 * tu - trip updates
                 * sa - service alerts
@@ -2116,6 +2146,7 @@ components:
       x-operation: true
       description: >
         Describes status of the Feed. Should be one of
+
           * `active` Feed should be used in public trip planners.
           * `deprecated` Feed is explicitly deprecated and should not be used in public trip planners.
           * `inactive` Feed hasn't been recently updated and should be used at risk of providing outdated information.
@@ -2133,6 +2164,7 @@ components:
       x-operation: true
       description: >
         Describes data type of a feed. Should be one of
+
           * `gtfs` GTFS feed.
           * `gtfs_rt` GTFS-RT feed.
           * `gbfs` GBFS feed.
@@ -2220,6 +2252,12 @@ components:
         default_value:
           $ref: "#/components/schemas/FeatureFlagValue"
           description: The flag's global default value.
+        disabled:
+          type: boolean
+          description: >
+            When true, the flag is hidden from the user-facing API. It remains visible and manageable through the Operations API.
+
+          default: false
     CreateFeatureFlagRequest:
       x-operation: true
       type: object
@@ -2247,6 +2285,12 @@ components:
         default_value:
           $ref: "#/components/schemas/FeatureFlagValue"
           description: The flag's global default value.
+        disabled:
+          type: boolean
+          description: >
+            When true, the flag is hidden from the user-facing API. It remains visible and manageable through the Operations API.
+
+          default: false
     UpdateFeatureFlagRequest:
       x-operation: true
       type: object
@@ -2262,6 +2306,11 @@ components:
         default_value:
           $ref: "#/components/schemas/FeatureFlagValue"
           description: Updated global default value. value_type cannot be changed.
+        disabled:
+          type: boolean
+          description: >
+            When true, the flag is hidden from the user-facing API. It remains visible and manageable through the Operations API.
+
     OperationUserProfile:
       x-operation: true
       type: object
@@ -2290,7 +2339,9 @@ components:
           example: "Acme Transit"
         features:
           type: array
-          description: Feature flags currently assigned to this user.
+          description: >
+            All feature flags with their resolved state for this user. Every flag is returned (including those the user has no explicit override for); user_value holds the per-user override when set, otherwise the flag's default_value applies.
+
           items:
             $ref: "#/components/schemas/UserFeatureFlagState"
           default: []

docs/UserServiceAPI.yaml

@@ -259,7 +259,10 @@ components:
           default: false
         features:
           type: array
-          description: Feature flags currently active for this user.
+          description: >
+            All active feature flags for this user. Each flag's value is resolved to the
+            user's override when set, otherwise the global default. Disabled flags are not
+            included.
           items:
             $ref: "#/components/schemas/FeatureFlag"
           default: []

functions-python/operations_api/src/feeds_operations/impl/models/operation_feature_flag_impl.py

@@ -35,4 +35,5 @@ def from_orm(cls, flag: FeatureFlag | None) -> OperationFeatureFlag | None:
             created_at=flag.created_at,
             value_type=flag.value_type,
             default_value=flag.default_value,
+            disabled=flag.disabled,
         )

functions-python/operations_api/src/feeds_operations/impl/models/operation_user_profile_impl.py

@@ -18,7 +18,7 @@
 from feeds_operations.impl.models.user_feature_flag_state_impl import (
     UserFeatureFlagStateImpl,
 )
-from shared.users_database_gen.sqlacodegen_models import AppUser
+from shared.users_database_gen.sqlacodegen_models import AppUser, FeatureFlag
 
 
 class OperationUserProfileImpl(OperationUserProfile):
@@ -28,16 +28,26 @@ class Config:
         from_attributes = True
 
     @classmethod
-    def from_orm(cls, user: AppUser | None) -> OperationUserProfile | None:
+    def from_orm(
+        cls,
+        user: AppUser | None,
+        all_flags: list[FeatureFlag] | None = None,
+    ) -> OperationUserProfile | None:
         if not user:
             return None
+        # Always return every feature flag with its default; overlay the user's
+        # value where an override exists. Users need not be linked to a flag.
+        overrides = {
+            uff.feature_flag_id: uff.value for uff in (user.user_feature_flags or [])
+        }
+        features = [
+            UserFeatureFlagStateImpl.from_flag(flag, overrides.get(flag.id))
+            for flag in (all_flags or [])
+        ]
         return cls(
             id=user.id,
             email=user.email,
             full_name=user.full_name,
             legacy_org_name=user.legacy_org_name,
-            features=[
-                UserFeatureFlagStateImpl.from_orm(uff)
-                for uff in (user.user_feature_flags or [])
-            ],
+            features=features,
         )