Fix onboarding shared OAuth provider persistence#1477
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR centralizes shared OAuth provider definitions in ChangesOAuth Shared Provider Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR fixes a bug where shared OAuth providers (Google, GitHub, Microsoft) selected during new-project onboarding were not being persisted to the project environment config. It refactors
Confidence Score: 4/5Safe to merge; the changes are logically consistent and the new regression test validates the complete two-stage updateConfig flow end-to-end. The refactoring correctly replaces hardcoded provider entries with a dynamic loop over SHARED_OAUTH_SIGN_IN_METHODS, and the new constant is computed accurately as the intersection of sharedProviders and OAUTH_SIGN_IN_METHODS. The regression test uses a stable shared mock so it can actually assert call counts across async renders. No logic changes to finalizeOnboarding itself, and the dev-environment branch (which intentionally omits isShared) is untouched. No files require special attention; shared.ts is the most load-bearing change and its type predicate correctly narrows the filtered array. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant WelcomeSlide
participant finalizeOnboarding
participant updateConfig
participant setProjectOnboardingStatus
User->>WelcomeSlide: Click "Get Started"
WelcomeSlide->>finalizeOnboarding: onFinish()
finalizeOnboarding->>finalizeOnboarding: persistOnboardingState()
finalizeOnboarding->>updateConfig: buildBranchConfigUpdate() [pushable: true]
updateConfig-->>finalizeOnboarding: true
alt "isDevelopmentEnvironment = false"
finalizeOnboarding->>updateConfig: "buildEnvironmentOAuthConfigUpdate()<br/>SHARED_OAUTH_SIGN_IN_METHODS loop [pushable: false]"
updateConfig-->>finalizeOnboarding: true
end
finalizeOnboarding->>setProjectOnboardingStatus: "completed"
finalizeOnboarding->>finalizeOnboarding: clearOnboardingState()
finalizeOnboarding->>User: onComplete()
Reviews (1): Last reviewed commit: "Fix onboarding shared OAuth provider per..." | Re-trigger Greptile |
![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
🤖 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
`@apps/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/project-onboarding-wizard.test.tsx:
- Around line 292-312: The test currently only awaits mockUpdateConfig inside
waitFor which can allow the completion assertions to race; update the test so
the assertions for mockUpdateConfig (including
expect(mockUpdateConfig).toHaveBeenCalledTimes(2) and
expect(mockUpdateConfig).toHaveBeenNthCalledWith(2, { ... })) and the completion
assertions for setStatus, clearOnboardingState, and onComplete are all placed
inside the same waitFor callback; ensure you reference the existing symbols
mockUpdateConfig, setStatus, clearOnboardingState, and onComplete so the test
synchronizes the full async tail and avoids flakiness.
🪄 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
Run ID: 8c75cc30-8410-4899-8297-ee610708d04c
📒 Files selected for processing (3)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/project-onboarding-wizard.test.tsxapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/project-onboarding-wizard.tsxapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/shared.ts
1 participant
Fixes Google/GitHub/Microsoft shared OAuth providers selected during new-project onboarding not being persisted into the project environment config.
Validation:
pnpm test run apps/dashboard/src/app/'(main)'/'(protected)'/'(outside-dashboard)'/new-project/page-client-parts/project-onboarding-wizard.test.tsxpnpm lint --fixpnpm --filter @stackframe/dashboard typecheckNote: full
pnpm typecheckcurrently fails in@stackframe/backendbecause generated Prisma artifacts are missing (@/generated/prisma/client), while dashboard typecheck passes.Link to Devin session: https://app.devin.ai/sessions/fe87aa598a524ab0822341324c03d90c
Requested by: @N2D4
Summary by CodeRabbit
Bug Fixes
Refactor
Tests