Skip to content

feat: enforce feature ownership project setting #7067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gagantrivedi wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat: enforce feature ownership project setting #7067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ca55b0d
feat: add project setting to enforce feature ownership (#4432)
gagantrivedi Mar 30, 2026
4e0d1e5
fixup: use admin_client_new fixture and extract _validate_enforce_fea…
gagantrivedi Mar 30, 2026
f9a448f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 30, 2026
188ae6c
fixup: fix test names to match 3-part naming convention
gagantrivedi Mar 30, 2026
9acd5b3
fixup: remove redundant master API key test
gagantrivedi Mar 31, 2026
5d60dba
fixup: remove redundant remove_owners test
gagantrivedi Mar 31, 2026
e67da98
fixup: skip auto-adding creator when owners are explicitly provided
gagantrivedi Apr 1, 2026
e6633d3
fixup: address PR review comments 2-5
gagantrivedi Apr 1, 2026
f6ce2cd
feat: migrated-owners-components-to-rtk
Zaimwa9 Apr 1, 2026
9a3dffb
feat: added-error-management-and-fixed-group-owners-mismatch
Zaimwa9 Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 75 additions & 5 deletions api/features/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,6 +31,7 @@
FeatureFlagCodeReferencesRepositoryCountSerializer,
)
from projects.models import Project
from users.models import FFAdminUser, UserPermissionGroup
from users.serializers import (
UserIdsSerializer,
UserListSerializer,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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."
)
Comment on lines +311 to +313
Copy link
Copy Markdown
Member

@khvn26 khvn26 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Does it make sense to include specific user email(s) in the response?

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."
)
Comment on lines +321 to +324
Copy link
Copy Markdown
Member

@khvn26 khvn26 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as here

return group_owners

def validate_name(self, name: str): # type: ignore[no-untyped-def]
view = self.context["view"]

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
29 changes: 29 additions & 0 deletions api/features/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()},
Expand Down
21 changes: 21 additions & 0 deletions api/projects/migrations/0028_add_enforce_feature_owners_to_project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.",
),
),
]
4 changes: 4 additions & 0 deletions api/projects/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions api/projects/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading