From ca55b0d064cd2217760c5e2bef117c674351f11e Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Mon, 30 Mar 2026 15:14:17 +0530 Subject: [PATCH 01/10] feat: add project setting to enforce feature ownership (#4432) Add an `enforce_feature_owners` boolean on the Project model. When enabled, feature creation requires at least one user or group owner. The owners/group_owners fields on CreateFeatureSerializer are now asymmetric PrimaryKeyRelatedFields (accept IDs on write, return nested objects on read). The remove-owners and remove-group-owners endpoints also prevent removing the last owner when enforcement is on. Frontend adds the project setting toggle, a FeatureOwnerSelect component for the creation modal, create-button validation, and owner chips in the feature modal header. --- api/features/serializers.py | 76 +++- api/features/views.py | 29 ++ ...8_add_enforce_feature_owners_to_project.py | 21 + api/projects/models.py | 4 + api/projects/serializers.py | 1 + .../unit/features/test_unit_features_views.py | 428 +++++++++++++++++- .../unit/projects/test_unit_projects_views.py | 45 ++ frontend/common/types/requests.ts | 6 +- frontend/common/types/responses.ts | 1 + .../web/components/FeatureOwnerSelect.tsx | 154 +++++++ .../components/feature-summary/FeatureRow.tsx | 46 +- .../components/FeatureUpdateSummary.tsx | 19 +- .../modals/create-feature/index.tsx | 10 + .../create-feature/tabs/CreateFeatureTab.tsx | 12 + .../tabs/FeatureSettingsTab.tsx | 22 + .../EnforceFeatureOwnershipSetting.tsx | 52 +++ .../sections/additional-settings/index.tsx | 3 + 17 files changed, 907 insertions(+), 22 deletions(-) create mode 100644 api/projects/migrations/0028_add_enforce_feature_owners_to_project.py create mode 100644 frontend/web/components/FeatureOwnerSelect.tsx create mode 100644 frontend/web/components/pages/project-settings/tabs/general-tab/sections/additional-settings/EnforceFeatureOwnershipSetting.tsx diff --git a/api/features/serializers.py b/api/features/serializers.py index a37c6b6ce7fb..dc23334dc8ec 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -3,6 +3,7 @@ from uuid import UUID import django.core.exceptions +from django.db import models from common.features.multivariate.serializers import ( MultivariateFeatureStateValueSerializer, ) @@ -10,6 +11,7 @@ CreateSegmentOverrideFeatureStateSerializer, FeatureStateValueSerializer, ) +from common.projects.permissions import VIEW_PROJECT from drf_spectacular.utils import extend_schema_field from drf_writable_nested import ( # type: ignore[attr-defined] WritableNestedModelSerializer, @@ -29,6 +31,7 @@ FeatureFlagCodeReferencesRepositoryCountSerializer, ) from projects.models import Project +from users.models import FFAdminUser, UserPermissionGroup from users.serializers import ( UserIdsSerializer, UserListSerializer, @@ -161,12 +164,30 @@ def validate_tags(self, tags: str) -> list[int]: raise serializers.ValidationError("Tag IDs must be integers.") +class _FeatureOwnersField(serializers.PrimaryKeyRelatedField[FFAdminUser]): + + def get_queryset(self) -> models.QuerySet[FFAdminUser]: + return FFAdminUser.objects.all() + + def to_representation(self, value: FFAdminUser) -> dict[str, Any]: + return UserListSerializer(value).data + + +class _FeatureGroupOwnersField(serializers.PrimaryKeyRelatedField[UserPermissionGroup]): + + def get_queryset(self) -> models.QuerySet[UserPermissionGroup]: + return UserPermissionGroup.objects.all() + + def to_representation(self, value: UserPermissionGroup) -> dict[str, Any]: + return UserPermissionGroupSummarySerializer(value).data + + class CreateFeatureSerializer(DeleteBeforeUpdateWritableNestedModelSerializer): multivariate_options = NestedMultivariateFeatureOptionSerializer( many=True, required=False ) - owners = UserListSerializer(many=True, read_only=True) - group_owners = UserPermissionGroupSummarySerializer(many=True, read_only=True) + owners = _FeatureOwnersField(many=True, required=False) + group_owners = _FeatureGroupOwnersField(many=True, required=False) environment_feature_state = serializers.SerializerMethodField() segment_feature_state = serializers.SerializerMethodField() @@ -240,12 +261,22 @@ def create(self, validated_data: dict) -> Feature: # type: ignore[type-arg] project = self.context["project"] self.validate_project_features_limit(project) + # Pop M2M fields before creating the instance (can't pass to Model.objects.create) + owners: list[FFAdminUser] = validated_data.pop("owners", []) + group_owners: list[UserPermissionGroup] = validated_data.pop("group_owners", []) + # Add the default(User creating the feature) owner of the feature # NOTE: pop the user before passing the data to create user = validated_data.pop("user", None) instance = super(CreateFeatureSerializer, self).create(validated_data) # type: ignore[no-untyped-call] if user and getattr(user, "is_master_api_key_user", False) is False: instance.owners.add(user) + + if owners: + instance.owners.add(*owners) + if group_owners: + instance.group_owners.add(*group_owners) + return instance # type: ignore[no-any-return] def validate_project_features_limit(self, project: Project) -> None: @@ -275,6 +306,26 @@ def validate_multivariate_options(self, multivariate_options): # type: ignore[n raise serializers.ValidationError("Invalid percentage allocation") return multivariate_options + 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." + ) + 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." + ) + return group_owners + def validate_name(self, name: str): # type: ignore[no-untyped-def] view = self.context["view"] @@ -317,8 +368,23 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: "Selected Tags must be from the same Project as current Feature" ) + self._validate_enforce_feature_owners(attrs) + return attrs + def _validate_enforce_feature_owners(self, attrs: dict[str, Any]) -> None: + project: Project = self.context["project"] + if ( + not self.instance + and project.enforce_feature_owners + and not attrs.get("owners") + and not attrs.get("group_owners") + ): + raise serializers.ValidationError( + "This project requires at least one owner or group owner " + "when creating a feature." + ) + @extend_schema_field(FeatureStateSerializerSmall(allow_null=True)) def get_environment_feature_state( # type: ignore[return] self, instance: Feature @@ -399,6 +465,9 @@ def update(self, feature: Feature, validated_data: dict[str, Any]) -> Feature: class UpdateFeatureSerializerWithMetadata(FeatureSerializerWithMetadata): """prevent users from changing certain values after creation""" + owners = _FeatureOwnersField(many=True, read_only=True) + group_owners = _FeatureGroupOwnersField(many=True, read_only=True) + class Meta(FeatureSerializerWithMetadata.Meta): read_only_fields = FeatureSerializerWithMetadata.Meta.read_only_fields + ( # type: ignore[assignment] "default_enabled", @@ -416,6 +485,9 @@ class ListFeatureSerializer(FeatureSerializerWithMetadata): class UpdateFeatureSerializer(ListFeatureSerializer): """prevent users from changing certain values after creation""" + owners = _FeatureOwnersField(many=True, read_only=True) + group_owners = _FeatureGroupOwnersField(many=True, read_only=True) + class Meta(ListFeatureSerializer.Meta): read_only_fields = ListFeatureSerializer.Meta.read_only_fields + ( # type: ignore[assignment] "default_enabled", diff --git a/api/features/views.py b/api/features/views.py index 152bb5223ff7..d885e0a71ba5 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -451,6 +451,11 @@ def remove_group_owners(self, request, *args, **kwargs): # type: ignore[no-unty feature = self.get_object() serializer = FeatureGroupOwnerInputSerializer(data=request.data) serializer.is_valid(raise_exception=True) + self._validate_owner_removal( + feature, + owners_to_remove=0, + group_owners_to_remove=len(serializer.validated_data["group_ids"]), + ) serializer.remove_group_owners(feature) response = Response(self.get_serializer(instance=feature).data) return response @@ -484,10 +489,34 @@ def remove_owners(self, request, *args, **kwargs): # type: ignore[no-untyped-de serializer.is_valid(raise_exception=True) feature = self.get_object() + self._validate_owner_removal( + feature, + owners_to_remove=len(serializer.validated_data["user_ids"]), + group_owners_to_remove=0, + ) serializer.remove_users(feature) return Response(self.get_serializer(instance=feature).data) + def _validate_owner_removal( + self, + feature: Feature, + owners_to_remove: int, + group_owners_to_remove: int, + ) -> None: + if not feature.project.enforce_feature_owners: + return + remaining = ( + feature.owners.count() + - owners_to_remove + + feature.group_owners.count() + - group_owners_to_remove + ) + if remaining < 1: + raise serializers.ValidationError( + "This project requires at least one owner or group owner per feature." + ) + @extend_schema( parameters=[GetInfluxDataQuerySerializer], responses={200: FeatureInfluxDataSerializer()}, diff --git a/api/projects/migrations/0028_add_enforce_feature_owners_to_project.py b/api/projects/migrations/0028_add_enforce_feature_owners_to_project.py new file mode 100644 index 000000000000..e19f2629a629 --- /dev/null +++ b/api/projects/migrations/0028_add_enforce_feature_owners_to_project.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.11 on 2026-03-27 03:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("projects", "0027_add_create_project_level_change_requests_permission"), + ] + + operations = [ + migrations.AddField( + model_name="project", + name="enforce_feature_owners", + field=models.BooleanField( + default=False, + help_text="Require at least one user or group owner when creating a feature.", + ), + ), + ] diff --git a/api/projects/models.py b/api/projects/models.py index 69443c19f1fa..f70e9ca8ee96 100644 --- a/api/projects/models.py +++ b/api/projects/models.py @@ -60,6 +60,10 @@ class Project(LifecycleModelMixin, SoftDeleteExportableModel): # type: ignore[d default=False, help_text="Prevent defaults from being set in all environments when creating a feature.", ) + enforce_feature_owners = models.BooleanField( + default=False, + help_text="Require at least one user or group owner when creating a feature.", + ) enable_realtime_updates = models.BooleanField( default=False, help_text="Enable this to trigger a realtime(sse) event whenever the value of a flag changes", diff --git a/api/projects/serializers.py b/api/projects/serializers.py index 1de2df20232e..70cd28c5a986 100644 --- a/api/projects/serializers.py +++ b/api/projects/serializers.py @@ -44,6 +44,7 @@ class Meta: "stale_flags_limit_days", "edge_v2_migration_status", "minimum_change_request_approvals", + "enforce_feature_owners", ) read_only_fields = ( "enable_dynamo_db", diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 1e5d0bf792c0..554e61da5acc 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -84,27 +84,17 @@ one_hour_ago = now - timedelta(hours=1) -def test_create_feature__with_owners_field__owners_is_read_only( +def test_create_feature__with_owners__assigns_specified_owners( project: Project, admin_client_original: APIClient, admin_user: FFAdminUser, ) -> None: # Given - default_value = "This is a value" + url = reverse("api-v1:projects:project-features-list", args=[project.id]) data = { "name": "test feature", - "initial_value": default_value, - "project": project.id, - "owners": [ - { - "id": 2, - "email": "fake_user@mail.com", - "first_name": "fake", - "last_name": "user", - } - ], + "owners": [admin_user.id], } - url = reverse("api-v1:projects:project-features-list", args=[project.id]) # When response = admin_client_original.post( @@ -113,9 +103,10 @@ def test_create_feature__with_owners_field__owners_is_read_only( # Then assert response.status_code == status.HTTP_201_CREATED - assert len(response.json()["owners"]) == 1 - assert response.json()["owners"][0]["id"] == admin_user.id - assert response.json()["owners"][0]["email"] == admin_user.email + response_owners = response.json()["owners"] + assert len(response_owners) == 1 + assert response_owners[0]["id"] == admin_user.id + assert response_owners[0]["email"] == admin_user.email @mock.patch("features.views.trigger_feature_state_change_webhooks") @@ -4594,3 +4585,408 @@ def test_create_feature__multivariate_options_provided__sets_type_to_multivariat # Then assert response.status_code == status.HTTP_201_CREATED assert response.json()["type"] == MULTIVARIATE + + +def test_create_feature__enforce_owners_enabled__no_owners__returns_400( + admin_client_original: APIClient, + project: Project, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = {"name": "test_feature_no_owners"} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "non_field_errors" in response.json() + assert "least one owner" in response.json()["non_field_errors"][0].lower() + + +def test_create_feature__enforce_owners_enabled__with_owners__returns_201( + admin_client_original: APIClient, + project: Project, + admin_user: FFAdminUser, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = { + "name": "test_feature_with_owners", + "owners": [admin_user.id], + } + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + feature = Feature.objects.get(name="test_feature_with_owners") + assert admin_user in feature.owners.all() + + +def test_create_feature__enforce_owners_enabled__with_group_owners__returns_201( + admin_client_original: APIClient, + project: Project, + organisation: Organisation, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + + group = UserPermissionGroup.objects.create( + name="Test Group", organisation=organisation + ) + + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = { + "name": "test_feature_with_group_owners", + "group_owners": [group.id], + } + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + feature = Feature.objects.get(name="test_feature_with_group_owners") + assert group in feature.group_owners.all() + + +def test_create_feature__enforce_owners_disabled__no_owners__returns_201( + admin_client_original: APIClient, + project: Project, +) -> None: + # Given — enforce_feature_owners defaults to False + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = {"name": "test_feature_no_enforcement"} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +def test_create_feature__owners_provided_without_enforcement__returns_201_with_owners( + admin_client_original: APIClient, + project: Project, + admin_user: FFAdminUser, +) -> None: + # Given — enforcement is off, but owners are still provided + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = { + "name": "test_feature_optional_owners", + "owners": [admin_user.id], + } + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + feature = Feature.objects.get(name="test_feature_optional_owners") + assert admin_user in feature.owners.all() + + +def test_create_feature__nonexistent_owner__returns_400( + admin_client_original: APIClient, + project: Project, +) -> None: + # Given + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = { + "name": "test_feature_invalid_owner", + "owners": [999999], + } + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "owners" in response.json() + + +def test_create_feature__group_owner_from_different_org__returns_400( + admin_client_original: APIClient, + project: Project, +) -> None: + # Given + other_org = Organisation.objects.create(name="Other Org") + other_group = UserPermissionGroup.objects.create( + name="Other Group", organisation=other_org + ) + + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = { + "name": "test_feature_wrong_org_group", + "group_owners": [other_group.id], + } + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "group_owners" in response.json() + + +def test_create_feature__owner_without_project_access__returns_400( + admin_client_original: APIClient, + project: Project, +) -> None: + # Given — create a user that does not belong to the project's organisation + other_user = FFAdminUser.objects.create_user(email="noaccess@example.com") # type: ignore[no-untyped-call] + + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = { + "name": "test_feature_no_access_user", + "owners": [other_user.id], + } + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "owners" in response.json() + + +def test_create_feature__enforce_owners_enabled_with_master_api_key__returns_400( + admin_master_api_key_client: APIClient, + project: Project, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = {"name": "test_feature_master_api_key_no_owners"} + + # When + response = admin_master_api_key_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "non_field_errors" in response.json() + + +def test_update_feature__owners_in_request_body__returns_200_without_changes( + admin_client_original: APIClient, + project: Project, + feature: Feature, + admin_user: FFAdminUser, + organisation: Organisation, +) -> None: + # Given + group = UserPermissionGroup.objects.create( + name="Test Group", organisation=organisation + ) + other_user = FFAdminUser.objects.create_user(email="other@example.com") # type: ignore[no-untyped-call] + other_user.add_organisation(organisation) + feature.owners.add(admin_user) + + url = reverse( + "api-v1:projects:project-features-detail", + args=[project.id, feature.id], + ) + data = { + "name": feature.name, + "owners": [other_user.id], + "group_owners": [group.id], + } + + # When + response = admin_client_original.patch( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then — owners and group_owners should be unchanged + assert response.status_code == status.HTTP_200_OK + feature.refresh_from_db() + assert list(feature.owners.all()) == [admin_user] + assert list(feature.group_owners.all()) == [] + + +def test_remove_owners__enforce_owners__last_owner__returns_400( + admin_client_original: APIClient, + project: Project, + feature: Feature, + admin_user: FFAdminUser, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + feature.owners.add(admin_user) + + url = reverse( + "api-v1:projects:project-features-remove-owners", + args=[project.id, feature.id], + ) + data = {"user_ids": [admin_user.id]} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + feature.refresh_from_db() + assert admin_user in feature.owners.all() + + +def test_remove_owners__enforce_owners__other_owners_remain__returns_200( + admin_client_original: APIClient, + project: Project, + feature: Feature, + admin_user: FFAdminUser, + organisation: Organisation, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + other_user = FFAdminUser.objects.create_user(email="other@example.com") # type: ignore[no-untyped-call] + other_user.add_organisation(organisation) + feature.owners.add(admin_user, other_user) + + url = reverse( + "api-v1:projects:project-features-remove-owners", + args=[project.id, feature.id], + ) + data = {"user_ids": [admin_user.id]} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_200_OK + feature.refresh_from_db() + assert admin_user not in feature.owners.all() + assert other_user in feature.owners.all() + + +def test_remove_owners__enforce_owners__group_owners_remain__returns_200( + admin_client_original: APIClient, + project: Project, + feature: Feature, + admin_user: FFAdminUser, + organisation: Organisation, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + group = UserPermissionGroup.objects.create( + name="Test Group", organisation=organisation + ) + feature.owners.add(admin_user) + feature.group_owners.add(group) + + url = reverse( + "api-v1:projects:project-features-remove-owners", + args=[project.id, feature.id], + ) + data = {"user_ids": [admin_user.id]} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then — allowed because group owner still exists + assert response.status_code == status.HTTP_200_OK + feature.refresh_from_db() + assert admin_user not in feature.owners.all() + assert group in feature.group_owners.all() + + +def test_remove_group_owners__enforce_owners__last_group_owner__returns_400( + admin_client_original: APIClient, + project: Project, + feature: Feature, + organisation: Organisation, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + group = UserPermissionGroup.objects.create( + name="Test Group", organisation=organisation + ) + feature.group_owners.add(group) + + url = reverse( + "api-v1:projects:project-features-remove-group-owners", + args=[project.id, feature.id], + ) + data = {"group_ids": [group.id]} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + feature.refresh_from_db() + assert group in feature.group_owners.all() + + +def test_remove_group_owners__enforce_owners__user_owners_remain__returns_200( + admin_client_original: APIClient, + project: Project, + feature: Feature, + admin_user: FFAdminUser, + organisation: Organisation, +) -> None: + # Given + project.enforce_feature_owners = True + project.save() + group = UserPermissionGroup.objects.create( + name="Test Group", organisation=organisation + ) + feature.owners.add(admin_user) + feature.group_owners.add(group) + + url = reverse( + "api-v1:projects:project-features-remove-group-owners", + args=[project.id, feature.id], + ) + data = {"group_ids": [group.id]} + + # When + response = admin_client_original.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then — allowed because user owner still exists + assert response.status_code == status.HTTP_200_OK + feature.refresh_from_db() + assert group not in feature.group_owners.all() + assert admin_user in feature.owners.all() diff --git a/api/tests/unit/projects/test_unit_projects_views.py b/api/tests/unit/projects/test_unit_projects_views.py index d2d22f6ac6bc..eb49b62401dd 100644 --- a/api/tests/unit/projects/test_unit_projects_views.py +++ b/api/tests/unit/projects/test_unit_projects_views.py @@ -1010,3 +1010,48 @@ def test_get_detailed_permissions__admin_viewing_other_user__returns_permissions "derived_from": {"groups": [], "roles": []}, } ] + + +def test_update_project__set_enforce_feature_owners__succeeds( + admin_client: APIClient, + project: Project, + organisation: Organisation, +) -> None: + # Given + assert project.enforce_feature_owners is False + url = reverse("api-v1:projects:project-detail", args=[project.id]) + data = { + "name": project.name, + "organisation": organisation.id, + "enforce_feature_owners": True, + } + + # When + response = admin_client.patch( + url, + data=json.dumps(data), + content_type="application/json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["enforce_feature_owners"] is True + project.refresh_from_db() + assert project.enforce_feature_owners is True + + +def test_list_projects__includes_enforce_feature_owners_field( + admin_client: APIClient, + project: Project, +) -> None: + # Given + url = reverse("api-v1:projects:project-list") + + # When + response = admin_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert len(response.json()) > 0 + assert "enforce_feature_owners" in response.json()[0] + assert response.json()[0]["enforce_feature_owners"] is False diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index fc48e4e17841..3f2bf7561ab7 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -33,6 +33,7 @@ export type UpdateProjectBody = { name: string hide_disabled_flags?: boolean prevent_flag_defaults?: boolean + enforce_feature_owners?: boolean enable_realtime_updates?: boolean minimum_change_request_approvals?: number | null stale_flags_limit_days?: number | null @@ -652,7 +653,10 @@ export type Req = { } createProjectFlag: { project_id: number - body: ProjectFlag + body: ProjectFlag & { + owners?: number[] + group_owners?: number[] + } } removeProjectFlag: { project_id: number diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 2265891e198c..e45d95b5de6c 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -217,6 +217,7 @@ export type Project = { use_edge_identities: boolean show_edge_identity_overrides_for_feature: boolean prevent_flag_defaults: boolean + enforce_feature_owners: boolean enable_realtime_updates: boolean max_segments_allowed?: number | null max_features_allowed?: number | null diff --git a/frontend/web/components/FeatureOwnerSelect.tsx b/frontend/web/components/FeatureOwnerSelect.tsx new file mode 100644 index 000000000000..f704cc047129 --- /dev/null +++ b/frontend/web/components/FeatureOwnerSelect.tsx @@ -0,0 +1,154 @@ +import React, { FC, useState } from 'react' +import { useGetUsersQuery } from 'common/services/useUser' +import { useGetGroupSummariesQuery } from 'common/services/useGroupSummary' +import AccountStore from 'common/stores/account-store' +import UserSelect from './UserSelect' +import GroupSelect from './GroupSelect' +import { IonIcon } from '@ionic/react' +import { close } from 'ionicons/icons' +import getUserDisplayName from 'common/utils/getUserDisplayName' +import PlanBasedBanner from './PlanBasedAccess' +import Utils from 'common/utils/utils' + +type FeatureOwnerSelectProps = { + selectedUserIds: number[] + selectedGroupIds: number[] + onUserIdsChange: (ids: number[]) => void + onGroupIdsChange: (ids: number[]) => void +} + +const FeatureOwnerSelect: FC = ({ + onGroupIdsChange, + onUserIdsChange, + selectedGroupIds, + selectedUserIds, +}) => { + const [showUsers, setShowUsers] = useState(false) + const [showGroups, setShowGroups] = useState(false) + + const organisationId = AccountStore.getOrganisation()?.id + const { data: users } = useGetUsersQuery( + { organisationId }, + { skip: !organisationId }, + ) + const { data: groups } = useGetGroupSummariesQuery( + { orgId: organisationId }, + { skip: !organisationId }, + ) + + const hasFlagOwnersPlan = Utils.getPlansPermission('FLAG_OWNERS') + + if (!hasFlagOwnersPlan) { + return ( + + ) + } + + const selectedUsers = + users?.filter((u) => selectedUserIds.includes(u.id)) ?? [] + const selectedGroups = + groups?.filter((g) => selectedGroupIds.includes(g.id)) ?? [] + + const handleAddUser = (id: number) => { + if (!selectedUserIds.includes(id)) { + onUserIdsChange([...selectedUserIds, id]) + } + } + + const handleRemoveUser = (id: number) => { + onUserIdsChange(selectedUserIds.filter((uid) => uid !== id)) + } + + const handleAddGroup = (id: number) => { + if (!selectedGroupIds.includes(id)) { + onGroupIdsChange([...selectedGroupIds, id]) + } + } + + const handleRemoveGroup = (id: number) => { + onGroupIdsChange(selectedGroupIds.filter((gid) => gid !== id)) + } + + return ( +
+
+ + + + + {selectedUsers.map((u) => ( + handleRemoveUser(u.id)} + className='chip mr-2' + > + {getUserDisplayName(u)} + + + + + ))} + {!selectedUsers.length && ( +
No users assigned
+ )} + + setShowUsers(!showUsers)} + /> +
+ +
+ + + + + {selectedGroups.map((g) => ( + handleRemoveGroup(g.id)} + className='chip mr-2' + > + {g.name} + + + + + ))} + {!selectedGroups.length && ( +
No groups assigned
+ )} + + setShowGroups(!showGroups)} + /> +
+
+ ) +} + +export default FeatureOwnerSelect diff --git a/frontend/web/components/feature-summary/FeatureRow.tsx b/frontend/web/components/feature-summary/FeatureRow.tsx index 23bf1c8bf1ef..4c98802dc19a 100644 --- a/frontend/web/components/feature-summary/FeatureRow.tsx +++ b/frontend/web/components/feature-summary/FeatureRow.tsx @@ -7,6 +7,7 @@ import ProjectStore from 'common/stores/project-store' import Constants from 'common/constants' import { useProtectedTags } from 'common/utils/useProtectedTags' import Icon from 'components/Icon' +import Tooltip from 'components/Tooltip' import FeatureValue from './FeatureValue' import FeatureAction, { FeatureActionProps } from './FeatureAction' import classNames from 'classnames' @@ -25,6 +26,7 @@ import AccountStore from 'common/stores/account-store' import CondensedFeatureRow from 'components/CondensedFeatureRow' import { useHistory } from 'react-router-dom' import { useGetHealthEventsQuery } from 'common/services/useHealthEvents' +import getUserDisplayName from 'common/utils/getUserDisplayName' import FeatureName from './FeatureName' import FeatureDescription from './FeatureDescription' import FeatureTags from './FeatureTags' @@ -224,9 +226,22 @@ const FeatureRow: FC = (props) => { tab, } + const ownerChips = [ + ...(projectFlag.owners ?? []).map((u) => ({ + id: `user-${u.id}`, + label: getUserDisplayName(u), + })), + ...(projectFlag.owner_groups ?? []).map((g) => ({ + id: `group-${g.id}`, + label: g.name, + })), + ] + openModal( - - {permission ? 'Edit Feature' : 'Feature'}: {projectFlag.name} + + + {permission ? 'Edit Feature' : 'Feature'}: {projectFlag.name} + + {ownerChips.length > 0 && ( +
+ + {ownerChips.slice(0, 3).map((chip) => ( + {chip.label}} + > + {chip.label} + + ))} + {ownerChips.length > 3 && ( + + +{ownerChips.length - 3} + + } + > + {ownerChips + .slice(3) + .map((c) => c.label) + .join(', ')} + + )} +
+ )} , , modalCssClass, diff --git a/frontend/web/components/modals/create-feature/components/FeatureUpdateSummary.tsx b/frontend/web/components/modals/create-feature/components/FeatureUpdateSummary.tsx index 72e5f759fa73..f2408f1c4de8 100644 --- a/frontend/web/components/modals/create-feature/components/FeatureUpdateSummary.tsx +++ b/frontend/web/components/modals/create-feature/components/FeatureUpdateSummary.tsx @@ -14,16 +14,20 @@ type FeatureUpdateSummaryProps = { regexValid: boolean featureLimitPercentage: number hasMetadataRequired: boolean + ownerIds?: number[] + groupOwnerIds?: number[] } const FeatureUpdateSummary: FC = ({ featureLimitPercentage, + groupOwnerIds, hasMetadataRequired, identity, invalid, isSaving, name, onCreateFeature, + ownerIds, projectId, regexValid, }) => { @@ -32,6 +36,10 @@ const FeatureUpdateSummary: FC = ({ { skip: !projectId }, ) const preventFlagDefaults = !!project?.prevent_flag_defaults + const enforceOwners = !!project?.enforce_feature_owners + const hasOwners = + (ownerIds?.length ?? 0) > 0 || (groupOwnerIds?.length ?? 0) > 0 + const missingRequiredOwners = enforceOwners && !hasOwners return ( <> @@ -46,6 +54,14 @@ const FeatureUpdateSummary: FC = ({ )} + {missingRequiredOwners && ( + + This project requires at least one user or group owner to be + assigned when creating a feature. Please assign an owner in the + settings above. + + )} +