From 0a8fea52f67813d9721077cd8d980b0d3fde64e4 Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Tue, 23 May 2023 20:35:17 +0000 Subject: [PATCH 1/6] wip --- coderd/database/queries.sql.go | 6 ++++ coderd/database/queries/workspaces.sql | 6 ++++ coderd/workspaces.go | 2 +- coderd/workspaces_test.go | 43 ++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 9955deb439ffd..914a2dbdce0a5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8333,6 +8333,12 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces -- @authorize_filter ORDER BY + (latest_build.completed_at IS NOT NULL AND + latest_build.canceled_at IS NULL AND + latest_build.error IS NULL AND + latest_build.transition = 'start'::workspace_transition) ASC, + owner_id ASC, + name, last_used_at DESC LIMIT CASE diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 3702bc94e20a6..ed787a93da54a 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -238,6 +238,12 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces -- @authorize_filter ORDER BY + (latest_build.completed_at IS NOT NULL AND + latest_build.canceled_at IS NULL AND + latest_build.error IS NULL AND + latest_build.transition = 'start'::workspace_transition) ASC, + owner_id ASC, + name, last_used_at DESC LIMIT CASE diff --git a/coderd/workspaces.go b/coderd/workspaces.go index e303fa31507f9..c9835c34f7181 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1016,7 +1016,7 @@ func convertWorkspaces(workspaces []database.Workspace, data workspaceData) ([]c )) } - sortWorkspaces(apiWorkspaces) + // sortWorkspaces(apiWorkspaces) return apiWorkspaces, nil } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index f3005b8eb4747..7d67230722e62 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -198,6 +198,49 @@ func TestAdminViewAllWorkspaces(t *testing.T) { require.Equal(t, 0, len(memberViewWorkspaces.Workspaces), "member in other org should see 0 workspaces") } +func TestWorkspacesSortOrder(t *testing.T) { + // the correct sorting order is: + // 1. first show workspaces that are currently running, + // 2. then sort by user_name, + // 3. then sort by last_used_at (descending), + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + firstUser := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, firstUser.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID) + workspace1 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { + ctr.Name = "test-workspace-sort-1" + }) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace1.LatestBuild.ID) + + workspace2 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { + ctr.Name = "test-workspace-sort-2" + }) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace2.LatestBuild.ID) + + build2 := coderdtest.CreateWorkspaceBuild(t, client, workspace2, database.WorkspaceTransitionStop) + coderdtest.AwaitWorkspaceBuildJob(t, client, build2.ID) + + workspace3 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { + ctr.Name = "test-workspace-sort-3" + }) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace3.LatestBuild.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + workspacesResponse, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) + require.NoError(t, err, "(first) fetch workspaces") + require.Equal(t, 3, len(workspacesResponse.Workspaces), "should be 3 workspaces present") + + require.Equal(t, codersdk.WorkspaceStatusRunning, workspacesResponse.Workspaces[0].LatestBuild.Status, "should be the first item in the list because it is running") + require.Equal(t, codersdk.WorkspaceStatusRunning, workspacesResponse.Workspaces[1].LatestBuild.Status) + require.Equal(t, codersdk.WorkspaceStatusStopped, workspacesResponse.Workspaces[2].LatestBuild.Status, "The stopped workspace should be last") + + require.Equal(t, workspace3.ID, workspacesResponse.Workspaces[0].ID, "If both are running, and have the same owner, sort by last used (in this case, the last one created)") +} + func TestPostWorkspacesByOrganization(t *testing.T) { t.Parallel() t.Run("InvalidTemplate", func(t *testing.T) { From b5aec66ef795f0eb854f6bb2a18e32e45f35f9de Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 24 May 2023 19:52:05 +0000 Subject: [PATCH 2/6] use updated sql --- coderd/database/queries.sql.go | 14 +++++++++----- coderd/database/queries/workspaces.sql | 14 +++++++++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 914a2dbdce0a5..3368f07ec645b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8172,7 +8172,11 @@ const getWorkspaces = `-- name: GetWorkspaces :many SELECT workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, COUNT(*) OVER () as count FROM - workspaces + workspaces +JOIN + users +ON + workspaces.owner_id = users.id LEFT JOIN LATERAL ( SELECT workspace_builds.transition, @@ -8333,12 +8337,12 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces -- @authorize_filter ORDER BY - (latest_build.completed_at IS NOT NULL AND + (latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND - latest_build.transition = 'start'::workspace_transition) ASC, - owner_id ASC, - name, + latest_build.transition = 'start'::workspace_transition) DESC, + LOWER(users.username) ASC, + LOWER(name) ASC, last_used_at DESC LIMIT CASE diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ed787a93da54a..ace404c7f3aa4 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -77,7 +77,11 @@ WHERE SELECT workspaces.*, COUNT(*) OVER () as count FROM - workspaces + workspaces +JOIN + users +ON + workspaces.owner_id = users.id LEFT JOIN LATERAL ( SELECT workspace_builds.transition, @@ -238,12 +242,12 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces -- @authorize_filter ORDER BY - (latest_build.completed_at IS NOT NULL AND + (latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND - latest_build.transition = 'start'::workspace_transition) ASC, - owner_id ASC, - name, + latest_build.transition = 'start'::workspace_transition) DESC, + LOWER(users.username) ASC, + LOWER(name) ASC, last_used_at DESC LIMIT CASE From 2feee9818bc044cb3e9e92546909dfe3cb7689f2 Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 24 May 2023 20:21:45 +0000 Subject: [PATCH 3/6] wip --- coderd/workspaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index c9835c34f7181..e303fa31507f9 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1016,7 +1016,7 @@ func convertWorkspaces(workspaces []database.Workspace, data workspaceData) ([]c )) } - // sortWorkspaces(apiWorkspaces) + sortWorkspaces(apiWorkspaces) return apiWorkspaces, nil } From f38e0c6393829e7267958a4fb88666ca11520dab Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 May 2023 17:39:22 +0200 Subject: [PATCH 4/6] Implement sorting in databasefake.go --- coderd/database/dbfake/databasefake.go | 54 +++++++++++++++ coderd/database/queries.sql.go | 3 +- coderd/database/queries/workspaces.sql | 3 +- coderd/workspaces.go | 28 -------- coderd/workspaces_internal_test.go | 94 -------------------------- coderd/workspaces_test.go | 30 ++++---- 6 files changed, 73 insertions(+), 139 deletions(-) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index 41800dbcd9e74..f02ba967edd2a 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -1329,6 +1329,60 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. workspaces = append(workspaces, workspace) } + // Sort workspaces (ORDER BY) + isRunning := func(build database.WorkspaceBuild, job database.ProvisionerJob) bool { + return job.CompletedAt.Valid && !job.CanceledAt.Valid && !job.Error.Valid && build.Transition == database.WorkspaceTransitionStart + } + + preloadedWorkspaceBuilds := map[uuid.UUID]database.WorkspaceBuild{} + preloadedProvisionerJobs := map[uuid.UUID]database.ProvisionerJob{} + preloadedUsers := map[uuid.UUID]database.User{} + + for _, w := range workspaces { + build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID) + if err != nil { + return nil, xerrors.Errorf("get latest build: %w", err) + } + preloadedWorkspaceBuilds[w.ID] = build + + job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID) + if err != nil { + return nil, xerrors.Errorf("get provisioner job: %w", err) + } + preloadedProvisionerJobs[w.ID] = job + + user, err := q.getUserByIDNoLock(w.OwnerID) + if err != nil { + return nil, xerrors.Errorf("get user: %w", err) + } + preloadedUsers[w.ID] = user + } + + sort.Slice(workspaces, func(i, j int) bool { + w1 := workspaces[i] + w2 := workspaces[j] + + // Order by: running first + w1IsRunning := isRunning(preloadedWorkspaceBuilds[w1.ID], preloadedProvisionerJobs[w1.ID]) + w2IsRunning := isRunning(preloadedWorkspaceBuilds[w2.ID], preloadedProvisionerJobs[w2.ID]) + + if w1IsRunning && !w2IsRunning { + return true + } + + if !w1IsRunning && w2IsRunning { + return false + } + + // Order by: usernames + if w1.ID != w2.ID { + return sort.StringsAreSorted([]string{preloadedUsers[w1.ID].Username, preloadedUsers[w2.ID].Username}) + } + + // Order by: workspace names + return sort.StringsAreSorted([]string{w1.Name, w2.Name}) + }) + beforePageCount := len(workspaces) if arg.Offset > 0 { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3368f07ec645b..91e8a85f7e5b0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8342,8 +8342,7 @@ ORDER BY latest_build.error IS NULL AND latest_build.transition = 'start'::workspace_transition) DESC, LOWER(users.username) ASC, - LOWER(name) ASC, - last_used_at DESC + LOWER(name) ASC LIMIT CASE WHEN $11 :: integer > 0 THEN diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ace404c7f3aa4..3819dbd640e66 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -247,8 +247,7 @@ ORDER BY latest_build.error IS NULL AND latest_build.transition = 'start'::workspace_transition) DESC, LOWER(users.username) ASC, - LOWER(name) ASC, - last_used_at DESC + LOWER(name) ASC LIMIT CASE WHEN @limit_ :: integer > 0 THEN diff --git a/coderd/workspaces.go b/coderd/workspaces.go index e303fa31507f9..627c431ae7964 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "net/http" - "sort" "strconv" "time" @@ -1015,36 +1014,9 @@ func convertWorkspaces(workspaces []database.Workspace, data workspaceData) ([]c &owner, )) } - - sortWorkspaces(apiWorkspaces) - return apiWorkspaces, nil } -func sortWorkspaces(workspaces []codersdk.Workspace) { - sort.Slice(workspaces, func(i, j int) bool { - iw := workspaces[i] - jw := workspaces[j] - - if iw.LatestBuild.Status == codersdk.WorkspaceStatusRunning && jw.LatestBuild.Status != codersdk.WorkspaceStatusRunning { - return true - } - - if jw.LatestBuild.Status == codersdk.WorkspaceStatusRunning && iw.LatestBuild.Status != codersdk.WorkspaceStatusRunning { - return false - } - - if iw.OwnerID != jw.OwnerID { - return iw.OwnerName < jw.OwnerName - } - - if jw.LastUsedAt.IsZero() && iw.LastUsedAt.IsZero() { - return iw.Name < jw.Name - } - return iw.LastUsedAt.After(jw.LastUsedAt) - }) -} - func convertWorkspace( workspace database.Workspace, workspaceBuild codersdk.WorkspaceBuild, diff --git a/coderd/workspaces_internal_test.go b/coderd/workspaces_internal_test.go index 62a5091539330..44c1699309c4c 100644 --- a/coderd/workspaces_internal_test.go +++ b/coderd/workspaces_internal_test.go @@ -4,7 +4,6 @@ import ( "testing" "time" - "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/database" @@ -81,96 +80,3 @@ func Test_calculateDeletingAt(t *testing.T) { }) } } - -func TestSortWorkspaces(t *testing.T) { - // the correct sorting order is: - // 1. first show workspaces that are currently running, - // 2. then sort by user_name, - // 3. then sort by last_used_at (descending), - t.Parallel() - - workspaceFactory := func(t *testing.T, name string, ownerID uuid.UUID, ownerName string, status codersdk.WorkspaceStatus, lastUsedAt time.Time) codersdk.Workspace { - t.Helper() - return codersdk.Workspace{ - ID: uuid.New(), - OwnerID: ownerID, - OwnerName: ownerName, - LatestBuild: codersdk.WorkspaceBuild{ - Status: status, - }, - Name: name, - LastUsedAt: lastUsedAt, - } - } - - userAuuid := uuid.New() - - workspaceRunningUserA := workspaceFactory(t, "running-userA", userAuuid, "userA", codersdk.WorkspaceStatusRunning, time.Now()) - workspaceRunningUserB := workspaceFactory(t, "running-userB", uuid.New(), "userB", codersdk.WorkspaceStatusRunning, time.Now()) - workspacePendingUserC := workspaceFactory(t, "pending-userC", uuid.New(), "userC", codersdk.WorkspaceStatusPending, time.Now()) - workspaceRunningUserA2 := workspaceFactory(t, "running-userA2", userAuuid, "userA", codersdk.WorkspaceStatusRunning, time.Now().Add(time.Minute)) - workspaceRunningUserZ := workspaceFactory(t, "running-userZ", uuid.New(), "userZ", codersdk.WorkspaceStatusRunning, time.Now()) - workspaceRunningUserA3 := workspaceFactory(t, "running-userA3", userAuuid, "userA", codersdk.WorkspaceStatusRunning, time.Now().Add(time.Hour)) - - testCases := []struct { - name string - input []codersdk.Workspace - expectedOrder []string - }{ - { - name: "Running workspaces should be first", - input: []codersdk.Workspace{ - workspaceRunningUserB, - workspacePendingUserC, - workspaceRunningUserA, - }, - expectedOrder: []string{ - "running-userA", - "running-userB", - "pending-userC", - }, - }, - { - name: "then sort by owner name", - input: []codersdk.Workspace{ - workspaceRunningUserZ, - workspaceRunningUserA, - workspaceRunningUserB, - }, - expectedOrder: []string{ - "running-userA", - "running-userB", - "running-userZ", - }, - }, - { - name: "then sort by last used at (recent first)", - input: []codersdk.Workspace{ - workspaceRunningUserA, - workspaceRunningUserA2, - workspaceRunningUserA3, - }, - expectedOrder: []string{ - "running-userA3", - "running-userA2", - "running-userA", - }, - }, - } - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - workspaces := tc.input - sortWorkspaces(workspaces) - - var resultNames []string - for _, workspace := range workspaces { - resultNames = append(resultNames, workspace.Name) - } - - require.Equal(t, tc.expectedOrder, resultNames, tc.name) - }) - } -} diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 7d67230722e62..df880a9475b42 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -199,31 +199,32 @@ func TestAdminViewAllWorkspaces(t *testing.T) { } func TestWorkspacesSortOrder(t *testing.T) { - // the correct sorting order is: - // 1. first show workspaces that are currently running, - // 2. then sort by user_name, - // 3. then sort by last_used_at (descending), t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) firstUser := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, firstUser.OrganizationID, nil) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID) + + // Workspace 1 should be running workspace1 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { - ctr.Name = "test-workspace-sort-1" + ctr.Name = "c-workspace" }) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace1.LatestBuild.ID) + // Workspace 2 should be stopped workspace2 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { - ctr.Name = "test-workspace-sort-2" + ctr.Name = "b-workspace" }) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace2.LatestBuild.ID) build2 := coderdtest.CreateWorkspaceBuild(t, client, workspace2, database.WorkspaceTransitionStop) coderdtest.AwaitWorkspaceBuildJob(t, client, build2.ID) + // Workspace 3 should be running workspace3 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { - ctr.Name = "test-workspace-sort-3" + ctr.Name = "a-workspace" }) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace3.LatestBuild.ID) @@ -232,13 +233,16 @@ func TestWorkspacesSortOrder(t *testing.T) { workspacesResponse, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err, "(first) fetch workspaces") - require.Equal(t, 3, len(workspacesResponse.Workspaces), "should be 3 workspaces present") + workspaces := workspacesResponse.Workspaces - require.Equal(t, codersdk.WorkspaceStatusRunning, workspacesResponse.Workspaces[0].LatestBuild.Status, "should be the first item in the list because it is running") - require.Equal(t, codersdk.WorkspaceStatusRunning, workspacesResponse.Workspaces[1].LatestBuild.Status) - require.Equal(t, codersdk.WorkspaceStatusStopped, workspacesResponse.Workspaces[2].LatestBuild.Status, "The stopped workspace should be last") - - require.Equal(t, workspace3.ID, workspacesResponse.Workspaces[0].ID, "If both are running, and have the same owner, sort by last used (in this case, the last one created)") + // the correct sorting order is: + // 1. Running workspaces + // 2. Sort by usernames + // 3. Sort by workspace names + require.Equal(t, 3, workspacesResponse.Count) + require.Equal(t, workspace3.Name, workspaces[0].Name) + require.Equal(t, workspace1.Name, workspaces[1].Name) + require.Equal(t, workspace2.Name, workspaces[2].Name) } func TestPostWorkspacesByOrganization(t *testing.T) { From eee8c982baa150ba11e377ec70e48a4893fc1d1f Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 May 2023 18:04:19 +0200 Subject: [PATCH 5/6] More fixes --- coderd/database/dbfake/databasefake.go | 16 ++++++++++------ coderd/workspaces_test.go | 23 +++++++++++++++-------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index f02ba967edd2a..c2e42f5c89fc7 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "reflect" "regexp" @@ -1340,22 +1341,25 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. for _, w := range workspaces { build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID) - if err != nil { + if err == nil { + preloadedWorkspaceBuilds[w.ID] = build + } else if !errors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get latest build: %w", err) } - preloadedWorkspaceBuilds[w.ID] = build job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID) - if err != nil { + if err == nil { + preloadedProvisionerJobs[w.ID] = job + } else if !errors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get provisioner job: %w", err) } - preloadedProvisionerJobs[w.ID] = job user, err := q.getUserByIDNoLock(w.OwnerID) - if err != nil { + if err == nil { + preloadedUsers[w.ID] = user + } else if !errors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get user: %w", err) } - preloadedUsers[w.ID] = user } sort.Slice(workspaces, func(i, j int) bool { diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index df880a9475b42..7218792ac837b 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -207,13 +207,13 @@ func TestWorkspacesSortOrder(t *testing.T) { coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID) - // Workspace 1 should be running + // c-workspace should be running workspace1 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { ctr.Name = "c-workspace" }) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace1.LatestBuild.ID) - // Workspace 2 should be stopped + // b-workspace should be stopped workspace2 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { ctr.Name = "b-workspace" }) @@ -222,7 +222,7 @@ func TestWorkspacesSortOrder(t *testing.T) { build2 := coderdtest.CreateWorkspaceBuild(t, client, workspace2, database.WorkspaceTransitionStop) coderdtest.AwaitWorkspaceBuildJob(t, client, build2.ID) - // Workspace 3 should be running + // a-workspace should be running workspace3 := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID, func(ctr *codersdk.CreateWorkspaceRequest) { ctr.Name = "a-workspace" }) @@ -230,19 +230,26 @@ func TestWorkspacesSortOrder(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - workspacesResponse, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err, "(first) fetch workspaces") workspaces := workspacesResponse.Workspaces + expected := []string{ + workspace3.Name, + workspace1.Name, + workspace2.Name, + } + + var actual []string + for _, w := range workspaces { + actual = append(actual, w.Name) + } + // the correct sorting order is: // 1. Running workspaces // 2. Sort by usernames // 3. Sort by workspace names - require.Equal(t, 3, workspacesResponse.Count) - require.Equal(t, workspace3.Name, workspaces[0].Name) - require.Equal(t, workspace1.Name, workspaces[1].Name) - require.Equal(t, workspace2.Name, workspaces[2].Name) + require.Equal(t, expected, actual) } func TestPostWorkspacesByOrganization(t *testing.T) { From cbfc6787defbc4916c92c9d0aa494f5e35984099 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 25 May 2023 18:06:44 +0200 Subject: [PATCH 6/6] sql fmt --- coderd/database/queries.sql.go | 8 ++++---- coderd/database/queries/workspaces.sql | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 91e8a85f7e5b0..de1a7231c3a8a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8337,10 +8337,10 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces -- @authorize_filter ORDER BY - (latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'start'::workspace_transition) DESC, + (latest_build.completed_at IS NOT NULL AND + latest_build.canceled_at IS NULL AND + latest_build.error IS NULL AND + latest_build.transition = 'start'::workspace_transition) DESC, LOWER(users.username) ASC, LOWER(name) ASC LIMIT diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 3819dbd640e66..fda15b2a0f9dc 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -242,10 +242,10 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces -- @authorize_filter ORDER BY - (latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'start'::workspace_transition) DESC, + (latest_build.completed_at IS NOT NULL AND + latest_build.canceled_at IS NULL AND + latest_build.error IS NULL AND + latest_build.transition = 'start'::workspace_transition) DESC, LOWER(users.username) ASC, LOWER(name) ASC LIMIT