feat: cohort CSV sync endpoint and cohort summary on segments - #8352
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
github-actions
Bot
added
api
📝 WalkthroughWalkthroughAdds CSV upload support for cohort membership synchronisation. The change parses and validates identifiers, reconciles membership states in batches, updates cohort versions, queues membership application, records metrics, and returns synchronisation counts. Cohort creation now accepts segment metadata. Segment responses now include related cohort details. The API specifications, observability catalogues, and unit tests cover these changes. Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟠 High · up to The CSV sync endpoint can return successfully while leaving the requested cohort membership unapplied, and concurrent uploads may produce an unintended combined membership set. The PR should not merge until transaction ordering and concurrency handling are fixed or explicitly accepted. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 18a749ae-5fa0-4688-817f-40381dc7f2de
📒 Files selected for processing (17)
api/cohorts/constants.pyapi/cohorts/dataclasses.pyapi/cohorts/exceptions.pyapi/cohorts/metrics.pyapi/cohorts/serializers.pyapi/cohorts/services.pyapi/cohorts/views.pyapi/segments/serializers.pyapi/segments/views.pyapi/tests/unit/cohorts/conftest.pyapi/tests/unit/cohorts/test_services.pyapi/tests/unit/cohorts/test_views.pyapi/tests/unit/segments/test_unit_segments_views.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mddocs/docs/deployment-self-hosting/observability/_metrics-catalogue.mdmcp/src/flagsmith_mcp/openapi.jsonopenapi.yaml
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
# Conflicts: # api/cohorts/serializers.py # api/cohorts/services.py # api/cohorts/views.py # api/tests/unit/cohorts/conftest.py # api/tests/unit/cohorts/test_services.py # api/tests/unit/cohorts/test_views.py # docs/docs/deployment-self-hosting/observability/_events-catalogue.md # openapi.yaml
github-actions
Bot
added
feature
Docker builds report
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
openapi.yaml (2)
29505-29531: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMark all
_SegmentCohortfields as read-only
_SegmentCohortSerializeris used only for the read-onlycohortrelationship, but itsenvironment,source_type,version, anddeletion_requested_atfields remain writable in the generated schema. Add these fields to the serializer’sread_only_fieldsso the nested schema matches its response-only contract andCohortSerializer.
19443-19457: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSet
CohortCsvSync.filetoformat: binary
CohortCsvSyncSerializer.fileis a DRFFileField, andsync-csvaccepts onlymultipart/form-data.format: uridescribes a URL string, so generated clients may send text instead of file bytes.api/cohorts/services.py (1)
204-204: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSchedule membership processing after the transaction commits.
TaskHandler.delaysaves the task directly to thetask_processordatabase. It does not register the save withtransaction.on_commit(). Therefore, whenTASK_PROCESSOR_DATABASE_URLis configured, the worker can run before the membership transaction commits.apply_pending_membershipscan find no rows, return, and leave the committed memberships pending until another task is enqueued.Wrap the task calls at lines 204 and 229 in
transaction.on_commit().Source: Learnings
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f49c3820-7c59-4395-b979-d3b54cd1db04
📒 Files selected for processing (9)
api/cohorts/serializers.pyapi/cohorts/services.pyapi/cohorts/views.pyapi/tests/unit/cohorts/conftest.pyapi/tests/unit/cohorts/test_services.pyapi/tests/unit/cohorts/test_views.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mdmcp/src/flagsmith_mcp/openapi.jsonopenapi.yaml
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19711 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results✅ oss · depot-ubuntu-latest-arm-16 — run #19711 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19711 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19711 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8352 +/- ##
========================================
Coverage 98.79% 98.79%
========================================
Files 1614 1616 +2
Lines 64988 65283 +295
========================================
+ Hits 64204 64499 +295
Misses 784 784 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Adds the API surface the dashboard needs to create segments from a CSV of identifiers, on top of the cohort CRUD from #8248.
CSV sync endpoint
POST /api/v1/environments/{api_key}/cohorts/{id}/sync-csv/: multipart upload withfile,identifier_column(0-based, defaults to the first column) andhas_header(defaults to true).202with{version, added, removed, unchanged, ignored: {empty, duplicates, too_long}}and queues the membership delta application inside the transaction, with a row lock on the cohort and a version bump.413.Cohort creation
POST .../cohorts/now accepts segment metadata (custom fields), applied to the managed segment.Segments API
cohortsummary (id,environment,source_type,version,deletion_requested_at), prefetched on the list view. This lets the dashboard tag CSV segments, route their deletion through the cohorts endpoint, and grey out segments awaiting drain.Observability
flagsmith_cohorts_csv_syncs_totalandflagsmith_cohorts_csv_sync_identifiers, newcohorts.csv.syncedevent, both documented in the observability catalogues.How did you test this code?
Recreates #8294, which GitHub auto-closed as merged during a stack reorder (its former base branch came to contain its head). Same content, reviewed there by @gagantrivedi.