fix(segment_membership): preserve system traits when seeding identities into ClickHouse - #8458
Conversation
…es into ClickHouse
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Docker builds report
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe identity mapper now reads optional Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change preserves cohort system traits during identity reseeding and includes stated unit and integration coverage. No current merge-blocking risk is established. 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: Team
Run ID: 284bfba5-9849-460d-8526-8e83415328ab
📒 Files selected for processing (3)
api/segment_membership/mappers.pyapi/tests/integration/segment_membership/test_segment_membership_clickhouse.pyapi/tests/unit/segment_membership/test_unit_segment_membership_mappers.py
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8458 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 1621 1621
Lines 66138 66158 +20
=======================================
+ Hits 65354 65374 +20
Misses 784 784 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ oss · depot-ubuntu-latest-16 — run #20152 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #20152 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20152 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20152 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20151 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #20151 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #20151 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20151 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
❌ private-cloud · depot-ubuntu-latest-16 — run #20149 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/change-request-test.pw.ts › Change Request Tests › Change requests can be created, approved, and published with four-eyes approval @enterprise ✅ oss · depot-ubuntu-latest-arm-16 — run #20149 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #20149 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
seed_organisation_identitiessilently wiped cohort membership: its mapper read onlyidentity_traits, and the seeded rows — stamped with the scan-startinserted_at— superseded older CDC-written rows (which carry cohortsystem_traits) at ReplacingMergeTree dedup. Any org that gained cohorts before its first seed, and any force re-seed of an org with cohorts, lost cohort membership in ClickHouse until each identity was next written through the CDC stream.map_identity_document_to_clickhouse_rownow mergessystem_traitsover the flattenedidentity_traits(system value wins on key clash), matching the precedence used at flag evaluation and by edge-api's CDC path (Flagsmith/edge-api#707). Asystem_traitsattribute stored as NULL counts as absent. The field is declared onflagsmith_schemas.dynamodb.Identityas of Flagsmith/flagsmith-common#271; this PR bumpsflagsmith-commonto 3.14.0 and reads it typed.How did you test this code?
test_unit_segment_membership_mappers.py: system wins over a spoofed user trait (with Decimal coercion), system-traits-only documents, and NULL-storedsystem_traits.test_segment_membership_clickhouse.pyagainst real ClickHouse: a CDC-written row carrying a cohort trait is superseded by a re-seed and the surviving row still carries the cohort.tests/unit/segment_membership/(67 passed) andtests/integration/segment_membership/(5 passed) suites, plus mypy.