-
Notifications
You must be signed in to change notification settings - Fork 881
fix: always return count of workspaces #12407
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
Conversation
Which adjustments do you think would be necessary? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sold on the technical summary row. While it's not strictly compatibility-breaking, it's something that older clients won't know how to handle properly.
That having been said, if it can be an opt-in flag, then it's not a problem at all. Maybe we add a &withSummary=true
query parameter to the /api/v2/workspaces
endpoint as well and plumb that through? We can make it the default after a couple of releases.
EDIT: I misinterpreted -- this is all happening on the backend, so clients don't come into it at all. I think this is fine then 👍
Oh, but API does not leak the technical row, right? From API perspective the only change is the "count" value. Is that what you meant? |
Basically this:
from your comment |
Yeah, I was wondering if the technical row would show up as a "dummy" workspace in the API response. But that does not appear to be the case, correct? |
That would be a developer error, and there is no point in doing this. Also, it is disabled by default. |
Related: #11795
Related: #12380
This PR ensures that
/workspaces
API endpoint always returns a count with the total number of workspaces. This is a simpler and alternative version to #12380.Once it is merged, @BrunoQuaresma can work on the final adjustments on the FE side.