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): Replace projectId with name and slug in workspace-role response. #432

Merged
merged 7 commits into from
Sep 17, 2024

Conversation

Nil2000
Copy link
Contributor

@Nil2000 Nil2000 commented Sep 15, 2024

User description

Description

Resolved according to the requirement and also added fixed test file as well

Fixes #429

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

Bug fix, Tests


Description

  • Enhanced the workspace-role service to include slug and name fields in the project data response.
  • Updated end-to-end tests to verify the presence of slug and name fields in the project data.
  • These changes ensure that the API response now provides more detailed project information, improving data accessibility.

Changes walkthrough 📝

Relevant files
Enhancement
workspace-role.service.ts
Enhance project data retrieval in workspace-role service 

apps/api/src/workspace-role/service/workspace-role.service.ts

  • Added slug and name fields to project selection in workspace-role
    service.
  • Updated include statements to fetch additional project details.
  • +25/-3   
    Tests
    workspace-role.e2e.spec.ts
    Update workspace-role tests for additional project fields

    apps/api/src/workspace-role/workspace-role.e2e.spec.ts

  • Updated tests to check for slug and name fields in project data.
  • Ensured test cases reflect changes in workspace-role response
    structure.
  • +12/-4   

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

    Code Duplication
    The project selection logic is repeated in three different places. Consider extracting this into a reusable function to improve maintainability.

    Copy link
    Contributor

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

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Type safety
    Use a more specific type for the query result instead of type casting

    Consider using a more specific type for the workspaceRole variable instead of
    casting it to WorkspaceRoleWithProjects.

    apps/api/src/workspace-role/service/workspace-role.service.ts [453-470]

    -const workspaceRole = (await this.prisma.workspaceRole.findUnique({
    +const workspaceRole = await this.prisma.workspaceRole.findUnique({
       where: {
         slug: workspaceRoleSlug
       },
       include: {
         projects: {
           select: {
             projectId: true,
             project: {
               select: {
                 slug: true,
                 name: true
               }
             }
           }
         }
       }
    -})) as WorkspaceRoleWithProjects
    +})
     
    +if (!workspaceRole) {
    +  // Handle the case when workspaceRole is null
    +  throw new NotFoundException(`Workspace role with slug ${workspaceRoleSlug} not found`)
    +}
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion enhances type safety by avoiding type casting, which can prevent potential runtime errors and improve code reliability.

    8
    Maintainability
    Extract repeated project selection logic into a reusable constant or function

    Consider using a constant or a reusable function for the project selection object to
    avoid code duplication and improve maintainability.

    apps/api/src/workspace-role/service/workspace-role.service.ts [98-107]

     projects: {
    -  select: {
    -    projectId: true,
    -    project: {
    -      select: {
    -        slug: true,
    -        name: true
    -      }
    -    }
    -  }
    +  select: PROJECT_SELECTION
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion improves maintainability by reducing code duplication, making it easier to manage changes to the project selection logic in one place.

    7
    Test improvement
    Create a helper function for project object comparison in tests

    Consider using a custom matcher or a helper function to compare the project objects
    in the test assertions, improving readability and reducing duplication.

    apps/api/src/workspace-role/workspace-role.e2e.spec.ts [604-620]

     expect(response.json()).toEqual({
       ...adminRole1,
       createdAt: expect.any(String),
       updatedAt: expect.any(String),
    -  projects: expect.arrayContaining([
    -    {
    -      projectId: projects[0].id,
    -      slug: projects[0].slug,
    -      name: projects[0].name
    -    },
    -    {
    -      projectId: projects[1].id,
    -      slug: projects[1].slug,
    -      name: projects[1].name
    -    }
    -  ])
    +  projects: expect.arrayContaining(
    +    projects.map(project => expectProjectMatch(project))
    +  )
     })
     
    +// Helper function (define outside the test)
    +function expectProjectMatch(project) {
    +  return expect.objectContaining({
    +    projectId: project.id,
    +    slug: project.slug,
    +    name: project.name
    +  });
    +}
    +
    Suggestion importance[1-10]: 6

    Why: This suggestion improves test readability and reduces duplication by using a helper function for project object comparison, making the tests easier to maintain.

    6

    @rajdip-b rajdip-b changed the title fix(API): Replace projectId with name and slug in workspace-role response. fix(api): Replace projectId with name and slug in workspace-role response. Sep 15, 2024
    @rajdip-b
    Copy link
    Member

    @Nil2000 could you please self assign the issue?

    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.

    For all the select blocks, unselect the projectId field and instead, select project.select.id. That way, things look more organized.

    @rajdip-b
    Copy link
    Member

    @Nil2000 the pr looks good. Would you be open to making the changes to api-client package in this PR? Since the changes are directly related.

    @Nil2000
    Copy link
    Contributor Author

    Nil2000 commented Sep 15, 2024

    Yeah sure, but I am not aware where I need to make the change.
    May be in the following file

    1. In workspace-role.types.d.ts interface WorkspaceRole

    Is that it? Or are there more changes left? 🤔

    @rajdip-b
    Copy link
    Member

    These are the places. You can run the tests using pnpm test:api-client

    @rajdip-b rajdip-b changed the title fix(api): Replace projectId with name and slug in workspace-role response. feat(api): Replace projectId with name and slug in workspace-role response. Sep 17, 2024
    @rajdip-b rajdip-b merged commit af06071 into keyshade-xyz:develop Sep 17, 2024
    7 checks passed
    @Nil2000 Nil2000 deleted the nilabhra-issue-429 branch September 20, 2024 07:00
    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.

    API: Replace projectId with name and slug in workspace-role response.
    2 participants