fix(api,api-client): Add environmentSlug in multiple places across the variable module #468

Nil2000 · 2024-09-26T09:50:44Z

Description

Add environmentSlug in multiple places across the variable module

Fixes #431

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.

…e variable module

codiumai-pr-agent-free · 2024-09-26T09:51:13Z

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Key issues to review Possible Typo There's a potential typo in the `enivronment` property name at line 19. It should be `environment`. Possible Typo There's a potential typo in the `enivronment` property name at line 19. It should be `environment`. Possible Typo There's a potential typo in the `enivronment` property name at line 113. It should be `environment`.

codiumai-pr-agent-free · 2024-09-26T09:51:35Z

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use a constant or enum for field names to improve code maintainability Consider using a constant or enum for the 'slug' property name to improve maintainability and reduce the risk of typos. apps/api/src/variable/service/variable.service.ts [124-128] environment: { select: { - slug: true + [EnvironmentFields.SLUG]: true } }, Apply this suggestion Suggestion importance[1-10]: 7 Why:	7
Maintainability	Extract repeated selection object into a constant to reduce duplication Consider extracting the repeated environment selection object into a reusable constant to reduce code duplication. apps/api/src/variable/service/variable.service.ts [253-257] -environment: { - select: { - slug: true - } -}, +environment: ENVIRONMENT_SELECT_OBJECT, Apply this suggestion Suggestion importance[1-10]: 7 Why:	7
Possible bug	✅ Correct the spelling of a property name to ensure accurate typing Suggestion Impact: The typo in the property name 'enivronment' was corrected to 'environment', as suggested. code diff: - enivronment: { + environment: { Fix the typo in the 'enivronment' property name to 'environment' to ensure correct typing and avoid potential bugs. packages/api-client/src/types/variable.types.d.ts [19-21] -enivronment: { +environment: { slug: string } Apply this suggestion Suggestion importance[1-10]: 7 Why:	7
Possible issue	✅ Fix a typo in a test assertion to ensure it correctly checks the property Suggestion Impact: The typo in the property name 'enivronment' was corrected to 'environment' in the test assertion, as suggested. code diff: - expect(variable.data.versions[0].enivronment.slug).toBe(environment.slug) + expect(variable.data.versions[0].environment.slug).toBe(environment.slug) Correct the typo in the 'enivronment' property name to 'environment' to match the corrected type definition and ensure the test passes. packages/api-client/tests/variable.spec.ts [113] -expect(variable.data.versions[0].enivronment.slug).toBe(environment.slug) +expect(variable.data.versions[0].environment.slug).toBe(environment.slug) Apply this suggestion Suggestion importance[1-10]: 7 Why:	7

💡 Need additional feedback ? start a PR chat

Nil2000 · 2024-09-26T10:10:23Z

Hi @rajdip-b , Require some help here.

Should this be environment-2-0 since we are passing environment2.id in environmentId field?

keyshade/apps/api/src/variable/variable.e2e.spec.ts

Lines 176 to 190 in beaf876

    
           it('should be able to create a variable', async () => { 
        
             const response = await app.inject({ 
        
               method: 'POST', 
        
               url: `/variable/${project1.slug}`, 
        
               payload: { 
        
                 name: 'Variable 3', 
        
                 note: 'Variable 3 note', 
        
                 rotateAfter: '24', 
        
                 entries: [ 
        
                   { 
        
                     value: 'Variable 3 value', 
        
                     environmentId: environment2.id 
        
                   } 
        
                 ] 
        
               },

rajdip-b

In places where we are selecting both environmentId and environment slug, its better to include it by using environment.id and environment.slug:

      environment: {
        id: true
        slug: true
      }

codecov · 2024-09-29T10:49:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.01%. Comparing base (ce50743) to head (7f83d64).
Report is 183 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop     #468       +/-   ##
============================================
- Coverage    91.71%   48.01%   -43.71%     
============================================
  Files          111      105        -6     
  Lines         2510     2743      +233     
  Branches       469      415       -54     
============================================
- Hits          2302     1317      -985     
- Misses         208     1426     +1218

Flag	Coverage Δ
api-e2e-tests	`48.01% <ø> (-43.71%)`	⬇️

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.

Nil2000 · 2024-09-29T11:16:25Z

In places where we are selecting both environmentId and environment slug, its better to include it by using environment.id and environment.slug:
      environment: {
        id: true
        slug: true
      }

Ok made these changes but did you notice about the environment2 test mentioned earlier? It is still coming.

rajdip-b

This step is failing: https://github.com/keyshade-xyz/keyshade/actions/runs/11092075120/job/30816652494?pr=468#step:10:486

Nil2000 · 2024-09-29T17:45:02Z

This step is failing: https://github.com/keyshade-xyz/keyshade/actions/runs/11092075120/job/30816652494?pr=468#step:10:486

Yeah already mentioned the issue this is always pointing to environment-1-0 slug or Environment 1 though I have pointed that to environment2. I am unable to get why this is happening. Can it be issue at the mapping part?

…e variable module

rajdip-b · 2024-10-13T14:32:07Z

@Nil2000 sorry about the delay, i have fixed the error. I would just need you to remove environmentId and have environment.id everywhere like i mentioned earlier.

rajdip-b · 2024-10-13T14:49:07Z

#486

This is a related issue that you can pick up.

rajdip-b · 2024-10-15T04:27:06Z

@Nil2000 can you please add this change I mentioned?

@Nil2000 sorry about the delay, i have fixed the error. I would just need you to remove environmentId and have environment.id everywhere like i mentioned earlier.

Nil2000 · 2024-10-15T04:33:22Z

@Nil2000 can you please add this change I mentioned?

@Nil2000 sorry about the delay, i have fixed the error. I would just need you to remove environmentId and have environment.id everywhere like i mentioned earlier.

Working on it. Would be done by today

Nil2000 · 2024-10-15T16:33:19Z

@rajdip-b The test issue is still present

rajdip-b

The last few changes that you made break a lot of things, and also isn't required. You are required to send back the data in the format that i have already mentioned in the comment above. This reqires you to just change how the environmentId is "included", and not make changes to the business logic.
I fixed the tests when I last updated the PR. You can also check that in https://github.com/keyshade-xyz/keyshade/actions/runs/11349562441/job/31565953148

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

rajdip-b

Just double-check the api-client package. The rest of the changes look solid.

packages/api-client/tests/variable.spec.ts

## [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 · 2024-10-24T12:11:22Z

🎉 This PR is included in version 2.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

fix(api,api-client) :Add environmentSlug in multiple places across th…

eb73d8f

…e variable module

Nil2000 requested a review from rajdip-b as a code owner September 26, 2024 09:50

codiumai-pr-agent-free bot added the Review effort [1-5]: 3 label Sep 26, 2024

Nil2000 changed the title ~~fix(api,api-client) :Add environmentSlug in multiple places across the variable module~~ fix(api,api-client): Add environmentSlug in multiple places across the variable module Sep 26, 2024

Merge branch 'develop' into d-431

beaf876

Nil2000 added 2 commits September 26, 2024 15:53

Some typo

298221d

Some typo

5abb06d

rajdip-b requested changes Sep 29, 2024

View reviewed changes

Nil2000 added 2 commits September 29, 2024 15:52

Fixed few typos:2

e66683e

Fixed few typos:3

6a3f063

Nil2000 added 2 commits September 29, 2024 16:22

Testing typos

f091bb6

Testing typos

1d8bf61

rajdip-b approved these changes Sep 29, 2024

View reviewed changes

rajdip-b requested changes Sep 29, 2024

View reviewed changes

Nil2000 and others added 10 commits September 29, 2024 23:15

Merge branch 'develop' into d-431

6ceb9a0

Testing typo fix

8d63588

Merge branch 'd-431' of https://github.com/Nil2000/keyshade into d-431

d8c35ff

fix(api,api-client) :Add environmentSlug in multiple places across th…

d409add

…e variable module

Some typo

92b1712

Some typo

67279ce

Fixed few typos:2

11e9148

Fixed few typos:3

6f27739

Testing typos

5cedec3

Testing typos

9663088

rajdip-b force-pushed the d-431 branch from d8c35ff to ea69bc2 Compare October 13, 2024 14:15

fixed test error

7f83d64

rajdip-b requested a review from kriptonian1 as a code owner October 13, 2024 14:31

rajdip-b added the hacktoberfest-accepted Your PR has this = Congrats! label Oct 13, 2024

reverted jest config

0de10c2

Nil2000 and others added 3 commits October 14, 2024 00:04

Merge branch 'develop' into d-431

2d66a55

Merge branch 'develop' into d-431

e90d34f

Resolved Merge conflict

3dd3bca

Nil2000 and others added 2 commits October 15, 2024 21:11

Merge branch 'develop' into d-431

21d3c2a

Merge branch 'd-431' of https://github.com/Nil2000/keyshade into d-431

8b3be35

rajdip-b requested changes Oct 15, 2024

View reviewed changes

Nil2000 commented Oct 15, 2024

View reviewed changes

apps/api/src/variable/variable.e2e.spec.ts Outdated Show resolved Hide resolved

Nil2000 commented Oct 15, 2024

View reviewed changes

apps/api/src/variable/variable.e2e.spec.ts Outdated Show resolved Hide resolved

Fixed some issues

f30df2b

Nil2000 force-pushed the d-431 branch 2 times, most recently from 0e85f44 to f30df2b Compare October 16, 2024 14:36

Merge branch 'develop' into d-431

0231e6e

rajdip-b requested changes Oct 16, 2024

View reviewed changes

packages/api-client/tests/variable.spec.ts Outdated Show resolved Hide resolved

packages/api-client/tests/variable.spec.ts Outdated Show resolved Hide resolved

Nil2000 added 2 commits October 17, 2024 09:23

Fixed few more typos

87e4da8

Merge branch 'd-431' of https://github.com/Nil2000/keyshade into d-431

4602ea6

rajdip-b approved these changes Oct 17, 2024

View reviewed changes

rajdip-b merged commit d970aff into keyshade-xyz:develop Oct 17, 2024
5 checks passed

rajdip-b added the released label Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(api,api-client): Add environmentSlug in multiple places across the variable module #468

fix(api,api-client): Add environmentSlug in multiple places across the variable module #468

Nil2000 commented Sep 26, 2024

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

codiumai-pr-agent-free bot commented Sep 26, 2024 •

edited

Loading

Nil2000 commented Sep 26, 2024 •

edited

Loading

rajdip-b left a comment •

edited

Loading

codecov bot commented Sep 29, 2024 •

edited

Loading

Nil2000 commented Sep 29, 2024

rajdip-b left a comment

Nil2000 commented Sep 29, 2024 •

edited

Loading

rajdip-b commented Oct 13, 2024

rajdip-b commented Oct 13, 2024

rajdip-b commented Oct 15, 2024

Nil2000 commented Oct 15, 2024

Nil2000 commented Oct 15, 2024

rajdip-b left a comment

rajdip-b left a comment

rajdip-b commented Oct 24, 2024

fix(api,api-client): Add environmentSlug in multiple places across the variable module #468

fix(api,api-client): Add environmentSlug in multiple places across the variable module #468

Conversation

Nil2000 commented Sep 26, 2024

Description

Dependencies

Future Improvements

Mentions

Screenshots of relevant screens

Developer's checklist

Documentation Update

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

PR Reviewer Guide 🔍

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

PR Code Suggestions ✨

Nil2000 commented Sep 26, 2024 • edited Loading

rajdip-b left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Sep 29, 2024 • edited Loading

Codecov Report

Nil2000 commented Sep 29, 2024

rajdip-b left a comment

Choose a reason for hiding this comment

Nil2000 commented Sep 29, 2024 • edited Loading

rajdip-b commented Oct 13, 2024

rajdip-b commented Oct 13, 2024

rajdip-b commented Oct 15, 2024

Nil2000 commented Oct 15, 2024

Nil2000 commented Oct 15, 2024

rajdip-b left a comment

Choose a reason for hiding this comment

rajdip-b left a comment

Choose a reason for hiding this comment

rajdip-b commented Oct 24, 2024

codiumai-pr-agent-free bot commented Sep 26, 2024 •

edited

Loading

Nil2000 commented Sep 26, 2024 •

edited

Loading

rajdip-b left a comment •

edited

Loading

codecov bot commented Sep 29, 2024 •

edited

Loading

Nil2000 commented Sep 29, 2024 •

edited

Loading