hotfix(onboarding): data-driven auto-detect for tenant Initial Setup steps#7
Conversation
Auto-complete the tenant "Initial Setup" onboarding steps from live data instead of relying solely on the backend `completedSteps`. A step reads as done the moment its underlying data exists (MSP profile filled, a customer/device/teammate added), and the hook writes that completion back. - New `useTenantOnboardingAutoDetect` hook: reads three schema-backed signals (MSP profile, customer count, connected-device count) in one Relay query plus the REST user count, and fires `completeTenantStepInBackground` when a step's condition holds but it isn't yet persisted. Returns `completedByData` so the card shows a step as done without waiting for the mutation round-trip. - New `tenantOnboardingAutoDetectRelayQuery` (single round-trip, no waterfall, no raw-POST GraphQL); device count filtered to ONLINE/OFFLINE only. - Wire the hook into the dashboard + Initial Setup card. - Add repo-scoped .gitignore (mirrors the monorepo frontend rules). NOTE: the client-side detect-and-write-back is a documented temporary stopgap until `tenantOnboardingProgress` computes completion authoritatively on the backend.
📝 WalkthroughWalkthroughThe PR adds onboarding auto-detection from tenant data, data-driven step rendering, an onboarding loading skeleton, dashboard Suspense integration, and repository ignore rules. ChangesOnboarding flow
Repository hygiene
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant InitialSetupCardContent
participant useTenantOnboardingAutoDetect
participant Relay
participant UsersAPI
participant Backend
InitialSetupCardContent->>useTenantOnboardingAutoDetect: request onboarding completion state
useTenantOnboardingAutoDetect->>Relay: fetch tenant, organization, and device signals
useTenantOnboardingAutoDetect->>UsersAPI: fetch user count
useTenantOnboardingAutoDetect->>Backend: persist one missing completed step
useTenantOnboardingAutoDetect-->>InitialSetupCardContent: return auto-detected steps
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
src/app/(app)/onboarding/components/initial-setup-card.tsx (1)
36-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo parallel step lists must stay manually in sync.
STEP_METAduplicates the same 4TenantOnboardingStepvalues (and their order) already defined inTENANT_ONBOARDING_STEPS(onboarding-steps.ts). Nothing enforces that adding/reordering a step in one array is mirrored in the other — a future step added only toTENANT_ONBOARDING_STEPS(used for counting) but notSTEP_META(used for rendering) would silently skew the "X/Y done" count against what's actually rendered, or vice versa.Consider deriving
STEP_META's ordering fromTENANT_ONBOARDING_STEPS(e.g. aRecord<TenantOnboardingStep, Omit<StepMeta, 'step'>>keyed lookup, thenTENANT_ONBOARDING_STEPS.map(step => ({ step, ...metaByStep[step] }))) so there's a single source of truth for which steps exist and in what order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/onboarding/components/initial-setup-card.tsx around lines 36 - 62, Derive the rendered STEP_META ordering from TENANT_ONBOARDING_STEPS instead of maintaining duplicate step values locally. Define metadata keyed by TenantOnboardingStep, then map TENANT_ONBOARDING_STEPS to add each step’s metadata while preserving the existing icons, titles, and descriptions, so counting and rendering share one source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts:
- Around line 123-145: The onboarding persistence effect can stall when
completeTenantStepInBackground fails because fired marks the step permanently
without triggering another attempt. Update the flow around fired and
completeTenantStepInBackground to handle settlement or errors, allowing the
current step to be removed/reset and the next satisfied step to be processed
during the same mount while preserving serialized completion.
In `@src/graphql/onboarding/tenant-onboarding-auto-detect-relay.ts`:
- Around line 15-17: Update the docblock for the tenant onboarding auto-detect
query to describe the caller’s actual store-and-network fetch policy instead of
network-only. Reference the existing use-tenant-onboarding-auto-detect behavior
and retain the explanation that avoids re-suspend thrashing with the sibling
useSuspenseQuery.
---
Nitpick comments:
In `@src/app/`(app)/onboarding/components/initial-setup-card.tsx:
- Around line 36-62: Derive the rendered STEP_META ordering from
TENANT_ONBOARDING_STEPS instead of maintaining duplicate step values locally.
Define metadata keyed by TenantOnboardingStep, then map TENANT_ONBOARDING_STEPS
to add each step’s metadata while preserving the existing icons, titles, and
descriptions, so counting and rendering share one source of truth.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dd9af419-5488-490d-85e0-76052b777522
📒 Files selected for processing (5)
.gitignoresrc/app/(app)/dashboard/components/dashboard-content.tsxsrc/app/(app)/onboarding/components/initial-setup-card.tsxsrc/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.tssrc/graphql/onboarding/tenant-onboarding-auto-detect-relay.ts
| // Steps whose completion mutation we've already sent this mount. Per-mount only — | ||
| // resets on remount, and the next visit re-derives from the backend `completedSteps`. | ||
| const fired = useRef<Set<TenantOnboardingStep>>(new Set()); | ||
|
|
||
| // Persist ONE step at a time: fire the first not-yet-persisted, not-yet-fired step; | ||
| // its mutation updates the store (tenant reference changes) which re-runs this effect | ||
| // for the next one. Serializing avoids firing all satisfied steps at once, where the | ||
| // concurrent completeTenantOnboardingStep responses (each returns the full | ||
| // `completedSteps` and overwrites the store, last-write-wins) could clobber a | ||
| // sibling's just-written step. | ||
| useEffect(() => { | ||
| if (!tenant) { | ||
| return; | ||
| } | ||
| const next = TENANT_ONBOARDING_STEPS.find( | ||
| step => completedByData.has(step) && !tenant.completedSteps.includes(step) && !fired.current.has(step), | ||
| ); | ||
| if (!next) { | ||
| return; | ||
| } | ||
| fired.current.add(next); | ||
| completeTenantStepInBackground(next); | ||
| }, [tenant, completedByData, completeTenantStepInBackground]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files and inspect the surrounding code.
git ls-files 'src/app/(app)/onboarding/hooks/*' 'src/app/(app)/onboarding/**' | sed -n '1,200p'
printf '\n--- use-tenant-onboarding-auto-detect.ts ---\n'
nl -ba src/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts | sed -n '1,220p'
printf '\n--- use-onboarding-mutations.ts ---\n'
nl -ba src/app/(app)/onboarding/hooks/use-onboarding-mutations.ts | sed -n '1,220p'Repository: flamingo-stack/openframe-frontend
Length of output: 1368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the previous command produced truncated output or files differ, inspect only the needed slices.
for f in \
src/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts \
src/app/(app)/onboarding/hooks/use-onboarding-mutations.ts
do
echo "--- $f (outline) ---"
ast-grep outline "$f" --view expanded || true
echo
doneRepository: flamingo-stack/openframe-frontend
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the exact lines around the effect and mutation callback.
sed -n '110,170p' src/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts
printf '\n'
sed -n '1,220p' src/app/(app)/onboarding/hooks/use-onboarding-mutations.tsRepository: flamingo-stack/openframe-frontend
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file1='src/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts'
file2='src/app/(app)/onboarding/hooks/use-onboarding-mutations.ts'
echo "--- $file1 ---"
nl -ba "$file1" | sed -n '1,220p'
echo
echo "--- $file2 ---"
nl -ba "$file2" | sed -n '1,220p'Repository: flamingo-stack/openframe-frontend
Length of output: 287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file1='src/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts'
file2='src/app/(app)/onboarding/hooks/use-onboarding-mutations.ts'
echo "--- $file1 ---"
cat -n "$file1" | sed -n '1,220p'
echo
echo "--- $file2 ---"
cat -n "$file2" | sed -n '1,220p'
echo
echo "--- references to completeTenantStepInBackground ---"
rg -n 'completeTenantStepInBackground|completeTenantOnboardingStep' src/app/(app)/onboarding -nRepository: flamingo-stack/openframe-frontend
Length of output: 8879
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file1='src/app/(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts'
file2='src/graphql/onboarding/use-onboarding-mutations.ts'
echo "--- $file1 (relevant slice) ---"
sed -n '123,145p' "$file1"
echo
echo "--- $file2 ---"
cat -n "$file2" | sed -n '1,220p'
echo
echo "--- references ---"
rg -n 'setTenant|completeTenantStepInBackground|onError|onCompleted|completeTenantOnboardingStep' src/graphql src/app/(app)/onboardingRepository: flamingo-stack/openframe-frontend
Length of output: 12092
A failed background completion can stall the rest of onboarding persistence for this mount.
fired.current is set before completeTenantStepInBackground(next) settles, and the background mutation never updates tenant on error. That leaves this effect with no trigger to advance to the next satisfied step, so one transient failure can keep later steps from being persisted until the next visit. Expose a settle/error path here or decouple advancement from the tenant ref change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(app)/onboarding/hooks/use-tenant-onboarding-auto-detect.ts around
lines 123 - 145, The onboarding persistence effect can stall when
completeTenantStepInBackground fails because fired marks the step permanently
without triggering another attempt. Update the flow around fired and
completeTenantStepInBackground to handle settlement or errors, allowing the
current step to be removed/reset and the next satisfied step to be processed
during the same mount while preserving serialized completion.
| * Fetched with `network-only` so every dashboard visit reflects current data. The user | ||
| * count is NOT here — it comes from the REST `api/users` list, whose `totalElements` | ||
| * matches Settings → Employees (the GraphQL `users` count did not). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Docblock says network-only, but the caller uses store-and-network.
This comment states the query is "Fetched with network-only so every dashboard visit reflects current data." However, use-tenant-onboarding-auto-detect.ts explicitly uses fetchPolicy: 'store-and-network' and documents why network-only was avoided (to prevent re-suspend thrashing against the sibling useSuspenseQuery). The docblock here is stale and contradicts the actual caller behavior.
📝 Proposed fix
- * Fetched with `network-only` so every dashboard visit reflects current data. The user
+ * Fetched with `store-and-network` (fresh on every mount, store-served on re-render — see
+ * `useTenantOnboardingAutoDetect`) so every dashboard visit reflects current data. The user📝 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.
| * Fetched with `network-only` so every dashboard visit reflects current data. The user | |
| * count is NOT here — it comes from the REST `api/users` list, whose `totalElements` | |
| * matches Settings → Employees (the GraphQL `users` count did not). | |
| * Fetched with `store-and-network` (fresh on every mount, store-served on re-render — see | |
| * `useTenantOnboardingAutoDetect`) so every dashboard visit reflects current data. The user | |
| * count is NOT here — it comes from the REST `api/users` list, whose `totalElements` | |
| * matches Settings → Employees (the GraphQL `users` count did not). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graphql/onboarding/tenant-onboarding-auto-detect-relay.ts` around lines
15 - 17, Update the docblock for the tenant onboarding auto-detect query to
describe the caller’s actual store-and-network fetch policy instead of
network-only. Reference the existing use-tenant-onboarding-auto-detect behavior
and retain the explanation that avoids re-suspend thrashing with the sibling
useSuspenseQuery.
2 participants
What
Auto-complete the tenant Initial Setup onboarding steps from live data instead of relying solely on the backend
completedSteps. A step reads as done the moment its underlying data exists (MSP profile filled, a customer/device/teammate added), and the hook persists that completion back.Changes
useTenantOnboardingAutoDetect— reads three schema-backed signals (MSP profile, customer count, connected-device count) in one Relay query plus the REST user count, and firescompleteTenantStepInBackgroundwhen a step's condition holds but it isn't yet persisted. ReturnscompletedByDataso the card shows a step as done without waiting for the mutation round-trip.tenantOnboardingAutoDetectRelayQuery— single round-trip (no request waterfall, no raw-POST GraphQL); device count filtered toONLINE/OFFLINEonly (archived/pending don't count).dashboard-content.tsx) and the Initial Setup card (initial-setup-card.tsx)..gitignore(mirrors the monorepo frontend rules).Note
The client-side detect-and-write-back is a documented temporary stopgap until
tenantOnboardingProgresscomputes step completion authoritatively on the backend. Rationale and known limitations are documented in the hook.Scope
This PR intentionally excludes the in-progress dashboard-activity tracking fixes (remote shell/control + resolve-ticket) — those ship separately.
Summary by CodeRabbit
New Features
Bug Fixes