Skip to content

feat: don't return 200 for deleted workspaces #1556

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

Merged
merged 4 commits into from
May 19, 2022
Merged

Conversation

deansheather
Copy link
Member

Adds a deleted query parameter which defaults to false on the get workspace by ID endpoint. The query param must match workspace.Deleted otherwise the request will fail.

Resolves #832

// `deleted` field on the workspace otherwise you will get a 410 Gone.
var (
deletedStr = r.URL.Query().Get("deleted")
deleted = false
Copy link
Member

Choose a reason for hiding this comment

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

nit: semantics: suggest renaming this to showDeleted

Copy link
Member Author

Choose a reason for hiding this comment

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

In the query parameter or just this variable name? I'm not a fan of capital letters in query params (or any URL stuff)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I was mainly thinking about the variable name, deleted=1 as a query parameter is fairly standard imo.

@johnstcn johnstcn self-requested a review May 19, 2022 08:55
@deansheather deansheather merged commit 4eb0bb6 into main May 19, 2022
@deansheather deansheather deleted the no-deleted-workspaces branch May 19, 2022 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

api: workspace by id does not return 404 when workspace deleted
2 participants