Feat/#111 change api auth #165
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThe changes add a new GitHub Actions workflow for Vercel preview deployments and update several authentication components. In the web app, login, sign-up, OTP verification, password reset, and forgot password flows have updated API endpoints, hooks, types, and styling. Several legacy components and schemas related to registration completion and animation have been removed. In the API package, new Zod schemas and type definitions have been introduced while outdated ones were removed. Internal dependency versions and import paths have been adjusted for improved consistency and type safety. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Vercel as Vercel API
Dev->>GH: Push commit (non-main branch)
GH->>GH: Checkout code
GH->>GH: Install Vercel CLI
GH->>Vercel: Pull environment configuration
GH->>Vercel: Build & deploy preview artifacts
Vercel-->>GH: Deployment result
sequenceDiagram
participant User as User
participant UI as OTP Form
participant Hook as usePostVerifyOtp
participant API as OTP Verification API
User->>UI: Enter OTP & Username
UI->>Hook: Submit verification request
Hook->>API: POST OTP data
API-->>Hook: Return token & user details
Hook-->>UI: Indicate success
UI->>User: Display success and redirect
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
Scope: all 2 workspace projects This error happened while installing a direct dependency of /tmp/eslint @repo/eslint-config is not in the npm registry, or you have no permission to fetch it. No authorization header was set for the request. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (37)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/apis/src/schema/api-response-schema.ts (1)
1-27: Consider adding generics and limiting allowed types where possible.
While this schema is flexible, you could enhance it by introducing more constraints or generics to strictly type thedataanderrorfields. That helps ensure better validation and reduces potential confusion about accepted shapes of the response.packages/apis/turbo/generators/templates/schema.hbs (1)
11-16: Typographical fixes & minor style nitpicks.
- There's a likely typo in
ResponseSchemaTransofrmed. Consider correcting it toResponseSchemaTransformed.- There's an extra semicolon
;;after the transform function.Below is a suggested fix:
-export const {{method}}{{pascalCase name}}ResponseSchemaTransofrmed = apiResponseSchema.extend({ +export const {{method}}{{pascalCase name}}ResponseSchemaTransformed = apiResponseSchema.extend({ data: z.object({ keyBody: z.string(), }), }).transform((data) => data);;packages/apis/src/services/core/accounts/users/login/post/post-login.schema.ts (1)
11-23: Validate property consistency in the extended schema:
You're extendingapiResponseSchemawith nested fields likeid,username, etc. Make sure each consumer of these fields is aware of the new structure. Also, watch for potential typos in your new name, since it was renamed topostLoginResponseSchemaTransformed.packages/apis/src/types/api.types.ts (1)
8-8: Ensure minimal overhead with additional imports:
Consider whether you needzdirectly in this file if its usage is primarily throughapiResponseSchema. Ifzis only used within the schema definition, you might not need to import it here..github/workflows/preview.yaml (1)
21-21: Missing newline at end of file.
Following YAML best practices, end the file with a newline to avoid potential parsing issues in some tools.Apply this diff to add a trailing newline:
- run: vercel deploy --prebuilt --token=${{ secrets.VERCEL_TOKEN }} + run: vercel deploy --prebuilt --token=${{ secrets.VERCEL_TOKEN }} +🧰 Tools
🪛 yamllint (1.35.1)
[error] 21-21: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (16)
.github/workflows/preview.yaml(1 hunks)apps/core/app/auth/login/_components/form/loginForm.tsx(3 hunks)apps/core/app/auth/login/page.tsx(2 hunks)apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx(2 hunks)apps/core/middleware.ts(1 hunks)packages/apis/package.json(2 hunks)packages/apis/src/instance/core-api.ts(1 hunks)packages/apis/src/instance/guest-api.ts(1 hunks)packages/apis/src/schema/api-response-schema.ts(1 hunks)packages/apis/src/services/core/accounts/users/login/post/post-login.schema.ts(1 hunks)packages/apis/src/services/core/accounts/users/login/post/post-login.ts(1 hunks)packages/apis/src/services/core/accounts/users/login/post/post-login.types.ts(1 hunks)packages/apis/src/services/core/accounts/users/login/post/use-post-login.ts(2 hunks)packages/apis/src/types/api.types.ts(2 hunks)packages/apis/turbo/generators/templates/hook.useMutation.hbs(1 hunks)packages/apis/turbo/generators/templates/schema.hbs(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/apis/src/instance/guest-api.ts
- apps/core/app/auth/login/page.tsx
- apps/core/middleware.ts
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/preview.yaml
13-13: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
.github/workflows/preview.yaml
[error] 21-21: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (20)
packages/apis/turbo/generators/templates/hook.useMutation.hbs (1)
22-22: Adopting the standard React hook naming convention
Renaming the hook touse{{pascalCase method}}{{pascalCase name}}is consistent with the usual lowercase starting pattern for custom React hooks. Nice work!packages/apis/package.json (2)
17-17: LGTM! Good addition of TypeScript definitions.Adding
@types/js-cookieimproves type safety for cookie handling in the authentication flow. The version^3.0.6correctly matches the major version of thejs-cookiedependency.
35-35: Verify the security implications of the version update.The change from fixed version
3.0.5to^3.0.5allows for minor version updates. Let's verify there are no security concerns with this change.✅ Verification successful
Version update to
^3.0.5appears safe to useBased on the verification results:
- No security vulnerabilities were found for js-cookie in GitHub's security advisories
- Version 3.0.5 is currently the latest release in the 3.x series, indicating no minor version updates are available yet
- TypeScript type definitions (@types/js-cookie) are maintained and compatible, with versions up to 3.0.6
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for security advisories and compatibility issues for js-cookie # Check npm for latest versions and security advisories npm view js-cookie versions --json | jq 'map(select(startswith("3.")))' npm audit js-cookie@^3.0.5 # Check GitHub security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "js-cookie") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' # Verify compatibility with @types/js-cookie npm view @types/js-cookie versions --json | jq 'map(select(startswith("3.")))'Length of output: 1195
packages/apis/src/services/core/accounts/users/login/post/post-login.types.ts (3)
1-2: Great use of type-only imports.
Usingimport typehelps ensure the imports are stripped away at compile time, reducing bundle size and clarifying that these are purely type definitions.
4-4: Correct reassignment of comment context.
Renaming the remark from "// Response" to "// Request" is consistent with the type namePostLoginRequest.
11-11: Correct label for the response type.
This adjustment from "// Request" to "// Response" aligns perfectly withPostLoginResponse.packages/apis/turbo/generators/templates/schema.hbs (1)
4-4: Clear labeling of the request schema.
Marking this as "// Request" ensures developers can quickly differentiate between the request and response schemas.packages/apis/src/services/core/accounts/users/login/post/use-post-login.ts (2)
1-8: Use of type-only imports is commendable.
This approach clarifies how the imports are being used, promoting cleaner type management and reducing bundling overhead.
21-21: Adoption of hook naming convention.
RenamingUsePostLogintousePostLoginaligns with the conventional React hook naming pattern.packages/apis/src/services/core/accounts/users/login/post/post-login.ts (2)
1-1: Good use of type-only imports forApiResponse:
Switching to type-only imports helps reduce import overhead and clarifies thatApiResponseis used strictly in a type context. This is a nice step toward cleaner and more optimized code.
6-6: Consistent type imports for request/response interfaces:
Usingimport typehere clarifies usage in a purely type-checking context, which is in line with best practices for TypeScript 4.5+ and helps avoid potential dead code.packages/apis/src/services/core/accounts/users/login/post/post-login.schema.ts (1)
1-1: Alignment with standard response schema:
ImportingapiResponseSchemashows good alignment with a standardized response structure. Just confirm that the rest of the codebase consistently uses and extends this shared schema to preserve unified validation rules.packages/apis/src/types/api.types.ts (2)
1-1: Structured approach to schema-driven typing:
AddingapiResponseSchemaimport keeps your error and response types aligned with zod validations. This is a clean approach that fosters consistency. Nice job!
30-30: Enhanced error-type alignment usingapiResponseSchema:
AligningApiErrorwith the validated structure fromapiResponseSchemahelps unify error handling. This ensures that all errors follow the same shape. Keep verifying that any custom error fields are captured in the schema.packages/apis/src/instance/core-api.ts (1)
3-3: Well-organized type imports:
ChangingAxiosErrorto a type import is a minor improvement that clarifies usage. This helps maintain a decluttered import approach and aligns with general TypeScript best practices in version 4.5+.apps/core/app/auth/login/_components/form/loginForm.tsx (4)
4-5: Switching to the updated login schema paths is good.
These new imports reflect the reorganized API module structure and help ensure your code aligns with the updated directory organization.
20-21: Secure default values.
Switching from hardcoded default credentials to empty strings is a best practice, as it prevents accidentally storing or displaying sensitive data.
31-35: Success callback improvement.
The callback appropriately logs the response and sets the auth token from the new nested propertyres.data.data.token. This reflects the updated schema. Make sure your server-side response consistently returnsres.data.data.tokento avoid undefined errors.
39-40: Enhanced error handling.
Using the nullish coalescing operator for fallback messaging helps gracefully handle unexpected error structures. Logging the error object ensures better visibility and debugging.apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx (1)
14-14: Use of type-only import.
This simplifies your compiled output by removing unused imports, thereby optimizing bundle size and clarifying that you’re importing types only.
| import { guestApi } from "#instance/guest-api"; | ||
|
|
||
| export const postLoginURL = () => path.join("/accounts/login/"); | ||
| export const postLoginURL = () => path.join("accounts/users/login/"); |
There was a problem hiding this comment.
💡 Codebase verification
Documentation needs to be updated to reflect the new endpoint path
The endpoint path change from /accounts/login/ to accounts/users/login/ has been properly implemented across all code references. However, the API documentation in docs/apis/authentication_api_documentation.md still references the old path and needs to be updated to maintain consistency.
- Update the endpoint path in
docs/apis/authentication_api_documentation.mdfromPOST /accounts/login/toPOST /accounts/users/login/
🔗 Analysis chain
Endpoint path change verification:
Changing the path from "/accounts/login/" to "accounts/users/login/" may have downstream effects on any code referencing the old endpoint. Please verify that all consumers of the login endpoint have been updated accordingly.
Let me search for references to both the old and new endpoint paths to ensure we have a complete picture.
The endpoint path change appears to be properly implemented across the codebase
The search results show that:
- The old path
/accounts/login/only appears in documentation (docs/apis/authentication_api_documentation.md) - The new path
accounts/users/loginis consistently used in:- Swagger API definition (
packages/apis/turbo/generators/swagger.json) - The endpoint implementation file (
packages/apis/src/services/core/accounts/users/login/post/post-login.ts) - Client-side components that consume the endpoint (
apps/core/app/auth/login/_components/form/loginForm.tsx)
- Swagger API definition (
Let me verify the documentation to ensure it's not actively used code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Ensure references to the old login endpoint are updated across the codebase.
rg --json "/accounts/login/"
Length of output: 913
Script:
#!/bin/bash
# Search for references to both old and new endpoint paths
echo "Searching for old path pattern..."
rg -A 2 "accounts/login"
echo -e "\nSearching for new path pattern..."
rg -A 2 "accounts/users/login"
# Search for any API client configurations or test files
echo -e "\nSearching for potential API client configurations..."
fd -e ts -e js -e json "api|client|config" --exec rg -l "login"
Length of output: 1856
Script:
#!/bin/bash
# Check if the documentation file is generated or hand-written
head -n 5 docs/apis/authentication_api_documentation.md
Length of output: 132
|
|
||
| export const coreApi = axios.create({ | ||
| baseURL: "http://37.152.182.32/", // You can set your base URL here | ||
| baseURL: "https://api.pixelgenius.design", // You can set your base URL here |
There was a problem hiding this comment.
💡 Codebase verification
Environment-specific configuration needed for API base URL
The codebase currently has hardcoded API URLs in multiple locations:
packages/apis/src/instance/core-api.ts:baseURL: "https://api.pixelgenius.design"packages/apis/src/instance/guest-api.ts:baseURL: "https://api.pixelgenius.design"packages/apis/turbo/generators/swagger.json:host: "api.pixelgenius.design"
There's no environment variable configuration for the API URL, which could make it difficult to switch between different environments (development, staging, production). Consider:
- Creating an environment configuration system using
.envfiles - Using environment variables like
VITE_API_URLorNEXT_PUBLIC_API_URL - Updating the axios instances to read from these environment variables
This will make the application more maintainable and easier to deploy across different environments.
🔗 Analysis chain
Check for environment-based baseURL usage:
Switching the baseURL to "https://api.pixelgenius.design" is fine, but ensure that any environment-specific requirements (e.g., staging vs. production) are handled appropriately. Consider using environment variables to avoid repeated changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for references to the old IP-based URL and confirm only the new domain-based URL is used.
rg "http://37\.152\.182\.32"
Length of output: 30
Script:
#!/bin/bash
# Search for any hardcoded URLs or baseURL configurations in the codebase
rg -i "baseURL|api\.pixelgenius\.design" -A 3 -B 3
Length of output: 2238
Script:
#!/bin/bash
# Look for environment variable usage related to API URLs
rg -i "process\.env.*api|env.*url" -A 2 -B 2
Length of output: 46
Script:
#!/bin/bash
# Check for configuration files that might contain environment-specific settings
fd -e config.js -e config.ts -e env -e env.example
Length of output: 174
| Deploy-Preview: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Outdated checkout action version.
Updating from actions/checkout@v2 to @v3 helps ensure future compatibility and includes performance improvements.
Apply this diff to fix the version:
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v3📝 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.
| - uses: actions/checkout@v2 | |
| - uses: actions/checkout@v3 |
🧰 Tools
🪛 actionlint (1.7.4)
13-13: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/apis/src/services/core/accounts/register/post/use-post-register.ts (1)
Inconsistent React Hook naming convention found in related components
While
usePostRegisterfollows the correct React hook naming convention (lowercase), there's an inconsistency in the related componentUsePostRegisterCompletewhich still uses PascalCase naming. This needs to be updated for consistency:
packages/apis/src/services/core/accounts/register/complete/post/use-post-register-complete.ts: RenameUsePostRegisterCompletetousePostRegisterCompleteapps/core/app/auth/signup/otp/_components/signup-otp-form.tsx: Update the import and usage ofUsePostRegisterCompletetousePostRegisterCompleteAlso, there's a typo in the variable name
muutationthat should be fixed tomutationin the signup-otp-form.tsx file.🔗 Analysis chain
Line range hint
21-31: Good rename to reflect React Hook conventions.Renaming the function from
UsePostRegistertousePostRegisterfollows a common React hook naming pattern and improves clarity. The mutation usage is consistent withreact-query.Below is a script to confirm that all code references to the old
UsePostRegistername have been updated:🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for the old function name to ensure no references remain rg "UsePostRegister" -A 5Length of output: 4026
🧹 Nitpick comments (3)
packages/apis/src/services/core/accounts/register/post/post-register.schema.ts (2)
15-27: Consider removing commented code or clarifying its usage.The
fullRegistrationSchemais fully commented out. If it's no longer needed, remove it to reduce clutter. Otherwise, clarify or document its intended future usage.
33-38: Typo in schema name ("Transofrmed" → "Transformed").Check for code references using the misspelled name and correct it.
Apply this diff to correct the spelling:
-export const postRegisterResponseSchemaTransofrmed = apiResponseSchema +export const postRegisterResponseSchemaTransformed = apiResponseSchemaapps/core/app/auth/signup/page.tsx (1)
35-42: Leverage descriptive logs
These logs clearly outline the workflow steps upon success. Make sure to remove or refine generic console statements in production to avoid clutter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (12)
apps/core/app/auth/_components/auth-card.tsx(1 hunks)apps/core/app/auth/layout.tsx(2 hunks)apps/core/app/auth/login/_components/form/loginForm.tsx(4 hunks)apps/core/app/auth/login/page.tsx(2 hunks)apps/core/app/auth/set-password/page.tsx(2 hunks)apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx(2 hunks)apps/core/app/auth/signup/otp/page.tsx(1 hunks)apps/core/app/auth/signup/page.tsx(4 hunks)apps/core/app/auth/template.tsx(1 hunks)packages/apis/src/services/core/accounts/register/post/post-register.schema.ts(1 hunks)packages/apis/src/services/core/accounts/register/post/post-register.ts(2 hunks)packages/apis/src/services/core/accounts/register/post/use-post-register.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/core/app/auth/_components/auth-card.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/core/app/auth/login/page.tsx
- apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx
- apps/core/app/auth/login/_components/form/loginForm.tsx
🔇 Additional comments (15)
apps/core/app/auth/template.tsx (1)
6-6: Good use of read-only prop for children.Defining
childrenas aReadonlytype helps ensure immutability and aligns with strong typing best practices in React.apps/core/app/auth/layout.tsx (2)
3-3: Great transition to Next.js Image component.Switching to the
<Image>component withfillproperly leverages Next.js image optimizations and responsive support. The usage ofclassName="absolute inset-0"ensures correct positioning. Consider using additional props likepriorityfor above-the-fold content or fine-tuning quality settings if needed.Also applies to: 18-22
12-14: Consistent read-only children.Using
Readonly<{ children: React.ReactNode }>keeps props immutable and matches the approach inTemplate, bolstering type safety and consistency.packages/apis/src/services/core/accounts/register/post/post-register.ts (2)
1-1: Imports look fine.The usage of
pathprovides better portability for building URLs, and the overall file structure is clear.
11-11: Endpoint change verification.You've updated the endpoint to
"/accounts/users/". Ensure all references to the old"/api/accounts/register"are modified accordingly.Below is a script to help confirm references to the old endpoint:
✅ Verification successful
No references to old endpoint found, change appears safe
Based on the search results:
- The old endpoint
/api/accounts/registeris not referenced anywhere in the codebase- The new endpoint
/accounts/users/is properly defined inpostRegisterURL- All related files (schemas, types, and components) are using the correct imports and there are no stale references
The endpoint change appears to be properly implemented without any lingering references to the old endpoint. The only other register-related endpoint found is
/accounts/register/complete/which is a separate functionality for registration completion.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for potential references to the old endpoint rg "/api/accounts/register" -A 5Length of output: 32
Script:
#!/bin/bash # Let's try a broader search for any references to register endpoints rg -i "register.*url|/register|accounts/.*register" -g "*.{ts,js,tsx,jsx,json}" -A 3Length of output: 5528
Script:
#!/bin/bash # Search for URL-related imports and usage ast-grep --pattern 'postRegisterURL'Length of output: 287
Script:
#!/bin/bash # Let's check test files specifically for any endpoint references fd -e test.ts -e spec.ts -e test.js -e spec.js --exec rg -l "register|accounts/users"Length of output: 87
apps/core/app/auth/signup/otp/page.tsx (1)
6-11: Parameter rename consistency check.Changing from
otptootpparameter.Below is a script to locate other references to
otpin the signup flow:✅ Verification successful
The
otpparameter change is part of a broader OTP verification flowThe codebase shows that the OTP functionality is still actively used across the authentication flow, particularly in signup verification. The change from
otptoKey findings:
- The OTP verification flow is intact in
apps/core/app/auth/signup/otp/_components/signup-otp-form.tsxwhich handles the actual OTP input- The parameter rename from
otpto- The OTP functionality is used in multiple places:
- Signup verification
- Password reset
- General authentication
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for references to 'otp' in the signup pages rg "otp" -A 5Length of output: 16541
packages/apis/src/services/core/accounts/register/post/post-register.schema.ts (1)
11-11: Renamed field maintains consistency.Renaming
confirmPasswordtoconfirm_passwordis consistent with your style. Ensure matching references across the codebase.apps/core/app/auth/set-password/page.tsx (3)
55-55: Confirm consistent error handling strategy
Using the nullish coalescing operator??is a more precise fallback strategy than||, which triggers for falsy values beyondnullorundefined. Good improvement.
106-106: Styling alignment with design system
Switching totext-muted-foregroundensures a consistent color scheme per new design standards.
114-114: Consistent usage of text-muted-foreground
The updated class name for confirm password mirrors the styling approach for the password field, enhancing uniformity.apps/core/app/auth/signup/page.tsx (5)
10-10: Lowercase naming for hooks
Renaming the function tousePostRegisteraligns well with React's hook naming conventions. Nice catch.
45-50: Consistent error fallback
Using??for handlingerr.response?.data.messageprovides a sensible default, ensuring users see a clear message if none is returned from the server.
113-115: Improved divider styling
Updating the divider's background color tobg-muted-foregroundaligns with the new color palette.
122-122: Background color standardization
Usingbg-backgroundon social login buttons improves visual consistency across the UI.
130-130: Uniform social login button styling
Applying the samebg-backgroundensures LinkedIn login button styling matches Google's, creating a cohesive look.
| {...register("confirm_password")} | ||
| error={errors.password?.message} |
There was a problem hiding this comment.
Fix error reference
Here, you register "confirm_password" but display errors.password?.message. The mismatch can confuse users, as mistakes in the "Confirm Password" field won't show correctly.
Proposed patch:
{...register("confirm_password")}
-error={errors.password?.message}
+error={errors.confirm_password?.message}📝 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.
| {...register("confirm_password")} | |
| error={errors.password?.message} | |
| {...register("confirm_password")} | |
| error={errors.confirm_password?.message} |
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx (1)
72-79: Add loading state handling during auto-submit.The auto-submit functionality should handle the loading state to prevent multiple submissions.
onChange={(value: string) => { setValue("otp", value); if (value.length === 6) { + if (mutation.isPending) return; const data = getValues(); handleSubmitForm(data); } }}apps/core/app/auth/set-password/page.tsx (1)
73-75: Replace hardcoded email with dynamic value.The email address is currently hardcoded which could be confusing for users.
- [email protected] + {username}
♻️ Duplicate comments (1)
apps/core/app/auth/signup/page.tsx (1)
97-98:⚠️ Potential issueFix error reference for confirm password field.
The error message reference is incorrect. You're using
errors.password?.messagefor the confirm password field.{...register("confirm_password")} -error={errors.password?.message} +error={errors.confirm_password?.message}
🧹 Nitpick comments (22)
packages/apis/src/services/core/accounts/users/verify_otp/post/post-verify-otp.ts (1)
11-11: Consider using URL path segments instead of path.join.Using
path.joinfor URL construction might cause issues on Windows systems due to backslash path separators. Consider using URL path segments or a dedicated URL construction utility.Apply this diff to improve URL construction:
-export const postVerifyOtpURL = () => path.join("accounts/users/verify_otp/"); +export const postVerifyOtpURL = () => "accounts/users/verify_otp/";packages/apis/src/services/core/accounts/users/verify_otp/post/use-post-verify-otp.ts (2)
8-8: Empty import statement should be removed.The empty import statement serves no purpose and should be removed for better code clarity.
-import {} from "./post-verify-otp.schema";
22-34: Consider adding retry configuration for better error handling.The mutation could benefit from retry configuration for transient failures, which is common in OTP verification scenarios.
export const usePostVerifyOtp = (props?: UsePostVerifyOtpProps) => { const mutation = useMutation< ApiResponse<PostVerifyOtpResponseTransformed>, ApiError, PostVerifyOtpRequest >({ mutationKey: postVerifyOtpQueryKey(), mutationFn: (data) => postVerifyOtp(data), + retry: (failureCount, error) => { + return failureCount < 3 && error.response?.status !== 400; + }, ...props, }); return mutation; };apps/core/app/auth/signup/otp/page.tsx (1)
106-106: Fix typo in the text.There's a typo in "didnt recieveddqqdqdqdqdqdqdqdqdq" that should be corrected.
-didnt recieveddqqdqdqdqdqdqdqdqdq code yet? +didn't receive code yet?packages/apis/src/services/core/accounts/users/register/post/post-register.schema.ts (2)
36-36: Fix typo in schema name.The response schema name contains a typo: "Transofrmed" should be "Transformed".
-export const postRegisterResponseSchemaTransofrmed = apiResponseSchema +export const postRegisterResponseSchemaTransformed = apiResponseSchema
44-45: Remove unnecessary transform.The transform function doesn't modify the data and can be removed.
}), - }) - .transform((data) => data); + });apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx (2)
45-45: Make success message more specific.The success message could be more specific about the completed action.
-toast.success("Registered successfully, Welcome to Pixel Genius!"); +toast.success("OTP verified successfully, Welcome to Pixel Genius!");
105-107: Improve countdown timer implementation.The countdown timer's initial value is hardcoded and resets on component re-render. Consider making it configurable and persistent.
+const RESEND_TIMEOUT = 120000; // 2 minutes +const [countdownTarget] = useState(() => Date.now() + RESEND_TIMEOUT); <p> didn't receive code yet?{" "} - <Countdown date={Date.now() + 120000} /> + <Countdown date={countdownTarget} /> </p>apps/core/app/auth/signup/page.tsx (2)
33-48: LGTM! Improved error handling with user feedback.The addition of error handling with toast notifications improves the user experience.
Consider simplifying query string construction.
The separate query string construction for username and email could be simplified.
- const usernameQuery = createQueryStringFromObject({ - username: context.username ?? "", - }); - const emailQuery = createQueryStringFromObject({ - email: context.email ?? "", - }); - - router.push(`/auth/signup/otp?${usernameQuery}&${emailQuery}`); + const query = createQueryStringFromObject({ + username: context.username ?? "", + email: context.email ?? "", + }); + router.push(`/auth/signup/otp?${query}`);
158-158: Consider enhancing the loading state.The current loading fallback is very basic. Consider using a loading skeleton that matches the form layout for a better user experience.
- <Suspense fallback={<div>Loading...</div>}> + <Suspense + fallback={ + <AuthCard> + <div className="animate-pulse space-y-4"> + <div className="h-8 bg-muted rounded w-3/4 mx-auto" /> + <div className="space-y-2"> + {[...Array(4)].map((_, i) => ( + <div key={i} className="h-12 bg-muted rounded" /> + ))} + </div> + <div className="h-10 bg-muted rounded" /> + </div> + </AuthCard> + } + >packages/apis/src/services/core/accounts/users/reset-password/post/post-reset-password.types.ts (2)
4-5: Fix swapped comments.The "Request" and "Response" comments are swapped. Line 4 should be "// Request" and line 11 should be "// Response".
Also applies to: 11-12
7-9: Fix typo in type name.The type name contains a typo:
PostResetPasswordRequestTransofrmedshould bePostResetPasswordRequestTransformed.-export type PostResetPasswordRequestTransofrmed = z.infer< +export type PostResetPasswordRequestTransformed = z.infer<packages/apis/src/services/core/accounts/users/reset-password/post/post-reset-password.schema.ts (2)
15-19: Fix typo in schema name.The schema name contains a typo:
postResetPasswordResponseSchemaTransofrmedshould bepostResetPasswordResponseSchemaTransformed.-export const postResetPasswordResponseSchemaTransofrmed = apiResponseSchema +export const postResetPasswordResponseSchemaTransformed = apiResponseSchema
12-12: Remove unnecessary transform functions.The transform functions don't modify the data and can be removed.
}) - .transform((data) => data);Also applies to: 19-19
packages/apis/src/services/core/accounts/users/reset-password/post/post-reset-password.ts (1)
16-16: Optimize response parsing.The response data is being parsed twice:
- First by the
requestHandler(line 22)- Then again manually (line 25)
Remove the redundant parsing.
const payloadParsed = schema.request.parse(props); const URL = postResetPasswordURL(); const response = await requestHandler( () => coreApi.post(URL, payloadParsed), schema.response._def.schema, ); - const dataParsed = schema.response.parse(response.data); - - return { ...response, data: dataParsed }; + return response;Also applies to: 25-27
packages/apis/src/services/core/accounts/users/reset-password/post/use-post-reset-password.ts (1)
8-8: Remove empty import statement.The empty import statement serves no purpose and should be removed.
-import {} from "./post-reset-password.schema";apps/core/app/auth/set-password/page.tsx (3)
30-30: Consider adding error handling for invalid username.The navigation guard could be enhanced to handle edge cases and provide user feedback.
- if (!username) router.replace("/auth/forget-password"); + if (!username) { + toast.error("Invalid or missing username"); + router.replace("/auth/forget-password"); + return null; + }
43-46: Add proper explanation for the biome-ignore comment.The lint rule ignore comment should include a clear explanation of why the exhaustive dependencies warning is being suppressed.
- // biome-ignore lint/correctness/useExhaustiveDependencies: <explanation> + // biome-ignore lint/correctness/useExhaustiveDependencies: setValue is from useForm and doesn't need to be in deps array
124-132: Consider enhancing loading state feedback.The button only shows a loading state, but users might benefit from more comprehensive feedback during the reset process.
<div className="pb-7 w-full"> + {mutation.isPending && ( + <p className="text-sm text-muted-foreground mb-2 text-center"> + Processing your password reset... + </p> + )} <Button isLoading={mutation.isPending} className="w-full text-lg font-bold bg-primary-600 hover:bg-primary-500" variant="secondary" > Reset </Button> </div>packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.schema.ts (2)
9-13: Remove unnecessary transform.The transform operation
transform((data) => data)is redundant as it simply returns the data without any modifications.-export const postForgetPasswordResponseSchemaTransofrmed = z - .object({ - message: z.string(), - }) - .transform((data) => data); +export const postForgetPasswordResponseSchemaTransofrmed = z.object({ + message: z.string(), +});
9-9: Fix typo in schema name.The schema name has a typo: "Transofrmed" should be "Transformed".
-export const postForgetPasswordResponseSchemaTransofrmed = z +export const postForgetPasswordResponseSchemaTransformed = zpackages/apis/src/services/core/accounts/users/forgot-password/post/use-post-forget-password.ts (1)
8-8: Remove empty import statement.The empty import statement is unnecessary and should be removed.
-import {} from "./post-forget-password.schema";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (34)
apps/core/app/(landing)/become-auther/_components/auther-form.tsx(0 hunks)apps/core/app/auth/forget-password/_components/form/forgetPasswordForm.tsx(2 hunks)apps/core/app/auth/layout.tsx(2 hunks)apps/core/app/auth/login/_components/form/loginForm.tsx(1 hunks)apps/core/app/auth/login/page.tsx(1 hunks)apps/core/app/auth/otp/page.tsx(0 hunks)apps/core/app/auth/set-password/page.tsx(3 hunks)apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx(3 hunks)apps/core/app/auth/signup/otp/page.tsx(2 hunks)apps/core/app/auth/signup/page.tsx(4 hunks)apps/core/app/auth/template.tsx(0 hunks)packages/apis/package.json(1 hunks)packages/apis/src/services/core/accounts/register/complete/post/post-register-complete.schema.ts(0 hunks)packages/apis/src/services/core/accounts/register/complete/post/post-register-complete.ts(0 hunks)packages/apis/src/services/core/accounts/register/complete/post/post-register-complete.types.ts(0 hunks)packages/apis/src/services/core/accounts/register/complete/post/use-post-register-complete.ts(0 hunks)packages/apis/src/services/core/accounts/register/post/post-register.schema.ts(0 hunks)packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.schema.ts(1 hunks)packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.ts(2 hunks)packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.types.ts(1 hunks)packages/apis/src/services/core/accounts/users/forgot-password/post/use-post-forget-password.ts(2 hunks)packages/apis/src/services/core/accounts/users/register/post/post-register.schema.ts(1 hunks)packages/apis/src/services/core/accounts/users/register/post/post-register.ts(1 hunks)packages/apis/src/services/core/accounts/users/register/post/post-register.types.ts(1 hunks)packages/apis/src/services/core/accounts/users/register/post/use-post-register.ts(2 hunks)packages/apis/src/services/core/accounts/users/reset-password/post/post-reset-password.schema.ts(1 hunks)packages/apis/src/services/core/accounts/users/reset-password/post/post-reset-password.ts(1 hunks)packages/apis/src/services/core/accounts/users/reset-password/post/post-reset-password.types.ts(1 hunks)packages/apis/src/services/core/accounts/users/reset-password/post/use-post-reset-password.ts(1 hunks)packages/apis/src/services/core/accounts/users/verify_otp/post/post-verify-otp.schema.ts(1 hunks)packages/apis/src/services/core/accounts/users/verify_otp/post/post-verify-otp.ts(1 hunks)packages/apis/src/services/core/accounts/users/verify_otp/post/post-verify-otp.types.ts(1 hunks)packages/apis/src/services/core/accounts/users/verify_otp/post/use-post-verify-otp.ts(1 hunks)packages/apis/src/services/core/forgot-password/post/post-forget-password.schema.ts(0 hunks)
💤 Files with no reviewable changes (9)
- apps/core/app/(landing)/become-auther/_components/auther-form.tsx
- apps/core/app/auth/template.tsx
- apps/core/app/auth/otp/page.tsx
- packages/apis/src/services/core/accounts/register/complete/post/post-register-complete.schema.ts
- packages/apis/src/services/core/accounts/register/complete/post/post-register-complete.ts
- packages/apis/src/services/core/accounts/register/post/post-register.schema.ts
- packages/apis/src/services/core/accounts/register/complete/post/post-register-complete.types.ts
- packages/apis/src/services/core/forgot-password/post/post-forget-password.schema.ts
- packages/apis/src/services/core/accounts/register/complete/post/use-post-register-complete.ts
✅ Files skipped from review due to trivial changes (2)
- packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.types.ts
- packages/apis/src/services/core/accounts/users/register/post/post-register.types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/core/app/auth/layout.tsx
- packages/apis/package.json
- apps/core/app/auth/login/_components/form/loginForm.tsx
- apps/core/app/auth/login/page.tsx
🔇 Additional comments (10)
packages/apis/src/services/core/accounts/users/register/post/use-post-register.ts (1)
1-11: LGTM!The changes follow best practices:
- Using type imports for better tree-shaking
- Following React hook naming convention
- Proper type safety with generics
Also applies to: 21-33
apps/core/app/auth/signup/otp/page.tsx (1)
12-14: Add validation for username format.The username extraction lacks validation, which could lead to invalid states.
const username = Array.isArray(searchParams.username) ? searchParams?.username[0] : searchParams.username; +if (username && username.length < 3) { + redirect("/auth/signup"); +}apps/core/app/auth/signup/page.tsx (2)
12-17: LGTM! Well-organized imports.The imports are logically grouped and utilize shared packages for types and schemas.
119-123: LGTM! Improved theme consistency.Good use of semantic color tokens (
bg-muted-foreground,bg-background) instead of hardcoded colors. This improves theme consistency and maintainability.Also applies to: 129-129, 137-137
packages/apis/src/services/core/accounts/users/register/post/post-register.ts (3)
13-27: Implementation looks good but verify test coverage.The implementation follows good practices with proper typing and validation. However, the removal of
isMock: trueoption might affect existing tests.
1-1: Verify path alias configuration for "#instance/core-api".The import uses a path alias. Ensure the path alias is properly configured in the TypeScript and build configuration.
11-11: Breaking Change: API endpoint has been modified.The registration endpoint has changed from "/api/accounts/register" to "/accounts/users/". This is a breaking change that requires careful coordination.
packages/apis/src/services/core/accounts/users/reset-password/post/use-post-reset-password.ts (1)
22-34: LGTM!The hook implementation is clean and follows React Query best practices.
packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.ts (1)
11-11: LGTM!The changes improve the code by:
- Following RESTful URL conventions with the new path
- Simplifying the request handler implementation
Also applies to: 20-24
apps/core/app/auth/forget-password/_components/form/forgetPasswordForm.tsx (1)
9-11: LGTM!The changes improve the code by:
- Following consistent module structure with updated import paths
- Using
router.replacefor better auth flow navigation- Maintaining proper type safety with type-only imports
Also applies to: 29-29
| export type PostVerifyOtpRequestTransofrmed = z.infer< | ||
| typeof postVerifyOtpSchema.request | ||
| >; |
There was a problem hiding this comment.
Fix typo in type name.
The type name contains a typo: PostVerifyOtpRequestTransofrmed should be PostVerifyOtpRequestTransformed.
Apply this diff to fix the typo:
-export type PostVerifyOtpRequestTransofrmed = z.infer<
+export type PostVerifyOtpRequestTransformed = z.infer<📝 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.
| export type PostVerifyOtpRequestTransofrmed = z.infer< | |
| typeof postVerifyOtpSchema.request | |
| >; | |
| export type PostVerifyOtpRequestTransformed = z.infer< | |
| typeof postVerifyOtpSchema.request | |
| >; |
| export const postVerifyOtpResponseSchemaTransofrmed = apiResponseSchema | ||
| .extend({ | ||
| data: z.object({ | ||
| id: z.number(), | ||
| username: z.string(), | ||
| email: z.string(), | ||
| is_active: z.boolean(), | ||
| is_email_verified: z.boolean(), | ||
| token: z.object({ | ||
| refresh: z.string(), | ||
| access: z.string(), | ||
| }), | ||
| }), | ||
| }) | ||
| .transform((data) => data); |
There was a problem hiding this comment.
Fix typo in schema name.
The schema name contains a typo: postVerifyOtpResponseSchemaTransofrmed should be postVerifyOtpResponseSchemaTransformed.
Apply this diff to fix the typo:
-export const postVerifyOtpResponseSchemaTransofrmed = apiResponseSchema
+export const postVerifyOtpResponseSchemaTransformed = apiResponseSchema📝 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.
| export const postVerifyOtpResponseSchemaTransofrmed = apiResponseSchema | |
| .extend({ | |
| data: z.object({ | |
| id: z.number(), | |
| username: z.string(), | |
| email: z.string(), | |
| is_active: z.boolean(), | |
| is_email_verified: z.boolean(), | |
| token: z.object({ | |
| refresh: z.string(), | |
| access: z.string(), | |
| }), | |
| }), | |
| }) | |
| .transform((data) => data); | |
| export const postVerifyOtpResponseSchemaTransformed = apiResponseSchema | |
| .extend({ | |
| data: z.object({ | |
| id: z.number(), | |
| username: z.string(), | |
| email: z.string(), | |
| is_active: z.boolean(), | |
| is_email_verified: z.boolean(), | |
| token: z.object({ | |
| refresh: z.string(), | |
| access: z.string(), | |
| }), | |
| }), | |
| }) | |
| .transform((data) => data); |
| We've sent the code to{" "} | ||
| <Link href="/auth/signup" className="underline"> | ||
| {email} | ||
| {searchParams.email} | ||
| </Link> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider masking part of the email for security.
Displaying the full email address could expose sensitive information. Consider partially masking it.
-{searchParams.email}
+{typeof searchParams.email === 'string' && searchParams.email.replace(/(.{2})(.*)(@.*)/, '$1***$3')}| .object({ | ||
| username: z.string().min(3, "Username must be at least 3 characters long"), | ||
| email: z.string().email("Invalid email format"), | ||
| password: z.string().min(8, "Password must be at least 8 characters long"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance password validation rules.
The current password validation only checks length. Consider adding more security requirements.
-password: z.string().min(8, "Password must be at least 8 characters long"),
+password: z.string()
+ .min(8, "Password must be at least 8 characters long")
+ .regex(/[A-Z]/, "Password must contain at least one uppercase letter")
+ .regex(/[a-z]/, "Password must contain at least one lowercase letter")
+ .regex(/[0-9]/, "Password must contain at least one number")
+ .regex(/[^A-Za-z0-9]/, "Password must contain at least one special character"),📝 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.
| password: z.string().min(8, "Password must be at least 8 characters long"), | |
| password: z.string() | |
| .min(8, "Password must be at least 8 characters long") | |
| .regex(/[A-Z]/, "Password must contain at least one uppercase letter") | |
| .regex(/[a-z]/, "Password must contain at least one lowercase letter") | |
| .regex(/[0-9]/, "Password must contain at least one number") | |
| .regex(/[^A-Za-z0-9]/, "Password must contain at least one special character"), |
| .object({ | ||
| new_password: z.string(), | ||
| confirm_password: z.string(), | ||
| otp: z.string(), | ||
| username: z.string(), | ||
| }) | ||
| .transform((data) => data); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add password validation and matching check.
The schema should include:
- Password validation rules (length, complexity, etc.)
- Validation to ensure
new_passwordmatchesconfirm_password
export const postResetPasswordRequestSchemaTransformed = z
.object({
- new_password: z.string(),
- confirm_password: z.string(),
+ new_password: z.string()
+ .min(8, "Password must be at least 8 characters")
+ .regex(/[A-Z]/, "Password must contain at least one uppercase letter")
+ .regex(/[a-z]/, "Password must contain at least one lowercase letter")
+ .regex(/[0-9]/, "Password must contain at least one number")
+ .regex(/[^A-Za-z0-9]/, "Password must contain at least one special character"),
+ confirm_password: z.string(),
otp: z.string(),
username: z.string(),
})
+ .refine((data) => data.new_password === data.confirm_password, {
+ message: "Passwords don't match",
+ path: ["confirm_password"],
+ })
.transform((data) => data);📝 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.
| .object({ | |
| new_password: z.string(), | |
| confirm_password: z.string(), | |
| otp: z.string(), | |
| username: z.string(), | |
| }) | |
| .transform((data) => data); | |
| export const postResetPasswordRequestSchemaTransformed = z | |
| .object({ | |
| new_password: z.string() | |
| .min(8, "Password must be at least 8 characters") | |
| .regex(/[A-Z]/, "Password must contain at least one uppercase letter") | |
| .regex(/[a-z]/, "Password must contain at least one lowercase letter") | |
| .regex(/[0-9]/, "Password must contain at least one number") | |
| .regex(/[^A-Za-z0-9]/, "Password must contain at least one special character"), | |
| confirm_password: z.string(), | |
| otp: z.string(), | |
| username: z.string(), | |
| }) | |
| .refine((data) => data.new_password === data.confirm_password, { | |
| message: "Passwords don't match", | |
| path: ["confirm_password"], | |
| }) | |
| .transform((data) => data); |
| PostResetPasswordResponseTransformed, | ||
| } from "./post-reset-password.types"; | ||
|
|
||
| export const postResetPasswordURL = () => path.join("accounts/users/reset-password/"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don't use path.join for URL construction.
Using path.join for URL paths can cause issues on Windows systems where the path separator is \. Use URL concatenation or template literals instead.
-export const postResetPasswordURL = () => path.join("accounts/users/reset-password/");
+export const postResetPasswordURL = () => "accounts/users/reset-password/";📝 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.
| export const postResetPasswordURL = () => path.join("accounts/users/reset-password/"); | |
| export const postResetPasswordURL = () => "accounts/users/reset-password/"; |
| export const postResetPassword = async ( | ||
| props?: PostResetPasswordRequest, | ||
| ): Promise<ApiResponse<PostResetPasswordResponseTransformed>> => { | ||
| const payloadParsed = schema.request.parse(props); | ||
|
|
||
| const URL = postResetPasswordURL(); | ||
|
|
||
| const response = await requestHandler( | ||
| () => coreApi.post(URL, payloadParsed), | ||
| schema.response._def.schema, | ||
| ); | ||
|
|
||
| const dataParsed = schema.response.parse(response.data); | ||
|
|
||
| return { ...response, data: dataParsed }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for schema parsing.
Add try-catch blocks to handle schema parsing errors gracefully.
export const postResetPassword = async (
props?: PostResetPasswordRequest,
): Promise<ApiResponse<PostResetPasswordResponseTransformed>> => {
+ try {
const payloadParsed = schema.request.parse(props);
const URL = postResetPasswordURL();
const response = await requestHandler(
() => coreApi.post(URL, payloadParsed),
schema.response._def.schema,
);
return response;
+ } catch (error) {
+ if (error instanceof z.ZodError) {
+ throw new Error(`Validation error: ${error.errors.map(e => e.message).join(", ")}`);
+ }
+ throw error;
+ }
};Committable suggestion skipped: line range outside the PR's diff.
| ApiResponse<PostForgetPasswordResponseTransformed>, | ||
| ApiError, | ||
| Omit<PostForgetPasswordRequest, "confirmPassword"> | ||
| PostForgetPasswordRequest |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify type compatibility.
The type parameter change from Omit<PostForgetPasswordRequest, "confirmPassword"> to PostForgetPasswordRequest suggests that confirmPassword is now required. Please verify this aligns with the schema definition and form implementation.
🏁 Script executed:
#!/bin/bash
# Description: Check if confirmPassword is used in the schema and form.
# Check schema definition
rg -A 5 "confirmPassword" packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.schema.ts
# Check form implementation
rg -A 5 "confirmPassword" apps/core/app/auth/forget-password/_components/form/forgetPasswordForm.tsxLength of output: 225
🏁 Script executed:
#!/bin/bash
# Verify definition of PostForgetPasswordRequest and check for "confirmPassword" in its context.
# Search for the definition of PostForgetPasswordRequest in the relevant directory
rg -n "interface PostForgetPasswordRequest" -A 10 packages/apis/src/services/core/accounts/users/forgot-password/post/
# Also, search for confirmPassword in the types file for any context
rg -n "confirmPassword" -A 10 packages/apis/src/services/core/accounts/users/forgot-password/post/Length of output: 219
🏁 Script executed:
#!/bin/bash
# Search the entire repository for the definition of PostForgetPasswordRequest and examine its properties.
rg -n "interface PostForgetPasswordRequest" -A 15
# Additionally, search the entire repository to see where "confirmPassword" is mentioned, to locate its usage and intended requirement.
rg -n "confirmPassword" -A 15Length of output: 2609
🏁 Script executed:
#!/bin/bash
# Search the entire repository for the definition of PostForgetPasswordRequest
rg -n "interface PostForgetPasswordRequest" -A 20Length of output: 51
Type Compatibility Issue: The forgot-password endpoint does not expect a confirmPassword field
After verifying with multiple repository searches, there’s no evidence (in either the schema or the form component) that the forgot-password flow uses or validates a confirmPassword field. In contrast, confirmPassword is exclusively referenced in the register schema (for password confirmation). Therefore, changing from Omit<PostForgetPasswordRequest, "confirmPassword"> to PostForgetPasswordRequest makes confirmPassword required where it shouldn’t be required according to the existing forgot-password implementation.
• The forgot-password schema (packages/apis/src/services/core/accounts/users/forgot-password/post/post-forget-password.schema.ts) produces no mention of confirmPassword.
• The forgot-password form (apps/core/app/auth/forget-password/_components/form/forgetPasswordForm.tsx) does not reference or handle a confirmPassword field.
The type change would force the inclusion of a field that isn’t used or expected, and this misalignment needs to be addressed.
mrbadri
force-pushed
the
feat/#111-change-api-auth
branch
from
c65ac19 to
8a9d007
Compare
Summary by CodeRabbit