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): Changed objects to classes #306

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

vr-varad
Copy link
Contributor

@vr-varad vr-varad commented Jul 1, 2024

User description

Description

The following files contained objects of the controller call implementation and I have converted the objects to classes made the functions static, and exported the instance of the class.

Fixes #299

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in the 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


Description

  • Converted AuthController, SecretController, and VariableController objects to classes.
  • Made the main methods in each controller (checkApiKeyValidity, fetchSecrets, fetchVariables) static.
  • Exported instances of the newly created classes.

Changes walkthrough 📝

Relevant files
Enhancement
auth.ts
Refactor AuthController to use class with static methods 

apps/cli/src/http/auth.ts

  • Converted AuthController object to AuthControllerClass class.
  • Made checkApiKeyValidity a static method.
  • Exported an instance of AuthControllerClass.
  • +18/-16 
    secret.ts
    Refactor SecretController to use class with static methods

    apps/cli/src/http/secret.ts

  • Converted SecretController object to SecretControllerClass class.
  • Made fetchSecrets a static method.
  • Exported an instance of SecretControllerClass.
  • +4/-2     
    variable.ts
    Refactor VariableController to use class with static methods

    apps/cli/src/http/variable.ts

  • Converted VariableController object to VariableControllerClass class.
  • Made fetchVariables a static method.
  • Exported an instance of VariableControllerClass.
  • +4/-2     

    💡 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 [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The conversion of objects to classes and making methods static could potentially alter the behavior if the methods were previously relying on instance-specific data. Ensure that these methods are indeed suitable to be static and do not require any instance-specific state.
    Refactoring Scope:
    Ensure that all parts of the application that were using these controllers have been updated to use the new class instances instead of the old object references.

    Copy link
    Contributor

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

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add error handling for the fetch operation to improve robustness

    Implement error handling for the fetch call to manage scenarios where the network request
    fails or returns an unexpected status.

    apps/cli/src/http/auth.ts [6-10]

    -const response = await fetch(`${baseUrl}/api/api-key/access/live-updates`, {
    -  headers: {
    -    'x-keyshade-token': apiKey
    +let response;
    +try {
    +  response = await fetch(`${baseUrl}/api/api-key/access/live-updates`, {
    +    headers: {
    +      'x-keyshade-token': apiKey
    +    }
    +  });
    +  if (!response.ok) {
    +    throw new Error(`API key is not valid for ${baseUrl}. Please check the key and try again.`);
       }
    -})
    +} catch (error) {
    +  Logger.error('Failed to fetch API key validity:', error);
    +  throw error;
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: Implementing error handling for the fetch call is crucial for managing scenarios where the network request fails or returns an unexpected status, thereby improving the robustness and reliability of the code.

    10
    Add validation for input parameters to prevent unnecessary API calls

    To ensure that the API key and base URL are not empty before making the request, add
    validation checks at the beginning of the checkApiKeyValidity method.

    apps/cli/src/http/auth.ts [4]

     static async checkApiKeyValidity(baseUrl: string, apiKey: string): Promise<void> {
    +  if (!baseUrl || !apiKey) {
    +    throw new Error('Base URL and API key must be provided.');
    +  }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding validation checks for baseUrl and apiKey ensures that the method does not proceed with invalid inputs, which is a best practice to avoid unnecessary API calls and potential errors.

    9
    Enhancement
    Improve error message clarity by including dynamic data

    Consider using a more descriptive error message that includes the baseUrl to help with
    debugging. This can be particularly useful in environments where multiple base URLs might
    be used.

    apps/cli/src/http/auth.ts [13-15]

     throw new Error(
    -  'API key is not valid. Please check the key and try again.'
    +  `API key is not valid for ${baseUrl}. Please check the key and try again.`
     )
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Including the baseUrl in the error message can significantly aid in debugging, especially in environments with multiple base URLs. This is a useful enhancement for clarity.

    8

    @vr-varad vr-varad changed the title Changed objects to classes fix(cli): changed objects to classes Jul 1, 2024
    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    • You don't need to change the name to include Class
    • You can get rid of the object initialization since static methods can be accessed without instantiation

    @rajdip-b rajdip-b added the foss hack Clustering all the curated issues for Foss Hack 2024 label Jul 1, 2024
    @vr-varad
    Copy link
    Contributor Author

    vr-varad commented Jul 2, 2024

    @rajdip-b Done.

    @rajdip-b rajdip-b changed the title fix(cli): changed objects to classes patch(cli): Changed objects to classes Jul 2, 2024
    @rajdip-b rajdip-b changed the title patch(cli): Changed objects to classes chore(cli): Changed objects to classes Jul 2, 2024
    @rajdip-b rajdip-b merged commit a495914 into keyshade-xyz:develop Jul 29, 2024
    4 checks passed
    rajdip-b pushed a commit that referenced this pull request Jul 29, 2024
    Co-authored-by: vr-varad <varadgupta21#gmail.com>
    rajdip-b pushed a commit that referenced this pull request Jul 29, 2024
    ## [2.3.0](v2.2.0...v2.3.0) (2024-07-29)
    
    ### 🚀 Features
    
    * **api:** Add pagination metadata to Environment module ([#382](#382)) ([9baa344](9baa344))
    * **api:** Add pagination metadata to Event module ([#394](#394)) ([60010b4](60010b4))
    * **api:** Add pagination metadata to Integration module ([#391](#391)) ([0372e36](0372e36))
    * **api:** Add pagination metadata to Project module ([#393](#393)) ([bc274fd](bc274fd))
    * **api:** Add pagination metadata to Secret module ([#389](#389)) ([c4cc667](c4cc667))
    * **api:** Add pagination metadata to Variable module ([#390](#390)) ([be6aabf](be6aabf))
    * **api:** Add pagination metadata to Workspace module  ([#387](#387)) ([a08c924](a08c924))
    * **api:** Add pagination metadata to Workspace Role module ([#388](#388)) ([d8e8f49](d8e8f49))
    * **api:** Create a paginate method ([#379](#379)) ([09576f1](09576f1))
    * **api:** Create endpoint for fetching all revisions of a [secure] ([#303](#303)) ([de2b602](de2b602))
    * **api:** Create endpoint for fetching all revisions of a variable ([#304](#304)) ([9abddc1](9abddc1))
    * **cli:** Improved the DX for list profile ([#334](#334)) ([6bff496](6bff496))
    * **platform:** Add warning sonner toast for invalid otp ([#335](#335)) ([21513f5](21513f5))
    
    ### 🐛 Bug Fixes
    
    * **cli:** Added parent directory check ([#359](#359)) ([538ea7f](538ea7f))
    * **platform:** Platform types fixes ([#374](#374)) ([8e9d9ff](8e9d9ff))
    
    ### 📚 Documentation
    
    * Added docker details in setting-things-up.md ([#358](#358)) ([ed5093a](ed5093a))
    * Update postman workspace link ([d6aba27](d6aba27))
    * Updated env and cli docs ([1213d2a](1213d2a))
    
    ### 🔧 Miscellaneous Chores
    
    * Added next backend url in .env.example ([5695254](5695254))
    * **api-client:** Added pagination structure ([a70e957](a70e957))
    * **api-client:** Fixed test script ([ad70819](ad70819))
    * **api-client:** Removed try-catch from tests in environment ([a64e48c](a64e48c))
    * **api:** Add user cache for optimization ([#386](#386)) ([8d730b5](8d730b5))
    * **api:** Alter cache rehydration interval ([f5f9eec](f5f9eec))
    * **api:** Fixed naming error in variable controller ([0c5a380](0c5a380))
    * **api:** Improve handling of edge cases for paginate module ([#402](#402)) ([8591487](8591487))
    * **api:** Minor updates to user service ([249d778](249d778))
    * **api:** Skip workspace creation when user is admin ([#376](#376)) ([13f6c59](13f6c59))
    * **ci:** Add docker check   ([#383](#383)) ([3119001](3119001))
    * **ci:** Add names to CI files ([1a7e5f6](1a7e5f6))
    * **ci:** Add validate CLI pipeline ([#373](#373)) ([a91df6c](a91df6c))
    * **ci:** Adding validate pipeline ([#372](#372)) ([23cf3b3](23cf3b3))
    * **ci:** Disabled platform and api deployments ([74d601a](74d601a))
    * **ci:** Fixed deployment scripts ([12e35db](12e35db))
    * **ci:** Fixed platform script ([d783f2a](d783f2a))
    * **CI:** Include migration deployment in API deploy pipeline ([dbd5222](dbd5222))
    * **CI:** Separated deployment and docker build jobs ([090e193](090e193))
    * **CI:** Setup inter-job dependency ([1756727](1756727))
    * **ci:** Update auto-assign.yaml ([#375](#375)) ([91e0ec1](91e0ec1))
    * **cli:** Changed objects to classes ([#306](#306)) ([c83f2db](c83f2db))
    * Removed Minio config ([8feb83a](8feb83a))
    * Updated deployment scripts and added health check in platform ([fcc1c3f](fcc1c3f))
    
    ### 🔨 Code Refactoring
    
    * **api:** Updated path of some endpoints in project controller ([9502678](9502678))
    * **api:** Updated Redis provider ([33491a1](33491a1))
    rajdip-b pushed a commit that referenced this pull request Jul 29, 2024
    ## [2.3.0](v2.2.0...v2.3.0) (2024-07-29)
    
    ### 🚀 Features
    
    * **api:** Add pagination metadata to Environment module ([#382](#382)) ([9baa344](9baa344))
    * **api:** Add pagination metadata to Event module ([#394](#394)) ([60010b4](60010b4))
    * **api:** Add pagination metadata to Integration module ([#391](#391)) ([0372e36](0372e36))
    * **api:** Add pagination metadata to Project module ([#393](#393)) ([bc274fd](bc274fd))
    * **api:** Add pagination metadata to Secret module ([#389](#389)) ([c4cc667](c4cc667))
    * **api:** Add pagination metadata to Variable module ([#390](#390)) ([be6aabf](be6aabf))
    * **api:** Add pagination metadata to Workspace module  ([#387](#387)) ([a08c924](a08c924))
    * **api:** Add pagination metadata to Workspace Role module ([#388](#388)) ([d8e8f49](d8e8f49))
    * **api:** Create a paginate method ([#379](#379)) ([09576f1](09576f1))
    * **api:** Create endpoint for fetching all revisions of a [secure] ([#303](#303)) ([de2b602](de2b602))
    * **api:** Create endpoint for fetching all revisions of a variable ([#304](#304)) ([9abddc1](9abddc1))
    * **cli:** Improved the DX for list profile ([#334](#334)) ([6bff496](6bff496))
    * **platform:** Add warning sonner toast for invalid otp ([#335](#335)) ([21513f5](21513f5))
    
    ### 🐛 Bug Fixes
    
    * **cli:** Added parent directory check ([#359](#359)) ([538ea7f](538ea7f))
    * **platform:** Platform types fixes ([#374](#374)) ([8e9d9ff](8e9d9ff))
    
    ### 📚 Documentation
    
    * Added docker details in setting-things-up.md ([#358](#358)) ([ed5093a](ed5093a))
    * Update postman workspace link ([d6aba27](d6aba27))
    * Updated env and cli docs ([1213d2a](1213d2a))
    
    ### 🔧 Miscellaneous Chores
    
    * Added next backend url in .env.example ([5695254](5695254))
    * **api-client:** Added pagination structure ([a70e957](a70e957))
    * **api-client:** Fixed test script ([ad70819](ad70819))
    * **api-client:** Removed try-catch from tests in environment ([a64e48c](a64e48c))
    * **api:** Add user cache for optimization ([#386](#386)) ([8d730b5](8d730b5))
    * **api:** Alter cache rehydration interval ([f5f9eec](f5f9eec))
    * **api:** Fixed naming error in variable controller ([0c5a380](0c5a380))
    * **api:** Improve handling of edge cases for paginate module ([#402](#402)) ([8591487](8591487))
    * **api:** Minor updates to user service ([249d778](249d778))
    * **api:** Skip workspace creation when user is admin ([#376](#376)) ([13f6c59](13f6c59))
    * **ci:** Add docker check   ([#383](#383)) ([3119001](3119001))
    * **ci:** Add names to CI files ([1a7e5f6](1a7e5f6))
    * **ci:** Add validate CLI pipeline ([#373](#373)) ([a91df6c](a91df6c))
    * **ci:** Adding validate pipeline ([#372](#372)) ([23cf3b3](23cf3b3))
    * **ci:** Disabled platform and api deployments ([74d601a](74d601a))
    * **ci:** Fixed deployment scripts ([12e35db](12e35db))
    * **ci:** Fixed platform script ([d783f2a](d783f2a))
    * **CI:** Include migration deployment in API deploy pipeline ([dbd5222](dbd5222))
    * **CI:** Separated deployment and docker build jobs ([090e193](090e193))
    * **CI:** Setup inter-job dependency ([1756727](1756727))
    * **ci:** Update auto-assign.yaml ([#375](#375)) ([91e0ec1](91e0ec1))
    * **cli:** Changed objects to classes ([#306](#306)) ([c83f2db](c83f2db))
    * Removed Minio config ([8feb83a](8feb83a))
    * Updated deployment scripts and added health check in platform ([fcc1c3f](fcc1c3f))
    
    ### 🔨 Code Refactoring
    
    * **api:** Updated path of some endpoints in project controller ([9502678](9502678))
    * **api:** Updated Redis provider ([33491a1](33491a1))
    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
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    CLI: Convert objects in http folder to classes
    2 participants