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

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

Closed
rajdip-b opened this issue Sep 14, 2024 · 7 comments · Fixed by #448
Closed

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

rajdip-b opened this issue Sep 14, 2024 · 7 comments · Fixed by #448
Assignees
Labels
difficulty: 1 good first issue Good for newcomers priority: medium released scope: api Everything related to the API type: enhancement New feature or request

Comments

@rajdip-b
Copy link
Member

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

  • Changes needs to be done to this function:
    async exportData(user: User, workspaceSlug: Workspace['slug']) {
    const workspace =
    await this.authorityCheckerService.checkAuthorityOverWorkspace({
    userId: user.id,
    entity: { slug: workspaceSlug },
    authorities: [Authority.WORKSPACE_ADMIN],
    prisma: this.prisma
    })
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const data: any = {}
    data.name = workspace.name
    data.description = workspace.description
    // Get all the roles of the workspace
    data.workspaceRoles = await this.prisma.workspaceRole.findMany({
    where: {
    workspaceId: workspace.id
    },
    select: {
    name: true,
    description: true,
    colorCode: true,
    hasAdminAuthority: true,
    authorities: true,
    projects: {
    select: {
    id: true
    }
    }
    }
    })
    // Get all projects, environments, variables and secrets of the workspace
    data.projects = await this.prisma.project.findMany({
    where: {
    workspaceId: workspace.id
    },
    select: {
    name: true,
    description: true,
    publicKey: true,
    privateKey: true,
    storePrivateKey: true,
    accessLevel: true,
    environments: {
    select: {
    name: true,
    description: true
    }
    },
    secrets: {
    select: {
    name: true,
    rotateAt: true,
    note: true,
    versions: {
    select: {
    value: true,
    version: true
    }
    }
    }
    },
    variables: {
    select: {
    name: true,
    note: true,
    versions: {
    select: {
    value: true,
    version: true
    }
    }
    }
    }
    }
    })
    return data
    }
  • Update the E2E tests in workspace.e2e.spec.ts to reflect this scenario.
@rajdip-b rajdip-b added type: enhancement New feature or request good first issue Good for newcomers scope: api Everything related to the API priority: medium difficulty: 1 labels Sep 14, 2024
@rajdip-b rajdip-b moved this to Todo in keyshade-api Sep 14, 2024
@Meeran-Tofiq
Copy link
Contributor

/attempt

Copy link

Assigned the issue to @Meeran-Tofiq!

@rajdip-b rajdip-b moved this from Todo to In progress in keyshade-api Sep 14, 2024
@Meeran-Tofiq
Copy link
Contributor

Meeran-Tofiq commented Sep 14, 2024

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
Copy link
Member Author

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
Copy link
Contributor

Meeran-Tofiq commented Sep 15, 2024

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
Copy link
Member Author

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.

@rajdip-b rajdip-b moved this from In progress to Under review in keyshade-api Sep 18, 2024
@github-project-automation github-project-automation bot moved this from Under review to Done in keyshade-api Sep 18, 2024
@rajdip-b rajdip-b moved this from Done to Queued for release in keyshade-api Sep 18, 2024
@rajdip-b
Copy link
Member Author

🎉 This issue has been resolved 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
Labels
difficulty: 1 good first issue Good for newcomers priority: medium released scope: api Everything related to the API type: enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants