feat(onboarding): Org-level onboarding experiment events - #8242
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
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:
📝 WalkthroughWalkthroughThe change adds optional organisation targeting keys and uses them in OpenFeature evaluation contexts. The frontend selects and stores onboarding variants and passes targeting keys during organisation creation. The API configures Flagsmith exposure and event processing, then tracks first environment evaluations. Production and staging ECS definitions provide event endpoints. OpenAPI schemas and event catalogue locations are updated. Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
github-actions
Bot
added
front-end
The onboarding flow decision is now made in the backend, per organisation. Serving a multivariate split assignment records the experiment exposure; the first SDK evaluation of any of the organisation's environments records the conversion, both against the organisation targeting key. - flagsmith 6.2.0 and openfeature-provider-flagsmith from its experimentation-tracking PR branch - events enabled on the Flagsmith-on-Flagsmith client via FLAGSMITH_ON_FLAGSMITH_SERVER_EVENTS_API_URL (set in staging and production ECS task definitions; never enabled in offline mode) - get_openfeature_client() attaches the provider's exposure hook, so any multivariate split evaluation records an exposure - onboarding_variant field on the organisation serialiser beep boop
The gate no longer evaluates the Flagsmith-on-Flagsmith flag client-side; the backend decides per organisation and the exposure is recorded server-side. beep boop
74e6760 to
015d7fd
Compare
github-actions
Bot
added
feature
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8242 +/- ##
==========================================
- Coverage 98.73% 98.72% -0.01%
==========================================
Files 1556 1558 +2
Lines 61894 62032 +138
==========================================
+ Hits 61111 61244 +133
- Misses 783 788 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f621a0df-02a6-4b18-8da0-dc4dd25e31d7
⛔ Files ignored due to path filters (1)
api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
api/app/settings/common.pyapi/environments/onboarding/services.pyapi/integrations/flagsmith/client.pyapi/organisations/serializers.pyapi/organisations/services.pyapi/pyproject.tomlapi/tests/unit/environments/onboarding/test_unit_environments_onboarding_services.pyapi/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.pyapi/tests/unit/organisations/test_unit_organisations_serializers.pyapi/tests/unit/organisations/test_unit_organisations_services.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mddocs/docs/integrating-with-flagsmith/CLI.mddocs/docs/integrating-with-flagsmith/legacy-cli.mddocs/docs/integrating-with-flagsmith/sdks/client-side-sdks/javascript.mddocs/docs/integrating-with-flagsmith/sdks/server-side.mdxfrontend/common/types/responses.tsfrontend/common/utils/getOnboardingVariant.tsfrontend/web/components/pages/onboarding/GettingStartedGate.tsxinfrastructure/aws/production/ecs-task-definition-admin-api.jsoninfrastructure/aws/production/ecs-task-definition-sdk-api.jsoninfrastructure/aws/production/ecs-task-definition-task-processor.jsoninfrastructure/aws/staging/ecs-task-definition-admin-api.jsoninfrastructure/aws/staging/ecs-task-definition-sdk-api.jsoninfrastructure/aws/staging/ecs-task-definition-task-processor.json
💤 Files with no reviewable changes (1)
- frontend/common/utils/getOnboardingVariant.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f621a0df-02a6-4b18-8da0-dc4dd25e31d7
⛔ Files ignored due to path filters (1)
api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
api/app/settings/common.pyapi/environments/onboarding/services.pyapi/integrations/flagsmith/client.pyapi/organisations/serializers.pyapi/organisations/services.pyapi/pyproject.tomlapi/tests/unit/environments/onboarding/test_unit_environments_onboarding_services.pyapi/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.pyapi/tests/unit/organisations/test_unit_organisations_serializers.pyapi/tests/unit/organisations/test_unit_organisations_services.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mddocs/docs/integrating-with-flagsmith/CLI.mddocs/docs/integrating-with-flagsmith/legacy-cli.mddocs/docs/integrating-with-flagsmith/sdks/client-side-sdks/javascript.mddocs/docs/integrating-with-flagsmith/sdks/server-side.mdxfrontend/common/types/responses.tsfrontend/common/utils/getOnboardingVariant.tsfrontend/web/components/pages/onboarding/GettingStartedGate.tsxinfrastructure/aws/production/ecs-task-definition-admin-api.jsoninfrastructure/aws/production/ecs-task-definition-sdk-api.jsoninfrastructure/aws/production/ecs-task-definition-task-processor.jsoninfrastructure/aws/staging/ecs-task-definition-admin-api.jsoninfrastructure/aws/staging/ecs-task-definition-sdk-api.jsoninfrastructure/aws/staging/ecs-task-definition-task-processor.json
💤 Files with no reviewable changes (1)
- frontend/common/utils/getOnboardingVariant.ts
🛑 Comments failed to post (3)
docs/docs/integrating-with-flagsmith/CLI.md (1)
31-40: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail curl -fsSL https://raw.githubusercontent.com/Flagsmith/flagsmith-cli/main/README.md | grep -nE 'pin.*install|release archive|install.sh'Repository: Flagsmith/flagsmith
Length of output: 647
Pin the installer source before making it the default installation path.
These copy-and-paste commands execute a mutable installer from the
mainbranch, which gives that script code execution on the user's machine if the repository or branch is compromised. Document the pinned installer as the recommended path and label themain-branch command as a quick-start or convenience option.Source: MCP tools
docs/docs/integrating-with-flagsmith/legacy-cli.md (1)
45-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail npx --yes `@flagsmith/cli` get --help | grep -E -- '--output|--api|--identity|ENVIRONMENT'Repository: Flagsmith/flagsmith
Length of output: 1180
🌐 Web query:
@flagsmith/cli legacy get command help ENVIRONMENT_API_KEY options --o -o💡 Result:
The Flagsmith CLI
getcommand is used to retrieve feature flags from the Flagsmith API and output them to a file [1][2]. TheENVIRONMENTargument represents the client-side environment key, which can be provided directly as an argument or via theFLAGSMITH_ENVIRONMENTenvironment variable [1][3]. The-o(or--output) option allows you to specify the destination file path for the output [1][2]. Usage syntax:flagsmith get [ENVIRONMENT] [-o <value>] [additional options][1][2] Available options for thegetcommand include: --o,--output=<value>: The file path to save the output (default:./flagsmith.json) [1][2]. --a,--api=<value>: The API URL to fetch feature flags from (default:https://edge.api.flagsmith.com/api/v1/) [1][2]. --i,--identity=<value>: The identity for which to fetch feature flags [1][2]. --e,--entity=<option>: Specify the entity to fetch, eitherflagsorenvironment(for local evaluation mode) [2]. --p,--pretty: Prettify the resulting JSON output [2]. Example usage:flagsmith get <ENVIRONMENT_API_KEY> -o ./my-flags.json[1][2] If you omit the environment key, the CLI will attempt to use the value set in theFLAGSMITH_ENVIRONMENTenvironment variable [1][3].Citations:
- 1: https://docs.flagsmith.com/integrating-with-flagsmith/CLI
- 2: https://github.com/Flagsmith/flagsmith-cli
- 3: https://docs.flagsmith.com/integrating-with-flagsmith/sdks/client-side-sdks/javascript
Make the examples match the legacy
getinterface.Use
<ENVIRONMENT_API_KEY>instead of<ENVIRONMENT_ID>, and use the declared short options-o,-a, and-irather than--o,--a, and--i.Suggested example changes
- $ flagsmith get <ENVIRONMENT_ID> + $ flagsmith get <ENVIRONMENT_API_KEY> - $ flagsmith get --o ./my-file.json + $ flagsmith get -o ./my-file.json - $ flagsmith get --a https://flagsmith.example.com/api/v1/ + $ flagsmith get -a https://flagsmith.example.com/api/v1/ - $ flagsmith get --i flagsmith_identity + $ flagsmith get -i flagsmith_identity📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.$ flagsmith get <ENVIRONMENT_API_KEY> $ flagsmith get -o ./my-file.json $ flagsmith get -a https://flagsmith.example.com/api/v1/ $ flagsmith get -i flagsmith_identitySource: MCP tools
docs/docs/integrating-with-flagsmith/sdks/server-side.mdx (1)
867-867: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail curl -fsSL https://raw.githubusercontent.com/Flagsmith/flagsmith-cli/main/README.md | grep -nE 'environment document|evaluate.*--js' curl -fsSL https://raw.githubusercontent.com/Flagsmith/flagsmith-nodejs-client/main/sdk/offline_handlers.ts | grep -nE 'buildEnvironmentModel|EnvironmentModel'Repository: Flagsmith/flagsmith
Length of output: 902
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail # Inspect the surrounding context and the referenced command in the documentation. sed -n '840,885p' docs/docs/integrating-with-flagsmith/sdks/server-side.mdx # Search for LocalFileHandler and flagsmith command references in the SDK docs/source files. rg -n "LocalFileHandler|flagsmith eval|--js|flagsmith environment document|environment document" docs docs/docs package.json -S || trueRepository: Flagsmith/flagsmith
Length of output: 16333
Use the environment-document command for
LocalFileHandler.
LocalFileHandlerreads a file and buildsEnvironmentModel.flagsmith eval --jswrites SDK resolution state for frontend SDKs, not the server-side local-evaluation environment document. The Node.js offline setup document soLocalFileHandleruses an invalid payload.Source: MCP tools
Also refreshes a stale line reference in the generated events catalogue. beep boop
When the routing timeout won the race, the late decision still wrote its variant and targeting key — read from whatever identity the SDK held by then, possibly the logged-in user. Decisions are now persisted only when they win the race. beep boop
github-actions
Bot
added
feature
emyller
left a comment
emyller
left a comment
There was a problem hiding this comment.
Backend looks great with only a few nits.
The render check already treats a missing decision as the legacy page, so the defaulted copy was redundant. beep boop
github-actions
Bot
added
feature
…ation The stored targeting key survived invite acceptance and keyless creation paths, so a user's first organisation created long after the entry decision could consume it. Any organisation membership now clears the key. beep boop
There was a problem hiding this comment.
Approving as the code is good, it fulfills the objective and better shipped than refactored.
But overall, at least for the frontend, I think a future rule of thumbs should be to encapsulate every experiment within a custom hook. Because the code is not long-lived and we'll need to clean it up at some point, it will make it way easier to just grep the hook/consumers and delete all at once.
Although this one is trickier by design given the current controller complexity (pure JS, no hook)
talissoncosta
left a comment
talissoncosta
left a comment
There was a problem hiding this comment.
Looks good from a FE perspective! Thanks for addressing the feedback.
5 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #8153.
The onboarding experiment splits on organisations. Both experiment events join on one identifier, chosen before the organisation exists.
The frontend makes the entry decision once, at routing time, under a server-assigned anonymous identity. The anonymous identifier is sent as
targeting_keywhen the organisation is created (in both arms).The backend stores
Organisation.targeting_key, and prefers it in the organisation's OpenFeature evaluation context. The first SDK evaluation of any of the organisation's environments tracks theenvironment.first_evaluatedconversion event against that key, from both first-evaluation feeders.Events are only sent when the new
FLAGSMITH_ON_FLAGSMITH_SERVER_EVENTS_API_URLsetting is configured (added to the staging and production ECS task definitions); FoF offline mode never sends events.Before the experiment starts: configure the
single_pagemultivariate split ononboarding_quickstart_flowin the production FoF environment only, after this is deployed. The dev/E2E environment keeps the flag flat, which keeps the E2E suites'hasFeaturegating equivalent to the entry decision.How did you test this code?
Added unit tests (API and frontend).