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.

Sign up for GitHub

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

Jump to bottom

fix(api): Replace the id with slug in the global-search service #455

Merged
merged 5 commits into from
Sep 22, 2024
Merged

fix(api): Replace the id with slug in the global-search service #455

merged 5 commits into from
Sep 22, 2024

Conversation

PriyobrotoKar
Copy link
Contributor

@PriyobrotoKar PriyobrotoKar commented Sep 21, 2024 •
edited by codiumai-pr-agent-free bot
Loading

User description

Description

Updated the response of the global search API with slug instead of id.

Fixes #426

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

Description

  • Updated the global search API to return slug instead of id in the response.
  • Modified multiple query methods in WorkspaceService to select slug instead of id.

Changes walkthrough 📝

Relevant files
Bug fix
workspace.service.ts
Replace `id` with `slug` in global search queries               

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

  • Replaced id with slug in the selection fields of multiple query
    methods.
  • Updated the global search service to use slug instead of id.
    		•  +4/-4     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @codiumai-pr-agent-free CodiumAI PR-Agent (Free)
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    API Change
    The PR changes the API response by replacing 'id' with 'slug'. This might break existing client implementations that rely on the 'id' field.

    @codiumai-pr-agent-free CodiumAI PR-Agent (Free)
    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Sep 21, 2024 •
    edited
    Loading

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    ✅ Extract common search logic into a separate method to reduce code duplication

    Consider extracting the common search logic into a separate method to reduce code
    duplication across similar queries.

    apps/api/src/workspace/service/workspace.service.ts [439-445]

    -{
-  where: {
-    id: { in: projectIds },
-    OR: [
-      { name: { contains: searchTerm, mode: 'insensitive' } },
-      { description: { contains: searchTerm, mode: 'insensitive' } }
-    ]
-  },
-  select: { slug: true, name: true, description: true }
-}
+this.createSearchQuery(projectIds, searchTerm, ['name', 'description'])

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    Why: Extracting common logic into a separate method significantly improves maintainability by reducing code duplication and enhancing readability.

    		8
    Type safety
    Add a type annotation to the query's select clause for improved type safety

    Consider adding a type annotation for the return value of the query to ensure type
    safety and improve code readability.

    apps/api/src/workspace/service/workspace.service.ts [445]

    -select: { slug: true, name: true, description: true }
+select: { slug: true, name: true, description: true } as const
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Adding a type annotation can improve type safety and readability, but the suggestion does not address a critical issue and the improvement is minor.

    		5

    💡 Need additional feedback ? start a PR chat

    rajdip-b
    rajdip-b requested changes Sep 21, 2024
    View reviewed changes
    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.

    Hey! Good job on this! Could you please also make the changes in the API Client package?

    @PriyobrotoKar
    Copy link
    Contributor Author

    Oh ok. Like in that global search test, I have to create some dummy resources (projects, environments, secrets and variables) and then make sure that the response array's length for each matches. Right?

    @rajdip-b
    Copy link
    Member

    All of that work is already done! You just need to update the response types both in the test and types file.

    @rajdip-b
    Copy link
    Member

    @PriyobrotoKar can you please apply this suggestion?
    image

    @PriyobrotoKar
    Copy link
    Contributor Author

    Yes sure! Doing it rn.

    @rajdip-b rajdip-b merged commit 74804b1 into keyshade-xyz:develop Sep 22, 2024
    5 checks passed
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    @PriyobrotoKar @Kiranchaudhary537
    fix(api): Replace the id with slug in the global-search service (keys…
    0cb3998 
    …hade-xyz#455)
    rajdip-b pushed a commit that referenced this pull request Oct 24, 2024
    @semantic-release-bot
    chore(release): 2.6.0 [skip ci]
    58d912f 
    ## [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
    Reviewers

    @rajdip-b rajdip-b rajdip-b approved these changes

    Assignees
    No one assigned
    Labels
    Bug fix released Review effort [1-5]: 2
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    API: Replace IDs in workspace global search response with slugs
    2 participants
    @PriyobrotoKar @rajdip-b