refactor: allow projectId prop for ProjectDataProvider#5164
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
Contributor
📝 WalkthroughWalkthroughReplaces the legacy onboarding UI with a new client-side multi-step onboarding wizard (create project → connect GitHub → select repo → configure deployment), removes the old OnboardingHeader, adds OnboardingStepHeader and ConfigureDeployment step, introduces DeploymentSettings, updates ProjectDataProvider to accept projectId prop, and adjusts GitHub callback/navigation and related UI behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (User)
participant Page as Server (OnboardingPage)
participant Client as Onboarding (Client)
participant API as trpc/API
participant GH as GitHub
Note over Page: Server checks workspace.betaFeatures.deployments
Browser->>Page: GET /{workspace}/projects/onboarding
Page-->>Browser: HTML (mounts Onboarding client via Suspense)
Browser->>Client: Create project action
Client->>API: createProject mutation
API-->>Client: returns projectId
Client->>Client: store projectId, advance step
Browser->>Client: Connect GitHub action
Client->>API: registerInstallation (initiates OAuth)
API->>GH: redirect for GitHub OAuth
GH-->>API: OAuth callback
API->>Browser: onSuccess triggers client navigation to ?step=select-repo&projectId=...
Client->>API: list repos / getInstallations
Browser->>Client: Select repo
Client->>API: link repo (mutateAsync)
API-->>Client: success -> invalidation (trpcUtils)
Client->>Client: advance to configure-deployment
Browser->>Client: Configure & Deploy
Client->>API: deploy mutation
API-->>Client: deploy result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
This was referenced Mar 2, 2026
19 tasks
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
mcstepp and
perkinsjr
as code owners
ogzhanolguncu
force-pushed
the
feat/next-onboarding-step
branch
from
8b8a6bb to
bad2566
Compare
ogzhanolguncu
force-pushed
the
feat/next-onboarding-step
branch
from
bad2566 to
aec8a3c
Compare
ogzhanolguncu
force-pushed
the
feat/next-onboarding-step
branch
from
aec8a3c to
6ba9cf1
Compare
graphite-app
bot
changed the base branch from
feat/onboarding-page
to
graphite-base/5164
ogzhanolguncu
force-pushed
the
feat/next-onboarding-step
branch
from
6ba9cf1 to
65370cb
Compare
ogzhanolguncu
force-pushed
the
graphite-base/5164
branch
from
f34edea to
09ccb61
Compare
Merge activity
|
chronark
reviewed
Mar 3, 2026
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx
Outdated
Show resolved
Hide resolved
chronark
reviewed
Mar 3, 2026
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx
Outdated
Show resolved
Hide resolved
chronark
requested changes
Mar 3, 2026
ogzhanolguncu
force-pushed
the
feat/next-onboarding-step
branch
from
65370cb to
bf06587
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/repo-list-item.tsx (1)
102-107:⚠️ Potential issue | 🟡 MinorGuard clicks while selecting to avoid duplicate submissions.
At Line 105 and Line 107, consider treating
loadingas a hard interaction lock in this component as well, so repeated clicks can’t re-fireonSelectduring in-flight selection.Suggested fix
<Button variant="outline" className="rounded-lg border-grayA-4 shadow-md transition-all h-7" - disabled={disabled || isLoading} + disabled={disabled || isLoading || loading} loading={loading} - onClick={() => onSelect(repo)} + onClick={() => { + if (loading) return; + onSelect(repo); + }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/repo-list-item.tsx around lines 102 - 107, The Button currently uses disabled={disabled || isLoading} but still passes onClick={() => onSelect(repo)} which allows repeated clicks during an in-flight selection; update the component to treat loading as a hard interaction lock by including the component's loading prop in the disabled condition (e.g., disabled when loading || isLoading || disabled) and add a runtime guard at the start of the onClick handler (check loading || isLoading || disabled and return early) before calling onSelect(repo) so duplicate submissions cannot be fired; locate the Button element and its onClick prop in repo-list-item.tsx to apply these changes.web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/github-settings/index.tsx (1)
55-75:⚠️ Potential issue | 🟠 MajorApply
readOnlybehavior consistently across all connection states.
readOnlyis only enforced in the"connected"branch. In"no-app"and"no-repo", users still get interactive actions, which breaks the read-only contract.Suggested minimal fix shape
switch (connectionState.status) { @@ case "no-app": + if (readOnly) { + return ( + <GitHubSettingCard chevronState="disabled"> + <SelectedConfig label="No repository connected" /> + </GitHubSettingCard> + ); + } return ( <GitHubSettingCard chevronState="disabled"> @@ case "no-repo": + if (readOnly) { + return ( + <GitHubSettingCard chevronState="disabled"> + <SelectedConfig label="No repository connected" /> + </GitHubSettingCard> + ); + } return <GitHubNoRepo projectId={projectId} installUrl={connectionState.installUrl} />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/github-settings/index.tsx around lines 55 - 75, When readOnly is true, ensure the "no-app" and "no-repo" branches mirror the non-interactive behavior used in the "connected" branch: in the "no-app" case render the ManageGitHubAppLink inside a GitHubSettingCard with chevronState="disabled" and make the ManageGitHubAppLink non-interactive (either pass a readOnly prop or use its "disabled"/outline variant without click handlers), and in the "no-repo" case render GitHubNoRepo in a disabled state (wrap it in GitHubSettingCard with chevronState="disabled" or pass readOnly to GitHubNoRepo so it shows non-interactive UI). Target the case "no-app", case "no-repo", the readOnly variable, and components GitHubSettingCard, ManageGitHubAppLink, and GitHubNoRepo when applying the change.
🧹 Nitpick comments (2)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsx (1)
16-16: Use a responsive container width.Line 16 uses a fixed width (
w-[900px]), which can overflow on narrower viewports. Preferw-full max-w-[900px](optionally with horizontal padding).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsx at line 16, The container uses a fixed className "w-[900px]" which can overflow on small viewports; update the JSX div that currently has className="w-[900px]" to use a responsive pattern such as "w-full max-w-[900px]" (optionally adding horizontal padding like "px-4") so the container scales on narrow screens while retaining the 900px maximum width.web/apps/dashboard/app/(app)/integrations/github/callback/page.tsx (1)
24-28: Consider using URLSearchParams for query parameters as a defensive best practice.While the backend enforces strict slug format validation (lowercase letters, numbers, and hyphens only) at workspace creation, using
URLSearchParamsand explicit encoding remains a defensive programming best practice and improves readability:Suggested refactor
const mutation = trpc.github.registerInstallation.useMutation({ onSuccess: (data) => { toast.success("GitHub App installed"); + const params = new URLSearchParams({ + step: "select-repo", + projectId: data.projectId, + }); - router.push( - `/${data.workspaceSlug}/projects/onboarding?step=select-repo&projectId=${data.projectId}`, - ); + router.push(`/${data.workspaceSlug}/projects/onboarding?${params.toString()}`); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/integrations/github/callback/page.tsx around lines 24 - 28, In onSuccess, build the redirect URL using URLSearchParams to safely encode query parameters instead of string interpolation; replace the router.push call that concatenates data.workspaceSlug and a query string with assembling params via new URLSearchParams({ step: 'select-repo', projectId: data.projectId }).toString() and then router.push(`/${data.workspaceSlug}/projects/onboarding?${params}`), referencing the onSuccess handler, router.push call, and data.workspaceSlug/data.projectId to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/index.tsx:
- Line 60: The step with id "select-repo" currently uses the wrong label text;
update the StepWizard.Step that has id="select-repo" (the element with
label="Connect GitHub") to use the correct label, e.g., change label="Connect
GitHub" to label="Select repository" (or "Select Repository") so the wizard
navigation displays the repository selection step name properly.
- Around line 16-21: Validate and normalize URL-driven wizard state by parsing
searchParams into a typed/validated step and projectId before setting state:
replace direct use of initialStep/initialProjectId with a small parsing layer
that maps the raw step string to an allowed StepId union (e.g., "create-project"
| "configure-deployment" | ...) and treats unknown values as the default step;
ensure that when mapping to defaultStepId or when initializing projectId state
(projectId, setProjectId) you enforce that steps which require a projectId
(e.g., configure-deployment, review) are not selected if projectId is null —
instead fall back to "create-project" or another safe step and only set
projectId state when the parsed projectId is a valid non-empty string. Ensure
this validation is applied where defaultStepId is computed and at initialization
so illegal URL states can't render null content.
In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/page.tsx:
- Around line 11-13: The workspace lookup uses db.query.workspaces.findFirst and
only filters by orgId and deletedAtM, so it can return the wrong workspace for
this [workspaceSlug] route; update the where clause in the workspace query (the
call to db.query.workspaces.findFirst that assigns workspace) to also filter by
the route param slug (e.g., add and(eq(table.slug, workspaceSlug)) or
equivalent) so the record is scoped to the current workspaceSlug while keeping
the existing orgId and isNull(table.deletedAtM) checks.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsx:
- Around line 19-21: The Deploy Button (the <Button> with type="submit") in this
component isn't wired to any handler or form target; either change it to a
regular button and add an onClick that calls a deploy handler (e.g.,
handleDeploy) or wrap the inputs in a <form> and implement an onSubmit handler
that performs the deploy action. Add a clearly named handler (handleDeploy or
onSubmitDeploy) inside the Configure Deployment component and wire it to the
Button via onClick or to the form via onSubmit, and ensure any required
props/state (deployment payload, loading state, navigation) are passed/updated
accordingly.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx:
- Around line 108-110: The dismiss button that renders <XMark> is missing an
accessible name; update the button (the element that calls
setIsBannerDismissed(true)) to provide an accessible label (e.g., add
aria-label="Dismiss" or aria-label="Close banner" or include visually hidden
text) so screen readers can announce its purpose while keeping the icon-only
visual; ensure the button still uses type="button" and retains its onClick
handler.
- Around line 76-93: The click handler handleSelectRepository awaits
selectRepoMutation.mutateAsync which can still reject after onError runs — wrap
the await in a try/catch so errors are handled instead of bubbling as unhandled
promise rejections: inside handleSelectRepository, keep
setMutatingRepoId(repo.id) and the finally that clears it, but change the try
block to try { await selectRepoMutation.mutateAsync(...); next(); } catch (err)
{ /* handle or log error; do not rethrow */ } finally { setMutatingRepoId(null);
} to ensure mutateAsync errors are caught; reference function
handleSelectRepository and method selectRepoMutation.mutateAsync.
---
Outside diff comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/github-settings/index.tsx:
- Around line 55-75: When readOnly is true, ensure the "no-app" and "no-repo"
branches mirror the non-interactive behavior used in the "connected" branch: in
the "no-app" case render the ManageGitHubAppLink inside a GitHubSettingCard with
chevronState="disabled" and make the ManageGitHubAppLink non-interactive (either
pass a readOnly prop or use its "disabled"/outline variant without click
handlers), and in the "no-repo" case render GitHubNoRepo in a disabled state
(wrap it in GitHubSettingCard with chevronState="disabled" or pass readOnly to
GitHubNoRepo so it shows non-interactive UI). Target the case "no-app", case
"no-repo", the readOnly variable, and components GitHubSettingCard,
ManageGitHubAppLink, and GitHubNoRepo when applying the change.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/repo-list-item.tsx:
- Around line 102-107: The Button currently uses disabled={disabled ||
isLoading} but still passes onClick={() => onSelect(repo)} which allows repeated
clicks during an in-flight selection; update the component to treat loading as a
hard interaction lock by including the component's loading prop in the disabled
condition (e.g., disabled when loading || isLoading || disabled) and add a
runtime guard at the start of the onClick handler (check loading || isLoading ||
disabled and return early) before calling onSelect(repo) so duplicate
submissions cannot be fired; locate the Button element and its onClick prop in
repo-list-item.tsx to apply these changes.
---
Nitpick comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsx:
- Line 16: The container uses a fixed className "w-[900px]" which can overflow
on small viewports; update the JSX div that currently has className="w-[900px]"
to use a responsive pattern such as "w-full max-w-[900px]" (optionally adding
horizontal padding like "px-4") so the container scales on narrow screens while
retaining the 900px maximum width.
In `@web/apps/dashboard/app/`(app)/integrations/github/callback/page.tsx:
- Around line 24-28: In onSuccess, build the redirect URL using URLSearchParams
to safely encode query parameters instead of string interpolation; replace the
router.push call that concatenates data.workspaceSlug and a query string with
assembling params via new URLSearchParams({ step: 'select-repo', projectId:
data.projectId }).toString() and then
router.push(`/${data.workspaceSlug}/projects/onboarding?${params}`), referencing
the onSuccess handler, router.push call, and data.workspaceSlug/data.projectId
to locate the change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/onboarding-header.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/data-provider.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/github-settings/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/page.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/onboarding-links.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/onboarding-step-header.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/page.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/connect-github.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/create-project.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/repo-list-item.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/skeleton.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/projects-client.tsxweb/apps/dashboard/app/(app)/integrations/github/callback/page.tsxweb/apps/dashboard/lib/trpc/routers/deploy/environment-settings/sentinel/update-middleware.tsweb/internal/ui/src/components/step-wizard/step-wizard.tsx
💤 Files with no reviewable changes (2)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/onboarding-header.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/index.tsx
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx
Show resolved
Hide resolved
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx
Outdated
Show resolved
Hide resolved
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/page.tsx
Show resolved
Hide resolved
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsx
Show resolved
Hide resolved
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx
Show resolved
Hide resolved
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx
Show resolved
Hide resolved
ogzhanolguncu
force-pushed
the
feat/next-onboarding-step
branch
from
5f2e4c4 to
fa20f12
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx (2)
76-93:⚠️ Potential issue | 🟠 MajorCatch
mutateAsyncrejection inhandleSelectRepository.Lines 82-89 can still reject and bubble from the click path;
onErrortoast does not consume the rejected promise.Suggested fix
const handleSelectRepository = async (repo: { id: number; fullName: string; installationId: number; }) => { setMutatingRepoId(repo.id); try { await selectRepoMutation.mutateAsync({ projectId, repositoryId: repo.id, repositoryFullName: repo.fullName, installationId: repo.installationId, }); next(); + } catch { + // Error toast is already handled in selectRepoMutation.onError } finally { setMutatingRepoId(null); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx around lines 76 - 93, handleSelectRepository currently awaits selectRepoMutation.mutateAsync which can reject and bubble up; wrap the await in a try/catch so rejections are consumed (call next() only on success) and surface an error toast in the catch instead of letting the promise reject, while keeping the existing finally block to call setMutatingRepoId(null). Specifically update the async body in handleSelectRepository to catch errors from selectRepoMutation.mutateAsync, call the same onError/toast logic in the catch, avoid rethrowing, and preserve calls to next() only after a successful mutate.
108-110:⚠️ Potential issue | 🟡 MinorAdd an accessible label to the icon-only dismiss button.
Line 108 renders only an icon; provide an accessible name so screen readers can announce its purpose.
Suggested fix
- <button type="button" onClick={() => setIsBannerDismissed(true)} className="ml-auto"> + <button + type="button" + aria-label="Dismiss GitHub connected banner" + onClick={() => setIsBannerDismissed(true)} + className="ml-auto" + > <XMark iconSize="sm-regular" /> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx around lines 108 - 110, The dismiss button currently renders only an icon (XMark) and lacks an accessible name; update the button in the select-repo component so screen readers can announce its purpose by adding an accessible label (e.g., aria-label="Dismiss banner" or aria-label="Close") to the <button> that calls setIsBannerDismissed(true), or include visually-hidden text inside the button for the same effect while keeping the XMark icon.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/apps/dashboard/app/`(app)/integrations/github/callback/page.tsx:
- Around line 26-28: The callback navigation uses router.push which leaves the
callback URL in history and can cause the useEffect guard around the
registerInstallation mutation (the mutation created in page.tsx that calls
registerInstallation) to re-run on remount; replace the router.push(...) call
with router.replace(...) so the callback URL is not kept in history, preventing
back-button remounts from re-triggering the registerInstallation mutation and
eliminating duplicate backend calls.
---
Duplicate comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsx:
- Around line 76-93: handleSelectRepository currently awaits
selectRepoMutation.mutateAsync which can reject and bubble up; wrap the await in
a try/catch so rejections are consumed (call next() only on success) and surface
an error toast in the catch instead of letting the promise reject, while keeping
the existing finally block to call setMutatingRepoId(null). Specifically update
the async body in handleSelectRepository to catch errors from
selectRepoMutation.mutateAsync, call the same onError/toast logic in the catch,
avoid rethrowing, and preserve calls to next() only after a successful mutate.
- Around line 108-110: The dismiss button currently renders only an icon (XMark)
and lacks an accessible name; update the button in the select-repo component so
screen readers can announce its purpose by adding an accessible label (e.g.,
aria-label="Dismiss banner" or aria-label="Close") to the <button> that calls
setIsBannerDismissed(true), or include visually-hidden text inside the button
for the same effect while keeping the XMark icon.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/onboarding-header.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/data-provider.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/github-settings/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/page.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/onboarding-links.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/onboarding-step-header.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/page.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/connect-github.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/create-project.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/repo-list-item.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/skeleton.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/projects-client.tsxweb/apps/dashboard/app/(app)/integrations/github/callback/page.tsxweb/apps/dashboard/lib/trpc/routers/deploy/environment-settings/sentinel/update-middleware.tsweb/internal/ui/src/components/step-wizard/step-wizard.tsx
💤 Files with no reviewable changes (2)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/onboarding-header.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/(onboarding)/index.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/github-settings/index.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/create-project.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/connect-github.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/data-provider.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/onboarding-step-header.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsx
- web/internal/ui/src/components/step-wizard/step-wizard.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/page.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/projects-client.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/select-repo/repo-list-item.tsx
- web/apps/dashboard/lib/trpc/routers/deploy/environment-settings/sentinel/update-middleware.ts
web/apps/dashboard/app/(app)/integrations/github/callback/page.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx (1)
20-29:⚠️ Potential issue | 🟠 MajorValidate URL-driven wizard state at the boundary.
The
stepandprojectIdare read directly from query params without validation. A URL like?step=configure-deploymentwithoutprojectIdwill navigate to that step but rendernullcontent (lines 82-91). Per coding guidelines, inputs should be parsed at boundaries into typed structures.🛡️ Suggested fix to validate step and guard navigation
+"use client"; +import { z } from "zod"; import { collection } from "@/lib/collections"; // ... other imports +const StepId = z.enum([ + "create-project", + "connect-github", + "select-repo", + "configure-deployment", +]); +type StepId = z.infer<typeof StepId>; export const Onboarding = () => { const existingProjects = useLiveQuery((q) => q.from({ project: collection.projects }), []); const isFirstProject = existingProjects.isLoading || existingProjects.data.length === 0; const searchParams = useSearchParams(); - const initialStep = searchParams.get("step") ?? undefined; - const initialProjectId = searchParams.get("projectId") ?? undefined; + const parsedStep = StepId.safeParse(searchParams.get("step")); + const initialProjectId = searchParams.get("projectId") ?? undefined; + + // Steps beyond create-project require a projectId + const stepsRequiringProjectId: StepId[] = ["connect-github", "select-repo", "configure-deployment"]; + const requestedStep = parsedStep.success ? parsedStep.data : undefined; + const canStartAtStep = + !requestedStep || + requestedStep === "create-project" || + Boolean(initialProjectId); + const initialStep = canStartAtStep ? requestedStep : "create-project"; const [projectId, setProjectId] = useState<string | null>(initialProjectId ?? null); return ( <div> - <StepWizard.Root defaultStepId={initialStep}> + <StepWizard.Root defaultStepId={initialStep}>As per coding guidelines: "Make illegal states unrepresentable by modeling domain with discriminated unions and parsing inputs at boundaries into typed structures."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/index.tsx around lines 20 - 29, Parse and validate the query params at the boundary: convert initialStep and initialProjectId into typed values (e.g., a Step discriminated union and a nullable ProjectId) before calling useState; ensure initialStep only becomes a valid step id for StepWizard.Root (reject unknown strings) and if a step that requires projectId (the ones used in the render logic that currently produce null) is requested without a valid initialProjectId, override to a safe default step (or clear to undefined) so projectId/useState and StepWizard.Root never enter an invalid state; update the initialization of projectId/setProjectId and defaultStepId to use these parsed/validated values and centralize this parsing adjacent to where initialStep/initialProjectId are read.
🧹 Nitpick comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsx (1)
77-77: Remove trailing whitespace after JSX element.There's an extra space after the
<SelectRepo />component that appears to be unintentional.✨ Suggested fix
- <SelectRepo projectId={projectId} />{" "} + <SelectRepo projectId={projectId} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/index.tsx at line 77, The JSX line rendering SelectRepo contains an unintended trailing whitespace token; remove the extra {" "} or any trailing space after the <SelectRepo projectId={projectId} /> component so the element is not followed by stray whitespace in the render output (update the JSX in the onboarding component where SelectRepo is used).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/index.tsx:
- Around line 20-29: Parse and validate the query params at the boundary:
convert initialStep and initialProjectId into typed values (e.g., a Step
discriminated union and a nullable ProjectId) before calling useState; ensure
initialStep only becomes a valid step id for StepWizard.Root (reject unknown
strings) and if a step that requires projectId (the ones used in the render
logic that currently produce null) is requested without a valid
initialProjectId, override to a safe default step (or clear to undefined) so
projectId/useState and StepWizard.Root never enter an invalid state; update the
initialization of projectId/setProjectId and defaultStepId to use these
parsed/validated values and centralize this parsing adjacent to where
initialStep/initialProjectId are read.
---
Nitpick comments:
In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/index.tsx:
- Line 77: The JSX line rendering SelectRepo contains an unintended trailing
whitespace token; remove the extra {" "} or any trailing space after the
<SelectRepo projectId={projectId} /> component so the element is not followed by
stray whitespace in the render output (update the JSX in the onboarding
component where SelectRepo is used).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/index.tsxweb/apps/dashboard/app/(app)/integrations/github/callback/page.tsx
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.
What does this PR do?
Refactors the project onboarding flow by restructuring the wizard steps and improving the user experience. The onboarding process now includes a new "configure deployment" step and moves from a route group structure to a dedicated onboarding page.
Key changes include:
(onboarding)route group to dedicated/onboardingpageType of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated