keyshade-xyz / keyshade

Realtime secret and configuration management tool, with the best in class security and seamless integration support
https://keyshade.xyz
Mozilla Public License 2.0
154 stars 84 forks source link

API: Remove `projectIds` from `Export Workspace Data` response. #425

Closed rajdip-b closed 14 hours ago

rajdip-b commented 5 days ago

Description

image

Calling /api/workspace/:workspaceSlug/export-data fetches us all the roles, projects, environments, variables and secrets of a workspace. Just that in the roles listing, we also get the projectIds associated to that role. We would want to exclude this selection.

Solution

Meeran-Tofiq commented 4 days ago

/attempt

github-actions[bot] commented 4 days ago

Assigned the issue to @Meeran-Tofiq!

Meeran-Tofiq commented 4 days ago

Hello, everyone!

I have already implemented the removal of the project IDs being exported.

In the issue it is stated that there are changes that need to be made to the E2E tests of workspace.e2e.spec.ts, however upon inspection, the tests do not contain anything that checks for project IDs being exported. Meaning that despite me removing them, all the tests in the API ran successfully.

If it is necessary for me to make new tests to make sure that there ISN'T project IDs in the exported data, I'd gladly do so. But I didn't want to fill the test file with unnecessary tests, unless instructed to otherwise.

Should I implement the new tests?

rajdip-b commented 4 days ago

I think in that case you can update the existing test to do a deep equality check for the presence of the fields (not the values). That should do it.

Meeran-Tofiq commented 3 days ago

Screenshot from 2024-09-15 14-38-32

Hello again. I believe this test already adequately tests for the existence of necessary keys in the returned object. I believe it will be unnecessary to make changes to it.

I am a beginner and I apologize if I'm going about this the wrong way.

If no extra tests are required, I will go ahead and make the pull request :)

thanks

rajdip-b commented 3 days ago

Hey! We do appreciate you pinging us with your suggestion. In fact, we encourage that highly.

Coming to your question, yes, this is present. What we also want is, just try to fetch the 0th element in the array and check if the required fields are present or not. That's all that we need.