From 33aaee17085139f211560dd740ec0d159da071cb Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 14:38:36 +0200 Subject: [PATCH 1/9] feat(cohorts): expose sync state and allow updating the managed segment --- .../migrations/0004_cohort_last_synced_at.py | 15 +++ api/cohorts/models.py | 1 + api/cohorts/serializers.py | 47 +++++++- api/cohorts/services.py | 6 +- api/cohorts/views.py | 6 ++ api/tests/unit/cohorts/test_views.py | 100 ++++++++++++++++++ 6 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 api/cohorts/migrations/0004_cohort_last_synced_at.py diff --git a/api/cohorts/migrations/0004_cohort_last_synced_at.py b/api/cohorts/migrations/0004_cohort_last_synced_at.py new file mode 100644 index 000000000000..c9bb9b6da7e0 --- /dev/null +++ b/api/cohorts/migrations/0004_cohort_last_synced_at.py @@ -0,0 +1,15 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("cohorts", "0003_cohort_sync_key"), + ] + + operations = [ + migrations.AddField( + model_name="cohort", + name="last_synced_at", + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/api/cohorts/models.py b/api/cohorts/models.py index 1efd43e478ff..c83e67aa1613 100644 --- a/api/cohorts/models.py +++ b/api/cohorts/models.py @@ -36,6 +36,7 @@ class Cohort(SoftDeleteExportableModel): external_id = models.CharField(max_length=255, null=True, blank=True) version = models.PositiveIntegerField(default=0) created_at = models.DateTimeField(auto_now_add=True) + last_synced_at = models.DateTimeField(null=True, blank=True) # Deletion drains memberships from the identity store first; the cohort is # only soft-deleted once drained. This marks it as awaiting that final step. deletion_requested_at = models.DateTimeField(null=True, blank=True) diff --git a/api/cohorts/serializers.py b/api/cohorts/serializers.py index e3c4fe639007..95892ae3d14e 100644 --- a/api/cohorts/serializers.py +++ b/api/cohorts/serializers.py @@ -1,11 +1,13 @@ import typing from django.core.files.uploadedfile import UploadedFile +from django.db.models import Count, Q +from drf_spectacular.utils import extend_schema_field from rest_framework import serializers from cohorts.constants import COHORT_CSV_MAX_FILE_SIZE_BYTES from cohorts.exceptions import CsvFileTooLargeError -from cohorts.models import Cohort, CohortSyncKey +from cohorts.models import Cohort, CohortMembershipState, CohortSyncKey from cohorts.services import create_cohort from environments.models import Environment from metadata.serializers import MetadataSerializer, MetadataSerializerMixin @@ -19,12 +21,19 @@ class Meta: model = Segment +class CohortMembershipCountsSerializer(serializers.Serializer): # type: ignore[type-arg] + applied = serializers.IntegerField(min_value=0) + pending_add = serializers.IntegerField(min_value=0) + pending_remove = serializers.IntegerField(min_value=0) + + class CohortSerializer(serializers.ModelSerializer[Cohort]): name = serializers.CharField(max_length=2000, source="segment.name") description = serializers.CharField( source="segment.description", required=False, allow_null=True ) metadata = MetadataSerializer(required=False, many=True, write_only=True) + membership_counts = serializers.SerializerMethodField() class Meta: model = Cohort @@ -38,8 +47,29 @@ class Meta: "source_type", "version", "created_at", + "last_synced_at", + "membership_counts", + ) + read_only_fields = ( + "segment", + "source_type", + "version", + "created_at", + "last_synced_at", + ) + + @extend_schema_field(CohortMembershipCountsSerializer) + def get_membership_counts(self, cohort: Cohort) -> dict[str, int]: + # Clients derive sync status and progress from these. + return cohort.memberships.aggregate( + applied=Count("id", filter=Q(state=CohortMembershipState.APPLIED)), + pending_add=Count( + "id", filter=Q(state=CohortMembershipState.PENDING_ADD) + ), + pending_remove=Count( + "id", filter=Q(state=CohortMembershipState.PENDING_REMOVE) + ), ) - read_only_fields = ("segment", "source_type", "version", "created_at") def validate(self, attrs: dict[str, typing.Any]) -> dict[str, typing.Any]: attrs = super().validate(attrs) @@ -64,6 +94,19 @@ def create(self, validated_data: dict[str, typing.Any]) -> Cohort: _SegmentMetadataHandler()._update_metadata(cohort.segment, metadata_data) return cohort + def update( + self, instance: Cohort, validated_data: dict[str, typing.Any] + ) -> Cohort: + # Only the managed segment's fields are updatable. + validated_data.pop("metadata", None) + segment_data = validated_data.pop("segment", {}) + if segment_data: + segment = instance.segment + for field, value in segment_data.items(): + setattr(segment, field, value) + segment.save(update_fields=list(segment_data)) + return instance + class CohortSyncKeySerializer(serializers.ModelSerializer[CohortSyncKey]): key = serializers.SerializerMethodField() diff --git a/api/cohorts/services.py b/api/cohorts/services.py index 5cdaf40c72b3..089f3f7138ca 100644 --- a/api/cohorts/services.py +++ b/api/cohorts/services.py @@ -404,8 +404,10 @@ def sync_cohort_memberships_from_csv( state=CohortMembershipState.PENDING_REMOVE, updated_at=timezone.now() ) - Cohort.objects.filter(id=cohort.id).update(version=F("version") + 1) - cohort.refresh_from_db(fields=["version"]) + Cohort.objects.filter(id=cohort.id).update( + version=F("version") + 1, last_synced_at=timezone.now() + ) + cohort.refresh_from_db(fields=["version", "last_synced_at"]) apply_cohort_membership_deltas.delay(kwargs={"cohort_id": cohort.id}) flagsmith_cohorts_csv_syncs_total.inc() diff --git a/api/cohorts/views.py b/api/cohorts/views.py index 42bca332aeee..03759226b26d 100644 --- a/api/cohorts/views.py +++ b/api/cohorts/views.py @@ -29,6 +29,9 @@ description="Create a cohort and the managed segment that targets it." ), retrieve=extend_schema(description="Retrieve a cohort."), + partial_update=extend_schema( + description="Update the cohort's managed segment name and description." + ), destroy=extend_schema( description=( "Request cohort deletion. Memberships are drained from identity " @@ -42,6 +45,7 @@ class CohortViewSet( mixins.ListModelMixin, mixins.CreateModelMixin, mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, mixins.DestroyModelMixin, ): serializer_class = CohortSerializer @@ -50,6 +54,8 @@ class CohortViewSet( model_class = Cohort lookup_field = "id" lookup_url_kwarg = "cohort_id" + # PATCH only: a cohort has no meaningful full replacement. + http_method_names = ["get", "post", "patch", "delete", "head", "options"] def get_queryset(self) -> QuerySet[Cohort]: # A cohort awaiting drain-then-delete is already gone from the diff --git a/api/tests/unit/cohorts/test_views.py b/api/tests/unit/cohorts/test_views.py index 9372b1cc6c04..090204bdc99d 100644 --- a/api/tests/unit/cohorts/test_views.py +++ b/api/tests/unit/cohorts/test_views.py @@ -166,6 +166,105 @@ def test_list_cohorts__deletion_requested_cohort__excluded( assert response.json()[0]["name"] == edge_cohort.segment.name +def test_retrieve_cohort__mixed_membership_states__returns_membership_counts( + staff_client: APIClient, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions( # type: ignore[call-arg] + [VIEW_ENVIRONMENT], environment_id=edge_cohort.environment_id + ) + memberships = { + "applied-1": CohortMembershipState.APPLIED, + "applied-2": CohortMembershipState.APPLIED, + "pending-add-1": CohortMembershipState.PENDING_ADD, + "pending-remove-1": CohortMembershipState.PENDING_REMOVE, + } + CohortMembership.objects.bulk_create( + CohortMembership(cohort=edge_cohort, identifier=identifier, state=state) + for identifier, state in memberships.items() + ) + url = reverse( + "api-v1:environments:cohorts:cohorts-detail", + args=[edge_cohort.environment.api_key, edge_cohort.id], + ) + + # When + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["membership_counts"] == { + "applied": 2, + "pending_add": 1, + "pending_remove": 1, + } + assert response.json()["last_synced_at"] is None + + +def test_update_cohort__staff_with_manage_segments__updates_segment_fields( + staff_client: APIClient, + dynamo_enabled_project: Project, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_project_permissions: WithProjectPermissionsCallable, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_project_permissions( # type: ignore[call-arg] + [MANAGE_SEGMENTS], project_id=dynamo_enabled_project.id + ) + with_environment_permissions( # type: ignore[call-arg] + [VIEW_ENVIRONMENT, MANAGE_SEGMENT_OVERRIDES], + environment_id=edge_cohort.environment_id, + ) + url = reverse( + "api-v1:environments:cohorts:cohorts-detail", + args=[edge_cohort.environment.api_key, edge_cohort.id], + ) + + # When + response = staff_client.patch( + url, + data={"name": "renamed", "description": "Updated description"}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + edge_cohort.segment.refresh_from_db() + assert edge_cohort.segment.name == "renamed" + assert edge_cohort.segment.description == "Updated description" + assert response.json()["name"] == "renamed" + assert response.json()["description"] == "Updated description" + + +def test_update_cohort__staff_without_permission__returns_403( + staff_client: APIClient, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions( # type: ignore[call-arg] + [VIEW_ENVIRONMENT], environment_id=edge_cohort.environment_id + ) + url = reverse( + "api-v1:environments:cohorts:cohorts-detail", + args=[edge_cohort.environment.api_key, edge_cohort.id], + ) + + # When + response = staff_client.patch(url, data={"name": "renamed"}, format="json") + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + edge_cohort.segment.refresh_from_db() + assert edge_cohort.segment.name != "renamed" + + def test_delete_cohort__staff_with_manage_segments__returns_202( staff_client: APIClient, dynamo_enabled_project: Project, @@ -501,6 +600,7 @@ def test_sync_csv__staff_with_manage_segments__returns_202_with_counts( assert all(m.state == CohortMembershipState.APPLIED for m in memberships) edge_cohort.refresh_from_db() assert edge_cohort.version == 1 + assert edge_cohort.last_synced_at is not None def test_sync_csv__without_permission__returns_403( From 91ef53a72b442f022a97b80bd1a9d7fdaecb4655 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:17:44 +0000 Subject: [PATCH 2/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/cohorts/serializers.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/api/cohorts/serializers.py b/api/cohorts/serializers.py index 95892ae3d14e..445338b7c2d9 100644 --- a/api/cohorts/serializers.py +++ b/api/cohorts/serializers.py @@ -63,9 +63,7 @@ def get_membership_counts(self, cohort: Cohort) -> dict[str, int]: # Clients derive sync status and progress from these. return cohort.memberships.aggregate( applied=Count("id", filter=Q(state=CohortMembershipState.APPLIED)), - pending_add=Count( - "id", filter=Q(state=CohortMembershipState.PENDING_ADD) - ), + pending_add=Count("id", filter=Q(state=CohortMembershipState.PENDING_ADD)), pending_remove=Count( "id", filter=Q(state=CohortMembershipState.PENDING_REMOVE) ), @@ -94,9 +92,7 @@ def create(self, validated_data: dict[str, typing.Any]) -> Cohort: _SegmentMetadataHandler()._update_metadata(cohort.segment, metadata_data) return cohort - def update( - self, instance: Cohort, validated_data: dict[str, typing.Any] - ) -> Cohort: + def update(self, instance: Cohort, validated_data: dict[str, typing.Any]) -> Cohort: # Only the managed segment's fields are updatable. validated_data.pop("metadata", None) segment_data = validated_data.pop("segment", {}) From 33c8cc631618fe7d6c5600bb8c99fb211c52d860 Mon Sep 17 00:00:00 2001 From: "flagsmith-engineering[bot]" Date: Wed, 26 Aug 2026 13:20:05 +0000 Subject: [PATCH 3/9] chore: Update documentation artefacts --- openapi.yaml | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/openapi.yaml b/openapi.yaml index c1ecffcc5321..df7d5da7cae9 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -2481,6 +2481,45 @@ paths: tags: - Environments x-flagsmith-minimum-plan: START_UP + patch: + operationId: api_v1_environments_cohorts_partial_update + description: Update the cohort's managed segment name and description. + parameters: + - name: cohort_id + in: path + description: A unique integer value identifying this cohort. + required: true + schema: + type: integer + - name: environment_api_key + in: path + required: true + schema: + type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/PatchedCohort' + application/x-www-form-urlencoded: + schema: + $ref: '#/components/schemas/PatchedCohort' + multipart/form-data: + schema: + $ref: '#/components/schemas/PatchedCohort' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Cohort' + security: + - tokenAuth: [] + - Master API Key: [] + tags: + - Environments + x-flagsmith-minimum-plan: START_UP delete: operationId: api_v1_environments_cohorts_destroy description: Request cohort deletion. Memberships are drained from identity data first; the cohort and its segment are deleted once drained. @@ -19558,6 +19597,16 @@ components: type: string format: date-time readOnly: true + last_synced_at: + type: + - string + - 'null' + format: date-time + readOnly: true + membership_counts: + allOf: + - $ref: '#/components/schemas/CohortMembershipCounts' + readOnly: true required: - name CohortCsvSync: @@ -19614,6 +19663,22 @@ components: - removed - unchanged - version + CohortMembershipCounts: + type: object + properties: + applied: + type: integer + minimum: 0 + pending_add: + type: integer + minimum: 0 + pending_remove: + type: integer + minimum: 0 + required: + - applied + - pending_add + - pending_remove CohortSyncKey: type: object properties: @@ -24507,6 +24572,52 @@ components: $ref: '#/components/schemas/VersionChangeSet' ignore_conflicts: type: boolean + PatchedCohort: + type: object + properties: + id: + type: integer + readOnly: true + uuid: + type: string + format: uuid + readOnly: true + name: + type: string + maxLength: 2000 + description: + type: + - string + - 'null' + metadata: + type: array + items: + $ref: '#/components/schemas/Metadata' + writeOnly: true + segment: + type: integer + readOnly: true + source_type: + allOf: + - $ref: '#/components/schemas/SourceTypeEnum' + readOnly: true + version: + type: integer + readOnly: true + created_at: + type: string + format: date-time + readOnly: true + last_synced_at: + type: + - string + - 'null' + format: date-time + readOnly: true + membership_counts: + allOf: + - $ref: '#/components/schemas/CohortMembershipCounts' + readOnly: true PatchedCreateUpdateUserEnvironmentPermission: type: object properties: From 1379c563f84dba6555c272846c99b86eee9d8a29 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 15:32:32 +0200 Subject: [PATCH 4/9] fix(cohorts): reorder last_synced_at migration after mixpanel source --- ...4_cohort_last_synced_at.py => 0005_cohort_last_synced_at.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename api/cohorts/migrations/{0004_cohort_last_synced_at.py => 0005_cohort_last_synced_at.py} (87%) diff --git a/api/cohorts/migrations/0004_cohort_last_synced_at.py b/api/cohorts/migrations/0005_cohort_last_synced_at.py similarity index 87% rename from api/cohorts/migrations/0004_cohort_last_synced_at.py rename to api/cohorts/migrations/0005_cohort_last_synced_at.py index c9bb9b6da7e0..58e481457084 100644 --- a/api/cohorts/migrations/0004_cohort_last_synced_at.py +++ b/api/cohorts/migrations/0005_cohort_last_synced_at.py @@ -3,7 +3,7 @@ class Migration(migrations.Migration): dependencies = [ - ("cohorts", "0003_cohort_sync_key"), + ("cohorts", "0004_mixpanel_source"), ] operations = [ From 3a0eaa7f464c9efd0f8f6463abee8ee46b997335 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 27 Aug 2026 11:03:24 +0200 Subject: [PATCH 5/9] fix(cohorts): annotate membership counts on the queryset and reject metadata on update --- api/cohorts/serializers.py | 18 ++++++- api/cohorts/views.py | 18 ++++++- api/tests/unit/cohorts/test_views.py | 80 ++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 4 deletions(-) diff --git a/api/cohorts/serializers.py b/api/cohorts/serializers.py index 445338b7c2d9..d970a90cb711 100644 --- a/api/cohorts/serializers.py +++ b/api/cohorts/serializers.py @@ -60,7 +60,14 @@ class Meta: @extend_schema_field(CohortMembershipCountsSerializer) def get_membership_counts(self, cohort: Cohort) -> dict[str, int]: - # Clients derive sync status and progress from these. + # Clients derive sync status and progress from these. The viewset + # annotates the counts; a freshly created cohort isn't annotated. + if (applied := getattr(cohort, "applied_count", None)) is not None: + return { + "applied": applied, + "pending_add": getattr(cohort, "pending_add_count", 0), + "pending_remove": getattr(cohort, "pending_remove_count", 0), + } return cohort.memberships.aggregate( applied=Count("id", filter=Q(state=CohortMembershipState.APPLIED)), pending_add=Count("id", filter=Q(state=CohortMembershipState.PENDING_ADD)), @@ -71,6 +78,14 @@ def get_membership_counts(self, cohort: Cohort) -> dict[str, int]: def validate(self, attrs: dict[str, typing.Any]) -> dict[str, typing.Any]: attrs = super().validate(attrs) + if self.instance is not None: + # Metadata is create-only; accepting it silently would misreport + # the PATCH as applied. + if "metadata" in attrs: + raise serializers.ValidationError( + {"metadata": "Metadata cannot be updated."} + ) + return attrs environment = Environment.objects.get( api_key=self.context["view"].kwargs["environment_api_key"] ) @@ -94,7 +109,6 @@ def create(self, validated_data: dict[str, typing.Any]) -> Cohort: def update(self, instance: Cohort, validated_data: dict[str, typing.Any]) -> Cohort: # Only the managed segment's fields are updatable. - validated_data.pop("metadata", None) segment_data = validated_data.pop("segment", {}) if segment_data: segment = instance.segment diff --git a/api/cohorts/views.py b/api/cohorts/views.py index 03759226b26d..def60664f272 100644 --- a/api/cohorts/views.py +++ b/api/cohorts/views.py @@ -1,4 +1,4 @@ -from django.db.models import QuerySet +from django.db.models import Count, Q, QuerySet from drf_spectacular.utils import extend_schema, extend_schema_view from rest_framework import mixins, status, viewsets from rest_framework.decorators import action @@ -10,7 +10,7 @@ from api.serializers import ErrorSerializer from cohorts import services -from cohorts.models import Cohort, CohortSyncKey +from cohorts.models import Cohort, CohortMembershipState, CohortSyncKey from cohorts.permissions import CohortPermission, CohortPlanPermission from cohorts.serializers import ( CohortCsvSyncResultSerializer, @@ -65,6 +65,20 @@ def get_queryset(self) -> QuerySet[Cohort]: .get_queryset() .filter(deletion_requested_at__isnull=True) .select_related("segment") + .annotate( + applied_count=Count( + "memberships", + filter=Q(memberships__state=CohortMembershipState.APPLIED), + ), + pending_add_count=Count( + "memberships", + filter=Q(memberships__state=CohortMembershipState.PENDING_ADD), + ), + pending_remove_count=Count( + "memberships", + filter=Q(memberships__state=CohortMembershipState.PENDING_REMOVE), + ), + ) .order_by("id") ) diff --git a/api/tests/unit/cohorts/test_views.py b/api/tests/unit/cohorts/test_views.py index 090204bdc99d..83f55d293d2c 100644 --- a/api/tests/unit/cohorts/test_views.py +++ b/api/tests/unit/cohorts/test_views.py @@ -1,3 +1,5 @@ +import typing + import pytest from common.environments.permissions import ( MANAGE_SEGMENT_OVERRIDES, @@ -28,6 +30,9 @@ WithProjectPermissionsCallable, ) +if typing.TYPE_CHECKING: + from pytest_django.fixtures import DjangoAssertNumQueries + def test_create_cohort__staff_with_manage_segments__returns_201( staff_client: APIClient, @@ -166,6 +171,48 @@ def test_list_cohorts__deletion_requested_cohort__excluded( assert response.json()[0]["name"] == edge_cohort.segment.name +def test_list_cohorts__multiple_cohorts__membership_counts_in_constant_queries( + staff_client: APIClient, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_environment_permissions: WithEnvironmentPermissionsCallable, + django_assert_num_queries: "DjangoAssertNumQueries", +) -> None: + # Given + environment = edge_cohort.environment + with_environment_permissions( # type: ignore[call-arg] + [VIEW_ENVIRONMENT], environment_id=environment.id + ) + CohortMembership.objects.create( + cohort=edge_cohort, identifier="applied-1", state=CohortMembershipState.APPLIED + ) + other_cohort = Cohort.objects.create( + environment=environment, + segment=Segment.objects.create(name="other", project=environment.project), + ) + CohortMembership.objects.bulk_create( + CohortMembership(cohort=other_cohort, identifier=identifier, state=state) + for identifier, state in { + "pending-add-1": CohortMembershipState.PENDING_ADD, + "pending-remove-1": CohortMembershipState.PENDING_REMOVE, + }.items() + ) + url = reverse( + "api-v1:environments:cohorts:cohorts-list", args=[environment.api_key] + ) + + # When + with django_assert_num_queries(10): + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert [row["membership_counts"] for row in response.json()] == [ + {"applied": 1, "pending_add": 0, "pending_remove": 0}, + {"applied": 0, "pending_add": 1, "pending_remove": 1}, + ] + + def test_retrieve_cohort__mixed_membership_states__returns_membership_counts( staff_client: APIClient, edge_cohort: Cohort, @@ -241,6 +288,39 @@ def test_update_cohort__staff_with_manage_segments__updates_segment_fields( assert response.json()["description"] == "Updated description" +def test_update_cohort__metadata_supplied__returns_400( + staff_client: APIClient, + dynamo_enabled_project: Project, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_project_permissions: WithProjectPermissionsCallable, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_project_permissions( # type: ignore[call-arg] + [MANAGE_SEGMENTS], project_id=dynamo_enabled_project.id + ) + with_environment_permissions( # type: ignore[call-arg] + [VIEW_ENVIRONMENT, MANAGE_SEGMENT_OVERRIDES], + environment_id=edge_cohort.environment_id, + ) + url = reverse( + "api-v1:environments:cohorts:cohorts-detail", + args=[edge_cohort.environment.api_key, edge_cohort.id], + ) + + # When + response = staff_client.patch( + url, data={"name": "renamed", "metadata": []}, format="json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"metadata": ["Metadata cannot be updated."]} + edge_cohort.segment.refresh_from_db() + assert edge_cohort.segment.name != "renamed" + + def test_update_cohort__staff_without_permission__returns_403( staff_client: APIClient, edge_cohort: Cohort, From 3287e6170bd07b080f310d20ac78ced5887cbc3e Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 27 Aug 2026 11:36:49 +0200 Subject: [PATCH 6/9] feat(cohorts): allow updating the managed segment's metadata via PATCH --- api/cohorts/serializers.py | 13 ++++++------- api/cohorts/views.py | 4 +++- api/tests/unit/cohorts/test_views.py | 24 ++++++++++++++++++------ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/api/cohorts/serializers.py b/api/cohorts/serializers.py index d970a90cb711..8effded71399 100644 --- a/api/cohorts/serializers.py +++ b/api/cohorts/serializers.py @@ -78,13 +78,9 @@ def get_membership_counts(self, cohort: Cohort) -> dict[str, int]: def validate(self, attrs: dict[str, typing.Any]) -> dict[str, typing.Any]: attrs = super().validate(attrs) - if self.instance is not None: - # Metadata is create-only; accepting it silently would misreport - # the PATCH as applied. - if "metadata" in attrs: - raise serializers.ValidationError( - {"metadata": "Metadata cannot be updated."} - ) + if self.instance is not None and "metadata" not in attrs: + # A partial update without metadata must not fail the + # required-metadata check. return attrs environment = Environment.objects.get( api_key=self.context["view"].kwargs["environment_api_key"] @@ -109,12 +105,15 @@ def create(self, validated_data: dict[str, typing.Any]) -> Cohort: def update(self, instance: Cohort, validated_data: dict[str, typing.Any]) -> Cohort: # Only the managed segment's fields are updatable. + metadata_data = validated_data.pop("metadata", None) segment_data = validated_data.pop("segment", {}) if segment_data: segment = instance.segment for field, value in segment_data.items(): setattr(segment, field, value) segment.save(update_fields=list(segment_data)) + if metadata_data is not None: + _SegmentMetadataHandler()._update_metadata(instance.segment, metadata_data) return instance diff --git a/api/cohorts/views.py b/api/cohorts/views.py index def60664f272..0257be59dd5f 100644 --- a/api/cohorts/views.py +++ b/api/cohorts/views.py @@ -30,7 +30,9 @@ ), retrieve=extend_schema(description="Retrieve a cohort."), partial_update=extend_schema( - description="Update the cohort's managed segment name and description." + description=( + "Update the cohort's managed segment name, description and metadata." + ) ), destroy=extend_schema( description=( diff --git a/api/tests/unit/cohorts/test_views.py b/api/tests/unit/cohorts/test_views.py index 83f55d293d2c..431cbc304ad4 100644 --- a/api/tests/unit/cohorts/test_views.py +++ b/api/tests/unit/cohorts/test_views.py @@ -288,11 +288,12 @@ def test_update_cohort__staff_with_manage_segments__updates_segment_fields( assert response.json()["description"] == "Updated description" -def test_update_cohort__metadata_supplied__returns_400( +def test_update_cohort__metadata_supplied__updates_segment_metadata( staff_client: APIClient, dynamo_enabled_project: Project, edge_cohort: Cohort, dynamodb_identity_wrapper: DynamoIdentityWrapper, + required_segment_metadata_field_for_dynamo_project: MetadataModelField, with_project_permissions: WithProjectPermissionsCallable, with_environment_permissions: WithEnvironmentPermissionsCallable, ) -> None: @@ -311,14 +312,25 @@ def test_update_cohort__metadata_supplied__returns_400( # When response = staff_client.patch( - url, data={"name": "renamed", "metadata": []}, format="json" + url, + data={ + "metadata": [ + { + "model_field": required_segment_metadata_field_for_dynamo_project.id, + "field_value": 10, + }, + ], + }, + format="json", ) # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json() == {"metadata": ["Metadata cannot be updated."]} - edge_cohort.segment.refresh_from_db() - assert edge_cohort.segment.name != "renamed" + assert response.status_code == status.HTTP_200_OK + metadata = Metadata.objects.get( + model_field=required_segment_metadata_field_for_dynamo_project + ) + assert metadata.object_id == edge_cohort.segment_id + assert metadata.field_value == "10" def test_update_cohort__staff_without_permission__returns_403( From 31429aa276e76c75cc9e784d40e60d22bb124734 Mon Sep 17 00:00:00 2001 From: "flagsmith-engineering[bot]" Date: Thu, 27 Aug 2026 10:11:25 +0000 Subject: [PATCH 7/9] chore: Update documentation artefacts --- openapi.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi.yaml b/openapi.yaml index df7d5da7cae9..27b59be73487 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -2483,7 +2483,7 @@ paths: x-flagsmith-minimum-plan: START_UP patch: operationId: api_v1_environments_cohorts_partial_update - description: Update the cohort's managed segment name and description. + description: 'Update the cohort''s managed segment name, description and metadata.' parameters: - name: cohort_id in: path From 70577781ba6cb4c3e669695b36f57ce55482f39e Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 27 Aug 2026 16:04:56 +0200 Subject: [PATCH 8/9] test(cohorts): split membership-count query assertion into rbac skipif twins --- api/tests/unit/cohorts/test_views.py | 52 ++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/api/tests/unit/cohorts/test_views.py b/api/tests/unit/cohorts/test_views.py index 431cbc304ad4..f6ccc4af38d8 100644 --- a/api/tests/unit/cohorts/test_views.py +++ b/api/tests/unit/cohorts/test_views.py @@ -6,6 +6,7 @@ VIEW_ENVIRONMENT, ) from common.projects.permissions import MANAGE_SEGMENTS +from django.conf import settings from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from django.utils import timezone @@ -171,12 +172,12 @@ def test_list_cohorts__deletion_requested_cohort__excluded( assert response.json()[0]["name"] == edge_cohort.segment.name -def test_list_cohorts__multiple_cohorts__membership_counts_in_constant_queries( +def _assert_list_cohorts_membership_counts_in_constant_queries( staff_client: APIClient, edge_cohort: Cohort, - dynamodb_identity_wrapper: DynamoIdentityWrapper, with_environment_permissions: WithEnvironmentPermissionsCallable, django_assert_num_queries: "DjangoAssertNumQueries", + num_queries: int, ) -> None: # Given environment = edge_cohort.environment @@ -202,7 +203,7 @@ def test_list_cohorts__multiple_cohorts__membership_counts_in_constant_queries( ) # When - with django_assert_num_queries(10): + with django_assert_num_queries(num_queries): response = staff_client.get(url) # Then @@ -213,6 +214,51 @@ def test_list_cohorts__multiple_cohorts__membership_counts_in_constant_queries( ] +@pytest.mark.skipif( + settings.IS_RBAC_INSTALLED is True, + reason="Skip this test if RBAC is installed", +) +def test_list_cohorts__multiple_cohorts_without_rbac__membership_counts_in_constant_queries( + staff_client: APIClient, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_environment_permissions: WithEnvironmentPermissionsCallable, + django_assert_num_queries: "DjangoAssertNumQueries", +) -> None: + # Given / When + # Then + _assert_list_cohorts_membership_counts_in_constant_queries( + staff_client, + edge_cohort, + with_environment_permissions, + django_assert_num_queries, + num_queries=10, + ) + + +@pytest.mark.skipif( + settings.IS_RBAC_INSTALLED is False, + reason="Skip this test if RBAC is not installed", +) +def test_list_cohorts__multiple_cohorts_with_rbac__membership_counts_in_constant_queries( + staff_client: APIClient, + edge_cohort: Cohort, + dynamodb_identity_wrapper: DynamoIdentityWrapper, + with_environment_permissions: WithEnvironmentPermissionsCallable, + django_assert_num_queries: "DjangoAssertNumQueries", +) -> None: # pragma: no cover + # Given / When + # Then + # RBAC's runtime role checks add two permission queries. + _assert_list_cohorts_membership_counts_in_constant_queries( + staff_client, + edge_cohort, + with_environment_permissions, + django_assert_num_queries, + num_queries=12, + ) + + def test_retrieve_cohort__mixed_membership_states__returns_membership_counts( staff_client: APIClient, edge_cohort: Cohort, From 94c805166058ba51486f8fab6a731687df4a77d3 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 28 Aug 2026 09:51:58 +0200 Subject: [PATCH 9/9] chore: regenerate events catalogue --- .../observability/_events-catalogue.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index ed27f15fcd46..8ef98355150a 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -86,7 +86,7 @@ Attributes: ### `cohorts.cohort.deleted` Logged at `info` from: - - `api/cohorts/services.py:457` + - `api/cohorts/services.py:459` Attributes: - `cohort.id` @@ -95,7 +95,7 @@ Attributes: ### `cohorts.cohort.deletion_requested` Logged at `info` from: - - `api/cohorts/services.py:441` + - `api/cohorts/services.py:443` Attributes: - `cohort.id` @@ -104,7 +104,7 @@ Attributes: ### `cohorts.csv.synced` Logged at `info` from: - - `api/cohorts/services.py:413` + - `api/cohorts/services.py:415` Attributes: - `adds.count`