Feat/#111 change api auth #164
Conversation
…nto feat/#111-change-api-auth
…nto feat/#111-change-api-auth
…genius/pixel-client into feat/#111-change-api-auth
WalkthroughThis pull request introduces several significant updates across the project's codebase, focusing on API structure, authentication flows, and development workflows. The changes include updating import statements to use type-only imports, modifying API endpoints and schemas, introducing a new Vercel preview deployment workflow, and standardizing naming conventions for hooks and functions. The modifications aim to improve type safety, enhance API response handling, and streamline the development and deployment processes. Changes
Sequence DiagramsequenceDiagram
participant Client
participant GithubActions
participant Vercel
Client->>GithubActions: Push to non-main branch
GithubActions->>Vercel: Install Vercel CLI
GithubActions->>Vercel: Pull environment info
GithubActions->>Vercel: Build project
GithubActions->>Vercel: Deploy preview
Vercel-->>Client: Preview deployment URL
Poem
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: 2
🧹 Nitpick comments (11)
.github/workflows/preview.yaml (3)
2-4: Consider adding VERCEL_TOKEN to the env block for consistency.While the token is properly used as a secret, consider defining it in the env block for better maintainability and consistency with other Vercel-related environment variables.
env: VERCEL_ORG_ID: ${{ secrets.VERCEL_ORG_ID }} VERCEL_PROJECT_ID: ${{ secrets.VERCEL_PROJECT_ID }} + VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }}
14-16: Consider caching the Vercel CLI installation.Adding caching for npm packages can speed up the workflow execution.
+ - uses: actions/setup-node@v4 + with: + node-version: 'lts/*' + cache: 'npm' - name: Install Vercel CLI run: npm install --global vercel@latest
20-21: Add deployment comment with preview URL and ensure proper file formatting.
- Consider adding a step to comment on the PR with the preview URL for easier access.
- Add a newline at the end of the file to comply with YAML standards.
- name: Deploy Project Artifacts to Vercel - run: vercel deploy --prebuilt --token=${{ secrets.VERCEL_TOKEN }} + run: | + DEPLOYMENT_URL=$(vercel deploy --prebuilt --token=${{ secrets.VERCEL_TOKEN }}) + echo "DEPLOYMENT_URL=$DEPLOYMENT_URL" >> $GITHUB_ENV + - name: Comment PR + uses: actions/github-script@v7 + with: + script: | + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: `✨ Preview deployed to: ${process.env.DEPLOYMENT_URL}` + })🧰 Tools
🪛 yamllint (1.35.1)
[error] 21-21: no new line character at the end of file
(new-line-at-end-of-file)
packages/apis/src/services/core/accounts/users/login/post/post-login.types.ts (1)
6-6: Typographical fix: “Transofrmed” → “Transformed”.
Although not newly introduced by these changes, consider correcting the spelling to maintain consistency and clarity.-export type PostLoginRequestTransofrmed = z.infer< +export type PostLoginRequestTransformed = z.infer<packages/apis/src/schema/api-response-schema.ts (2)
3-9: Data property allows broad usage.
This union type covers arrays, records, null, and undefined, providing flexibility but minimal type safety. If you require strongly typed data, consider narrowing the type or using generics.
10-16: Consider a structured error approach.
Bothmessageanderrorsupport various null/undefined states, which might complicate consistent usage. Clarifying which field is set during errors vs. successes could improve readability and usage predictability.packages/apis/turbo/generators/templates/schema.hbs (1)
12-16: Resolve minor naming inconsistencies and extra semicolon.
There appears to be a typo (“Transofrmed”) and an extra semicolon. Consider removing the extra semicolon and correcting the name.-export const {{method}}{{pascalCase name}}ResponseSchemaTransofrmed = apiResponseSchema.extend({ +export const {{method}}{{pascalCase name}}ResponseSchemaTransformed = apiResponseSchema.extend({ ... }).transform((data) => data);; ^ remove extra semicolonpackages/apis/src/services/core/accounts/users/login/post/post-login.ts (1)
12-12: Consider replacingpath.joinfor URLsIf
path.joinis meant for constructing URLs rather than file system paths, it may produce incorrect results on certain operating systems. Instead, consider simple string concatenation or a library built for URL manipulation (e.g.,new URL).-export const postLoginURL = () => path.join("accounts/users/login/"); +export const postLoginURL = () => "accounts/users/login/";packages/apis/src/services/core/accounts/users/login/post/post-login.schema.ts (1)
11-23: Typographical discrepancy in schema nameThe property
export const postLoginResponseSchemaTransofrmedmisspells "transformed" as "transofrmed". Consider correcting to avoid confusion.-export const postLoginResponseSchemaTransofrmed = apiResponseSchema +export const postLoginResponseSchemaTransformed = apiResponseSchemaapps/core/app/auth/login/_components/form/loginForm.tsx (1)
39-40: Error object logging
Logging the entire error object may reveal sensitive data. Consider logging only the pertinent information or removing this console statement in production environments.apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx (1)
49-49: String literal usage
Changing from template literal to a simple string for the route is a small improvement in readability. It's now clearer that the string is static.
📜 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)
- apps/core/app/auth/login/page.tsx
- packages/apis/src/instance/guest-api.ts
- 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 (22)
packages/apis/package.json (2)
17-17: LGTM! Good practice adding TypeScript types.Adding
@types/js-cookieas a dev dependency is a good practice for TypeScript projects, and the version^3.0.6correctly matches the major version of thejs-cookiedependency.
35-35: Verify the version change implications.Changing from fixed version
3.0.5to^3.0.5allows minor version updates. Since this package is used for authentication, let's verify the version change is secure.Run the following script to check for security advisories and changelog:
✅ Verification successful
Version change to
^3.0.5for js-cookie is safeBased on the verification results:
- No security vulnerabilities reported for js-cookie package
- Latest version in 3.0.x range is 3.0.5 (released on 2023-04-24)
- All 3.0.x releases were made within a short timeframe (April 2023), suggesting stability in the minor version range
The caret (
^) will only allow updates to patch versions within 3.0.x range, and since 3.0.5 is currently the latest patch version with no reported vulnerabilities, this change is secure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for security advisories and changelog for js-cookie # Check npm for latest versions and their publish dates curl -s https://registry.npmjs.org/js-cookie | jq '.time | del(.created, .modified)' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "js-cookie") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 1748
.github/workflows/preview.yaml (1)
5-8: LGTM! Appropriate trigger configuration for preview deployments.The workflow correctly triggers on pushes to all branches except main, which is the ideal setup for preview deployments.
packages/apis/src/instance/core-api.ts (2)
3-3: Use of type-only import is a nice optimization.
This change clarifies thatAxiosErroris only used as a type, potentially improving compile-time efficiency and code clarity in TypeScript ≥3.8.
10-10: Ensure consistent usage of the new base URL across the codebase.
While updating the base URL to a secure domain is beneficial, verify that there are no remaining references to the deprecated address in the codebase, especially in other modules or environment variables.✅ Verification successful
Base URL is consistently used across the codebase
The base URL
https://api.pixelgenius.designis consistently used in all relevant locations:
packages/apis/src/instance/core-api.tspackages/apis/src/instance/guest-api.tspackages/apis/turbo/generators/swagger.jsonNo references to any deprecated addresses were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Searching for any leftover references to the old site address rg -A 3 "37\.152\.182\.32"Length of output: 28
Script:
#!/bin/bash # Search for any URLs in the codebase to ensure consistency rg -i "http[s]?://[^\s)\"']+" --type ts --type js # Also search for potential environment variables or configuration files fd -e env -e config -e json -e yaml -e yml | xargs catLength of output: 73957
Script:
#!/bin/bash # Search for any references to pixelgenius.design in the codebase rg -i "pixelgenius\.design"Length of output: 346
packages/apis/src/services/core/accounts/users/login/post/post-login.types.ts (3)
1-2: Use of type-only imports is clean and efficient.
This change aligns with best practices to reduce import overhead and improves clarity by signifying that these imports are strictly for types.
4-4: Correct comment context for PostLoginRequest.
Switching from “Response” to “Request” accurately reflects that this type describes request data.
11-11: Correct comment context for PostLoginResponse.
Switching from “Request” to “Response” accurately denotes that this type describes response data.packages/apis/src/schema/api-response-schema.ts (1)
17-27: Optional or nullable meta structure.
Allowing bothmeta: nulland.optional()can result in multiple ways to represent a missing meta. Verify that your calling code consistently handles these cases.packages/apis/turbo/generators/templates/schema.hbs (2)
1-1: Proper import usage of apiResponseSchema.
The reference toapiResponseSchemaensures consistency with the global response format.
4-4: Clarity in labeling Request vs. Response.
Update confirms that this schema definition correctly represents the request payload.packages/apis/src/services/core/accounts/users/login/post/use-post-login.ts (2)
1-1: Type-only imports for clarity.
Using type imports clearly communicates that these symbols are used only for type-checking, improving maintainability.Also applies to: 8-8
21-21: Appropriate hook naming convention.
Renaming the hook tousePostLoginfollows React best practices for hook naming.packages/apis/src/services/core/accounts/users/login/post/post-login.ts (2)
1-1: Good usage of type-only importUsing
type { ApiResponse }for type-only imports is efficient, reducing any bundler overhead when referencing TypeScript types.
6-6: Consistent with best practices for type-only importsThis aligns with TypeScript guidelines on type import separation, clarifying that these references are for type declarations only.
packages/apis/src/services/core/accounts/users/login/post/post-login.schema.ts (1)
1-1: Validate your server response carefullyThe usage of
apiResponseSchemais a good approach to standardizing response schemas. Verify that the backend consistently returns the structured fields defined byapiResponseSchema.✅ Verification successful
apiResponseSchemais consistently used with proper validationBased on the search results, I can verify that
apiResponseSchemais being used consistently and properly:
The schema is well-defined in
packages/apis/src/schema/api-response-schema.tswith:
- A standardized structure for
datafield that can handle arrays, records, null, or undefined- A
messagefield that accepts string, null, or undefined- An
errorfieldThe implementation in
post-login.schema.tscorrectly:
- Imports and extends the base
apiResponseSchema- Defines specific data structure for login response with proper field validations
- Uses Zod's type-safe schema validation
The schema is also used in:
- API type definitions (
api.types.ts) for error handling- Generator templates ensuring consistent usage across new endpoints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Attempt to find references to 'apiResponseSchema' usage across the repo # and ensure consistent patterns with server responses rg -A 8 "apiResponseSchema"Length of output: 4987
packages/apis/src/types/api.types.ts (1)
1-1: Check correctness ofApiErrorshapeUsing
z.input<typeof apiResponseSchema>guards the error shape with your Zod schema. Ensure your server actually returns the fields enforced byapiResponseSchema; otherwise, the type might mismatch reality during runtime.Also applies to: 8-8, 30-30
✅ Verification successful
Let me check the schema definition to verify the error shape.
API error shape is correctly enforced by the schema
The
apiResponseSchemainpackages/apis/src/schema/api-response-schema.tsproperly defines the error shape with:
datafield as array, record, null or undefinedmessagefield as string, null or undefinederrorfield as record or arrayThe
ApiErrortype correctly uses this schema throughAxiosError<z.input<typeof apiResponseSchema>>, and all API endpoints consistently extend this base schema (as seen in templates and implementations likepostLoginResponseSchemaTransofrmed).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure errors are shaped according to the Zod schema rg -A 5 "ApiError"Length of output: 20328
Script:
#!/bin/bash # Find the schema definition rg -A 10 "apiResponseSchema"Length of output: 5746
packages/apis/turbo/generators/templates/hook.useMutation.hbs (1)
22-22: Proper React hook naming conventionRenaming from
Use{{pascalCase method}}{{pascalCase name}}touse{{pascalCase method}}{{pascalCase name}}follows React’s recommended hook naming convention. Great improvement for consistency.apps/core/app/auth/login/_components/form/loginForm.tsx (3)
4-5: Ensure type consistency and correct imports
The shift to a specific type import on line 5 is good practice, ensuring no unneeded runtime import overhead. Verify that thepostLoginSchema(line 4) and its request property match the structure of yourPostLoginRequestso that the form validation remains consistent.
20-21: Security improvement by removing hardcoded credentials
Changing the default credentials to empty strings is a good step for security, preventing accidental exposure of sensitive information.
31-31: Appropriate naming convention
RenamingUsePostLogintousePostLoginaligns with widely accepted hook naming conventions in React. Nice work!apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx (1)
14-14: Use of type-only import
Switching to a type-only import helps reduce bundle size and clarifies that no runtime values are being imported. This is a beneficial micro-optimization.
| Deploy-Preview: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 |
There was a problem hiding this comment.
Update actions/checkout to the latest version.
The current version (v2) is outdated. Update to v4 for improved security and performance.
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v4📝 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@v4 |
🧰 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)
| console.log(res); | ||
| toast.success("Logged in successfully"); | ||
| setAuthTokens(res.data); | ||
| setAuthTokens(res.data.data.token); |
There was a problem hiding this comment.
Potential exposure of sensitive data in logs
Logging the entire response object (which may contain auth tokens) could risk leakage in logs. Consider removing or sanitizing console.log to avoid storing tokens in logs.
- console.log(res);
toast.success("Logged in successfully");
- setAuthTokens(res.data.data.token);
+ // console.log(res); // Remove or sanitize sensitive data
setAuthTokens(res.data.data.token);Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Security
Chores