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

chore(cli): Add Zod validation to parseInput function #362

Merged

Conversation

sujal-sakpal
Copy link
Contributor

@sujal-sakpal sujal-sakpal commented Jul 17, 2024

User description

Fixes #361

Description:
Hi ,

I've added Zod validation to the profile creation command. The parseInput method now validates user input for name, apiKey, baseUrl, and setDefault fields using predefined schemas.

Changes Made:

Implemented Zod validation schemas for:

name: Must be alphanumeric with no spaces.
apiKey: Should start with ks_ and contain only letters and numbers.
baseUrl: Valid URL or empty.
setDefault: Optional boolean.
Updated parseInput to validate input before processing.

Added a basic implementation of the text function for user input collection.

Testing:

Validated user inputs to ensure correct validation.
Handled cases where inputs may be missing or incorrect.
Please review and let me know if you have any questions or suggestions. Thanks!


PR Type

Enhancement, Tests


Description

  • Added Zod validation schemas to ensure proper input validation for profile creation.
  • Updated the parseInput method to use the new validation schemas.
  • Implemented a basic text function to handle user input collection.

Changes walkthrough 📝

Relevant files
Enhancement
create.profile.ts
Add Zod validation and input parsing enhancements               

apps/cli/src/commands/profile/create.profile.ts

  • Added Zod validation schemas for name, apiKey, baseUrl, and
    setDefault.
  • Updated parseInput method to validate input using the defined schemas.
  • Implemented a basic text function for user input collection.
  • +44/-23 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Validation Logic
    The validation logic for baseUrl allows an empty string, which might not be intended as valid input for a URL. Consider if an empty string should be valid or if it should be handled differently.

    Dummy Implementation
    The text function is currently implemented with a dummy return value. Ensure to replace this with actual logic to collect user input.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Jul 17, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add error handling for input validation to manage exceptions gracefully

    Consider adding error handling for the parse method of the inputSchema. Currently,
    if the validation fails, it will throw an error which is not caught, potentially
    causing the application to crash or behave unexpectedly.

    apps/cli/src/commands/profile/create.profile.ts [112]

    -const parsedData = inputSchema.parse({ name, apiKey, baseUrl, setDefault });
    +let parsedData;
    +try {
    +  parsedData = inputSchema.parse({ name, apiKey, baseUrl, setDefault });
    +} catch (error) {
    +  console.error('Validation error:', error);
    +  throw new Error('Input validation failed');
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: Adding error handling for the input validation is crucial as it prevents the application from crashing due to unhandled exceptions, thus improving the robustness and reliability of the code.

    10
    Possible issue
    Ensure the optional baseUrl is a valid URL if provided

    The baseUrlSchema should ensure that the URL is not just any string but a valid URL
    format. The current implementation allows an empty string or any string when the URL
    is optional. It's better to ensure that if a URL is provided, it must be valid.

    apps/cli/src/commands/profile/create.profile.ts [73]

    -const baseUrlSchema = z.string().url().or(z.string().length(0)).optional();
    +const baseUrlSchema = z.string().url().optional();
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion improves the validation logic by ensuring that if a URL is provided, it must be valid, which enhances data integrity and prevents potential issues with invalid URLs.

    8
    Enhancement
    ✅ Replace the dummy return in the text function with actual input collection logic
    Suggestion Impact:The commit replaced the dummy return in the text function with actual logic to collect user input using the `text` function with a message and placeholder.

    code diff:

    -async function text(options: { message: string, placeholder: string }): Promise<string> {
    -  // Implement your actual input collection logic here
    -  return ''; // Dummy return, replace with actual user input

    The text function currently returns a dummy empty string, which is not practical for
    a real-world application. Replace the dummy return with actual logic to collect user
    input, or if this is placeholder code, ensure to implement it before merging.

    apps/cli/src/commands/profile/create.profile.ts [86]

    -return ''; // Dummy return, replace with actual user input
    +return await prompt(options.message, { default: options.placeholder });
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion addresses a placeholder that needs to be replaced with actual logic, which is important for the functionality of the application. However, it may be less critical if the placeholder is intended for future implementation.

    7
    Expand the allowed characters in nameSchema to include underscores

    The regex for nameSchema allows alphanumeric characters but does not permit
    underscores or other common characters that might be valid in certain contexts.
    Consider expanding the character set if applicable.

    apps/cli/src/commands/profile/create.profile.ts [72]

    -const nameSchema = z.string().regex(/^[a-zA-Z0-9]+$/, 'Name must contain only letters and numbers without spaces.');
    +const nameSchema = z.string().regex(/^[a-zA-Z0-9_]+$/, 'Name must contain only letters, numbers, and underscores without spaces.');
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Allowing underscores in the nameSchema can make the application more flexible and user-friendly, but it is a minor enhancement compared to other suggestions.

    6

    apps/cli/src/commands/profile/create.profile.ts Outdated Show resolved Hide resolved
    apps/cli/src/commands/profile/create.profile.ts Outdated Show resolved Hide resolved
    apps/cli/src/commands/profile/create.profile.ts Outdated Show resolved Hide resolved
    @rajdip-b rajdip-b added the foss hack Clustering all the curated issues for Foss Hack 2024 label Jul 18, 2024
    @rajdip-b
    Copy link
    Member

    rajdip-b commented Aug 2, 2024

    @sujal-sakpal any progress on this?

    @rajdip-b rajdip-b force-pushed the feature/add-zod-validation branch from e40687e to 49ad77f Compare September 1, 2024 09:59
    @rajdip-b rajdip-b changed the title Add Zod validation to parseInput function chore(cli): Add Zod validation to parseInput function Sep 1, 2024
    @rajdip-b rajdip-b force-pushed the feature/add-zod-validation branch 2 times, most recently from a3ca22d to 680df07 Compare September 1, 2024 10:47
    @rajdip-b rajdip-b force-pushed the feature/add-zod-validation branch from 680df07 to 9b188e5 Compare September 4, 2024 15:15
    @rajdip-b rajdip-b merged commit 4a166c7 into keyshade-xyz:develop Sep 4, 2024
    6 checks passed
    rajdip-b added a commit that referenced this pull request Sep 5, 2024
    Co-authored-by: rajdip-b <agentR47@gmail.com>
    rajdip-b pushed a commit that referenced this pull request Sep 5, 2024
    ## [2.4.0](v2.3.0...v2.4.0) (2024-09-05)
    
    ### 🚀 Features
    
    * **api-client:** Create controller for Event module ([#399](#399)) ([122df35](122df35))
    * **api-client:** Create controller for Integration module ([#397](#397)) ([697d38b](697d38b))
    * **api-client:** Create controller for Project module ([#370](#370)) ([fa25866](fa25866))
    * **api-client:** Create controller for Secret module ([#396](#396)) ([7e929c0](7e929c0))
    * **api-client:** Create controller for Variable module ([#395](#395)) ([3e114d9](3e114d9))
    * **api:** Add global search in workspace ([c49962b](c49962b))
    * **api:** Add max page size ([#377](#377)) ([ed18eb0](ed18eb0))
    * **cli:** Add functionality to operate on Environments ([#324](#324)) ([4c6f3f8](4c6f3f8))
    * **cli:** Quit on decryption failure ([#381](#381)) ([1349d15](1349d15))
    
    ### 🐛 Bug Fixes
    
    * **api-client:** Fixed broken export ([096df2c](096df2c))
    * **api:** Add NotFound exception on passing an invalid roleId while inviting user in workspace ([#408](#408)) ([ab441db](ab441db))
    * **cli:** Fixed missing module ([f7a091f](f7a091f))
    * **platform:**  Build failure in platform ([#385](#385)) ([90dcb2c](90dcb2c))
    
    ### 🔧 Miscellaneous Chores
    
    * Add api client build script and updated CI ([da0e27a](da0e27a))
    * **api:** Reorganized import using path alias ([d5befd1](d5befd1))
    * **ci:** Update CLI CI name ([8f4c456](8f4c456))
    * **cli:** Add Zod validation to parseInput function ([#362](#362)) ([34e6c39](34e6c39))
    * Fixed api client tests and rearranged controllers ([1307604](1307604))
    * Housekeeping ([c5f1330](c5f1330))
    * **platform:** Added strict null check ([072254f](072254f))
    * **web:** Added strict null check ([7e12b47](7e12b47))
    
    ### 🔨 Code Refactoring
    
    * **api:** Update logic for forking projects ([#398](#398)) ([4cf3838](4cf3838))
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    ## [2.4.0](keyshade-xyz/keyshade@v2.3.0...v2.4.0) (2024-09-05)
    
    ### 🚀 Features
    
    * **api-client:** Create controller for Event module ([keyshade-xyz#399](keyshade-xyz#399)) ([122df35](keyshade-xyz@122df35))
    * **api-client:** Create controller for Integration module ([keyshade-xyz#397](keyshade-xyz#397)) ([697d38b](keyshade-xyz@697d38b))
    * **api-client:** Create controller for Project module ([keyshade-xyz#370](keyshade-xyz#370)) ([fa25866](keyshade-xyz@fa25866))
    * **api-client:** Create controller for Secret module ([keyshade-xyz#396](keyshade-xyz#396)) ([7e929c0](keyshade-xyz@7e929c0))
    * **api-client:** Create controller for Variable module ([keyshade-xyz#395](keyshade-xyz#395)) ([3e114d9](keyshade-xyz@3e114d9))
    * **api:** Add global search in workspace ([c49962b](keyshade-xyz@c49962b))
    * **api:** Add max page size ([keyshade-xyz#377](keyshade-xyz#377)) ([ed18eb0](keyshade-xyz@ed18eb0))
    * **cli:** Add functionality to operate on Environments ([keyshade-xyz#324](keyshade-xyz#324)) ([4c6f3f8](keyshade-xyz@4c6f3f8))
    * **cli:** Quit on decryption failure ([keyshade-xyz#381](keyshade-xyz#381)) ([1349d15](keyshade-xyz@1349d15))
    
    ### 🐛 Bug Fixes
    
    * **api-client:** Fixed broken export ([096df2c](keyshade-xyz@096df2c))
    * **api:** Add NotFound exception on passing an invalid roleId while inviting user in workspace ([keyshade-xyz#408](keyshade-xyz#408)) ([ab441db](keyshade-xyz@ab441db))
    * **cli:** Fixed missing module ([f7a091f](keyshade-xyz@f7a091f))
    * **platform:**  Build failure in platform ([keyshade-xyz#385](keyshade-xyz#385)) ([90dcb2c](keyshade-xyz@90dcb2c))
    
    ### 🔧 Miscellaneous Chores
    
    * Add api client build script and updated CI ([da0e27a](keyshade-xyz@da0e27a))
    * **api:** Reorganized import using path alias ([d5befd1](keyshade-xyz@d5befd1))
    * **ci:** Update CLI CI name ([8f4c456](keyshade-xyz@8f4c456))
    * **cli:** Add Zod validation to parseInput function ([keyshade-xyz#362](keyshade-xyz#362)) ([34e6c39](keyshade-xyz@34e6c39))
    * Fixed api client tests and rearranged controllers ([1307604](keyshade-xyz@1307604))
    * Housekeeping ([c5f1330](keyshade-xyz@c5f1330))
    * **platform:** Added strict null check ([072254f](keyshade-xyz@072254f))
    * **web:** Added strict null check ([7e12b47](keyshade-xyz@7e12b47))
    
    ### 🔨 Code Refactoring
    
    * **api:** Update logic for forking projects ([keyshade-xyz#398](keyshade-xyz#398)) ([4cf3838](keyshade-xyz@4cf3838))
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    enhancement foss hack Clustering all the curated issues for Foss Hack 2024 Review effort [1-5]: 2 Tests
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    CLI: Parse profile create input using zod
    2 participants