Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(schema, api-client): Migrate user types and schemas to @keyshade/schema #535

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 13, 2024

User description

Description

@keyshade/schema

  • Add schemas and types for user.
  • Add tests for user schemas.
  • Export user schemas from index and its types from index.types

@keyshade/api-client

  • Remove existing user.types.
  • Change imports for user types to @keyshade/schema.

Related to #519

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Enhancement, Tests


Description

  • Migrated user types and schemas from api-client to @keyshade/schema.
  • Updated UserController in api-client to use types from @keyshade/schema.
  • Removed user type definitions from api-client.
  • Added Zod schemas and inferred types for user-related data in @keyshade/schema.
  • Added comprehensive tests for user schemas in @keyshade/schema.

Changes walkthrough 📝

Relevant files
Enhancement
user.ts
Update user controller to use schema package                         

packages/api-client/src/controllers/user.ts

  • Updated import statements to use @keyshade/schema.
  • Removed imports from @api-client/types/user.types.
  • +1/-1     
    user.types.d.ts
    Remove user types from api-client                                               

    packages/api-client/src/types/user.types.d.ts

    • Removed all user-related type definitions.
    +0/-33   
    index.ts
    Export user schemas from schema index                                       

    packages/schema/src/index.ts

    • Added export for user schemas.
    +5/-2     
    index.types.ts
    Export user types from schema index                                           

    packages/schema/src/index.types.ts

    • Added export for user types.
    +3/-0     
    user.ts
    Add user schemas to schema package                                             

    packages/schema/src/user/user.ts

    • Added Zod schemas for user-related requests and responses.
    +42/-0   
    user.types.ts
    Add user types to schema package                                                 

    packages/schema/src/user/user.types.ts

    • Added inferred types from user schemas.
    +38/-0   
    Tests
    user.spec.ts
    Add tests for user schemas                                                             

    packages/schema/tests/user.spec.ts

    • Added tests for user-related schemas.
    +181/-0 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    519 - Fully compliant

    Compliant requirements:

    • Created user folder in schema package
    • Added user.ts with zod schemas
    • Added user.types.ts with inferred types
    • Updated index.ts exports
    • Updated api-client to use schema types
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Schema Validation
    Verify that the email validation in GetSelfResponseSchema and UpdateSelfRequestSchema follows security best practices and handles all edge cases

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Nov 13, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    ✅ Add length validation constraints to string fields to ensure data integrity

    Add minimum and maximum length constraints to the OTP string validation to ensure it
    matches your application's OTP format requirements.

    packages/schema/src/user/user.ts [31-33]

     export const ValidateEmailChangeOTPRequestSchema = z.object({
    -  otp: z.string()
    +  otp: z.string().min(6).max(6)
     })

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    Why: Adding specific length constraints for OTP validation is crucial for security and data integrity, as it ensures the OTP matches the expected format and prevents potential validation bypass.

    8
    Possible issue
    Add URL format validation to ensure valid URL inputs

    Add URL validation for the profilePictureUrl field to ensure it contains a valid URL
    format.

    packages/schema/src/user/user.ts [16-21]

     export const UpdateSelfRequestSchema = z.object({
       name: z.string().optional(),
    -  profilePictureUrl: z.string().optional(),
    +  profilePictureUrl: z.string().url().optional(),
       isOnboardingFinished: z.boolean().optional(),
       email: z.string().email().optional()
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding URL validation for profile picture URLs is essential for preventing invalid data and potential security issues related to malformed URLs.

    8
    Best practice
    Add input validation constraints to prevent invalid data lengths

    Add string length constraints to the name field to prevent extremely long or empty
    names.

    packages/schema/src/user/user.ts [16-21]

     export const UpdateSelfRequestSchema = z.object({
    -  name: z.string().optional(),
    +  name: z.string().min(1).max(100).optional(),
       profilePictureUrl: z.string().optional(),
       isOnboardingFinished: z.boolean().optional(),
       email: z.string().email().optional()
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding length constraints for the name field is important to prevent data issues and potential abuse, ensuring the stored names are reasonable and manageable.

    7

    💡 Need additional feedback ? start a PR chat

    @rajdip-b rajdip-b merged commit c24695e into keyshade-xyz:develop Nov 18, 2024
    5 checks passed
    @muntaxir4 muntaxir4 deleted the user-schema branch November 18, 2024 12:01
    rajdip-b pushed a commit that referenced this pull request Dec 3, 2024
    ## [2.8.0](v2.7.0...v2.8.0) (2024-12-03)
    
    ### 🚀 Features
    
    * **api:** Add workspace removal notification email template ([#476](#476)) ([40b754f](40b754f))
    * **cli:** Store `metrics_enabled` key in profile config ([#536](#536)) ([9283b22](9283b22))
    * **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([#557](#557)) ([126d024](126d024))
    * **platform:** Added screen for CREATE NEW PROJECT ([#540](#540)) ([b644633](b644633))
    * **platform:** Updated the empty state of dashboard ([#522](#522)) ([28739d9](28739d9))
    * **schema, api-client:** Migrate auth types to @keyshade/schema ([#532](#532)) ([d880098](d880098))
    * **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([#546](#546)) ([a3267de](a3267de))
    * **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([#547](#547)) ([08868c3](08868c3))
    * **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([#538](#538)) ([c468af0](c468af0))
    * **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([#539](#539)) ([bc3100b](bc3100b))
    * **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([#535](#535)) ([c24695e](c24695e))
    * **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([#545](#545)) ([0ee8f9a](0ee8f9a))
    * **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([#569](#569)) ([4398969](4398969))
    * **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([#568](#568)) ([9efbf2d](9efbf2d))
    * **schema:** Add User type inference from UserSchema ([#574](#574)) ([84c1db5](84c1db5))
    
    ### 🐛 Bug Fixes
    
    * **api:** Incorrect oauth redirect url ([58d96e5](58d96e5))
    * **platform:** Resolve loading SVG blocking input field interaction ([#571](#571)) ([30f4f65](30f4f65))
    
    ### 📚 Documentation
    
    * Add pictures to Bruno setup ([#541](#541)) ([210c0fd](210c0fd))
    * Migrate to Bruno ([#525](#525)) ([1793d92](1793d92))
    
    ### 🔧 Miscellaneous Chores
    
    * **ci:** Add script to validate schema package ([59e4280](59e4280))
    * Fixed codecov client version ([a998ae4](a998ae4))
    * **package:** Fixed tests and did housekeeping ([#544](#544)) ([40008e3](40008e3))
    * Update test coverage settings ([5b27e32](5b27e32))
    * Update Turbo to 2.3.1 ([#564](#564)) ([3a63823](3a63823))
    * **web:** Update dockerfile ([10d9cc5](10d9cc5))
    
    ### 🔨 Code Refactoring
    
    * **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([#520](#520)) ([7c8ee5d](7c8ee5d))
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants