Skip to content

Commit e4fa212

Browse files
mtojekmtojek
authored
fix: always return count of workspaces (#12407)
1 parent 0016b02 commit e4fa212

File tree

6 files changed

+217
-57
lines changed

6 files changed

+217
-57
lines changed

‎coderd/database/dbmem/dbmem.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ func mapAgentStatus(dbAgent database.WorkspaceAgent, agentInactiveDisconnectTime
345345
return status
346346
}
347347

348-
func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspaces []database.Workspace, count int64) []database.GetWorkspacesRow {
348+
func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspaces []database.Workspace, count int64, withSummary bool) []database.GetWorkspacesRow { //nolint:revive // withSummary flag ensures the extra technical row
349349
rows := make([]database.GetWorkspacesRow, 0, len(workspaces))
350350
for _, w := range workspaces {
351351
wr := database.GetWorkspacesRow{
@@ -389,6 +389,12 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac
389389

390390
rows = append(rows, wr)
391391
}
392+
if withSummary {
393+
rows = append(rows, database.GetWorkspacesRow{
394+
Name: "**TECHNICAL_ROW**",
395+
Count: count,
396+
})
397+
}
392398
return rows
393399
}
394400

@@ -8278,12 +8284,12 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
82788284
}
82798285
if arg.Limit > 0 {
82808286
if int(arg.Limit) > len(workspaces) {
8281-
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount)), nil
8287+
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount), arg.WithSummary), nil
82828288
}
82838289
workspaces = workspaces[:arg.Limit]
82848290
}
82858291

8286-
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount)), nil
8292+
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount), arg.WithSummary), nil
82878293
}
82888294

82898295
func (q *FakeQuerier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, prepared rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {

‎coderd/database/modelqueries.go

+6
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
231231
arg.RequesterID,
232232
arg.Offset,
233233
arg.Limit,
234+
arg.WithSummary,
234235
)
235236
if err != nil {
236237
return nil, err
@@ -258,6 +259,11 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
258259
&i.TemplateName,
259260
&i.TemplateVersionID,
260261
&i.TemplateVersionName,
262+
&i.Username,
263+
&i.LatestBuildCompletedAt,
264+
&i.LatestBuildCanceledAt,
265+
&i.LatestBuildError,
266+
&i.LatestBuildTransition,
261267
&i.Count,
262268
); err != nil {
263269
return nil, err

‎coderd/database/queries.sql.go

+106-36
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

+75-18
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,17 @@ WHERE
7777
);
7878

7979
-- name: GetWorkspaces :many
80+
WITH filtered_workspaces AS (
8081
SELECT
8182
workspaces.*,
8283
COALESCE(template.name, 'unknown') as template_name,
8384
latest_build.template_version_id,
8485
latest_build.template_version_name,
85-
COUNT(*) OVER () as count
86+
users.username as username,
87+
latest_build.completed_at as latest_build_completed_at,
88+
latest_build.canceled_at as latest_build_canceled_at,
89+
latest_build.error as latest_build_error,
90+
latest_build.transition as latest_build_transition
8691
FROM
8792
workspaces
8893
JOIN
@@ -266,23 +271,75 @@ WHERE
266271
END
267272
-- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces
268273
-- @authorize_filter
269-
ORDER BY
270-
-- To ensure that 'favorite' workspaces show up first in the list only for their owner.
271-
CASE WHEN workspaces.owner_id = @requester_id AND workspaces.favorite THEN 0 ELSE 1 END ASC,
272-
(latest_build.completed_at IS NOT NULL AND
273-
latest_build.canceled_at IS NULL AND
274-
latest_build.error IS NULL AND
275-
latest_build.transition = 'start'::workspace_transition) DESC,
276-
LOWER(users.username) ASC,
277-
LOWER(workspaces.name) ASC
278-
LIMIT
279-
CASE
280-
WHEN @limit_ :: integer > 0 THEN
281-
@limit_
282-
END
283-
OFFSET
284-
@offset_
285-
;
274+
), filtered_workspaces_order AS (
275+
SELECT
276+
fw.*
277+
FROM
278+
filtered_workspaces fw
279+
ORDER BY
280+
-- To ensure that 'favorite' workspaces show up first in the list only for their owner.
281+
CASE WHEN owner_id = @requester_id AND favorite THEN 0 ELSE 1 END ASC,
282+
(latest_build_completed_at IS NOT NULL AND
283+
latest_build_canceled_at IS NULL AND
284+
latest_build_error IS NULL AND
285+
latest_build_transition = 'start'::workspace_transition) DESC,
286+
LOWER(username) ASC,
287+
LOWER(name) ASC
288+
LIMIT
289+
CASE
290+
WHEN @limit_ :: integer > 0 THEN
291+
@limit_
292+
END
293+
OFFSET
294+
@offset_
295+
), filtered_workspaces_order_with_summary AS (
296+
SELECT
297+
fwo.*
298+
FROM
299+
filtered_workspaces_order fwo
300+
-- Return a technical summary row with total count of workspaces.
301+
-- It is used to present the correct count if pagination goes beyond the offset.
302+
UNION ALL
303+
SELECT
304+
'00000000-0000-0000-0000-000000000000'::uuid, -- id
305+
'0001-01-01 00:00:00+00'::timestamp, -- created_at
306+
'0001-01-01 00:00:00+00'::timestamp, -- updated_at
307+
'00000000-0000-0000-0000-000000000000'::uuid, -- owner_id
308+
'00000000-0000-0000-0000-000000000000'::uuid, -- organization_id
309+
'00000000-0000-0000-0000-000000000000'::uuid, -- template_id
310+
false, -- deleted
311+
'**TECHNICAL_ROW**', -- name
312+
'', -- autostart_schedule
313+
0, -- ttl
314+
'0001-01-01 00:00:00+00'::timestamp, -- last_used_at
315+
'0001-01-01 00:00:00+00'::timestamp, -- dormant_at
316+
'0001-01-01 00:00:00+00'::timestamp, -- deleting_at
317+
'never'::automatic_updates, -- automatic_updates
318+
false, -- favorite
319+
-- Extra columns added to `filtered_workspaces`
320+
'', -- template_name
321+
'00000000-0000-0000-0000-000000000000'::uuid, -- template_version_id
322+
'', -- template_version_name
323+
'', -- username
324+
'0001-01-01 00:00:00+00'::timestamp, -- latest_build_completed_at,
325+
'0001-01-01 00:00:00+00'::timestamp, -- latest_build_canceled_at,
326+
'', -- latest_build_error
327+
'start'::workspace_transition -- latest_build_transition
328+
WHERE
329+
@with_summary :: boolean = true
330+
), total_count AS (
331+
SELECT
332+
count(*) AS count
333+
FROM
334+
filtered_workspaces
335+
)
336+
SELECT
337+
fwos.*,
338+
tc.count
339+
FROM
340+
filtered_workspaces_order_with_summary fwos
341+
CROSS JOIN
342+
total_count tc;
286343

287344
-- name: GetWorkspaceByOwnerIDAndName :one
288345
SELECT

‎coderd/workspaces.go

+20
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
173173
// the workspace owner_id when ordering the rows.
174174
filter.RequesterID = apiKey.UserID
175175

176+
// We need the technical row to present the correct count on every page.
177+
filter.WithSummary = true
178+
176179
workspaceRows, err := api.Database.GetAuthorizedWorkspaces(ctx, filter, prepared)
177180
if err != nil {
178181
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -181,6 +184,23 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
181184
})
182185
return
183186
}
187+
if len(workspaceRows) == 0 {
188+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
189+
Message: "Internal error fetching workspaces.",
190+
Detail: "Workspace summary row is missing.",
191+
})
192+
return
193+
}
194+
if len(workspaceRows) == 1 {
195+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspacesResponse{
196+
Workspaces: []codersdk.Workspace{},
197+
Count: int(workspaceRows[0].Count),
198+
})
199+
return
200+
}
201+
// Skip technical summary row
202+
workspaceRows = workspaceRows[:len(workspaceRows)-1]
203+
184204
if len(workspaceRows) == 0 {
185205
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspacesResponse{
186206
Workspaces: []codersdk.Workspace{},

‎coderd/workspaces_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -1753,6 +1753,7 @@ func TestOffsetLimit(t *testing.T) {
17531753
})
17541754
require.NoError(t, err)
17551755
require.Len(t, ws.Workspaces, 0)
1756+
require.Equal(t, ws.Count, 3) // can't find workspaces, but count is non-zero
17561757
}
17571758

17581759
func TestWorkspaceUpdateAutostart(t *testing.T) {

0 commit comments

Comments
 (0)