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(api): Fetch total count of environments, secrets and variables in project #434

Conversation

unamdev0
Copy link
Contributor

@unamdev0 unamdev0 commented Sep 15, 2024

User description

Description

Sending total Count of env,variable and secrets when fetching project data for a workspace

Fixes #308

Screenshots of response

Screenshot 2024-09-14 at 12 27 27 PM

Further improvement

  • [] Need to update postman
    Sample response
   "items": [
       {
           "id": "c6d92085-0e85-4986-a1d3-7923583efa22",
           "name": "Project 1",
           "slug": "project-1-oa2j9",
           "description": "Dummy project 1",
           "createdAt": "2024-09-13T15:45:42.014Z",
           "updatedAt": "2024-09-13T15:45:42.014Z",
           "storePrivateKey": true,
           "isDisabled": false,
           "accessLevel": "PRIVATE",
           "isForked": false,
           "lastUpdatedById": "cm0v65bm00000130sigtvry7y",
           "workspaceId": "3b41add3-4e14-4866-a6e8-0669d1d7b08b",
           "forkedFromId": null,
           "totalEnvironmentsOfProject": 3,
           "totalVariablesOfProject": 1,
           "totalSecretsOfProject": 1
       },
       {
           "id": "45dae533-8556-4621-b841-43502fe85752",
           "name": "Project 2",
           "slug": "project-2-o9rl4",
           "description": "Dummy project 1",
           "createdAt": "2024-09-13T16:25:40.877Z",
           "updatedAt": "2024-09-13T16:25:40.877Z",
           "storePrivateKey": true,
           "isDisabled": false,
           "accessLevel": "PRIVATE",
           "isForked": false,
           "lastUpdatedById": "cm0v65bm00000130sigtvry7y",
           "workspaceId": "3b41add3-4e14-4866-a6e8-0669d1d7b08b",
           "forkedFromId": null,
           "totalEnvironmentsOfProject": 3,
           "totalVariablesOfProject": 0,
           "totalSecretsOfProject": 0
       },
       {
           "id": "5392bcb8-dcd7-41d3-9c78-1adfa0818f1b",
           "name": "Project 3",
           "slug": "project-3-b8i47",
           "description": "Dummy project 1",
           "createdAt": "2024-09-13T16:27:25.038Z",
           "updatedAt": "2024-09-13T16:27:25.038Z",
           "storePrivateKey": true,
           "isDisabled": false,
           "accessLevel": "PRIVATE",
           "isForked": false,
           "lastUpdatedById": "cm0v65bm00000130sigtvry7y",
           "workspaceId": "3b41add3-4e14-4866-a6e8-0669d1d7b08b",
           "forkedFromId": null,
           "totalEnvironmentsOfProject": 1,
           "totalVariablesOfProject": 0,
           "totalSecretsOfProject": 0
       }
   ],
   "metadata": {
       "page": 0,
       "perPage": 10,
       "pageCount": 1,
       "totalCount": 3,
       "links": {
           "self": "/project/all/my-first-workspace-rbqs2?page=0&limit=10&sort=name&order=asc&search=project 1",
           "first": "/project/all/my-first-workspace-rbqs2?page=0&limit=10&sort=name&order=asc&search=project 1",
           "previous": null,
           "next": null,
           "last": "/project/all/my-first-workspace-rbqs2?page=0&limit=10&sort=name&order=asc&search=project 1"
       }
   }
}

PR Type

Tests, Enhancement


Description

  • Enhanced the project fetching functionality to include total counts of environments, variables, and secrets for each project.
  • Implemented permission checks to ensure only authorized data is counted.
  • Added end-to-end tests to verify the correct fetching of environments, variables, and secrets for projects within a workspace.

Changes walkthrough 📝

Relevant files
Tests
project.e2e.spec.ts
Add tests for fetching environments, variables, and secrets in
projects

apps/api/src/project/project.e2e.spec.ts

  • Added a new project for testing environment, secret, and variable
    fetching.
  • Implemented a test to verify fetching of all environments, variables,
    and secrets for projects in a workspace.
  • Added setup for environments, secrets, and variables in the test.
  • +91/-1   
    Enhancement
    project.service.ts
    Enhance project fetching with total counts of environments, variables,
    and secrets

    apps/api/src/project/service/project.service.ts

  • Modified project fetching to include total counts of environments,
    variables, and secrets.
  • Implemented logic to calculate these totals with permission checks.
  • Refactored project fetching logic to accommodate new data.
  • +54/-1   

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

    Performance Concern
    The new implementation fetches all environments, secrets, and variables for each project, which could be inefficient for projects with many items. Consider using aggregation queries or optimizing the database calls.

    Error Handling
    The new code doesn't include error handling for database queries. Consider adding try-catch blocks or error handling mechanisms to manage potential failures gracefully.

    @unamdev0 unamdev0 force-pushed the feature/total-count-of-env-var-secret-in-project branch from 6559d45 to c694d0c Compare September 15, 2024 09:48
    Copy link
    Contributor

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

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Performance
    Optimize database queries by fetching all required counts in a single query

    Consider using a single database query to fetch all required counts instead of
    multiple separate queries for each environment.

    apps/api/src/project/service/project.service.ts [818-863]

    -const allEnvs = await this.prisma.environment.findMany({
    -  where: { projectId: project.id }
    +const projectCounts = await this.prisma.environment.findMany({
    +  where: { projectId: project.id },
    +  select: {
    +    id: true,
    +    slug: true,
    +    _count: {
    +      select: {
    +        secrets: {
    +          where: { projectId: project.id }
    +        },
    +        variables: {
    +          where: { projectId: project.id }
    +        }
    +      }
    +    }
    +  }
     })
     
    -const envPromises = allEnvs.map(async (env) => {
    +for (const env of projectCounts) {
       const hasRequiredPermission =
         await this.authorityCheckerService.checkAuthorityOverEnvironment({
           userId: user.id,
           entity: { slug: env.slug },
           authorities: [Authority.READ_ENVIRONMENT],
           prisma: this.prisma
         })
     
       if (hasRequiredPermission) {
         totalEnvironmentsOfProject += 1
    +    totalSecretsOfProject += env._count.secrets
    +    totalVariablesOfProject += env._count.variables
    +  }
    +}
     
    -    const [secretCount, variableCount] = await Promise.all([
    -      this.prisma.secret.count({
    -        where: {
    -          projectId: project.id,
    -          versions: { some: { environmentId: env.id } }
    -        }
    -      }),
    -      this.prisma.variable.count({
    -        where: {
    -          projectId: project.id,
    -          versions: { some: { environmentId: env.id } }
    -        }
    -      })
    -    ])
    -
    -    totalSecretsOfProject += secretCount
    -    totalVariablesOfProject += variableCount
    -  }
    -})
    -
    -await Promise.all(envPromises)
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion offers a substantial performance improvement by reducing the number of database queries, which can lead to faster execution and reduced load on the database.

    9
    ✅ Optimize performance by parallelizing the fetching of environment, secret, and variable counts

    Consider using Promise.all() to parallelize the fetching of environment, secret, and
    variable counts for better performance.

    apps/api/src/project/service/project.service.ts [822-863]

     const envPromises = allEnvs.map(async (env) => {
       const hasRequiredPermission =
         await this.authorityCheckerService.checkAuthorityOverEnvironment({
           userId: user.id,
           entity: { slug: env.slug },
           authorities: [Authority.READ_ENVIRONMENT],
           prisma: this.prisma
         })
     
       if (hasRequiredPermission) {
         totalEnvironmentsOfProject += 1
    -
    -    const [secretCount, variableCount] = await Promise.all([
    +    return Promise.all([
           this.prisma.secret.count({
             where: {
               projectId: project.id,
               versions: { some: { environmentId: env.id } }
             }
           }),
           this.prisma.variable.count({
             where: {
               projectId: project.id,
               versions: { some: { environmentId: env.id } }
             }
           })
         ])
    -
    -    totalSecretsOfProject += secretCount
    -    totalVariablesOfProject += variableCount
       }
    +  return [0, 0]
     })
     
    -await Promise.all(envPromises)
    +const counts = await Promise.all(envPromises)
    +totalSecretsOfProject = counts.reduce((sum, [secretCount]) => sum + secretCount, 0)
    +totalVariablesOfProject = counts.reduce((sum, [, variableCount]) => sum + variableCount, 0)
     

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies an opportunity to optimize performance by parallelizing asynchronous operations, which can significantly reduce execution time in scenarios with multiple environments.

    8
    Enhancement
    ✅ Enhance test assertions to verify both count and content of fetched project data

    Consider adding assertions to verify the actual content of the fetched items, not
    just their counts.

    apps/api/src/project/project.e2e.spec.ts [604-610]

     expect(response.statusCode).toBe(200)
     expect(response.json().items.length).toEqual(1)
    -//One environment is added by default
    -expect(response.json().items[0].totalEnvironmentsOfProject).toEqual(2)
     
    -expect(response.json().items[0].totalVariablesOfProject).toEqual(2)
    -expect(response.json().items[0].totalSecretsOfProject).toEqual(2)
    +const project = response.json().items[0]
    +expect(project.totalEnvironmentsOfProject).toEqual(2)
    +expect(project.totalVariablesOfProject).toEqual(2)
    +expect(project.totalSecretsOfProject).toEqual(2)
     
    +// Verify project details
    +expect(project.name).toEqual('Project4')
    +expect(project.description).toEqual('Project for testing if all environments,secrets and keys are being fetched or not')
    +
    +// Verify that sensitive data is not included
    +expect(project).not.toHaveProperty('privateKey')
    +expect(project).not.toHaveProperty('publicKey')
    +

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    Why: This suggestion enhances the robustness of the test by verifying the content of the fetched data, ensuring that the test checks more than just the counts, which improves test coverage.

    7
    Best practice
    ✅ Improve test case naming for better clarity and understanding

    Consider using a more descriptive name for the test case to clearly indicate what is
    being tested.

    apps/api/src/project/project.e2e.spec.ts [530]

    -it('should be able fetch all environments,variables,secrets of all projects of a workspace', async () => {
    +it('should fetch correct counts of environments, variables, and secrets for projects in a workspace', async () => {
     

    [Suggestion has been applied]

    Suggestion importance[1-10]: 5

    Why: The suggestion improves code readability and maintainability by making the test case name more descriptive, though it does not address a critical issue.

    5

    @unamdev0 unamdev0 changed the title fetching total count of env,secret and variables when fetching projects of a workspace Feature(api): fetching total count of env,secret and variables when fetching projects of a workspace Sep 15, 2024
    @rajdip-b rajdip-b changed the title Feature(api): fetching total count of env,secret and variables when fetching projects of a workspace feat(api): Fetch total count of environments, secrets and variables in project Sep 15, 2024
    Copy link

    codecov bot commented Sep 15, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 87.93%. Comparing base (ce50743) to head (d32f207).
    Report is 150 commits behind head on develop.

    Additional details and impacted files
    @@             Coverage Diff             @@
    ##           develop     #434      +/-   ##
    ===========================================
    - Coverage    91.71%   87.93%   -3.78%     
    ===========================================
      Files          111      105       -6     
      Lines         2510     2719     +209     
      Branches       469      412      -57     
    ===========================================
    + Hits          2302     2391      +89     
    - Misses         208      328     +120     
    Flag Coverage Δ
    api-e2e-tests 87.93% <100.00%> (-3.78%) ⬇️

    Flags with carried forward coverage won't be shown. Click here to find out more.

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @rajdip-b
    Copy link
    Member

    Hold up a min, I'm making some changes.

    @rajdip-b
    Copy link
    Member

    I have updated the method by which projects are fetched. Doesn't directly affect your changes. Please make the required changes now.

    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! Nice work!

    apps/api/src/project/service/project.service.ts Outdated Show resolved Hide resolved
    @unamdev0 unamdev0 force-pushed the feature/total-count-of-env-var-secret-in-project branch from 690911e to 92567c5 Compare September 16, 2024 13:32
    @rajdip-b rajdip-b merged commit 0c9e50a into keyshade-xyz:develop Sep 16, 2024
    4 checks passed
    @unamdev0 unamdev0 deleted the feature/total-count-of-env-var-secret-in-project branch September 16, 2024 13:44
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    …n project (keyshade-xyz#434)
    
    Co-authored-by: codiumai-pr-agent-free[bot] <138128286+codiumai-pr-agent-free[bot]@users.noreply.github.com>
    Co-authored-by: rajdip-b <agentR47@gmail.com>
    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.

    API: Send Total numbers of Environments, Variables, and Secrets
    2 participants