Skip to content

fix(cohorts): Populate rules_data on a cohort's segment - #8579

Open
khvn26 wants to merge 3 commits into
mainfrom
fix/cohort-segment-rules-data
Open

khvn26 wants to merge 3 commits into
mainfrom
fix/cohort-segment-rules-data

Conversation

@khvn26

@khvn26 khvn26 commented Sep 23, 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

Closes #8507.

How did you test this code?

Unit tests.

@vercel

vercel Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

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

3 Skipped Deployments
Project Deployment Actions Updated
docs Ignored Ignored Preview Sep 23, 2026 12:14pm UTC
flagsmith-frontend-preview Ignored Ignored Preview Sep 23, 2026 12:14pm UTC
flagsmith-frontend-staging Ignored Ignored Preview Sep 23, 2026 12:14pm UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e1be25ab-7e90-4ecb-9a85-9b16105418ce

📥 Commits

Reviewing files that changed from the base of the PR and between 3a887a5 and 42ec82e.

📒 Files selected for processing (2)
  • api/cohorts/migrations/0006_backfill_cohort_segment_rules_data.py
  • api/tests/unit/cohorts/test_migrations.py

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

create_cohort now creates the cohort before its managed segment. The segment receives rules_data with an ALL rule and an IS_SET condition on the cohort’s system trait. The cohort is then linked to the segment and saved. A data migration backfills eligible existing cohort segments. Tests cover the creation flow and migration. Six event catalogue source references are updated.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 42ec8

New cohorts now get rule data on their managed segment, and a data migration backfills existing live cohort segments with the same rule. That covers both requirements from the linked issue. The migration leaves deleted, unmanaged and already-populated segments unchanged. No outstanding issues block merging.


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 fix docs Documentation updates labels Sep 23, 2026
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.80%. Comparing base (f5345d1) to head (42ec82e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8579   +/-   ##
=======================================
  Coverage   98.80%   98.80%           
=======================================
  Files        1631     1633    +2     
  Lines       66920    66974   +54     
=======================================
+ Hits        66123    66177   +54     
  Misses        797      797           

☔ 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.

A cohort creates its segment's rule and condition as rows, and is the
one path that leaves `Segment.rules_data` unset. The backfill migration
only covered segments that existed when it ran, so every cohort created
since carries none.

That is invisible today, since evaluation reads the rows. It stops being
invisible the moment anything reads the field instead: a segment with no
rules matches nobody, so cohort membership would quietly stop applying.

Build the cohort unsaved to learn the trait key its membership is keyed
on, so the segment can be created carrying its rules in one write.
@khvn26
khvn26 force-pushed the fix/cohort-segment-rules-data branch from 2c745eb to 83f58f6 Compare September 23, 2026 11:58
@github-actions github-actions Bot added fix and removed fix docs Documentation updates labels Sep 23, 2026
@github-actions github-actions Bot added the docs Documentation updates label Sep 23, 2026
@khvn26
khvn26 marked this pull request as ready for review September 23, 2026 12:03
@khvn26
khvn26 requested review from a team as code owners September 23, 2026 12:03
@khvn26
khvn26 requested review from matthewelwell and removed request for a team September 23, 2026 12:03
@github-actions github-actions Bot removed fix docs Documentation updates labels Sep 23, 2026
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-8579 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-8579 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-api-test:pr-8579 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-private-cloud:pr-8579 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith:pr-8579 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-api:pr-8579 Finished ✅ Results ✅

@github-actions github-actions Bot added the fix label Sep 23, 2026

@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: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2ab1e43b-ef9a-493e-a6c3-cbc9d4571354

📥 Commits

Reviewing files that changed from the base of the PR and between f5345d1 and 3a887a5.

📒 Files selected for processing (3)
  • api/cohorts/services.py
  • api/tests/unit/cohorts/test_services.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread api/cohorts/services.py
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20690 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  1 minute
commit  42ec82e
info  🔄 Run: #20690 (attempt 1)

🗂️ Previous results
✅ private-cloud · depot-ubuntu-latest-16 — run #20690 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  4 passed

Details

stats  4 tests across 4 suites
duration  58.6 seconds
commit  42ec82e
info  🔄 Run: #20690 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #20690 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  39.9 seconds
commit  42ec82e
info  🔄 Run: #20690 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #20690 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  34.6 seconds
commit  42ec82e
info  🔄 Run: #20690 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20688 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  3 passed

Details

stats  3 tests across 3 suites
duration  51.7 seconds
commit  3a887a5
info  🔄 Run: #20688 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-16 — run #20688 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  3 passed

Details

stats  3 tests across 3 suites
duration  56.2 seconds
commit  3a887a5
info  🔄 Run: #20688 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #20688 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  46.4 seconds
commit  3a887a5
info  🔄 Run: #20688 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #20688 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  41.8 seconds
commit  3a887a5
info  🔄 Run: #20688 (attempt 1)

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Visual Regression

19 screenshots compared. See report for details.
View full report

@github-actions github-actions Bot added docs Documentation updates and removed fix labels Sep 23, 2026
@github-actions github-actions Bot added fix and removed docs Documentation updates labels Sep 23, 2026

This branch was successfully deployed

1 active (outdated) deployment
Preview – docs — 3a887a51 Deployed Sep 23, 2026 by vercel[bot]
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 fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cohort segments lack rules_data

1 participant