feat(multivariate): add per-feature unique key to multivariate options#7698
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 55061 +90
=======================================
+ Hits 54161 54251 +90
Misses 810 810 ☔ View full report in Codecov by Harness. 🚀 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
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
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?
Address review: 2000 characters is excessive for an analytics identifier. Shrink `key` to 255 characters and validate it as a slug (letters, numbers, underscores, hyphens) using Django's built-in validator, which DRF copies to the serializer field. Also move the field's description from a code comment to `help_text`. The unreleased migration 0009 is updated in place.
github-actions
Bot
added
feature
Address review: an empty string key would act as a unique value of its own under the (feature, key) constraint, allowing only one blank key per feature with a confusing error. Clients that mean "no key" must omit the field or send null; an explicit empty string is now rejected with a 400. Existing clients are unaffected: the field is optional, and omitting it stores null. The unreleased migration 0009 is updated in place.
github-actions
Bot
added
feature
…ndpoint Address review: NestedMultivariateFeatureOptionSerializer is reused as a writable nested serializer by CreateFeatureSerializer, which made `key` writable through the feature endpoints without uniqueness validation. Make `key` read-only in the nested serializer, keeping it writable only in MultivariateFeatureOptionSerializer where its uniqueness is validated.
github-actions
Bot
added
feature
Address review: add coverage for updating keys on multivariate options — setting a new key, keeping an option's own key without colliding with itself, and rejecting a sibling's key with a 400.
github-actions
Bot
added
feature
github-actions
Bot
added
feature
Setting a key via PUT is already covered by the create round-trip and the remaining update tests (own-key self-collision and sibling-duplicate).
github-actions
Bot
added
feature
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.