OAuth provider client and dashboard #792
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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
WalkthroughThe changes introduce comprehensive support for managing OAuth providers across backend, shared interfaces, server and client app implementations, and dashboard UI. This includes CRUD API enhancements, schema updates, new TypeScript types, caching and hook-based access patterns, UI dialogs for management, and corresponding test and type updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardUI
participant ClientApp
participant BackendAPI
participant ServerApp
User->>DashboardUI: Open user detail page
DashboardUI->>ClientApp: Fetch user & OAuth providers
ClientApp->>BackendAPI: listOAuthProviders(userId)
BackendAPI->>ServerApp: listServerOAuthProviders(userId)
ServerApp-->>BackendAPI: OAuth providers list
BackendAPI-->>ClientApp: OAuth providers list
ClientApp-->>DashboardUI: OAuth providers list
DashboardUI->>User: Display OAuth providers table
User->>DashboardUI: Add/Edit/Delete OAuth provider
DashboardUI->>ClientApp: create/update/delete OAuth provider
ClientApp->>BackendAPI: Corresponding CRUD API call
BackendAPI->>ServerApp: Corresponding server CRUD method
ServerApp-->>BackendAPI: Result or error
BackendAPI-->>ClientApp: Result or error
ClientApp-->>DashboardUI: Update UI / Show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
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