OAuth provider client and dashboard #792
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
Show resolved
Hide resolved
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
Outdated
Show resolved
Hide resolved
|
😱 Found 4 issues. Time to roll up your sleeves! 😱 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
There was a problem hiding this comment.
Greptile Summary
I cannot provide a technical review for this pull request because no changed files were provided in the context. The PR is titled "OAuth provider client and dashboard" which suggests it involves OAuth provider functionality and dashboard components, but without access to the actual code changes, I cannot analyze what specific modifications were made, how they integrate with the existing codebase, or their technical implementation.
To provide an accurate review, I would need to see the modified files, including any changes to OAuth provider configurations, dashboard components, API endpoints, database schemas, or related functionality.
Confidence score: 0/5
• Cannot assess safety as no code changes are visible
• Score reflects inability to review due to missing file contents rather than code quality issues
• All files would need attention since none are visible for review
No files reviewed, no comments
WalkthroughAdded provider_config_id to OAuth provider CRUD responses, extended schemas and shared interfaces, introduced OAuth provider management in client/server app implementations and dashboard UI, and updated tests and types to support CRUD, caching, hooks, and UI dialogs for user-associated OAuth providers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant DashboardUI
participant ClientApp
participant BackendAPI
participant ServerApp
Note over DashboardUI,ClientApp: Read flow (list)
User->>DashboardUI: Open user detail page
DashboardUI->>ClientApp: request user + oauth providers
ClientApp->>BackendAPI: GET /api/latest/oauth-providers?user_id=...
BackendAPI->>ServerApp: listServerOAuthProviders(userId)
ServerApp-->>BackendAPI: [providers with provider_config_id]
BackendAPI-->>ClientApp: [providers]
ClientApp-->>DashboardUI: render list
Note over DashboardUI,ClientApp: Create / Update / Delete flow
User->>DashboardUI: Create/Edit/Delete provider
DashboardUI->>ClientApp: create/update/delete call
ClientApp->>BackendAPI: POST/PATCH/DELETE /api/latest/oauth-providers
BackendAPI->>ServerApp: create/update/deleteServerOAuthProvider(...)
ServerApp-->>BackendAPI: result (or known error)
BackendAPI-->>ClientApp: result (includes provider_config_id)
ClientApp-->>DashboardUI: update UI / show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/stack-shared/src/interface/client-interface.ts (1)
739-739: Fix inconsistent return type in deleteOAuthProviderThe method signature indicates it returns
Promise<void>but the implementation returnsawait response.json(). For a delete operation, it should either return void or the method signature should be updated to match the actual return type.Apply this fix to make the implementation consistent with the signature:
- return await response.json(); + // Delete operations typically don't return dataOr update the method signature if a response is actually needed:
- ): Promise<void> { + ): Promise<any> {packages/stack-shared/src/interface/server-interface.ts (1)
750-762: Fix return statement inconsistency!The method signature indicates
Promise<void>but line 761 returnsawait response.json(). For a void return type, the implementation should not return any value.Apply this fix:
async deleteServerOAuthProvider( userId: string, providerId: string, ): Promise<void> { - const response = await this.sendServerRequest( + await this.sendServerRequest( urlString`/oauth-providers/${userId}/${providerId}`, { method: "DELETE", }, null, ); - return await response.json(); }
♻️ Duplicate comments (1)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
828-838: Violation of API naming convention: Using camelCase parameters in API interfacesAccording to the naming convention rule, parameters used in API communication should use snake_case. The parameters 'allowSignIn' and 'allowConnectedAccounts' should be 'allow_sign_in' and 'allow_connected_accounts' for consistency with HTTP API naming conventions.
🧹 Nitpick comments (3)
packages/template/src/lib/stack-app/index.ts (1)
54-58: Minor nitpick: Unnecessary export reorderingThe reordering of permission-related exports doesn't appear to serve a functional purpose and could make the git history less clear.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx (2)
1113-1113: Use template literal for compound keyFor better readability and consistency with modern JavaScript practices.
- <TableRow key={provider.id + '-' + provider.accountId}> + <TableRow key={`${provider.id}-${provider.accountId}`}>
1231-1232: Replace empty div spacer with proper flex gapThe empty div appears to be used as a spacer. Consider using the parent's flex gap or adding a specific spacing class for better maintainability.
<ContactChannelsSection user={user} /> <OAuthProvidersSection user={user} /> - <div /> <MetadataSection user={user} />If additional spacing is needed between OAuthProvidersSection and MetadataSection, consider adjusting the parent's
gap-6class or adding margin to the MetadataSection component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/backend/src/app/api/latest/oauth-providers/crud.tsx(4 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/oauth-providers.test.ts(11 hunks)packages/stack-shared/src/interface/client-interface.ts(3 hunks)packages/stack-shared/src/interface/crud/oauth-providers.ts(2 hunks)packages/stack-shared/src/interface/server-interface.ts(5 hunks)packages/stack-shared/src/schema-fields.ts(1 hunks)packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts(5 hunks)packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts(6 hunks)packages/template/src/lib/stack-app/apps/interfaces/server-app.ts(2 hunks)packages/template/src/lib/stack-app/index.ts(2 hunks)packages/template/src/lib/stack-app/users/index.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/stack-shared/src/interface/crud/oauth-providers.ts (1)
packages/stack-shared/src/schema-fields.ts (1)
oauthProviderProviderConfigIdSchema(504-504)
packages/stack-shared/src/interface/server-interface.ts (1)
packages/stack-shared/src/interface/crud/oauth-providers.ts (1)
OAuthProviderCrud(86-86)
packages/stack-shared/src/interface/client-interface.ts (3)
packages/stack-shared/src/sessions.ts (1)
InternalSession(51-212)packages/stack-shared/src/interface/crud/oauth-providers.ts (1)
OAuthProviderCrud(86-86)packages/stack-shared/src/utils/objects.tsx (1)
filterUndefined(368-370)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (5)
packages/template/src/lib/stack-app/apps/implementations/common.ts (2)
createCache(22-27)useAsyncCache(145-190)packages/stack-shared/src/interface/crud/oauth-providers.ts (1)
OAuthProviderCrud(86-86)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1395-1397)KnownErrors(1399-1508)packages/stack-shared/src/utils/results.tsx (1)
error(36-41)packages/template/src/lib/stack-app/users/index.ts (1)
ServerOAuthProvider(33-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (24)
packages/stack-shared/src/schema-fields.ts (1)
504-504: Clean schema field additionThe new
oauthProviderProviderConfigIdSchemafollows the established patterns in this file with appropriate Yup validation, OpenAPI metadata, and clear documentation.packages/stack-shared/src/interface/crud/oauth-providers.ts (1)
8-8: Schema integration is consistent across the stackThe addition of
provider_config_idto both the import (line 8) and client read schema (line 20) is correct and consistent with the broader implementation. The backend CRUD handlers inapps/backend/src/app/api/latest/oauth-providers/crud.tsxreturn this field in all relevant operations (onCreate, onRead, onList, onUpdate), and the E2E tests validate its presence in API responses.Also applies to: 20-20
apps/e2e/tests/backend/endpoints/api/v1/oauth-providers.test.ts (1)
49-49: Comprehensive test coverage for provider_config_id fieldThe test updates consistently validate the presence of
provider_config_idacross all OAuth provider CRUD operations (create, read, list, update). This ensures the new field is properly returned by the API in all scenarios and maintains test coverage for the enhanced OAuth provider functionality.Also applies to: 93-93, 141-141, 405-405, 531-531, 541-541, 569-569, 597-597, 660-660, 683-683, 881-881
apps/backend/src/app/api/latest/oauth-providers/crud.tsx (1)
157-157: Clean implementation of provider_config_id in CRUD responsesThe addition of
provider_config_idto all OAuth provider CRUD operation responses is well-implemented:
- Read/List/Update operations correctly use the existing
oauthAccount.configOAuthProviderId- Create operation appropriately uses the input
data.provider_config_id- Consistent field structure across all operations ensures API coherence
Also applies to: 197-197, 308-308, 404-404
packages/template/src/lib/stack-app/index.ts (1)
99-100: Appropriate addition of OAuth provider type exportsThe new
OAuthProviderandServerOAuthProvidertype exports correctly expose the OAuth provider functionality to consumers of the Stack template library, completing the OAuth provider management feature integration.packages/template/src/lib/stack-app/apps/interfaces/server-app.ts (2)
1-2: LGTM: Import additions are appropriateThe new imports for
KnownErrors,Result, andServerOAuthProviderare correctly added and necessary for the newcreateOAuthProvidermethod.Also applies to: 5-5
52-59: Well-designed OAuth provider creation methodThe
createOAuthProvidermethod follows established patterns in the interface with:
- Comprehensive parameter set covering all necessary OAuth provider fields
- Proper Result-based error handling with specific error type for account ID conflicts
- Consistent async method signature
- Appropriate return type using
ServerOAuthProviderpackages/template/src/lib/stack-app/users/index.ts (3)
19-45: Well-structured OAuth provider type definitionsThe
OAuthProviderandServerOAuthProvidertypes are properly designed with:
- Appropriate readonly properties for immutable data
- Consistent field naming and types across both variants
- Logical difference in
accountIdoptionality (optional for client, required for server)- Proper Result-based error handling in update methods with specific error types
- Clean async method signatures for update and delete operations
252-256: Consistent OAuth provider methods in UserExtraThe new OAuth provider methods follow the established patterns in the codebase:
- Proper React-like hook naming with
useprefix- Consistent async/sync method pairing (
listOAuthProviders/useOAuthProviders)- Appropriate null handling for single item access methods
- Clean separation between list and individual item operations
351-355: Proper server-side OAuth provider methodsThe ServerBaseUser OAuth provider methods correctly mirror the client-side methods while returning appropriate
ServerOAuthProvidertypes:
- Consistent method naming and signatures with UserExtra
- Proper server type returns instead of client types
- Maintains the established server vs client pattern throughout the codebase
- Complete CRUD access for server-side OAuth provider management
packages/stack-shared/src/interface/client-interface.ts (3)
20-20: Necessary import additionThe
OAuthProviderCrudimport is properly added and required for the OAuth provider method type definitions.
677-677: Improved consistency in OAuth provider methodsThe OAuth provider methods now consistently:
- Require
InternalSessionparameter for proper authentication- Use
sendClientRequestfor uniform request handling- Follow established patterns with other authenticated methods in the interface
Also applies to: 713-713, 730-730
689-707: Well-typed OAuth provider update methodThe
updateOAuthProvidermethod properly usesOAuthProviderCrud['Client']types for both input data and return value, ensuring type safety and consistency with the CRUD interface.packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (7)
4-4: LGTM!The import for
OAuthProviderCrudfollows the established pattern and is properly organized with other CRUD imports.
28-28: Import looks good!The addition of
ServerOAuthProviderto the imports is consistent with the OAuth provider management feature being implemented.
158-162: Cache implementation follows established patterns!The
_serverOAuthProvidersCacheis properly implemented using the standard cache pattern with correct typing and parameter handling.
233-269: Well-structured OAuth provider conversion method!The
_serverOAuthProviderFromCrudmethod correctly implements:
- Proper mapping of all CRUD properties
- Error handling for known errors using Result pattern
- Cache refresh after mutations
- Type-safe async methods matching the
ServerOAuthProviderinterface
602-624: OAuth provider user methods implemented correctly!The implementation properly follows established patterns:
- React hooks are appropriately platform-guarded
useMemooptimization for React hooks- Consistent list/get/use method patterns
- Proper cache usage and data transformation
1025-1025: Cache refresh properly integrated!Adding OAuth providers cache refresh to
_refreshUsersensures data consistency when user data is refreshed.
1029-1055: Robust OAuth provider creation method!The
createOAuthProviderimplementation includes:
- Comprehensive parameter validation through TypeScript types
- Proper error handling for known errors using Result pattern
- Cache refresh for data consistency
- Correct parameter mapping to API snake_case format
packages/stack-shared/src/interface/server-interface.ts (4)
15-15: Import properly added!The
OAuthProviderCrudimport follows the established pattern for CRUD type imports.
696-711: Type refactoring improves consistency!The method now uses centralized CRUD types, which improves maintainability and type safety.
714-729: CRUD type updates are consistent!The
listServerOAuthProvidersmethod properly uses the centralized CRUD types for its return value.
731-748: Update method types properly refactored!The
updateServerOAuthProvidermethod now uses centralized CRUD types for both parameters and return values.
…jects/stack into oauth-providers-dashboard
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx (3)
951-955: Allow clearing the email on update (don’t silently restore the old value).Current fallback prevents clearing. Pass undefined to keep, or empty string to clear, based on the form value.
- result = await provider.update({ - email: values.email?.trim() || provider.email, + const nextEmail = values.email === undefined ? undefined : values.email.trim(); + const updateRes = await provider.update({ + email: nextEmail, allowSignIn: values.allowSignIn, allowConnectedAccounts: values.allowConnectedAccounts, });
907-916: Use schema validation for accountId instead of manual early return.Rely on yup and show a field error rather than silently returning.
- accountId: yup.string() - .defined("Account ID is required") + accountId: yup.string() + .trim() + .min(1, "Account ID is required") .label("Account ID") @@ - // Create new provider - if (!values.accountId.trim()) return; + // Create new provider (schema ensures non-empty accountId)
1163-1168: Ask for confirmation before deleting an OAuth provider.Avoid accidental destructive actions.
- onClick: async () => { - await provider.delete(); - }, + onClick: async () => { + if (confirm("Delete this OAuth provider? This action cannot be undone.")) { + await provider.delete(); + } + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
For blocking alerts and errors in UI, do not use toast notifications; use alerts instead
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Keep hover/click animations snappy; avoid pre-transition delays on hover and apply transitions after the action (e.g., fade-out on hover end)
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
🧬 Code graph analysis (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx (7)
packages/template/src/lib/stack-app/users/index.ts (1)
ServerOAuthProvider(34-46)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(27-34)packages/stack-ui/src/components/ui/use-toast.tsx (2)
useToast(195-195)toast(195-195)apps/dashboard/src/components/form-fields.tsx (1)
SelectField(229-266)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1570-1572)KnownErrors(1574-1696)apps/dashboard/src/components/form-dialog.tsx (1)
SmartFormDialog(11-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (6)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx (6)
881-899: Confirm custom yup method.nonEmptyexists; otherwise use.trim().required().If
.nonEmptyisn’t registered, validation will not run and the form may accept empty provider IDs.If not available, apply:
- providerId: yup.string() - .defined("Provider is required") - .nonEmpty("Provider is required") + providerId: yup.string() + .trim() + .required("Provider is required")
965-972: Verify create payload field name:providerConfigIdvsprovider_config_id.APIs in this PR introduce
provider_config_id; ensure the client call expects camelCaseproviderConfigId. If the API expects snake_case, the request will fail.
932-937: Confirmprovider.typeequals the provider configidused in the selector.Defaulting
providerIdtoprovider.typeassumes parity withproject.config.oauthProviders[].id. Verify consistency across all providers.
1082-1090: Edit flow LGTM.Launching edit dialog with the selected provider is correct.
1099-1110: Rendering and keying rows looks good.Clear columns and a stable key (
id-accountId) are appropriate.Also applies to: 1112-1114
1231-1232: Good integration point.Placing OAuth providers between contact channels and metadata aligns with the page’s flow.
| if (KnownErrors.OAuthProviderAccountIdAlreadyUsedForSignIn.isInstance(result.error)) { | ||
| toast({ | ||
| title: "Account Already Connected", | ||
| description: `A ${providerType} provider with account ID "${accountId}" already exists (possibly for a different user)`, | ||
| variant: "destructive", | ||
| }); | ||
| } else { | ||
| console.error(result.error); | ||
| toast({ | ||
| title: "Error", | ||
| description: `An unexpected error occurred while ${operation} the OAuth provider.`, | ||
| variant: "destructive", | ||
| }); | ||
| } | ||
| return 'prevent-close'; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify result handling and replace blocking error toasts with inline Alerts (prevents runtime crash + matches dashboard guideline).
- Risk:
resultis assumed to always havestatus;createOAuthProvidermay throw or returnvoid, leading toresult.statuscrash. - Guideline: For blocking errors, avoid toasts; show an inline Alert in the dialog.
Apply:
@@
-import {
+import {
Accordion,
@@
Typography,
+ Alert,
+ AlertDescription,
+ AlertTitle,
cn,
useToast
} from "@stackframe/stack-ui";
@@
-function OAuthProviderDialog(props: OAuthProviderDialogProps) {
+function OAuthProviderDialog(props: OAuthProviderDialogProps) {
const stackAdminApp = useAdminApp();
const project = stackAdminApp.useProject();
- const { toast } = useToast();
+ // useToast not needed for errors here; show section-local Alert instead
+ const [formError, setFormError] = useState<string | null>(null);
@@
- const handleSubmit = async (values: yup.InferType<typeof formSchema>) => {
- let result;
+ const handleSubmit = async (values: yup.InferType<typeof formSchema>) => {
+ setFormError(null);
+ let err: unknown | null = null;
if (isEditMode && provider) {
// Update existing provider
- result = await provider.update({
+ const updateRes = await provider.update({
email: values.email?.trim() || provider.email,
allowSignIn: values.allowSignIn,
allowConnectedAccounts: values.allowConnectedAccounts,
});
+ if (updateRes.status === "error") {
+ err = updateRes.error;
+ }
} else {
// Create new provider
- if (!values.accountId.trim()) return;
+ if (!values.accountId.trim()) {
+ setFormError("Account ID is required.");
+ return 'prevent-close';
+ }
const providerConfig = availableProviders.find((p: any) => p.id === values.providerId);
if (!providerConfig) {
throw new StackAssertionError(`Provider config not found for ${values.providerId}`);
}
- result = await stackAdminApp.createOAuthProvider({
- userId: props.user.id,
- providerConfigId: providerConfig.id,
- accountId: values.accountId.trim(),
- email: values.email?.trim() || "",
- allowSignIn: values.allowSignIn,
- allowConnectedAccounts: values.allowConnectedAccounts,
- });
+ try {
+ await stackAdminApp.createOAuthProvider({
+ userId: props.user.id,
+ providerConfigId: providerConfig.id,
+ accountId: values.accountId.trim(),
+ email: values.email?.trim() || "",
+ allowSignIn: values.allowSignIn,
+ allowConnectedAccounts: values.allowConnectedAccounts,
+ });
+ } catch (e) {
+ err = e;
+ }
}
- // Handle errors for both create and update operations
- if (result.status === "error") {
- const providerType = isEditMode && provider ? provider.type : values.providerId;
- const accountId = isEditMode && provider ? provider.accountId : values.accountId;
- const operation = isEditMode ? "updating" : "adding";
-
- if (KnownErrors.OAuthProviderAccountIdAlreadyUsedForSignIn.isInstance(result.error)) {
- toast({
- title: "Account Already Connected",
- description: `A ${providerType} provider with account ID "${accountId}" already exists (possibly for a different user)`,
- variant: "destructive",
- });
- } else {
- console.error(result.error);
- toast({
- title: "Error",
- description: `An unexpected error occurred while ${operation} the OAuth provider.`,
- variant: "destructive",
- });
- }
- return 'prevent-close';
- }
+ if (err) {
+ const providerType = isEditMode && provider ? provider.type : values.providerId;
+ const accountId = isEditMode && provider ? provider.accountId : values.accountId;
+ const operation = isEditMode ? "updating" : "adding";
+ if (KnownErrors.OAuthProviderAccountIdAlreadyUsedForSignIn.isInstance(err)) {
+ setFormError(`A ${providerType} provider with account ID "${accountId}" already exists (possibly for a different user).`);
+ } else {
+ console.error(err);
+ setFormError(`An unexpected error occurred while ${operation} the OAuth provider.`);
+ }
+ return 'prevent-close';
+ }
};
@@
return (
<SmartFormDialog
title={isEditMode ? "Edit OAuth Provider" : "Add OAuth Provider"}
- description={isEditMode ? "Update the OAuth provider settings." : "Connect a new OAuth provider to this user account."}
+ description={
+ <div className="space-y-2">
+ <div>{isEditMode ? "Update the OAuth provider settings." : "Connect a new OAuth provider to this user account."}</div>
+ {formError && (
+ <Alert variant="destructive">
+ <AlertTitle>Error</AlertTitle>
+ <AlertDescription>{formError}</AlertDescription>
+ </Alert>
+ )}
+ </div>
+ }
open={props.open}
onOpenChange={props.onOpenChange}
formSchema={schemaWithDefaults}
onSubmit={handleSubmit}
/>
);Also applies to: 1003-1010, 16-43, 870-875
| const oauthProviders = user.useOAuthProviders(); | ||
| const [isAddProviderDialogOpen, setIsAddProviderDialogOpen] = useState(false); | ||
| const [editingProvider, setEditingProvider] = useState<ServerOAuthProvider | null>(null); | ||
| const { toast } = useToast(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Show section-level Alert for toggle errors instead of toasts (blocking error).
Per dashboard guideline, use Alerts for blocking errors. Keep success toasts if desired.
function OAuthProvidersSection({ user }: OAuthProvidersSectionProps) {
const oauthProviders = user.useOAuthProviders();
const [isAddProviderDialogOpen, setIsAddProviderDialogOpen] = useState(false);
const [editingProvider, setEditingProvider] = useState<ServerOAuthProvider | null>(null);
const { toast } = useToast();
+ const [sectionError, setSectionError] = useState<string | null>(null);
@@
const handleProviderUpdate = async (provider: ServerOAuthProvider, updates: { allowSignIn?: boolean, allowConnectedAccounts?: boolean }) => {
const result = await provider.update(updates);
if (result.status === "error") {
if (KnownErrors.OAuthProviderAccountIdAlreadyUsedForSignIn.isInstance(result.error)) {
- toast({
- title: "Account Already Connected",
- description: `A ${provider.type} provider with account ID "${provider.accountId}" is already connected for this user.`,
- variant: "destructive",
- });
+ setSectionError(`A ${provider.type} provider with account ID "${provider.accountId}" is already connected for this user.`);
} else {
- const settingType = updates.allowSignIn !== undefined ? "sign-in" : "connected accounts";
- toast({
- title: "Error",
- description: `Failed to update ${settingType} setting.`,
- variant: "destructive",
- });
+ const settingType = updates.allowSignIn !== undefined ? "sign-in" : "connected accounts";
+ setSectionError(`Failed to update ${settingType} setting.`);
}
} else {
+ setSectionError(null);
let successMessage = "";
@@
<div className="flex items-center justify-between">
@@
</div>
+ {sectionError && (
+ <Alert variant="destructive">
+ <AlertTitle>Error</AlertTitle>
+ <AlertDescription>{sectionError}</AlertDescription>
+ </Alert>
+ )}Also applies to: 1021-1036, 1074-1081
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
around lines 1015-1019 (and similarly for the handler blocks at 1021-1036 and
1074-1081), replace the blocking-error toast usage with a section-level Alert:
add a local state like errorMessage (string | null) and set it when the
toggle/save action fails instead of calling toast.error; render an Alert
component in the users/providers section that displays errorMessage and provides
a retry/dismiss action that clears the state; keep success toasts unchanged and
clear errorMessage on successful retry/save.
| let successMessage = ""; | ||
| if (updates.allowSignIn !== undefined) { | ||
| successMessage = `Sign-in ${provider.allowSignIn ? 'disabled' : 'enabled'} for ${provider.type} provider.`; | ||
| } else if (updates.allowConnectedAccounts !== undefined) { | ||
| successMessage = `Connected accounts ${provider.allowConnectedAccounts ? 'disabled' : 'enabled'} for ${provider.type} provider.`; | ||
| } | ||
| toast({ |
There was a problem hiding this comment.
Fix toggle success message: message is inverted when update mutates provider in-place.
Use the requested update value, not the (possibly already mutated) provider state.
- if (updates.allowSignIn !== undefined) {
- successMessage = `Sign-in ${provider.allowSignIn ? 'disabled' : 'enabled'} for ${provider.type} provider.`;
- } else if (updates.allowConnectedAccounts !== undefined) {
- successMessage = `Connected accounts ${provider.allowConnectedAccounts ? 'disabled' : 'enabled'} for ${provider.type} provider.`;
- }
+ if (updates.allowSignIn !== undefined) {
+ successMessage = `Sign-in ${updates.allowSignIn ? 'enabled' : 'disabled'} for ${provider.type} provider.`;
+ } else if (updates.allowConnectedAccounts !== undefined) {
+ successMessage = `Connected accounts ${updates.allowConnectedAccounts ? 'enabled' : 'disabled'} for ${provider.type} provider.`;
+ }📝 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.
| let successMessage = ""; | |
| if (updates.allowSignIn !== undefined) { | |
| successMessage = `Sign-in ${provider.allowSignIn ? 'disabled' : 'enabled'} for ${provider.type} provider.`; | |
| } else if (updates.allowConnectedAccounts !== undefined) { | |
| successMessage = `Connected accounts ${provider.allowConnectedAccounts ? 'disabled' : 'enabled'} for ${provider.type} provider.`; | |
| } | |
| toast({ | |
| let successMessage = ""; | |
| if (updates.allowSignIn !== undefined) { | |
| successMessage = `Sign-in ${updates.allowSignIn ? 'enabled' : 'disabled'} for ${provider.type} provider.`; | |
| } else if (updates.allowConnectedAccounts !== undefined) { | |
| successMessage = `Connected accounts ${updates.allowConnectedAccounts ? 'enabled' : 'disabled'} for ${provider.type} provider.`; | |
| } | |
| toast({ |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
around lines 1038-1044, the success message uses provider.allowSignIn /
provider.allowConnectedAccounts which may have been mutated by the update,
causing the message to be inverted; change the logic to derive enabled/disabled
from the requested update values (updates.allowSignIn and
updates.allowConnectedAccounts) when those keys are defined (fall back to
provider.type for the provider name as before), e.g. use the boolean from
updates to decide "enabled" vs "disabled" so the message reflects the action
requested rather than the possibly mutated provider state.
Important
Enhances OAuth provider management with CRUD operations, UI components, and schema updates across client and server interfaces.
provider_config_idto OAuth provider CRUD operations incrud.tsxandpage-client.tsx.OAuthProviderDialogandOAuthProvidersSectioncomponents inpage-client.tsxfor managing OAuth providers.oauth-providers.test.tsto includeprovider_config_id.OAuthProviderCrudtoclient-interface.tsandserver-interface.tsfor CRUD operations.StackClientInterfaceandStackServerInterfaceto handle OAuth provider operations.oauthProviderProviderConfigIdSchematoschema-fields.ts.oauth-providers.tsto includeprovider_config_idin schemas.client-app-impl.tsandserver-app-impl.tsto integrate OAuth provider management.OAuthProviderandServerOAuthProvidertypes tousers/index.ts.server-app.tsandindex.tsto export new types and functions.This description was created by
for 4a540dd. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation