From c68c12a88a7fdfa3ab1046d62c20ca5aee197bf1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Apr 2026 01:09:35 +0000 Subject: [PATCH] fix: include specific IDs in owner validation error messages Address review feedback from PR #7067: - Include user IDs in error message when users don't have project access - Include group IDs in error message when groups belong to wrong organisation This helps users understand exactly which owners/groups are invalid. --- api/features/serializers.py | 30 ++++++++++++------- .../unit/features/test_unit_features_views.py | 2 ++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/api/features/serializers.py b/api/features/serializers.py index 84c8ddbac235..6ca299c582f4 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -306,22 +306,32 @@ def validate_multivariate_options(self, multivariate_options): # type: ignore[n def validate_owners(self, owners: list[FFAdminUser]) -> list[FFAdminUser]: project: Project = self.context["project"] - for user in owners: - if not user.has_project_permission(VIEW_PROJECT, project): - raise serializers.ValidationError( - "Some users do not have access to this project." - ) + invalid_users = [ + user + for user in owners + if not user.has_project_permission(VIEW_PROJECT, project) + ] + if invalid_users: + invalid_user_ids = [user.id for user in invalid_users] + raise serializers.ValidationError( + f"Users with ids {invalid_user_ids} do not have access to this project." + ) return owners def validate_group_owners( self, group_owners: list[UserPermissionGroup] ) -> list[UserPermissionGroup]: project: Project = self.context["project"] - for group in group_owners: - if group.organisation_id != project.organisation_id: - raise serializers.ValidationError( - "Some groups do not belong to this project's organisation." - ) + invalid_groups = [ + group + for group in group_owners + if group.organisation_id != project.organisation_id + ] + if invalid_groups: + invalid_group_ids = [group.id for group in invalid_groups] + raise serializers.ValidationError( + f"Groups with ids {invalid_group_ids} do not belong to this project's organisation." + ) return group_owners def validate_name(self, name: str): # type: ignore[no-untyped-def] diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index d4e95211f581..dd3d72292219 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -4750,6 +4750,7 @@ def test_create_feature__group_owner_from_different_org__returns_400( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST assert "group_owners" in response.json() + assert str(other_group.id) in response.json()["group_owners"][0] def test_create_feature__owner_without_project_access__returns_400( @@ -4773,6 +4774,7 @@ def test_create_feature__owner_without_project_access__returns_400( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST assert "owners" in response.json() + assert str(other_user.id) in response.json()["owners"][0] def test_update_feature__owners_in_request_body__returns_200_without_changes(