feat(cohorts): expose sync state and allow updating the managed segment - #8386
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe cohort model now stores an optional Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The PR adds cohort synchronization state and allows updating the managed segment. It is mergeable with explicit owner awareness because synchronization timestamps may advance before downstream membership application completes, combined updates may partially persist on failure, and concurrent creation may still produce duplicate active source cohorts. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
Docker builds report
|
|
@themis-blindfold review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c13fdc1e-9773-40da-a188-8f929dfc71eb
📒 Files selected for processing (8)
api/cohorts/migrations/0004_cohort_last_synced_at.pyapi/cohorts/models.pyapi/cohorts/serializers.pyapi/cohorts/services.pyapi/cohorts/views.pyapi/tests/unit/cohorts/test_views.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mdopenapi.yaml
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
![themis-blindfold[bot]](https://avatars.githubusercontent.com/in/4278417?s=60&v=4){kind=small}
⚖️ Themis review: 🟠 Fix before mergeTL;DR: The new sync-state response makes cohort list requests issue an additional membership aggregate for every cohort, and the new PATCH contract accepts
🟠 Majors
📝 Walkthrough
🧪 How to verify
Product take: Sync progress and inline cohort editing are a solid dashboard improvement, but list performance and a silently ineffective documented input would undermine that experience at scale. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. A little cohort status goes a long way—once it stops counting one cohort at a time · reviewed at c30addb |
c30addb to
a5f02fe
Compare
github-actions
Bot
added
feature
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: de270ccb-8280-40d8-8f50-ea2dd8c9a723
📒 Files selected for processing (6)
api/cohorts/migrations/0005_cohort_last_synced_at.pyapi/cohorts/models.pyapi/cohorts/serializers.pyapi/cohorts/services.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mdopenapi.yaml
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8386 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 1620 1621 +1
Lines 66018 66102 +84
=======================================
+ Hits 65234 65318 +84
Misses 784 784 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19903 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #19903 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19903 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19903 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19889 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19889 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19889 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19889 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19887 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19887 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19887 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19887 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
github-actions
Bot
added
docs
github-actions
Bot
added
docs
1dcbb18 to
7cf092c
Compare
github-actions
Bot
added
docs
|
@themis-blindfold review |
⚖️ Themis review: ✅ Ship itThe cohort detail and list responses now expose synchronisation progress without an N+1 query path, and PATCH preserves the managed-segment and metadata semantics. No new actionable defects found. CI was still running at review time; completed CodeQL, pre-commit, documentation, and preview checks succeeded, with no completed failures.
⚖️ Acknowledged
📝 Walkthrough
🧪 How to verify
Product take: Solid dashboard-enabling improvement: users can see cohort sync progress and correct cohort labels without leaving the cohort flow. 🧭 Assumptions & unverified claims
The cohorts can now report their whereabouts without sending the database on a scavenger hunt · reviewed at 7cf092c |
for more information, see https://pre-commit.ci
…etadata on update
7cf092c to
94c8051
Compare
github-actions
Bot
added
feature
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Backend support for the CSV cohort detail view:
membership_counts(applied,pending_add,pending_remove) so the dashboard can derive sync status and progress.PATCHon a cohort updates the managed segment'snameanddescription.PUTis not exposed.Cohort.last_synced_at, stamped on every CSV sync.How did you test this code?
Unit tests for the new counts, update permissions and the
last_synced_atstamp. Manually QA'd with the stacked dashboard branch (feat/cohort-resync-frontend).