feat(teams): onboarding step for creating team and inviting members#2811
Conversation
…ms-onboarding-step
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds an interactive onboarding lesson for inviting team members to a workspace. It extends the onboarding system to support dynamic lesson inclusion via PostHog feature flags and introduces a new team creation mutation with email invitations. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@js/app/packages/app/component/interactive-onboarding/InteractiveOnboarding.tsx`:
- Around line 106-116: The invite-team lesson is being filtered out early
because inviteTeamEnabled() is evaluated synchronously when allLessons() is
passed into createOnboardingState, so async PostHog flags aren't reflected; to
fix, stop relying on a static filter at state-creation: keep LESSONS/include
invite-team in allLessons() or remove the invite-team gating there, and instead
gate the invite-team step at render-time (wrap the invite-team step/component
with a conditional Show when={inviteTeamEnabled().enabled} or equivalent) so the
lesson appears once the flag resolves; alternatively, if you prefer state-driven
behavior, change createOnboardingState to accept a reactive definitions accessor
(or defer creating the onboarding state until inviteTeamEnabled().enabled is
resolved) and update usages of createOnboardingState and allLessons to use the
reactive accessor.
In
`@js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx`:
- Around line 79-87: The focus call in addEmailField uses requestAnimationFrame
to query by id (`invite-email-${newIndex}`) but races with Solid's render
(setEmails and <Index> commit), causing focus to sometimes fail; change
addEmailField to set the new email, then obtain a stable reference from the
rendered input (e.g., provide a ref from the <Index> child or store refs in a
map keyed by index) and call focus on that ref once the component has committed
(alternatively await Solid's rendering by using queueMicrotask or a render-await
utility before focusing); update references for `addEmailField`, `setEmails`,
and the input identification logic to use the ref-based focus approach instead
of getElementById + requestAnimationFrame.
- Line 43: The declared signal submitted (createSignal(false)) is unused but
setSubmitted(true) is called in handleSubmit; either remove submitted and all
setSubmitted calls (clean up the unused state) or wire submitted into the
component validation UX by reading submitted() where validation/error UI is
rendered (e.g., only show field errors or validation messages when submitted()
is true). Update the code in invite-team.tsx to replace/remove references
accordingly (adjust handleSubmit, any error display logic, and imports if
createSignal becomes unused).
- Around line 366-371: The onContinue handler currently queries the DOM with
document.getElementById(INVITE_FORM_ID) which breaks if the component is
rendered multiple times; replace this brittle id lookup by using a React ref or
an injected submit callback: add a formRef (e.g., const formRef =
useRef<HTMLFormElement|null>(null)) and attach it to the form element, or accept
a prop like onRequestSubmit and call that from onContinue; then change
onContinue to call formRef.current?.requestSubmit() or props.onRequestSubmit()
and remove the direct use of INVITE_FORM_ID so the component no longer depends
on global DOM ids.
- Around line 12-21: inviteFormSchema.emails currently filters out empty strings
with .transform(...) before validating, so validation errors (from the piped
z.array) report indices in the filtered array and don't line up with the UI
inputs (emails()), causing newErrors.emails[error.path[1]] to mark the wrong
input. Fix by removing the array-level .transform and instead validate the
original array elements in-place: change the element schema to allow/skip empty
strings (e.g. treat empty as valid/no-op) and only run email validation on
non-empty strings, or keep the transform but map validation error indices back
to the original emails() indices before assigning to newErrors.emails; update
code paths that reference inviteFormSchema.emails, error.path[1], and
newErrors.emails accordingly.
- Around line 12-21: Update inviteFormSchema to use Zod 4 top-level validators:
replace the email validator inside the emails pipeline from
z.string().email('Invalid email address') to z.email('Invalid email address'),
and ensure the teamName validations remain as z.string().min(1, 'Team name is
required').max(50, 'Team name is too long') (or keep the existing chaining) so
the schema uses z.email for email validation while preserving the
transform(pipe) logic on the emails array in inviteFormSchema.
- Around line 222-234: Replace the nested ternary used to compute the class for
the element with id "team-name-counter" by extracting the logic into a small
helper (e.g., a function like counterColor(len: number)) or an if/else chain:
use teamName().length and TEAM_NAME_MAX_LENGTH inside that helper to return
'text-failure-ink' when length > TEAM_NAME_MAX_LENGTH, 'text-alert-ink' when
length > TEAM_NAME_MAX_LENGTH - 10, and 'text-ink/40' otherwise, then call that
helper in the className (cn(..., counterColor(teamName().length))) to remove the
nested ternary.
- Around line 348-357: The SkipAction button currently has no onClick behavior;
wire it to the onboarding flow by either (A) extending
LessonDefinition.secondaryAction to accept props and have
InteractiveOnboarding.tsx pass the advance/skip callbacks into the Dynamic
component so SkipAction receives and calls a provided skip/advance prop, or (B)
read the shared onboarding store/context inside SkipAction and call the existing
advanceLesson/skip method directly; locate the SkipAction component in
invite-team.tsx and update it to invoke the onboarding advance/skip handler when
clicked, and adjust InteractiveOnboarding.tsx and the secondaryAction type
signature accordingly so the handler is passed through.
- Around line 73-77: canAddEmail currently returns true when emails() is empty
because lastEmail is undefined and undefined?.trim() !== '' evaluates true;
update canAddEmail (and its use of emails(), currentEmails, lastEmail) to
explicitly handle empty arrays by returning false when currentEmails.length ===
0, or equivalently return Boolean(lastEmail && lastEmail.trim() !== ''), so the
"Add another" button is only enabled when the last email exists and has
non-whitespace content; also check removeEmail usage if you prefer to prevent it
from producing an empty array, but the immediate fix is to change canAddEmail as
described.
In `@js/app/packages/queries/team/teams.ts`:
- Around line 168-180: The current mutationFn (the create+invite sequence using
authServiceClient.createTeam and authServiceClient.inviteToTeam) can leave the
client cache stale when invite fails after team creation; update the mutation so
that onSettled always calls invalidateUserTeams() to reconcile the cache, and
modify mutationFn to detect partial success (wrap inviteToTeam in its own
try/catch or return a result object indicating inviteFailure) so you can avoid
rolling back the created team in onError when createTeam succeeded (or surface a
clear "team created but invites failed" message) rather than removing the
server-side team; ensure references to mutationFn, createTeam, inviteToTeam,
onError, onSettled and invalidateUserTeams are used to locate and implement
these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12d89768-b4e0-4742-b5dc-9cd4ee48a104
📒 Files selected for processing (7)
js/app/packages/app/component/interactive-onboarding/InteractiveOnboarding.tsxjs/app/packages/app/component/interactive-onboarding/lessons/index.tsjs/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsxjs/app/packages/app/component/interactive-onboarding/types.tsjs/app/packages/core/constant/featureFlags.tsjs/app/packages/queries/team/index.tsjs/app/packages/queries/team/teams.ts
| const inviteFormSchema = z.object({ | ||
| teamName: z | ||
| .string() | ||
| .min(1, 'Team name is required') | ||
| .max(50, 'Team name is too long'), | ||
| emails: z | ||
| .array(z.string()) | ||
| .transform((emails) => emails.filter((e) => e.trim() !== '')) | ||
| .pipe(z.array(z.string().email('Invalid email address'))), | ||
| }); |
There was a problem hiding this comment.
Validation error indices don't match UI field indices after .transform filter.
inviteFormSchema.emails runs:
.transform((emails) => emails.filter((e) => e.trim() !== ''))— drops blank rows.pipe(z.array(z.string().email(...)))— validates the filtered array
The resulting error.path[1] indices reference positions in the filtered array, not the original emails() array that drives the UI. Example — with emails() === ['', 'bad@', 'ok@co'], the filter produces ['bad@', 'ok@co'], validation fails at index 0, and your handler sets newErrors.emails[0], which decorates the first (empty) input in the UI instead of the invalid one at UI index 1.
Either validate the original array (skip-empty inside the element schema) or map filtered indices back to UI indices before assigning errors:
🛠️ Suggested schema rewrite preserving indices
- emails: z
- .array(z.string())
- .transform((emails) => emails.filter((e) => e.trim() !== ''))
- .pipe(z.array(z.string().email('Invalid email address'))),
+ emails: z.array(
+ z
+ .string()
+ .refine(
+ (v) => v.trim() === '' || /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(v),
+ 'Invalid email address'
+ )
+ ),Then at submit time, strip empties from the payload separately (you already do the non-empty check on Line 180).
Also applies to: 147-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx`
around lines 12 - 21, inviteFormSchema.emails currently filters out empty
strings with .transform(...) before validating, so validation errors (from the
piped z.array) report indices in the filtered array and don't line up with the
UI inputs (emails()), causing newErrors.emails[error.path[1]] to mark the wrong
input. Fix by removing the array-level .transform and instead validate the
original array elements in-place: change the element schema to allow/skip empty
strings (e.g. treat empty as valid/no-op) and only run email validation on
non-empty strings, or keep the transform but map validation error indices back
to the original emails() indices before assigning to newErrors.emails; update
code paths that reference inviteFormSchema.emails, error.path[1], and
newErrors.emails accordingly.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
In Zod 4, is z.string().email() deprecated in favor of z.email(), and does it still function correctly?
💡 Result:
Yes, in Zod 4, z.string.email is deprecated in favor of the top-level z.email. The method form still exists and functions correctly at runtime, but it is marked as deprecated in TypeScript JSDoc to encourage use of the new top-level API for better tree-shaking and conciseness.
Citations:
- 1: [v4]
z.string().trim().email()should not deplecated colinhacks/zod#4850 - 2: https://v4.zod.dev/v4/changelog?id=deprecates-email-etc
- 3: https://zod.dev/v4/changelog
- 4: https://v4.zod.dev/api?id=emails
- 5: https://dev.to/pockit_tools/migrating-to-zod-4-the-complete-guide-to-breaking-changes-performance-gains-and-new-features-3ll0
- 6: https://v4.zod.dev/v4?id=custom-email-regex
- 7: https://zod.dev/v4
- 8: https://v4.zod.dev/v4
- 9: https://www.zod.dev/v4/changelog?id=deprecates-email-etc
🏁 Script executed:
# First, check if the file exists and get its total line count
wc -l "js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx"Repository: macro-inc/macro
Length of output: 139
🏁 Script executed:
# Read the relevant sections of the file
sed -n '1,150p' "js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx" | cat -nRepository: macro-inc/macro
Length of output: 5548
🏁 Script executed:
# Check lines 126-130 and 136 more carefully
sed -n '120,145p' "js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx" | cat -nRepository: macro-inc/macro
Length of output: 1009
🏁 Script executed:
# Search for all occurrences of .email( in the file to ensure we catch all instances
rg '\.email\(' "js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx" -nRepository: macro-inc/macro
Length of output: 209
Modernize to Zod 4 top-level validation functions.
In Zod 4, z.string().email() is deprecated in favor of the top-level z.email() function for better tree-shaking and a more concise API. The chained form still works correctly at runtime but is marked as deprecated in TypeScript. Consider updating lines 20 and 136 to use z.email(). The same modernization applies to z.string().min() and z.string().max() (use z.string().min(1).max(50) as-is, or consider z.string() chaining for consistency).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx`
around lines 12 - 21, Update inviteFormSchema to use Zod 4 top-level validators:
replace the email validator inside the emails pipeline from
z.string().email('Invalid email address') to z.email('Invalid email address'),
and ensure the teamName validations remain as z.string().min(1, 'Team name is
required').max(50, 'Team name is too long') (or keep the existing chaining) so
the schema uses z.email for email validation while preserving the
transform(pipe) logic on the emails array in inviteFormSchema.
| const canAddEmail = () => { | ||
| const currentEmails = emails(); | ||
| const lastEmail = currentEmails[currentEmails.length - 1]; | ||
| return lastEmail?.trim() !== ''; | ||
| }; |
There was a problem hiding this comment.
canAddEmail returns true when the list is empty.
If emails() ever becomes [] (e.g. after removing the last row via removeEmail), currentEmails[currentEmails.length - 1] is undefined, undefined?.trim() !== '' evaluates to true, and the "Add another" button becomes enabled — which is inconsistent with its intent ("don't let the user add more until the last one has content"). Today removeEmail isn't guarded against emptying the array, so this is reachable.
🛠️ Proposed fix
- const canAddEmail = () => {
- const currentEmails = emails();
- const lastEmail = currentEmails[currentEmails.length - 1];
- return lastEmail?.trim() !== '';
- };
+ const canAddEmail = () => {
+ const currentEmails = emails();
+ if (currentEmails.length === 0) return true;
+ const lastEmail = currentEmails[currentEmails.length - 1];
+ return lastEmail.trim() !== '';
+ };📝 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.
| const canAddEmail = () => { | |
| const currentEmails = emails(); | |
| const lastEmail = currentEmails[currentEmails.length - 1]; | |
| return lastEmail?.trim() !== ''; | |
| }; | |
| const canAddEmail = () => { | |
| const currentEmails = emails(); | |
| if (currentEmails.length === 0) return true; | |
| const lastEmail = currentEmails[currentEmails.length - 1]; | |
| return lastEmail.trim() !== ''; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx`
around lines 73 - 77, canAddEmail currently returns true when emails() is empty
because lastEmail is undefined and undefined?.trim() !== '' evaluates true;
update canAddEmail (and its use of emails(), currentEmails, lastEmail) to
explicitly handle empty arrays by returning false when currentEmails.length ===
0, or equivalently return Boolean(lastEmail && lastEmail.trim() !== ''), so the
"Add another" button is only enabled when the last email exists and has
non-whitespace content; also check removeEmail usage if you prefer to prevent it
from producing an empty array, but the immediate fix is to change canAddEmail as
described.
| const addEmailField = () => { | ||
| if (!canAddEmail() || isPending()) return; | ||
| const newIndex = emails().length; | ||
| setEmails((prev) => [...prev, '']); | ||
| requestAnimationFrame(() => { | ||
| const input = document.getElementById(`invite-email-${newIndex}`); | ||
| input?.focus(); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Focus-by-id after requestAnimationFrame races with Solid's render.
setEmails queues a reactive update, but the DOM element with id invite-email-${newIndex} isn't guaranteed to exist in the next animation frame (Solid batches updates and <Index> adds nodes on commit, which may not finish by the next RAF). On slower devices or under load the focus silently no-ops.
Safer: set focus via a ref obtained from <Index> child, or use queueMicrotask after awaiting a render. Minor polish rather than a blocker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx`
around lines 79 - 87, The focus call in addEmailField uses requestAnimationFrame
to query by id (`invite-email-${newIndex}`) but races with Solid's render
(setEmails and <Index> commit), causing focus to sometimes fail; change
addEmailField to set the new email, then obtain a stable reference from the
rendered input (e.g., provide a ref from the <Index> child or store refs in a
map keyed by index) and call focus on that ref once the component has committed
(alternatively await Solid's rendering by using queueMicrotask or a render-await
utility before focusing); update references for `addEmailField`, `setEmails`,
and the input identification logic to use the ref-based focus approach instead
of getElementById + requestAnimationFrame.
| onContinue: () => { | ||
| const form = document.getElementById( | ||
| INVITE_FORM_ID | ||
| ) as HTMLFormElement | null; | ||
| form?.requestSubmit(); | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Form lookup by id couples the lesson to DOM queries — OK, but brittle.
Using document.getElementById(INVITE_FORM_ID) works but couples onContinue to the rendered DOM. If this component is ever rendered twice (e.g., test mode replay), the id collision would break submission. Not a blocker for v1.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/interactive-onboarding/lessons/invite-team.tsx`
around lines 366 - 371, The onContinue handler currently queries the DOM with
document.getElementById(INVITE_FORM_ID) which breaks if the component is
rendered multiple times; replace this brittle id lookup by using a React ref or
an injected submit callback: add a formRef (e.g., const formRef =
useRef<HTMLFormElement|null>(null)) and attach it to the form element, or accept
a prop like onRequestSubmit and call that from onContinue; then change
onContinue to call formRef.current?.requestSubmit() or props.onRequestSubmit()
and remove the direct use of INVITE_FORM_ID so the component no longer depends
on global DOM ids.
| mutationFn: async ({ name, emails }: CreateTeamWithInvitesArgs) => { | ||
| const team = await throwOnErr(() => | ||
| authServiceClient.createTeam({ name }) | ||
| ); | ||
|
|
||
| if (emails && emails.length > 0) { | ||
| await throwOnErr(() => | ||
| authServiceClient.inviteToTeam(team.id, { emails }) | ||
| ); | ||
| } | ||
|
|
||
| return team; | ||
| }, |
There was a problem hiding this comment.
Partial-success leaves the user-teams cache permanently out of sync with the server.
The mutation performs two sequential calls: createTeam then inviteToTeam. If the team is created successfully but the invite call throws, onError rolls back the cache to previousTeams (removing the team) and the "Team created" toast never fires — yet the team does exist on the server. No invalidateUserTeams() runs on the error path, so the stale cache persists until a full refresh.
At minimum, invalidate on settle so the cache reconciles with the server regardless of outcome:
🛠️ Proposed fix
onError: (error, _args, context) => {
console.error('Failed to create team', error);
toast.failure('Failed to create team');
if (context?.previousTeams) {
queryClient.setQueryData(
teamKeys.userTeams.queryKey,
context.previousTeams
);
}
},
+ onSettled: () => {
+ invalidateUserTeams();
+ },Even better, distinguish create-failure from invite-failure inside mutationFn so you can surface a clearer message ("Team created but failed to send invites") and skip the rollback when the team did get created.
Also applies to: 226-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/queries/team/teams.ts` around lines 168 - 180, The current
mutationFn (the create+invite sequence using authServiceClient.createTeam and
authServiceClient.inviteToTeam) can leave the client cache stale when invite
fails after team creation; update the mutation so that onSettled always calls
invalidateUserTeams() to reconcile the cache, and modify mutationFn to detect
partial success (wrap inviteToTeam in its own try/catch or return a result
object indicating inviteFailure) so you can avoid rolling back the created team
in onError when createTeam succeeded (or surface a clear "team created but
invites failed" message) rather than removing the server-side team; ensure
references to mutationFn, createTeam, inviteToTeam, onError, onSettled and
invalidateUserTeams are used to locate and implement these changes.
1 participant
No description provided.