Add granular signup qualifier values backwards-compatibly#90348
Open
neil-marcellini wants to merge 1 commit into
Open
Add granular signup qualifier values backwards-compatibly#90348neil-marcellini wants to merge 1 commit into
neil-marcellini wants to merge 1 commit into
Conversation
Introduces two new signup qualifier values that the landing page can send
without breaking older App clients:
- vsb-1-4 — replaces the historical "1-9 employees" landing page bracket
- smb-5-plus — replaces the historical "10+ employees" landing page bracket
Old App clients string-match exclusively on `vsb` / `smb`, so unknown
values fall through to the default onboarding flow with the full company
size list. Updated clients recognize the new strings via shared
`isVsbQualifier` / `isSmbQualifier` helpers and:
- Pre-set `companySize=MICRO_SMALL` ("1-4") for `vsb-1-4` so stored data
matches what the user said. Legacy `vsb` keeps mapping to `MICRO`.
- Show the employees step for `smb-5-plus` with `1-4` hidden but `5-10`
still visible. Legacy `smb` continues to hide both small-team options.
Tests cover the helpers, the `OnboardingFlow` pre-set logic, and the
`OnboardingEmployees` filter behavior under each qualifier.
Co-authored-by: Cursor <cursoragent@cursor.com>
melvin-bot
Bot
requested review from
a team and
thelullabyy
and removed request for
a team
|
@thelullabyy Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
melvin-bot
Bot
requested review from
JmillsExpensify and
abzokhattab
and removed request for
a team
|
@abzokhattab Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // VSB/SMB have fixed suffixes; individual (null) is handled via purposeSuffixes. | ||
| // VSB_1_4 follows the same flow as legacy VSB (the company size is pre-set so the | ||
| // employees step is skipped). SMB_5_PLUS follows the same flow as legacy SMB. | ||
| const qualifierSuffixes = { |
Contributor
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The suffix arrays for VSB and VSB_1_4 are identical, and the suffix arrays for SMB and SMB_5_PLUS are also identical. If the flow for either group changes, both entries must be updated in lockstep, which is error-prone.
Extract the shared arrays into named constants and reference them from both keys:
const VSB_SUFFIXES = [ONBOARDING.ACCOUNTING, ONBOARDING.INTERESTED_FEATURES];
const SMB_SUFFIXES = [ONBOARDING.EMPLOYEES, ONBOARDING.ACCOUNTING, ONBOARDING.INTERESTED_FEATURES];
const qualifierSuffixes = {
[ONBOARDING_SIGNUP_QUALIFIERS.VSB]: VSB_SUFFIXES,
[ONBOARDING_SIGNUP_QUALIFIERS.VSB_1_4]: VSB_SUFFIXES,
[ONBOARDING_SIGNUP_QUALIFIERS.SMB]: SMB_SUFFIXES,
[ONBOARDING_SIGNUP_QUALIFIERS.SMB_5_PLUS]: SMB_SUFFIXES,
[ONBOARDING_SIGNUP_QUALIFIERS.INDIVIDUAL]: null,
} satisfies Record<ValueOf<typeof ONBOARDING_SIGNUP_QUALIFIERS>, OnboardingScreen[] | null>;Reviewed at: ce78964 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Contributor
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Contributor
|
@neil-marcellini Kindly finish/tick all the checkbox in PR desc |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(Neil's AI agent)
Explanation of Change
Adds two new signup qualifier values that the landing page can send without breaking older App clients, in preparation for splitting the landing page brackets from
1-9 / 10+into1-4 / 5+(Web-Expensify #52332 and a follow-up landing page PR).New qualifier values (in
CONST.ONBOARDING_SIGNUP_QUALIFIERS):VSB_1_4→ wire-formatvsb-1-4— landing page "1-4 employees"SMB_5_PLUS→ wire-formatsmb-5-plus— landing page "5+ employees"Both fit the existing
_SIMPLENAMEWAF rule ([\w@., -]{1,64}) on Web-Expensify, so no input rule change is needed for them to passsignUpvalidation.Why this approach is safe for old clients:
The App keys all behavior off exact string matches like
signupQualifier === 'vsb'/=== 'smb'. An old App build that hasn't picked up this PR will seevsb-1-4/smb-5-plus, returnfalsefrom every isVsb/isSmb check, and fall into the default onboarding flow that shows the full company-size list. Slightly more clicks for those users but no missing options, no wrong routing, no broken state.Updated client behavior (this PR):
src/libs/SignupQualifierUtils.tswithisVsbQualifier()/isSmbQualifier()helpers that treat the legacy and granular values as members of the same family. Replaces 14 inline=== VSB/=== SMBchecks across 8 onboarding files with the helpers so future qualifier additions don't fan out across the codebase.OnboardingFlow.ts: pre-setscompanySize = MICRO_SMALL("1-4") forvsb-1-4so the stored data matches what the user said on the landing page. Legacyvsbcontinues to map toMICRO("1-10") for backwards compatibility.BaseOnboardingEmployees.tsx: undersmb-5-plus, only1-4is hidden —5-10stays visible because the user said "5+", not "10+". Legacysmbcontinues to hide both1-4and5-10.getOnboardingStepCounter.ts: adds the new qualifiers to the exhaustivequalifierSuffixesmap so the step counter shows the right total for users on the new flow.vsb/smbkeep their historical meaning forever at the data layer — historical NVPs, marketing dashboards, and Meta SignUp event audiences keyed on them remain accurate.Companion PRs (deploy in any order — each is independently safe):
valueattributes (vsb-1-4/smb-5-plus) in addition to the new label text. As-is the PR only changes the visible labels and would break the in-app flow for users picking "5+".Marketing/growth heads-up needed: the new strings will start appearing in the Meta
SignUpconversion event (window.fbq('trackCustom', 'SignUp', {qualifier})inhomepage/js/signin.jsx) and in theintroSelected.signupQualifierNVP. New analytics queries need to handle the four-value space (vsb,smb,vsb-1-4,smb-5-plus).Fixed Issues
$ https://github.com/Expensify/Expensify/issues/613539
Tests
(Neil's AI agent)
Automated tests (run with
npx jest tests/unit/libs/SignupQualifierUtilsTest.ts tests/unit/libs/OnboardingFlowTest.ts tests/ui/OnboardingEmployees.tsx):SignupQualifierUtilsTest.ts(12 tests) — verifies legacy and granular values are correctly grouped, INDIVIDUAL/null/undefined are not, and the wire-format strings matchvsb-1-4/smb-5-plusexactly.OnboardingFlowTest.ts(5 tests) — verifiesvsb-1-4pre-setscompanySize = "1-4", legacyvsbkeeps pre-setting"1-10",smb-5-plussets MANAGE_TEAM purpose without pre-setting a size, and routes to the employees step.tests/ui/OnboardingEmployees.tsx(3 tests, 1 new) — adds a case verifying thatsmb-5-pluskeeps5-10visible while hiding1-4and the legacy1-10.TDD verification was done by reverting the implementation, confirming the new tests fail (they did — 5 explicit failures plus compile errors on the new CONST keys), then reapplying the implementation and watching all 19 tests pass.
Manual dev testing (cannot exercise the new path end-to-end until the landing page is updated to send the new strings, since
signUpis the only producer ofsignupQualifier). Verified existing legacy paths still work:signupQualifier=smb(legacy SMB path) → the employees step still hides 1-4, 5-10, and 1-10.signupQualifier=vsb(legacy VSB path) → still skips the employees step and pre-setscompanySize=1-10, still routes to accounting.Once the landing-page PR is up I'll add a follow-up dev video showing the new
vsb-1-4andsmb-5-pluspaths.Offline tests
No offline-specific behavior changes — signup qualifiers are written during online signup and read from the locally-cached onboarding NVP afterwards, which already works offline.
QA Steps
(Neil's AI agent)
This PR is a no-op until the landing page starts sending the new strings, so QA on the existing legacy paths is sufficient to confirm no regressions:
vsb).smb).PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)STYLE.md) were followedisVsbQualifier/isSmbQualifierhelpers replace duplicated inline string equality checks; same pattern as similar utility modules insrc/libs/OnboardingEmployees,OnboardingFlow, and the new helper unit testsmainbranch was merged into this PR after a review, I tested again — branch is up to date with main as of branch creationMade with Cursor