Skip to content

feat(cohorts): add environment cohort CRUD API - #8248

Merged
gagantrivedi merged 23 commits into
mainfrom
feat/cohort-crud
Aug 12, 2026
Merged

feat(cohorts): add environment cohort CRUD API#8248
gagantrivedi merged 23 commits into
mainfrom
feat/cohort-crud

Conversation

@gagantrivedi

@gagantrivedi gagantrivedi commented Aug 10, 2026

Copy link
Copy Markdown
Member

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Contributes to cohort sync (follow-up to #8211, #8212, #8213).

Adds the environment-scoped cohort CRUD API:

  • POST/GET/DELETE on /api/v1/environments/<api_key>/cohorts/; create builds a managed segment with a single IS_SET condition on the cohort's system trait key.
  • Deletion drains memberships from the identity store before soft-deleting: rows flip to pending_remove and the applier finishes the soft delete once drained (deletion_requested_at marks the wait).
  • Segments backing a live cohort reject direct dashboard edits, deletes and clones (403), and carry a read-only managed_by: "cohort" marker so the dashboard can render them differently. Cloning them is blocked too.
  • Non-edge projects are rejected with 400 DynamoNotEnabledError (edge-identities precedent) — core/Postgres identity support branches out later.
  • The API requires the start-up plan or above (require_minimum_plan; self-hosted deployments are exempt as usual), plus MANAGE_SEGMENTS (project) and MANAGE_SEGMENT_OVERRIDES (environment) to create/delete.
  • cohort.created / cohort.deletion_requested / cohort.deleted events.

How did you test this code?

Unit tests for the create/delete services, applier task deletion branch, views, permissions (including plan gating via saas_mode), and segment edit protection; the full segments view suite still passes.

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Aug 12, 2026 9:01am
flagsmith-frontend-preview Ignored Ignored Preview Aug 12, 2026 9:01am
flagsmith-frontend-staging Ignored Ignored Preview Aug 12, 2026 9:01am

Request Review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Adds environment-scoped cohort CRUD endpoints with Startup plan and environment permission checks. Adds cohort creation with linked segments, rules, and conditions. Adds deferred deletion that drains memberships before removing the cohort and segment. Excludes deletion-pending cohorts from listings. Prevents changes to cohort-managed segments. Adds migrations, OpenAPI definitions, event catalogue entries, MCP schema updates, and unit tests.

Estimated code review effort: 4 (Complex) | ~60 minutes


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added api Issue related to the REST API docs Documentation updates feature New feature or request and removed docs Documentation updates labels Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.73%. Comparing base (c761517) to head (23d2d60).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #8248    +/-   ##
========================================
  Coverage   98.72%   98.73%            
========================================
  Files        1556     1567    +11     
  Lines       61929    62376   +447     
========================================
+ Hits        61141    61588   +447     
  Misses        788      788            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added feature New feature or request docs Documentation updates and removed feature New feature or request docs Documentation updates labels Aug 10, 2026
@gagantrivedi
gagantrivedi marked this pull request as ready for review August 10, 2026 08:23
@gagantrivedi
gagantrivedi requested review from a team as code owners August 10, 2026 08:23
@github-actions github-actions Bot added the feature New feature or request label Aug 11, 2026
@github-actions github-actions Bot added the docs Documentation updates label Aug 11, 2026
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 11, 2026
@gagantrivedi
gagantrivedi requested a review from Zaimwa9 August 11, 2026 08:00
@github-actions github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 11, 2026
@Zaimwa9

Zaimwa9 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

@themis-blindfold review

Comment thread api/cohorts/views.py
@themis-blindfold

Copy link
Copy Markdown
Contributor

⚖️ Themis review: 🧹 Ship it, nits inside

Well-structured CRUD API for environment-scoped cohorts. The create/delete lifecycle is clean: creation wraps segment + rule + condition + cohort in one atomic block, and deletion drains memberships before soft-deleting. Segment edit protection via check_object_permissions is the right DRF hook, and the queryset hides deletion-pending cohorts. Permission layering (plan gate + MANAGE_SEGMENTS + edge-sync guard) is thorough. CI is fully green.

Area Score
🎯 Correctness 4/5
🧪 Test coverage 4/5
📐 Code quality 4/5
🚀 Product impact 4/5

🧹 Nits

  • api/cohorts/services.py:118 — no OpenFeature flag check; see inline comment.

⚖️ Acknowledged

  • api/cohorts/services.py:144cohort.id after delete() survives because Cohort is a SoftDeleteObject — accepted by @gagantrivedi.
  • api/cohorts/services.pycohort.deletion_requested event ordering — thread resolved by @coderabbitai[bot].
  • api/cohorts/views.py:36 — viewset docstring — thread resolved by @coderabbitai[bot].
  • openapi.yaml:2145 — delete response status 202 — thread resolved by @coderabbitai[bot].
  • api/cohorts/views.py:49 — edge sync gate consistency — thread resolved by @coderabbitai[bot].
  • api/cohorts/services.py — task hand-off durability — thread resolved by @gagantrivedi.
📝 Walkthrough
  • Cohort CRUD API — new CohortViewSet with list/create/retrieve/delete, environment-scoped under /api/v1/environments/<key>/cohorts/, gated by plan + MANAGE_SEGMENTS + edge-sync.
  • Cohort servicescreate_cohort builds a managed segment with an IS_SET condition on the cohort's system trait key; delete_cohort marks deletion, flips memberships to PENDING_REMOVE, and dispatches the applier task.
  • Segment protectionmanaged_by field on Segment, check_object_permissions override blocks update/delete/clone on cohort-managed segments.
  • Task extensionapply_cohort_membership_deltas now calls finalise_cohort_deletion after draining memberships when deletion_requested_at is set.
  • Tests — view tests for CRUD, permissions, plan gating; service tests for create/delete lifecycle; task test for drain-then-finalise; segment tests for managed-segment protection.
  • Docs/specs — openapi.yaml, MCP openapi.json, and events catalogue updated.
🧪 How to verify
  1. Run the full cohort test suite: pytest api/tests/unit/cohorts/ -v
  2. Run segment view tests to confirm managed-segment protection: pytest api/tests/unit/segments/test_unit_segments_views.py -v -k cohort
  3. Verify no migration conflicts: python api/manage.py showmigrations cohorts segments | grep '\[ \]' should be empty.
  4. Confirm the OpenAPI spec regenerates cleanly: python api/manage.py spectacular --validate (if available).

Automate: The existing CI already covers unit tests, coverage, and E2E.

Product take: This is the control-plane API for cohort sync — a meaningful product capability that lets users manage segment membership from uploaded identifier lists. It's a solid building block; the real user value lands when the ingestion API (upload) ships on top of this.

🧭 Assumptions & unverified claims
  • The review assumes Flagsmith's task processor writes task rows to the same database, so delay() inside transaction.atomic() commits atomically with the cohort state change. The task processor source is in an external package (flagsmith-common) and was not inspectable in this environment.
  • The review assumes SoftDeleteManager (from django-softdelete) filters out soft-deleted objects by default, which is standard for the package but was not verified against the installed version's source.

A feature that needs plan gating, edge gating, and a drain queue just to say goodbye — cohort deletion has more ceremony than a royal wedding. · reviewed at faa346b

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8d9459f1-b266-4f9d-8b98-8bdb902ed771

📥 Commits

Reviewing files that changed from the base of the PR and between b45d482 and faa346b.

📒 Files selected for processing (12)
  • api/cohorts/serializers.py
  • api/cohorts/services.py
  • api/segments/migrations/0031_segment_managed_by.py
  • api/segments/models.py
  • api/segments/serializers.py
  • api/segments/views.py
  • api/tests/unit/cohorts/test_services.py
  • api/tests/unit/cohorts/test_views.py
  • api/tests/unit/segments/test_unit_segments_views.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md
  • mcp/src/flagsmith_mcp/openapi.json
  • openapi.yaml

Comment thread api/cohorts/services.py
Comment thread api/segments/migrations/0031_segment_managed_by.py
Comment thread api/tests/unit/cohorts/test_views.py
Comment thread mcp/src/flagsmith_mcp/openapi.json
Comment thread api/cohorts/permissions.py Outdated
Comment thread api/segments/views.py

@Zaimwa9 Zaimwa9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Happily approving!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants