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(workspace-resolver): prevent deletion of demo workspaces (#2207) #3068

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

khakimov
Copy link
Contributor

CleanShot 2566-12-19 at 10 25 36@2x

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @khakimov,
Left 1 comment :)


// Check if the id is in the list of demo workspaceIds
if (demoWorkspaceIds.includes(id)) {
throw new Error('Cannot delete a demo workspace.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khakimov could you throw a ForbiddenException instead? This will (ongoing work) be parsed by our exception handler and nicely sent back through graphql response.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added checks in deleteUser as well to prevent removing current user from the demo workspace.

…sages (twentyhq#2207)

- ForbiddenException messages for attempts to delete users and workspaces associated with demo accounts
Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Maybe people will stop deleting our demo workspace now :D

@charlesBochet charlesBochet merged commit 351dc64 into twentyhq:main Dec 20, 2023
11 checks passed
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.

None yet

2 participants