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(cli): Implemented pagination support #453

Merged
merged 8 commits into from
Sep 21, 2024

Conversation

Nil2000
Copy link
Contributor

@Nil2000 Nil2000 commented Sep 20, 2024

User description

Description

Implemented pagination support

Fixes #442

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, Dependencies


Description

  • Implemented pagination support for both environment and workspace listing commands in the CLI.
  • Added new command options for pagination, sorting, and searching.
  • Updated the package dependencies to include @keyshade/api-client.
  • Updated the pnpm-lock.yaml to reflect changes in dependencies and TypeScript version.

Changes walkthrough 📝

Relevant files
Enhancement
list.environment.ts
Add pagination support to environment listing command       

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

  • Added pagination options to the command.
  • Updated the action method to handle new options.
  • Enhanced environment fetching with pagination parameters.
  • +35/-4   
    list.workspace.ts
    Add pagination support to workspace listing command           

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

  • Added pagination options to the command.
  • Updated the action method to handle new options.
  • Enhanced workspace fetching with pagination parameters.
  • +42/-2   
    Dependencies
    package.json
    Update package dependencies for CLI                                           

    apps/cli/package.json

    • Added @keyshade/api-client as a workspace dependency.
    +2/-1     
    pnpm-lock.yaml
    Update pnpm lock file with new dependencies                           

    pnpm-lock.yaml

  • Updated TypeScript version in lock file.
  • Added @keyshade/api-client to lock file.
  • +13/-143

    💡 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

    Error Handling
    The action method doesn't handle potential errors from the API call. Consider adding error handling and user feedback for failed requests.

    Error Handling
    The action method doesn't handle potential errors from the API call. Consider adding error handling and user feedback for failed requests.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Sep 20, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Validate pagination input parameters before making API calls

    Consider adding input validation for the pagination options to ensure they are
    within acceptable ranges before passing them to the API call.

    apps/cli/src/commands/environment/list.environment.ts [60-64]

     const { page, limit, order, sort, search } = options
     if (!projectSlug) {
       Logger.error('Project slug is required')
       return
     }
    +if (page && (isNaN(Number(page)) || Number(page) < 1)) {
    +  Logger.error('Invalid page number')
    +  return
    +}
    +if (limit && (isNaN(Number(limit)) || Number(limit) < 1)) {
    +  Logger.error('Invalid limit')
    +  return
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding input validation is crucial for preventing potential errors and ensuring that the API receives valid data, which enhances the robustness of the application.

    9
    Error handling
    ✅ Add error handling for API call failures to improve user feedback
    Suggestion Impact:The commit added error handling for failed API calls by logging an error message when fetching workspaces fails.

    code diff:

         } else {
           Logger.error(`Failed fetching workspaces: ${error.message}`)

    Consider adding error handling for the case when the API call fails, to provide more
    informative feedback to the user.

    apps/cli/src/commands/workspace/list.workspace.ts [51-63]

     const { success, data, error } =
       await ControllerInstance.getInstance().workspaceController.getWorkspacesOfUser(
         {
           page,
           limit,
           order,
           sort,
           search
         },
         this.headers
       )
     
     if (success) {
    +  // Existing success handling
    +} else {
    +  Logger.error('Failed to fetch workspaces:', error?.message || 'Unknown error')
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Implementing error handling provides users with informative feedback when API calls fail, improving the user experience and aiding in debugging.

    8
    Enhancement
    ✅ Use object destructuring for function parameters to improve code clarity
    Suggestion Impact:The suggestion to use object destructuring was indirectly implemented by spreading the options object in the function call, which simplifies the code and achieves a similar goal of improving readability.

    code diff:

       async action({ args, options }: CommandActionData): Promise<void> {
         const [projectSlug] = args
    -    const { page, limit, order, sort, search } = options
    +
         if (!projectSlug) {
           Logger.error('Project slug is required')
           return
    @@ -75,7 +50,7 @@
           data: environments,
           error
         } = await environmentController.getAllEnvironmentsOfProject(
    -      { projectSlug, page, limit, order, sort, search },
    +      { projectSlug, ...options },

    Consider using object destructuring for the options parameter in the action method
    to improve code readability and maintainability.

    apps/cli/src/commands/environment/list.environment.ts [58-60]

    -async action({ args, options }: CommandActionData): Promise<void> {
    +async action({ args, options: { page, limit, order, sort, search } }: CommandActionData): Promise<void> {
       const [projectSlug] = args
    -  const { page, limit, order, sort, search } = options
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves code readability and maintainability by using object destructuring, which is a common practice in modern JavaScript and TypeScript.

    7
    Best practice
    Add type annotations to destructured options for improved type safety

    Consider adding type annotations for the destructured options to ensure type safety
    and improve code maintainability.

    apps/cli/src/commands/workspace/list.workspace.ts [49]

    -const { page, limit, order, sort, search } = options
    +const { page, limit, order, sort, search }: {
    +  page?: number;
    +  limit?: number;
    +  order?: 'ASC' | 'DESC';
    +  sort?: string;
    +  search?: string;
    +} = options
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Adding type annotations enhances type safety and maintainability, ensuring that the code adheres to TypeScript's type-checking capabilities.

    6

    💡 Need additional feedback ? start a PR chat

    @Nil2000
    Copy link
    Contributor Author

    Nil2000 commented Sep 20, 2024

    @rajdip-b Can you help what part has created this build errors?

    Edit: Solved

    apps/cli/package.json Outdated Show resolved Hide resolved
    apps/cli/src/commands/environment/list.environment.ts Outdated Show resolved Hide resolved
    apps/cli/src/commands/environment/list.environment.ts Outdated Show resolved Hide resolved
    apps/cli/src/commands/environment/list.environment.ts Outdated Show resolved Hide resolved
    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.

    I'm holding this up till i get my issue merged.

    apps/cli/src/util/pagination-options.ts Outdated Show resolved Hide resolved
    apps/cli/src/util/pagination-options.ts Outdated Show resolved Hide resolved
    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.

    LGTM

    @rajdip-b
    Copy link
    Member

    I'll merge #451 by tonight.

    @Nil2000
    Copy link
    Contributor Author

    Nil2000 commented Sep 21, 2024

    I'll merge #451 by tonight.

    Ok thanks 🙏

    @rajdip-b
    Copy link
    Member

    @Nil2000 hey bro, can you now get the remaining stuff fixed?

    @Nil2000
    Copy link
    Contributor Author

    Nil2000 commented Sep 21, 2024

    @Nil2000 hey bro, can you now get the remaining stuff fixed?

    Did not get you. Can you be specific about what were you mentioning?

    @rajdip-b rajdip-b merged commit feb1806 into keyshade-xyz:develop Sep 21, 2024
    4 checks passed
    @Nil2000 Nil2000 deleted the d-442 branch September 21, 2024 14:03
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    rajdip-b pushed a commit that referenced this pull request Oct 24, 2024
    ## [2.6.0](v2.5.0...v2.6.0) (2024-10-24)
    
    ### 🚀 Features
    
    * **api:**  Add icon and remove description field from workspace ([#435](#435)) ([a99c0db](a99c0db))
    * **api-client:** Added workspace-membership and related tests ([#452](#452)) ([6a1c091](6a1c091))
    * **api-client:** Create controller for User module ([#484](#484)) ([f9d8e83](f9d8e83))
    * **api:** Add prod env schema in env file ([#436](#436)) ([21c3004](21c3004))
    * **api:** Add resend otp implementation ([#445](#445)) ([4dc6aa1](4dc6aa1))
    * **api:** Fetch total count of environments, [secure]s and variables in project ([#434](#434)) ([0c9e50a](0c9e50a))
    * **api:** Replace `projectId` with `name` and `slug` in workspace-role response.  ([#432](#432)) ([af06071](af06071))
    * **cli:** Add functionality to operate on Secrets ([#504](#504)) ([1b4bf2f](1b4bf2f))
    * **cli:** Add project command ([#451](#451)) ([70448e1](70448e1))
    * **cli:** Add workspace operations ([#441](#441)) ([ed38d22](ed38d22))
    * **cli:** implement commands to get, list, update, and delete, workspace roles ([#469](#469)) ([957ea8d](957ea8d))
    * **cli:** Implemented pagination support ([#453](#453)) ([feb1806](feb1806))
    * **cli:** Secret scan ([#438](#438)) ([85cb8ab](85cb8ab))
    * **cli:** Update environment command outputs ([f4af874](f4af874))
    * **platform:** Clearing email field after waitlisting the user email ([#481](#481)) ([256d659](256d659))
    * Remove project IDs from workspace role export data and update tests ([#448](#448)) ([8fdb328](8fdb328))
    * **web:** Configured extra check for waitlisted users already in the list and created toast message for them ([#492](#492)) ([2ddd0ef](2ddd0ef))
    * **web:** show the toast only when email add successfully ([#490](#490)) ([783c411](783c411))
    
    ### 🐛 Bug Fixes
    
    * **api,api-client:** Add environmentSlug in multiple places across the variable module ([#468](#468)) ([d970aff](d970aff))
    * **api:** Replace the id with slug in the global-search service ([#455](#455)) ([74804b1](74804b1))
    * **platform:** Fixed duplicate Google Logo UI fix  ([#450](#450)) ([fb0d6f7](fb0d6f7))
    * resolve footer website name cut-off or overlap issue ([#444](#444)) ([fe03ba2](fe03ba2))
    * **web:** Horizontal Scrolling issue on the website ([#440](#440)) ([655177b](655177b))
    
    ### 📚 Documentation
    
    * Add documentation for environment in CLI ([#462](#462)) ([dad7394](dad7394))
    * Add documentation for project in CLI ([#466](#466)) ([341fb32](341fb32))
    * Add documentation for scan in CLI ([#461](#461)) ([72281e6](72281e6))
    * Add documentation for workspace command ([#464](#464)) ([4aad8a2](4aad8a2))
    * Add instructions for resetting the local Prisma database ([#495](#495)) ([#501](#501)) ([b07ea17](b07ea17))
    * Added docker support documentation ([#465](#465)) ([bc04be4](bc04be4))
    * Added documentation for running the platform ([#473](#473)) ([8b8386b](8b8386b))
    * Added missing mappings to pages ([5de9fd8](5de9fd8))
    * Fix Documentation Hyperlink and update expired Discord invite link ([#496](#496)) ([5a10e39](5a10e39))
    * Updated CLI docs ([#460](#460)) ([c7e0f13](c7e0f13))
    
    ### 🔧 Miscellaneous Chores
    
    * Add more logging to Sentry init ([#470](#470)) ([de4925d](de4925d))
    * **api:** Optimise API docker image size ([#360](#360)) ([ea40dc1](ea40dc1))
    * **api:** Updated lockfile ([a968e78](a968e78))
    * **CI:** Add [secure] scan validation ([f441262](f441262))
    * **cli:** Update controller invocation in environment commands ([#477](#477)) ([596bd1a](596bd1a))
    * Minor changes to variables ([fe01ca6](fe01ca6))
    * **[secure]-scan:** Failing lint issues ([#507](#507)) ([48f45df](48f45df))
    * **[secure]-scan:** Formatted files ([5884833](5884833))
    * Update .env.example ([70ad4f7](70ad4f7))
    * Updated scripts ([9eb76a7](9eb76a7))
    * **web:** email validation ([#487](#487)) ([e8e737a](e8e737a))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.6.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    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.

    CLI: Implement pagination support
    2 participants