feat(multivariate): add per-feature unique key to multivariate options#7698
feat(multivariate): add per-feature unique key to multivariate options#7698gagantrivedi wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
github-actions
Bot
added
api
gagantrivedi
force-pushed
the
feat/multivariate-option-key
branch
from
d7fb119 to
c2adfb4
Compare
github-actions
Bot
added
feature
Add a nullable, per-feature-unique `key` to MultivariateFeatureOption so each variant carries a stable, human-readable identifier. This is the first slice towards attributing experimentation exposure data to the specific variant a user was shown. - Model: nullable `key` CharField with a (feature, key) unique_together constraint. NULLs remain distinct in Postgres, so the key stays optional while non-null keys are unique per feature. - Serializer: expose `key`, and enforce uniqueness manually to return a clean 400 (the auto-generated UniqueTogetherValidator would otherwise force `key` to be required on create).
gagantrivedi
force-pushed
the
feat/multivariate-option-key
branch
from
c2adfb4 to
df85009
Compare
github-actions
Bot
added
feature
gagantrivedi
changed the title
github-actions
Bot
added
feature
Docker builds report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7698 +/- ##
=======================================
Coverage 98.52% 98.52%
=======================================
Files 1444 1445 +1
Lines 54971 55021 +50
=======================================
+ Hits 54161 54211 +50
Misses 810 810 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise
Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Flagged 2 possible loopholes but otherwise I haven't identified other leaks into existing MV features.
It's mostly about reviewing serializers using the NestedMultivariateFeatureOptionSerializer and an alignment on empty string values.
It would be nice to also add tests on updating keys in MV options and then maybe add some coverage around CreateFeatureSerializer to sleep at peace?
| ) | ||
|
|
||
| # A stable, human-readable identifier for the variant. | ||
| key = models.CharField(max_length=2000, null=True, blank=True) |
| # Then | ||
| assert ( | ||
| MultivariateFeatureOption.objects.filter( | ||
| feature=feature, key__isnull=True |
There was a problem hiding this comment.
This one is only testing null and not "" which would trigger the constraint. Do we want to allow users to use "" and make them unique or normalize them into null ?
| "string_value", | ||
| "boolean_value", | ||
| "default_percentage_allocation", | ||
| "key", |
There was a problem hiding this comment.
So, this serializer is also used in CreateFeatureSerializer, that would make key writable there too right.
Just for context, the frontend is only creating MV options via the mv-options dedicated endpoint but I believe (not tested it though) that it would make it writable in the feature endpoint too without validation.
We should make it read_only here and writable only in the MultivariateFeatureOptionSerializer
3 participants
docs/if required so people know about the feature. (deferred — the key isn't yet surfaced to SDKs/UI; docs will land with the SDK/experimentation slices.)Changes
Contributes to
Adds a nullable, per-feature-unique
keytoMultivariateFeatureOption, giving each variant a stable, human-readable identifier (e.g.control,variant_a). This is the foundation for attributing experimentation exposure/metrics to the specific variant a user was shown.features/multivariate/models.py): nullablekeyCharField (max_length=2000, mirroringFeature.name) with a("feature", "key")unique_together. Postgres treatsNULLas distinct, so the key stays optional now while non-null keys are unique per feature — no not-null enforcement, no backfill.0009): single additive migration (field + constraint), zero data migration.features/multivariate/serializers.py): exposeskey. The(feature, key)unique_togethermakes DRF both attach aUniqueTogetherValidatorand mark the fieldrequired=True— either of which would break the existing no-key create flow — so the auto-validator andrequiredare overridden, and uniqueness is enforced manually to return a clean400(the DB constraint remains the final guard).Out of scope (follow-up slices): threading
keythrough the flag engine + environment document, the SDK flag-responsevariant_key, experimentation consumingkeyfor per-variant attribution, and the frontend variation-key input/display.How did you test this code?
Automated tests, written TDD-first (Given/When/Then, both auth types via
admin_client_new):tests/unit/.../test_unit_multivariate_models.py):keydefaults toNone; duplicate non-null key on the same feature →IntegrityError; multiple null keys on the same feature allowed; same key across different features allowed.tests/integration/.../test_integration_multivariate.py): create withkeyround-trips the value; duplicate key →400with a clear message.Verification run locally:
ruff check,ruff format --check, andmypy(strict, incl. tests): clean.