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

refactor(cli): Refactored profile commands into readable blocks #331

Merged
merged 1 commit into from
Jul 7, 2024

Conversation

rajdip-b
Copy link
Member

@rajdip-b rajdip-b commented Jul 7, 2024

PR Type

Enhancement, Bug fix


Description

  • Refactored profile-related commands to improve readability and maintainability.
  • Added utility functions for profile operations: checkProfileExists, checkIsDefaultProfile, and getDefaultProfile.
  • Improved error handling and code structure in profile commands.
  • Renamed methods for better clarity and consistency.

Changes walkthrough 📝

Relevant files
Enhancement
base.command.ts
Refactor profile default retrieval in BaseCommand               

apps/cli/src/commands/base.command.ts

  • Added getDefaultProfile import.
  • Replaced direct access to profiles.default with
    getDefaultProfile(profiles).
  • +2/-1     
    create.profile.ts
    Refactor profile creation logic and method names                 

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

  • Refactored profile creation logic into parseInput method.
  • Renamed setProfileData to setProfileConfigData.
  • Moved profile creation steps into action method.
  • +25/-16 
    delete.profile.ts
    Refactor profile deletion with utility functions                 

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

  • Added checkProfileExists and checkIsDefaultProfile utility functions.
  • Refactored profile deletion logic to use new utility functions.
  • Moved confirmation logic to a separate method makeConfirmation.
  • +12/-22 
    list.profile.ts
    Refactor profile listing and default profile retrieval     

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

  • Added getDefaultProfile import.
  • Refactored profile listing to use printProfile method.
  • Moved profile data into a class property.
  • +26/-14 
    update.profile.ts
    Refactor profile update with utility functions                     

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

  • Added checkProfileExists and checkIsDefaultProfile utility functions.
  • Refactored profile update logic to use new utility functions.
  • Moved profile update steps into updateProfileData method.
  • +14/-9   
    use.profile.ts
    Refactor profile usage with utility function                         

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

  • Added checkProfileExists utility function.
  • Refactored profile usage logic to use new utility function.
  • +2/-4     
    profile.ts
    Add profile utility functions                                                       

    apps/cli/src/util/profile.ts

  • Added utility functions: checkProfileExists, checkIsDefaultProfile,
    and getDefaultProfile.
  • +25/-0   

    💡 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: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Possible Bug:
    The deletion of the 'default' key in profile configurations should be handled carefully to ensure that it does not affect other operations unexpectedly. This is seen in the list.profile.ts where delete this.profiles.default is used.

    Refactoring Concern:
    In create.profile.ts, the method setProfileData was renamed to setProfileConfigData, but the documentation and method calls in other parts of the application should be checked to ensure they are updated accordingly.

    Error Handling:
    The new utility functions like checkProfileExists throw an error directly when a profile is not found if the spinner s is not provided. This could lead to unhandled exceptions if not properly caught wherever these functions are used.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add error handling for the fetchProfileConfig call

    Consider adding error handling for the fetchProfileConfig call to manage potential
    failures gracefully.

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

    -this.profiles = await fetchProfileConfig()
    +try {
    +  this.profiles = await fetchProfileConfig()
    +} catch (error) {
    +  outro('Failed to fetch profile configuration')
    +  return
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding error handling for fetchProfileConfig is crucial as it deals with external configuration loading, which can fail and should be managed gracefully.

    8
    Use the delete operator to remove the profile property entirely

    Instead of setting this.profiles[profile] to undefined, use the delete operator to remove
    the property entirely.

    apps/cli/src/commands/profile/delete.profile.ts [39]

    -this.profiles[profile] = undefined
    +delete this.profiles[profile]
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion to use the delete operator instead of setting to undefined is correct and aligns with best practices for property removal in JavaScript objects.

    6
    Possible issue
    Add a null check for defaultProfileName to prevent potential runtime errors

    Instead of directly accessing profiles[defaultProfileName], consider adding a null check
    to ensure defaultProfileName is not undefined or null, which would prevent potential
    runtime errors.

    apps/cli/src/commands/base.command.ts [147-148]

     const defaultProfileName = getDefaultProfile(profiles)
    -const defaultProfile = profiles[defaultProfileName]
    +const defaultProfile = defaultProfileName ? profiles[defaultProfileName] : undefined
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a null check for defaultProfileName before accessing profiles[defaultProfileName] is a good practice to avoid runtime errors if defaultProfileName is undefined.

    7
    Add a null check before deleting the default property from this.profiles

    Consider adding a check to ensure this.profiles is not null or undefined before attempting
    to delete the default property.

    apps/cli/src/commands/profile/list.profile.ts [38]

    -delete this.profiles.default
    +if (this.profiles) {
    +  delete this.profiles.default
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Ensuring this.profiles is not null or undefined before attempting to delete the default property prevents potential runtime errors.

    7

    @rajdip-b rajdip-b merged commit b29ac91 into develop Jul 7, 2024
    5 checks passed
    @rajdip-b rajdip-b deleted the refactor/profile-command branch July 7, 2024 13:56
    rajdip-b pushed a commit that referenced this pull request Jul 11, 2024
    ## [2.2.0](v2.1.0...v2.2.0) (2024-07-11)
    
    ### 🚀 Features
    
    * **api-client:** Added API Client package ([#346](#346)) ([6734e1e](6734e1e))
    * **api:** Updated API key ([fbac312](fbac312))
    * **platform:** View [secure]s ([#313](#313)) ([97c4541](97c4541))
    * **web:** Add Pricing Page ([#243](#243)) ([2c7f1d6](2c7f1d6))
    
    ### 📚 Documentation
    
    * **cli:** Added docs for the CLI package ([#329](#329)) ([edad166](edad166))
    * **cli:** Added usage docs ([#330](#330)) ([b6963d5](b6963d5))
    * Update Discord link ([871b6cd](871b6cd))
    * Update README.md ([e66fcd2](e66fcd2))
    * **web:** Add documentation about our web package ([#268](#268)) ([3d848e7](3d848e7))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Updated response types in environment service ([b8a3ddd](b8a3ddd))
    * **ci:** Added release scripts for platform and api ([02dae60](02dae60))
    * **CI:** Updated action plugin versions ([88bb317](88bb317))
    * **CI:** Updated pnpm version in CI file ([2692e88](2692e88))
    * **platform:** Fixed env parsing in platform ([d6ffafa](d6ffafa))
    * **web:** Update Terms and Conditions and Privacy Policy ([#282](#282)) ([d621dcb](d621dcb))
    
    ### 🔨 Code Refactoring
    
    * **api:** Update [secure] and variable fetching endpoints ([7d9acd0](7d9acd0))
    * **cli:** Refactored profile commands into readable blocks ([#331](#331)) ([4a8a089](4a8a089))
    * **cli:** Updated configuration commands and mechanism ([#310](#310)) ([9079b6d](9079b6d))
    rajdip-b pushed a commit that referenced this pull request Jul 11, 2024
    ## [2.2.0](v2.1.0...v2.2.0) (2024-07-11)
    
    ### 🚀 Features
    
    * **api-client:** Added API Client package ([#346](#346)) ([6734e1e](6734e1e))
    * **api:** Updated API key ([fbac312](fbac312))
    * **platform:** View [secure]s ([#313](#313)) ([97c4541](97c4541))
    * **web:** Add Pricing Page ([#243](#243)) ([2c7f1d6](2c7f1d6))
    
    ### 📚 Documentation
    
    * **cli:** Added docs for the CLI package ([#329](#329)) ([edad166](edad166))
    * **cli:** Added usage docs ([#330](#330)) ([b6963d5](b6963d5))
    * Update Discord link ([871b6cd](871b6cd))
    * Update README.md ([e66fcd2](e66fcd2))
    * **web:** Add documentation about our web package ([#268](#268)) ([3d848e7](3d848e7))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Updated response types in environment service ([b8a3ddd](b8a3ddd))
    * **ci:** Added release scripts for platform and api ([02dae60](02dae60))
    * **CI:** Updated action plugin versions ([88bb317](88bb317))
    * **CI:** Updated pnpm version in CI file ([2692e88](2692e88))
    * **platform:** Fixed env parsing in platform ([d6ffafa](d6ffafa))
    * **web:** Update Terms and Conditions and Privacy Policy ([#282](#282)) ([d621dcb](d621dcb))
    
    ### 🔨 Code Refactoring
    
    * **api:** Update [secure] and variable fetching endpoints ([7d9acd0](7d9acd0))
    * **cli:** Refactored profile commands into readable blocks ([#331](#331)) ([4a8a089](4a8a089))
    * **cli:** Updated configuration commands and mechanism ([#310](#310)) ([9079b6d](9079b6d))
    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.

    1 participant