refactor(app): extract provider connect auth views#681
Conversation
📝 WalkthroughWalkthroughThis pull request extracts OAuth/API authentication views, OAuth auto-callback handling, and prompt form logic from ChangesProvider connection flow extraction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 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 |
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/components/dialog-connect-provider-auth-views.tsx, packages/app/src/components/dialog-connect-provider-auto-view.tsx, packages/app/src/components/dialog-connect-provider-error.ts, packages/app/src/components/dialog-connect-provider-prompt-view.tsx, packages/app/src/components/dialog-connect-provider-source.test.ts, packages/app/src/components/dialog-connect-provider.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request refactors the DialogConnectProvider component by modularizing its internal view components and error formatting logic into separate files, including ProviderApiAuthView, ProviderOAuthCodeView, ProviderOAuthAutoView, and ProviderOAuthPromptsView. A new test suite was also introduced to verify the extraction. A high-severity reactivity issue was identified in ProviderOAuthPromptsView where synchronous store updates could cause the item() memo to re-compute prematurely, leading to incorrect indexing during prompt transitions.
Perf delta summaryComparator: pass
|
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 `@packages/app/src/components/dialog-connect-provider-auth-views.tsx`:
- Around line 36-45: The API auth submit block should be wrapped in a try-catch
so errors are surfaced to the form; around the call to
globalSDK.client.auth.set(...) (the block using props.provider().id and apiKey)
add a try block and catch any thrown error, and in the catch call
setFormStore("error", formatProviderConnectError(err, props.provider(), "api"))
to mirror the OAuth path, then return (or exit) without calling
props.onComplete() on failure so the form shows the error.
🪄 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: CHILL
Plan: Pro Plus
Run ID: c0fce2f9-93fd-4e82-8fa5-092b3f8e4115
📒 Files selected for processing (8)
packages/app/src/components/dialog-connect-provider-auth-views.tsxpackages/app/src/components/dialog-connect-provider-auto-view.tsxpackages/app/src/components/dialog-connect-provider-error.tspackages/app/src/components/dialog-connect-provider-prompt-state.tspackages/app/src/components/dialog-connect-provider-prompt-view.test.tspackages/app/src/components/dialog-connect-provider-prompt-view.tsxpackages/app/src/components/dialog-connect-provider-source.test.tspackages/app/src/components/dialog-connect-provider.tsx
Summary
Extracts owner files from the oversized provider connection dialog:
dialog-connect-provider-auth-views.tsxowns API-key and OAuth code auth viewsdialog-connect-provider-api-auth.tsowns API-key auth submit success/failure handlingdialog-connect-provider-auto-view.tsxowns OAuth auto-code completiondialog-connect-provider-prompt-view.tsxowns OAuth prompt sequencing UIdialog-connect-provider-prompt-state.tsowns prompt select transition statedialog-connect-provider-error.tsowns provider auth error formattingdialog-connect-provider-source.test.tsand focused helper tests lock moved behavior and source boundariesdialog-connect-provider.tsxdrops from 654 LOC ondevto 352 LOC in this PR. The new production owner files are all under 200 LOC.Why
This is a #604 settings-lane frontend governance slice. The provider connection dialog mixed provider/method state, prompt sequencing, API-key auth, OAuth code auth, auto OAuth completion, and error formatting in one file. This PR keeps the state machine and navigation in the parent while moving auth views/helpers into small owner files.
Related Issue
Part of #604.
Human Review Status
Ready for merge. Review follow-ups from Gemini and CodeRabbit were addressed, all review threads are resolved, and the latest fresh-eyes review found no actionable P0/P1/P2/P3 issues.
Review Focus
DialogConnectProviderstill owns method selection, auth state, back navigation, and completion toast.Topology
devcodex/dialog-connect-provider-ownersTouched Files
packages/app/src/components/dialog-connect-provider.tsxpackages/app/src/components/dialog-connect-provider-auth-views.tsxpackages/app/src/components/dialog-connect-provider-api-auth.tspackages/app/src/components/dialog-connect-provider-api-auth.test.tspackages/app/src/components/dialog-connect-provider-auto-view.tsxpackages/app/src/components/dialog-connect-provider-error.tspackages/app/src/components/dialog-connect-provider-prompt-state.tspackages/app/src/components/dialog-connect-provider-prompt-view.tsxpackages/app/src/components/dialog-connect-provider-prompt-view.test.tspackages/app/src/components/dialog-connect-provider-source.test.tsRisk Notes
Risk is prop wiring between the parent auth state machine and the extracted prompt/API/OAuth views. Review follow-ups also touched API-key submit failure behavior and OAuth prompt select transition state. No dependency, migration, credential format, persistence, or platform behavior changes are intended.
How To Verify
Screenshots or Recordings
Not captured. Electron manual verification was not run; this PR is a component extraction plus error-handling review follow-up with no Electron/preload/native surface change. The provider API-key connect user path was covered by the web E2E listed above.
Checklist
dev, and my PR title and commit messages use Conventional Commits in English