diff --git a/api/features/serializers.py b/api/features/serializers.py index a37c6b6ce7fb..84c8ddbac235 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -10,6 +10,8 @@ CreateSegmentOverrideFeatureStateSerializer, FeatureStateValueSerializer, ) +from common.projects.permissions import VIEW_PROJECT +from django.db import models 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,28 @@ 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 +259,22 @@ def create(self, validated_data: dict) -> Feature: # type: ignore[type-arg] project = self.context["project"] self.validate_project_features_limit(project) - # Add the default(User creating the feature) owner of the feature - # NOTE: pop the user before passing the data to create + # 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", []) + 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: + + if owners: + instance.owners.add(*owners) + elif user and getattr(user, "is_master_api_key_user", False) is False: + # Auto-add the creating user as owner only when no explicit owners provided instance.owners.add(user) + + 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 +304,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 +366,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 +463,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 +483,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..d4e95211f581 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -84,38 +84,29 @@ 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_client_new: 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( + response = admin_client_new.post( url, data=json.dumps(data), content_type="application/json" ) # 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,355 @@ 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_new: 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_new.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_new: 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_new.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_new: 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_new.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_new: 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_new.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_new: 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_new.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_new: 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_new.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_new: 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_new.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_new: 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_new.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_update_feature__owners_in_request_body__returns_200_without_changes( + admin_client_new: 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_new.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_new: 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_new.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_group_owners_remain__returns_200( + admin_client_new: 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_new.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_new: 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_new.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_new: 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_new.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..03747f288934 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__default_enforce_feature_owners__returns_false( + 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/services/useProjectFlag.ts b/frontend/common/services/useProjectFlag.ts index efc34282f70a..4c713111f0d5 100644 --- a/frontend/common/services/useProjectFlag.ts +++ b/frontend/common/services/useProjectFlag.ts @@ -38,6 +38,28 @@ export const projectFlagService = service }) .injectEndpoints({ endpoints: (builder) => ({ + addFlagGroupOwners: builder.mutation< + Res['projectFlag'], + Req['addFlagGroupOwners'] + >({ + invalidatesTags: (res) => [{ id: res?.id, type: 'ProjectFlag' }], + query: ({ feature_id, project_id, ...body }) => ({ + body, + method: 'POST', + url: `projects/${project_id}/features/${feature_id}/add-group-owners/`, + }), + }), + addFlagOwners: builder.mutation( + { + invalidatesTags: (res) => [{ id: res?.id, type: 'ProjectFlag' }], + query: ({ feature_id, project_id, ...body }) => ({ + body, + method: 'POST', + url: `projects/${project_id}/features/${feature_id}/add-owners/`, + }), + }, + ), + createProjectFlag: builder.mutation< Res['projectFlag'], Req['createProjectFlag'] @@ -52,6 +74,7 @@ export const projectFlagService = service url: `projects/${query.project_id}/features/`, }), }), + getFeatureList: builder.query({ providesTags: (_res, _meta, req) => [ { @@ -133,6 +156,30 @@ export const projectFlagService = service }, }), + removeFlagGroupOwners: builder.mutation< + Res['projectFlag'], + Req['removeFlagGroupOwners'] + >({ + invalidatesTags: (res) => [{ id: res?.id, type: 'ProjectFlag' }], + query: ({ feature_id, project_id, ...body }) => ({ + body, + method: 'POST', + url: `projects/${project_id}/features/${feature_id}/remove-group-owners/`, + }), + }), + + removeFlagOwners: builder.mutation< + Res['projectFlag'], + Req['removeFlagOwners'] + >({ + invalidatesTags: (res) => [{ id: res?.id, type: 'ProjectFlag' }], + query: ({ feature_id, project_id, ...body }) => ({ + body, + method: 'POST', + url: `projects/${project_id}/features/${feature_id}/remove-owners/`, + }), + }), + removeProjectFlag: builder.mutation({ invalidatesTags: [ { id: 'LIST', type: 'ProjectFlag' }, @@ -208,10 +255,14 @@ export async function createProjectFlag( } export const { + useAddFlagGroupOwnersMutation, + useAddFlagOwnersMutation, useCreateProjectFlagMutation, useGetFeatureListQuery, useGetProjectFlagQuery, useGetProjectFlagsQuery, + useRemoveFlagGroupOwnersMutation, + useRemoveFlagOwnersMutation, useRemoveProjectFlagMutation, useUpdateProjectFlagMutation, } = projectFlagService diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index fc48e4e17841..f9be110890f9 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,12 +653,35 @@ export type Req = { } createProjectFlag: { project_id: number - body: ProjectFlag + body: ProjectFlag & { + owners?: number[] + group_owners?: number[] + } } removeProjectFlag: { project_id: number flag_id: number } + addFlagOwners: { + project_id: number + feature_id: number + user_ids: number[] + } + removeFlagOwners: { + project_id: number + feature_id: number + user_ids: number[] + } + addFlagGroupOwners: { + project_id: number + feature_id: number + group_ids: number[] + } + removeFlagGroupOwners: { + project_id: number + feature_id: number + group_ids: number[] + } updateEnvironment: { id: number; body: Environment } createCloneIdentityFeatureStates: { environment_id: string diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 2265891e198c..1ba22e5a5ad0 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 @@ -623,7 +624,7 @@ export type ProjectFlag = { num_identity_overrides: number | null num_segment_overrides: number | null owners: User[] - owner_groups: UserGroupSummary[] + group_owners: UserGroupSummary[] metadata: Metadata[] | [] project: number tags: number[] @@ -635,7 +636,6 @@ export type ProjectFlag = { last_successful_repository_scanned_at: string last_feature_found_at: string }[] - environment_feature_state?: FeatureState } export type FeatureListProviderData = { diff --git a/frontend/common/utils/getUserDisplayName.tsx b/frontend/common/utils/getUserDisplayName.tsx index ec49935670e6..8c8fe6d38b6d 100644 --- a/frontend/common/utils/getUserDisplayName.tsx +++ b/frontend/common/utils/getUserDisplayName.tsx @@ -1,7 +1,7 @@ import { AccountModel, User } from 'common/types/responses' export default function ( - user: AccountModel | undefined, + user: AccountModel | User | undefined, defaultName = 'Unknown', ) { if (!user) { diff --git a/frontend/web/components/FlagOwnerGroups.js b/frontend/web/components/FlagOwnerGroups.js deleted file mode 100644 index 433341ec39c5..000000000000 --- a/frontend/web/components/FlagOwnerGroups.js +++ /dev/null @@ -1,91 +0,0 @@ -import React, { Component } from 'react' -import data from 'common/data/base/_data' -import UserSelect from './UserSelect' -import ConfigProvider from 'common/providers/ConfigProvider' -import Icon from './Icon' -import { close } from 'ionicons/icons' -import { IonIcon } from '@ionic/react' -import GroupSelect from './GroupSelect' -import { getProjectFlag } from 'common/services/useProjectFlag' -import { getStore } from 'common/store' -import ConnectedGroupSelect from './ConnectedGroupSelect' -import SettingsButton from './SettingsButton' - -class TheComponent extends Component { - state = {} - componentDidMount() { - this.getData() - } - - getData = () => { - getProjectFlag(getStore(), { - id: this.props.id, - project: this.props.projectId, - }).then((res) => { - const groupOwners = (res.data.group_owners || []).map((v) => v.id) - this.setState({ groupOwners }) - }) - } - - addOwner = (id) => { - this.setState({ groupOwners: (this.state.groupOwners || []).concat(id) }) - data.post( - `${Project.api}projects/${this.props.projectId}/features/${this.props.id}/add-group-owners/`, - { - group_ids: [id], - }, - ) - } - - removeOwner = (id) => { - this.setState({ - groupOwners: (this.state.groupOwners || []).filter((v) => v !== id), - }) - data.post( - `${Project.api}projects/${this.props.projectId}/features/${this.props.id}/remove-group-owners/`, - { - group_ids: [id], - }, - ) - } - - getGroupOwners = (users, groupOwners) => - users ? users.filter((v) => groupOwners.includes(v.id)) : [] - - render() { - return ( - - {({ groups }) => { - return ( -
- - this.setState({ showUsers: !this.state.showUsers }) - } - /> - } - onClick={() => { - this.setState({ showUsers: true }) - }} - > - Assigned groups - -
- ) - }} - - ) - } -} - -export default ConfigProvider(TheComponent) diff --git a/frontend/web/components/FlagOwnerGroups.tsx b/frontend/web/components/FlagOwnerGroups.tsx new file mode 100644 index 000000000000..a48efb57b5a7 --- /dev/null +++ b/frontend/web/components/FlagOwnerGroups.tsx @@ -0,0 +1,43 @@ +import React, { FC, useState } from 'react' +import ConnectedGroupSelect from './ConnectedGroupSelect' +import SettingsButton from './SettingsButton' +import AccountStore from 'common/stores/account-store' + +type FlagOwnerGroupsProps = { + selectedIds: number[] + onAdd: (id: number) => void + onRemove: (id: number) => void +} + +const FlagOwnerGroups: FC = ({ + onAdd, + onRemove, + selectedIds, +}) => { + const [showGroups, setShowGroups] = useState(false) + + return ( +
+ onAdd(id)} + onRemove={(id: number) => onRemove(id)} + onToggle={() => setShowGroups(!showGroups)} + /> + } + onClick={() => setShowGroups(true)} + > + Assigned groups + +
+ ) +} + +export default FlagOwnerGroups diff --git a/frontend/web/components/FlagOwners.js b/frontend/web/components/FlagOwners.js deleted file mode 100644 index 66bf7ba4e1d8..000000000000 --- a/frontend/web/components/FlagOwners.js +++ /dev/null @@ -1,108 +0,0 @@ -import React, { Component } from 'react' -import data from 'common/data/base/_data' -import UserSelect from './UserSelect' -import ConfigProvider from 'common/providers/ConfigProvider' -import Icon from './Icon' -import { close } from 'ionicons/icons' -import { IonIcon } from '@ionic/react' -import { getProjectFlag } from 'common/services/useProjectFlag' -import { getStore } from 'common/store' -import SettingsButton from './SettingsButton' -import OrganisationProvider from 'common/providers/OrganisationProvider' -import getUserDisplayName from 'common/utils/getUserDisplayName' - -class TheComponent extends Component { - state = {} - componentDidMount() { - this.getData() - } - - getData = () => { - getProjectFlag(getStore(), { - id: this.props.id, - project: this.props.projectId, - }).then((res) => { - const owners = (res.data.owners || []).map((v) => v.id) - this.setState({ owners }) - }) - } - - addOwner = (id) => { - this.setState({ owners: (this.state.owners || []).concat(id) }) - data.post( - `${Project.api}projects/${this.props.projectId}/features/${this.props.id}/add-owners/`, - { - user_ids: [id], - }, - ) - } - - removeOwner = (id) => { - this.setState({ owners: (this.state.owners || []).filter((v) => v !== id) }) - data.post( - `${Project.api}projects/${this.props.projectId}/features/${this.props.id}/remove-owners/`, - { - user_ids: [id], - }, - ) - } - - getOwners = (users, owners) => - users ? users.filter((v) => owners.includes(v.id)) : [] - - render() { - return ( - - {({ users }) => { - const ownerUsers = this.getOwners(users, this.state.owners || []) - return ( -
- - {ownerUsers.map((u) => ( - this.removeOwner(u.id)} - className='chip mr-2' - > - - {getUserDisplayName(u)} - - - - - - ))} - {!ownerUsers.length && ( -
This flag has no assigned users
- )} - - } - feature={'FLAG_OWNERS'} - onClick={() => { - this.setState({ showUsers: !this.state.showUsers }) - }} - > - Assigned users - - - - this.setState({ showUsers: !this.state.showUsers }) - } - /> -
- ) - }} - - ) - } -} - -export default ConfigProvider(TheComponent) diff --git a/frontend/web/components/FlagOwners.tsx b/frontend/web/components/FlagOwners.tsx new file mode 100644 index 000000000000..9fa5840ddfda --- /dev/null +++ b/frontend/web/components/FlagOwners.tsx @@ -0,0 +1,70 @@ +import React, { FC, useState } from 'react' +import UserSelect from './UserSelect' +import { IonIcon } from '@ionic/react' +import { close } from 'ionicons/icons' +import SettingsButton from './SettingsButton' +import { useGetUsersQuery } from 'common/services/useUser' +import AccountStore from 'common/stores/account-store' +import getUserDisplayName from 'common/utils/getUserDisplayName' + +type FlagOwnersProps = { + selectedIds: number[] + onAdd: (id: number) => void + onRemove: (id: number) => void +} + +const FlagOwners: FC = ({ onAdd, onRemove, selectedIds }) => { + const [showUsers, setShowUsers] = useState(false) + + const organisationId = AccountStore.getOrganisation()?.id + const { data: users } = useGetUsersQuery( + { organisationId }, + { skip: !organisationId }, + ) + + const ownerUsers = users + ? users.filter((v) => selectedIds.includes(v.id)) + : [] + + return ( +
+ + {ownerUsers.map((u) => ( + onRemove(u.id)} + className='chip mr-2' + > + + {getUserDisplayName(u)} + + + + + + ))} + {!ownerUsers.length &&
This flag has no assigned users
} + + } + feature={'FLAG_OWNERS'} + onClick={() => setShowUsers(!showUsers)} + > + Assigned users + + + setShowUsers(!showUsers)} + /> +
+ ) +} + +export default FlagOwners diff --git a/frontend/web/components/UserSelect.tsx b/frontend/web/components/UserSelect.tsx index d7116a1b13e9..df0a9c822ffd 100644 --- a/frontend/web/components/UserSelect.tsx +++ b/frontend/web/components/UserSelect.tsx @@ -9,14 +9,14 @@ interface UserSelectProps { value: any[] isOpen: boolean onToggle: () => void - disabled: boolean - onRemove?: (id: string) => void - onAdd?: (id: string) => void + disabled?: boolean + onRemove?: (id: number) => void + onAdd?: (id: number) => void onChange?: (value: any[]) => void } export const UserSelect: React.FC = ({ - disabled, + disabled = false, isOpen, onAdd, onChange, diff --git a/frontend/web/components/feature-summary/FeatureRow.tsx b/frontend/web/components/feature-summary/FeatureRow.tsx index 23bf1c8bf1ef..07713f7a5bb3 100644 --- a/frontend/web/components/feature-summary/FeatureRow.tsx +++ b/frontend/web/components/feature-summary/FeatureRow.tsx @@ -25,6 +25,8 @@ 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 { useGetProjectQuery } from 'common/services/useProject' +import getUserDisplayName from 'common/utils/getUserDisplayName' import FeatureName from './FeatureName' import FeatureDescription from './FeatureDescription' import FeatureTags from './FeatureTags' @@ -102,6 +104,12 @@ const FeatureRow: FC = (props) => { { skip: !projectFlag?.project }, ) + const { data: projectData } = useGetProjectQuery( + { id: projectId }, + { skip: !projectId }, + ) + const enforceFeatureOwners = !!projectData?.enforce_feature_owners + useEffect(() => { const { feature } = Utils.fromParam() const { id } = projectFlag @@ -224,9 +232,24 @@ const FeatureRow: FC = (props) => { tab, } + const ownerChips = enforceFeatureOwners + ? [ + ...(projectFlag.owners ?? []).map((u) => ({ + id: `user-${u.id}`, + label: getUserDisplayName(u), + })), + ...(projectFlag.group_owners ?? []).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} + + ))} + {ownerChips.length > 3 && ( + +{ownerChips.length - 3} + )} +
+ )} , , modalCssClass, diff --git a/frontend/web/components/import-export/FeatureImport.tsx b/frontend/web/components/import-export/FeatureImport.tsx index bb50bc0bf530..b4ee3bef3a02 100644 --- a/frontend/web/components/import-export/FeatureImport.tsx +++ b/frontend/web/components/import-export/FeatureImport.tsx @@ -176,6 +176,7 @@ const FeatureExport: FC = ({ projectId }) => { return { created_date: createdDate, default_enabled: importItem.enabled, + group_owners: [], id: i, initial_value: importItem.value, isNew: true, @@ -185,7 +186,6 @@ const FeatureExport: FC = ({ projectId }) => { name: importItem.name, num_identity_overrides: 0, num_segment_overrides: 0, - owner_groups: [], owners: [], project: ProjectStore.model!.id, tags: [], 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. + + )} +