fix(onboarding): let segment overrides match the entry decision - #8288
fix(onboarding): let segment overrides match the entry decision#8288talissoncosta wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to The PR enables onboarding segment overrides to match the entry decision by passing the signup email as a transient trait. A localized type-suppression cleanup remains advisable, but no actionable merge-blocking risk remains. 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
the
front-end
457e7f4 to
d1b2b10
Compare
The entry decision identifies with an empty identifier and no traits, so no trait-based segment override on onboarding_quickstart_flow could ever match it. That is why the flagsmith_team override never applied, and the team never saw the variant it targets. Pass the signed-in user's email as a transient trait so overrides that match on email can apply. Email is the only trait available before the organisation exists, so overrides on plan or organisation still cannot match. Bucketing is unaffected: the percentage split is keyed on the identifier the API assigns, not on traits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d1b2b10 to
87605bd
Compare
![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: Pro Plus
Run ID: b4f3e352-5384-4e49-89b0-c15b0a6ec1d7
📒 Files selected for processing (2)
frontend/common/utils/onboardingEntry.tsfrontend/web/components/App.js
| // @ts-expect-error transient is missing from the SDK's identify type | ||
| await flagsmith.identify('', {}, true) | ||
| await flagsmith.identify('', email ? { email } : {}, true) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
npm pack --silent --ignore-scripts \
--pack-destination "$tmp_dir" \
'`@flagsmith/flagsmith`@12.1.0' >/dev/null
tarball="$(find "$tmp_dir" -maxdepth 1 -name '*.tgz' -print -quit)"
tar -xzf "$tarball" -C "$tmp_dir"
rg -n -C 6 'identify|transient' "$tmp_dir/package"
rg -n -C 4 'flagsmith\.identify|decideOnboardingEntry' frontendRepository: Flagsmith/flagsmith
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
npm pack --silent --ignore-scripts \
--pack-destination "$tmp_dir" \
'`@flagsmith/flagsmith`@12.1.0' >/dev/null
tarball="$(find "$tmp_dir" -maxdepth 1 -name '*.tgz' -print -quit)"
tar -xzf "$tarball" -C "$tmp_dir"
printf '%s\n' '--- package declarations ---'
rg -n -C 8 'identify\s*\(' "$tmp_dir/package" --glob '*.d.ts' --glob '*.ts' --glob '*.js' --glob '*.mjs' | head -n 240
printf '%s\n' '--- transient handling ---'
rg -n -C 10 'transient' "$tmp_dir/package" --glob '*.d.ts' --glob '*.ts' --glob '*.js' --glob '*.mjs' | head -n 260
printf '%s\n' '--- repository dependency and call site ---'
rg -n -C 5 '`@flagsmith/flagsmith`|flagsmith\.identify|decideOnboardingEntry' frontend package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null | head -n 180Repository: Flagsmith/flagsmith
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
npm pack --silent --ignore-scripts \
--pack-destination "$tmp_dir" \
'`@flagsmith/flagsmith`@12.1.0' >/dev/null
tarball="$(find "$tmp_dir" -maxdepth 1 -name '*.tgz' -print -quit)"
tar -tzf "$tarball" | grep -E '\.(d\.ts|ts)$' | head -n 80
tar -xzf "$tarball" -C "$tmp_dir"
printf '%s\n' '--- identify declarations ---'
find "$tmp_dir/package" -type f \( -name '*.d.ts' -o -name '*.ts' \) -print0 |
xargs -0 grep -n -H -E 'identify[[:space:]]*\(' || true
printf '%s\n' '--- package metadata ---'
python3 - "$tmp_dir/package/package.json" <<'PY'
import json, sys
with open(sys.argv[1]) as f:
data = json.load(f)
for key in ("name", "version", "types", "typings", "main", "module"):
print(f"{key}: {data.get(key)}")
PY
printf '%s\n' '--- repository dependency and call site ---'
rg -n -C 4 '`@flagsmith/flagsmith`|flagsmith\.identify|decideOnboardingEntry' \
frontend package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null | head -n 180Repository: Flagsmith/flagsmith
Length of output: 15134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
npm pack --silent --ignore-scripts \
--pack-destination "$tmp_dir" \
'`@flagsmith/flagsmith`@12.1.0' >/dev/null
tarball="$(find "$tmp_dir" -maxdepth 1 -name '*.tgz' -print -quit)"
tar -xzf "$tarball" -C "$tmp_dir"
printf '%s\n' '--- public declarations ---'
for file in "$tmp_dir/package/index.d.ts" \
"$tmp_dir/package/flagsmith-core.d.ts" \
"$tmp_dir/package/types.d.ts" \
"$tmp_dir/package/src/flagsmith-core.d.ts"; do
if [ -f "$file" ]; then
echo "--- $file"
cat -n "$file" | grep -E -C 8 'identify|transient' | head -n 180 || true
fi
done
printf '%s\n' '--- package documentation ---'
find "$tmp_dir/package" -maxdepth 1 -type f -iname 'readme*' -print0 |
xargs -0 grep -n -i -C 8 'transient' || true
printf '%s\n' '--- relevant source methods ---'
python3 - "$tmp_dir/package/src/flagsmith-core.ts" <<'PY'
import sys
path = sys.argv[1]
lines = open(path).read().splitlines()
for start, end in ((600, 630), (740, 775)):
print(f"--- {path}:{start}-{end}")
for number in range(start, min(end, len(lines)) + 1):
print(f"{number}: {lines[number-1]}")
PYRepository: Flagsmith/flagsmith
Length of output: 4226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- transient-related source locations ---'
rg -n -C 4 '\btransient\b' . \
-g '*.py' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' \
-g '!frontend/package-lock.json' -g '!**/node_modules/**' \
| head -n 320Repository: Flagsmith/flagsmith
Length of output: 27428
Replace the broad type suppression with a narrow type fix.
@flagsmith/flagsmith 12.1.0 declares only two identify parameters, although its implementation supports the third transient parameter. Add a narrow type augmentation or update the SDK declaration. The empty identifier and true correctly create a fully transient identity, so the email trait is not persisted.
1 participant
Changes
Contributes to #7540
Signing up with a
@flagsmith.comemail gave me the legacy flow, even though theflagsmith_teamsegment ononboarding_quickstart_flowshould have put me into the new one.The segment was fine. The entry decision identifies with no traits (
flagsmith.identify('', {}, true)), and segment overrides match on traits, so none could ever apply. This passes the user's email through as a transient trait.Bucketing is unchanged: the split is keyed on the identifier the API assigns, not on traits.
How did you test this code?
Reproduced the symptom by signing up with a
@flagsmith.comemail. Unit tests and eslint pass.Not confirmed end to end: sign up with an email in the
flagsmith_teamsegment and check you land on the new onboarding.