From 5d898452d50d6a51aefddbf4e41745ebb715ef59 Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 31 Oct 2022 14:56:22 +0000 Subject: [PATCH 1/7] chore: refactor workspace count to sinngle route --- cli/configssh.go | 6 +- cli/list.go | 8 +- cli/ssh.go | 6 +- coderd/coderd.go | 1 - coderd/database/databasefake/databasefake.go | 150 ------------------ coderd/database/modelqueries.go | 21 --- coderd/database/querier.go | 2 - coderd/database/queries.sql.go | 154 ------------------- coderd/database/queries/workspaces.sql | 129 ---------------- coderd/templates_test.go | 8 +- coderd/users_test.go | 12 +- coderd/workspaceapps_test.go | 6 +- coderd/workspaces.go | 62 ++------ coderd/workspaces_test.go | 81 ++++------ codersdk/workspaces.go | 46 +----- site/src/api/typesGenerated.ts | 16 +- 16 files changed, 77 insertions(+), 631 deletions(-) diff --git a/cli/configssh.go b/cli/configssh.go index 373257403110d..000c0f8965bd0 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -70,7 +70,7 @@ type sshWorkspaceConfig struct { } func sshFetchWorkspaceConfigs(ctx context.Context, client *codersdk.Client) ([]sshWorkspaceConfig, error) { - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: codersdk.Me, }) if err != nil { @@ -78,8 +78,8 @@ func sshFetchWorkspaceConfigs(ctx context.Context, client *codersdk.Client) ([]s } var errGroup errgroup.Group - workspaceConfigs := make([]sshWorkspaceConfig, len(workspaces)) - for i, workspace := range workspaces { + workspaceConfigs := make([]sshWorkspaceConfig, len(res.Workspaces)) + for i, workspace := range res.Workspaces { i := i workspace := workspace errGroup.Go(func() error { diff --git a/cli/list.go b/cli/list.go index 4a136f8afbc0a..c06ed5922de25 100644 --- a/cli/list.go +++ b/cli/list.go @@ -91,11 +91,11 @@ func list() *cobra.Command { } filter.Owner = myUser.Username } - workspaces, err := client.Workspaces(cmd.Context(), filter) + res, err := client.Workspaces(cmd.Context(), filter) if err != nil { return err } - if len(workspaces) == 0 { + if len(res.Workspaces) == 0 { _, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Prompt.String()+"No workspaces found! Create one:") _, _ = fmt.Fprintln(cmd.ErrOrStderr()) _, _ = fmt.Fprintln(cmd.ErrOrStderr(), " "+cliui.Styles.Code.Render("coder create ")) @@ -112,8 +112,8 @@ func list() *cobra.Command { } now := time.Now() - displayWorkspaces = make([]workspaceListRow, len(workspaces)) - for i, workspace := range workspaces { + displayWorkspaces = make([]workspaceListRow, len(res.Workspaces)) + for i, workspace := range res.Workspaces { displayWorkspaces[i] = workspaceListRowFromWorkspace(now, usersByID, workspace) } diff --git a/cli/ssh.go b/cli/ssh.go index b4d4f6420da78..5e564c8302fd2 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -225,17 +225,17 @@ func getWorkspaceAndAgent(ctx context.Context, cmd *cobra.Command, client *coder err error ) if shuffle { - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: codersdk.Me, }) if err != nil { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, err } - if len(workspaces) == 0 { + if len(res.Workspaces) == 0 { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.New("no workspaces to shuffle") } - workspace, err = cryptorand.Element(workspaces) + workspace, err = cryptorand.Element(res.Workspaces) if err != nil { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, err } diff --git a/coderd/coderd.go b/coderd/coderd.go index 43ae621be144b..4a8c208561cb2 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -519,7 +519,6 @@ func New(options *Options) *API { apiKeyMiddleware, ) r.Get("/", api.workspaces) - r.Get("/count", api.workspaceCount) r.Route("/{workspace}", func(r chi.Router) { r.Use( httpmw.ExtractWorkspaceParam(options.Database), diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index c6900c316ec1d..c6bd1aabadb4b 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -790,156 +790,6 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. return workspaces, nil } -func (q *fakeQuerier) GetWorkspaceCount(ctx context.Context, arg database.GetWorkspaceCountParams) (int64, error) { - count, err := q.GetAuthorizedWorkspaceCount(ctx, arg, nil) - return count, err -} - -//nolint:gocyclo -func (q *fakeQuerier) GetAuthorizedWorkspaceCount(ctx context.Context, arg database.GetWorkspaceCountParams, authorizedFilter rbac.AuthorizeFilter) (int64, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - workspaces := make([]database.Workspace, 0) - for _, workspace := range q.workspaces { - if arg.OwnerID != uuid.Nil && workspace.OwnerID != arg.OwnerID { - continue - } - - if arg.OwnerUsername != "" { - owner, err := q.GetUserByID(ctx, workspace.OwnerID) - if err == nil && !strings.EqualFold(arg.OwnerUsername, owner.Username) { - continue - } - } - - if arg.TemplateName != "" { - template, err := q.GetTemplateByID(ctx, workspace.TemplateID) - if err == nil && !strings.EqualFold(arg.TemplateName, template.Name) { - continue - } - } - - if !arg.Deleted && workspace.Deleted { - continue - } - - if arg.Name != "" && !strings.Contains(strings.ToLower(workspace.Name), strings.ToLower(arg.Name)) { - continue - } - - if arg.Status != "" { - build, err := q.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID) - if err != nil { - return 0, xerrors.Errorf("get latest build: %w", err) - } - - job, err := q.GetProvisionerJobByID(ctx, build.JobID) - if err != nil { - return 0, xerrors.Errorf("get provisioner job: %w", err) - } - - switch arg.Status { - case "pending": - if !job.StartedAt.Valid { - continue - } - - case "starting": - if !job.StartedAt.Valid && - !job.CanceledAt.Valid && - job.CompletedAt.Valid && - time.Since(job.UpdatedAt) > 30*time.Second || - build.Transition != database.WorkspaceTransitionStart { - continue - } - - case "running": - if !job.CompletedAt.Valid && - job.CanceledAt.Valid && - job.Error.Valid || - build.Transition != database.WorkspaceTransitionStart { - continue - } - - case "stopping": - if !job.StartedAt.Valid && - !job.CanceledAt.Valid && - job.CompletedAt.Valid && - time.Since(job.UpdatedAt) > 30*time.Second || - build.Transition != database.WorkspaceTransitionStop { - continue - } - - case "stopped": - if !job.CompletedAt.Valid && - job.CanceledAt.Valid && - job.Error.Valid || - build.Transition != database.WorkspaceTransitionStop { - continue - } - - case "failed": - if (!job.CanceledAt.Valid && !job.Error.Valid) || - (!job.CompletedAt.Valid && !job.Error.Valid) { - continue - } - - case "canceling": - if !job.CanceledAt.Valid && job.CompletedAt.Valid { - continue - } - - case "canceled": - if !job.CanceledAt.Valid && !job.CompletedAt.Valid { - continue - } - - case "deleted": - if !job.StartedAt.Valid && - job.CanceledAt.Valid && - !job.CompletedAt.Valid && - time.Since(job.UpdatedAt) > 30*time.Second || - build.Transition != database.WorkspaceTransitionDelete { - continue - } - - case "deleting": - if !job.CompletedAt.Valid && - job.CanceledAt.Valid && - job.Error.Valid && - build.Transition != database.WorkspaceTransitionDelete { - continue - } - - default: - return 0, xerrors.Errorf("unknown workspace status in filter: %q", arg.Status) - } - } - - if len(arg.TemplateIds) > 0 { - match := false - for _, id := range arg.TemplateIds { - if workspace.TemplateID == id { - match = true - break - } - } - if !match { - continue - } - } - - // If the filter exists, ensure the object is authorized. - if authorizedFilter != nil && !authorizedFilter.Eval(workspace.RBACObject()) { - continue - } - workspaces = append(workspaces, workspace) - } - - return int64(len(workspaces)), nil -} - func (q *fakeQuerier) GetWorkspaceByID(_ context.Context, id uuid.UUID) (database.Workspace, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 56cd7ff074622..8ccbe5f14f6ca 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -112,7 +112,6 @@ func (q *sqlQuerier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([ type workspaceQuerier interface { GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) - GetAuthorizedWorkspaceCount(ctx context.Context, arg GetWorkspaceCountParams, authorizedFilter rbac.AuthorizeFilter) (int64, error) } // GetAuthorizedWorkspaces returns all workspaces that the user is authorized to access. @@ -167,23 +166,3 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa } return items, nil } - -func (q *sqlQuerier) GetAuthorizedWorkspaceCount(ctx context.Context, arg GetWorkspaceCountParams, authorizedFilter rbac.AuthorizeFilter) (int64, error) { - // In order to properly use ORDER BY, OFFSET, and LIMIT, we need to inject the - // authorizedFilter between the end of the where clause and those statements. - filter := strings.Replace(getWorkspaceCount, "-- @authorize_filter", fmt.Sprintf(" AND %s", authorizedFilter.SQLString(rbac.NoACLConfig())), 1) - // The name comment is for metric tracking - query := fmt.Sprintf("-- name: GetAuthorizedWorkspaceCount :one\n%s", filter) - row := q.db.QueryRowContext(ctx, query, - arg.Deleted, - arg.Status, - arg.OwnerID, - arg.OwnerUsername, - arg.TemplateName, - pq.Array(arg.TemplateIds), - arg.Name, - ) - var count int64 - err := row.Scan(&count) - return count, err -} diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 25ab45c21df96..ec145e06b253a 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -111,8 +111,6 @@ type sqlcQuerier interface { GetWorkspaceBuildsCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceBuild, error) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Workspace, error) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWorkspaceByOwnerIDAndNameParams) (Workspace, error) - // this duplicates the filtering in GetWorkspaces - GetWorkspaceCount(ctx context.Context, arg GetWorkspaceCountParams) (int64, error) GetWorkspaceCountByUserID(ctx context.Context, ownerID uuid.UUID) (int64, error) GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, ids []uuid.UUID) ([]GetWorkspaceOwnerCountsByTemplateIDsRow, error) GetWorkspaceResourceByID(ctx context.Context, id uuid.UUID) (WorkspaceResource, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 58f89cad9fd70..54e4f26970f56 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5871,160 +5871,6 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo return i, err } -const getWorkspaceCount = `-- name: GetWorkspaceCount :one -SELECT - COUNT(*) as count -FROM - workspaces -LEFT JOIN LATERAL ( - SELECT - workspace_builds.transition, - provisioner_jobs.started_at, - provisioner_jobs.updated_at, - provisioner_jobs.canceled_at, - provisioner_jobs.completed_at, - provisioner_jobs.error - FROM - workspace_builds - LEFT JOIN - provisioner_jobs - ON - provisioner_jobs.id = workspace_builds.job_id - WHERE - workspace_builds.workspace_id = workspaces.id - ORDER BY - build_number DESC - LIMIT - 1 -) latest_build ON TRUE -WHERE - -- Optionally include deleted workspaces - workspaces.deleted = $1 - AND CASE - WHEN $2 :: text != '' THEN - CASE - WHEN $2 = 'pending' THEN - latest_build.started_at IS NULL - WHEN $2 = 'starting' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'start'::workspace_transition - - WHEN $2 = 'running' THEN - 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 - - WHEN $2 = 'stopping' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'stop'::workspace_transition - - WHEN $2 = 'stopped' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'stop'::workspace_transition - - WHEN $2 = 'failed' THEN - (latest_build.canceled_at IS NOT NULL AND - latest_build.error IS NOT NULL) OR - (latest_build.completed_at IS NOT NULL AND - latest_build.error IS NOT NULL) - - WHEN $2 = 'canceling' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NULL - - WHEN $2 = 'canceled' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NOT NULL - - WHEN $2 = 'deleted' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NOT NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'delete'::workspace_transition - - WHEN $2 = 'deleting' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'delete'::workspace_transition - - ELSE - true - END - ELSE true - END - -- Filter by owner_id - AND CASE - WHEN $3 :: uuid != '00000000-00000000-00000000-00000000' THEN - owner_id = $3 - ELSE true - END - -- Filter by owner_name - AND CASE - WHEN $4 :: text != '' THEN - owner_id = (SELECT id FROM users WHERE lower(username) = lower($4) AND deleted = false) - ELSE true - END - -- Filter by template_name - -- There can be more than 1 template with the same name across organizations. - -- Use the organization filter to restrict to 1 org if needed. - AND CASE - WHEN $5 :: text != '' THEN - template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($5) AND deleted = false) - ELSE true - END - -- Filter by template_ids - AND CASE - WHEN array_length($6 :: uuid[], 1) > 0 THEN - template_id = ANY($6) - ELSE true - END - -- Filter by name, matching on substring - AND CASE - WHEN $7 :: text != '' THEN - name ILIKE '%' || $7 || '%' - ELSE true - END - -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaceCount - -- @authorize_filter -` - -type GetWorkspaceCountParams struct { - Deleted bool `db:"deleted" json:"deleted"` - Status string `db:"status" json:"status"` - OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` - OwnerUsername string `db:"owner_username" json:"owner_username"` - TemplateName string `db:"template_name" json:"template_name"` - TemplateIds []uuid.UUID `db:"template_ids" json:"template_ids"` - Name string `db:"name" json:"name"` -} - -// this duplicates the filtering in GetWorkspaces -func (q *sqlQuerier) GetWorkspaceCount(ctx context.Context, arg GetWorkspaceCountParams) (int64, error) { - row := q.db.QueryRowContext(ctx, getWorkspaceCount, - arg.Deleted, - arg.Status, - arg.OwnerID, - arg.OwnerUsername, - arg.TemplateName, - pq.Array(arg.TemplateIds), - arg.Name, - ) - var count int64 - err := row.Scan(&count) - return count, err -} - const getWorkspaceCountByUserID = `-- name: GetWorkspaceCountByUserID :one SELECT COUNT(id) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 15e35391f7ed7..6e7f0436dbff8 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -145,135 +145,6 @@ OFFSET @offset_ ; --- this duplicates the filtering in GetWorkspaces --- name: GetWorkspaceCount :one -SELECT - COUNT(*) as count -FROM - workspaces -LEFT JOIN LATERAL ( - SELECT - workspace_builds.transition, - provisioner_jobs.started_at, - provisioner_jobs.updated_at, - provisioner_jobs.canceled_at, - provisioner_jobs.completed_at, - provisioner_jobs.error - FROM - workspace_builds - LEFT JOIN - provisioner_jobs - ON - provisioner_jobs.id = workspace_builds.job_id - WHERE - workspace_builds.workspace_id = workspaces.id - ORDER BY - build_number DESC - LIMIT - 1 -) latest_build ON TRUE -WHERE - -- Optionally include deleted workspaces - workspaces.deleted = @deleted - AND CASE - WHEN @status :: text != '' THEN - CASE - WHEN @status = 'pending' THEN - latest_build.started_at IS NULL - WHEN @status = 'starting' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'start'::workspace_transition - - WHEN @status = 'running' THEN - 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 - - WHEN @status = 'stopping' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'stop'::workspace_transition - - WHEN @status = 'stopped' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'stop'::workspace_transition - - WHEN @status = 'failed' THEN - (latest_build.canceled_at IS NOT NULL AND - latest_build.error IS NOT NULL) OR - (latest_build.completed_at IS NOT NULL AND - latest_build.error IS NOT NULL) - - WHEN @status = 'canceling' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NULL - - WHEN @status = 'canceled' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NOT NULL - - WHEN @status = 'deleted' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NOT NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'delete'::workspace_transition - - WHEN @status = 'deleting' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'delete'::workspace_transition - - ELSE - true - END - ELSE true - END - -- Filter by owner_id - AND CASE - WHEN @owner_id :: uuid != '00000000-00000000-00000000-00000000' THEN - owner_id = @owner_id - ELSE true - END - -- Filter by owner_name - AND CASE - WHEN @owner_username :: text != '' THEN - owner_id = (SELECT id FROM users WHERE lower(username) = lower(@owner_username) AND deleted = false) - ELSE true - END - -- Filter by template_name - -- There can be more than 1 template with the same name across organizations. - -- Use the organization filter to restrict to 1 org if needed. - AND CASE - WHEN @template_name :: text != '' THEN - template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower(@template_name) AND deleted = false) - ELSE true - END - -- Filter by template_ids - AND CASE - WHEN array_length(@template_ids :: uuid[], 1) > 0 THEN - template_id = ANY(@template_ids) - ELSE true - END - -- Filter by name, matching on substring - AND CASE - WHEN @name :: text != '' THEN - name ILIKE '%' || @name || '%' - ELSE true - END - -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaceCount - -- @authorize_filter -; - -- name: GetWorkspaceByOwnerIDAndName :one SELECT * diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 885d683fb0b46..d7825725ec1a9 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -621,9 +621,9 @@ func TestTemplateMetrics(t *testing.T) { Entries: []codersdk.DAUEntry{}, }, daus, "no DAUs when stats are empty") - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) - assert.Zero(t, workspaces[0].LastUsedAt) + assert.Zero(t, res.Workspaces[0].LastUsedAt) conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{ Logger: slogtest.Make(t, nil).Named("tailnet"), @@ -672,9 +672,9 @@ func TestTemplateMetrics(t *testing.T) { "BuildTimeStats never loaded", ) - workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) + res, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) assert.WithinDuration(t, - database.Now(), workspaces[0].LastUsedAt, time.Minute, + database.Now(), res.Workspaces[0].LastUsedAt, time.Minute, ) } diff --git a/coderd/users_test.go b/coderd/users_test.go index bb55b909faf5b..b7688d6ce8587 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1279,11 +1279,11 @@ func TestWorkspacesByUser(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: codersdk.Me, }) require.NoError(t, err) - require.Len(t, workspaces, 0) + require.Len(t, res.Workspaces, 0) }) t.Run("Access", func(t *testing.T) { t.Parallel() @@ -1313,13 +1313,13 @@ func TestWorkspacesByUser(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - workspaces, err := newUserClient.Workspaces(ctx, codersdk.WorkspaceFilter{Owner: codersdk.Me}) + res, err := newUserClient.Workspaces(ctx, codersdk.WorkspaceFilter{Owner: codersdk.Me}) require.NoError(t, err) - require.Len(t, workspaces, 0) + require.Len(t, res.Workspaces, 0) - workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{Owner: codersdk.Me}) + res, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{Owner: codersdk.Me}) require.NoError(t, err) - require.Len(t, workspaces, 1) + require.Len(t, res.Workspaces, 1) }) } diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index c37f8f5c5f574..c9f29bd84aa28 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -618,11 +618,11 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { me, err := client.User(ctx, codersdk.Me) require.NoError(t, err, "get current user details") - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: codersdk.Me, }) require.NoError(t, err, "get workspaces") - require.Len(t, workspaces, 1, "expected 1 workspace") + require.Len(t, res.Workspaces, 1, "expected 1 workspace") appHost, err := client.GetAppHost(ctx) require.NoError(t, err, "get app host") @@ -631,7 +631,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { AppSlug: appName, Port: port, AgentName: proxyTestAgentName, - WorkspaceName: workspaces[0].Name, + WorkspaceName: res.Workspaces[0].Name, Username: me.Username, }.String() diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 0295dc29d5e56..559cdbaab3e3d 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -136,76 +136,40 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { return } - data, err := api.workspaceData(ctx, workspaces) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace resources.", - Detail: err.Error(), - }) - return - } - - wss, err := convertWorkspaces(workspaces, data) + // run the query again to get the total count for frontend pagination + filter.Offset = 0 + filter.Limit = 0 + all, err := api.Database.GetAuthorizedWorkspaces(ctx, filter, sqlFilter) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error converting workspaces.", + Message: "Internal error fetching workspaces.", Detail: err.Error(), }) return } + count := len(all) - httpapi.Write(ctx, rw, http.StatusOK, wss) -} - -func (api *API) workspaceCount(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - apiKey := httpmw.APIKey(r) - - queryStr := r.URL.Query().Get("q") - filter, errs := workspaceSearchQuery(queryStr, codersdk.Pagination{}) - if len(errs) > 0 { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid audit search query.", - Validations: errs, - }) - return - } - - if filter.OwnerUsername == "me" { - filter.OwnerID = apiKey.UserID - filter.OwnerUsername = "" - } - - sqlFilter, err := api.HTTPAuth.AuthorizeSQLFilter(r, rbac.ActionRead, rbac.ResourceWorkspace.Type) + data, err := api.workspaceData(ctx, workspaces) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error preparing sql filter.", + Message: "Internal error fetching workspace resources.", Detail: err.Error(), }) return } - countFilter := database.GetWorkspaceCountParams{ - Deleted: filter.Deleted, - OwnerUsername: filter.OwnerUsername, - OwnerID: filter.OwnerID, - Name: filter.Name, - Status: filter.Status, - TemplateIds: filter.TemplateIds, - TemplateName: filter.TemplateName, - } - - count, err := api.Database.GetAuthorizedWorkspaceCount(ctx, countFilter, sqlFilter) + wss, err := convertWorkspaces(workspaces, data) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace count.", + Message: "Internal error converting workspaces.", Detail: err.Error(), }) return } - httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceCountResponse{ - Count: count, + httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspacesResponse{ + Workspaces: wss, + Count: count, }) } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 06d98ce87352d..a9cbd666247b3 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -143,7 +143,7 @@ func TestAdminViewAllWorkspaces(t *testing.T) { firstWorkspaces, err := other.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err, "(first) fetch workspaces") - require.ElementsMatch(t, otherWorkspaces, firstWorkspaces) + require.ElementsMatch(t, otherWorkspaces.Workspaces, firstWorkspaces.Workspaces) } func TestPostWorkspacesByOrganization(t *testing.T) { @@ -662,27 +662,27 @@ func TestWorkspaceFilterManual(t *testing.T) { defer cancel() // full match - ws, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Name: workspace.Name, }) require.NoError(t, err) - require.Len(t, ws, 1, workspace.Name) - require.Equal(t, workspace.ID, ws[0].ID) + require.Len(t, res.Workspaces, 1, workspace.Name) + require.Equal(t, workspace.ID, res.Workspaces[0].ID) // partial match - ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ Name: workspace.Name[1 : len(workspace.Name)-2], }) require.NoError(t, err) - require.Len(t, ws, 1) - require.Equal(t, workspace.ID, ws[0].ID) + require.Len(t, res.Workspaces, 1) + require.Equal(t, workspace.ID, res.Workspaces[0].ID) // no match - ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ Name: "$$$$", }) require.NoError(t, err) - require.Len(t, ws, 0) + require.Len(t, res.Workspaces, 0) }) t.Run("Template", func(t *testing.T) { t.Parallel() @@ -699,17 +699,17 @@ func TestWorkspaceFilterManual(t *testing.T) { defer cancel() // empty - ws, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) - require.Len(t, ws, 2) + require.Len(t, res.Workspaces, 2) // single template - ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ Template: template.Name, }) require.NoError(t, err) - require.Len(t, ws, 1) - require.Equal(t, workspace.ID, ws[0].ID) + require.Len(t, res.Workspaces, 1) + require.Equal(t, workspace.ID, res.Workspaces[0].ID) }) t.Run("Status", func(t *testing.T) { t.Parallel() @@ -732,7 +732,7 @@ func TestWorkspaceFilterManual(t *testing.T) { // filter finds both running workspaces ws1, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) - require.Len(t, ws1, 2) + require.Len(t, ws1.Workspaces, 2) // stop workspace1 build1 := coderdtest.CreateWorkspaceBuild(t, client, workspace1, database.WorkspaceTransitionStop) @@ -743,8 +743,8 @@ func TestWorkspaceFilterManual(t *testing.T) { Status: "running", }) require.NoError(t, err) - require.Len(t, ws2, 1) - require.Equal(t, workspace2.ID, ws2[0].ID) + require.Len(t, ws2.Workspaces, 1) + require.Equal(t, workspace2.ID, ws2.Workspaces[0].ID) // stop workspace2 build2 := coderdtest.CreateWorkspaceBuild(t, client, workspace2, database.WorkspaceTransitionStop) @@ -755,7 +755,7 @@ func TestWorkspaceFilterManual(t *testing.T) { Status: "running", }) require.NoError(t, err) - require.Len(t, ws3, 0) + require.Len(t, ws3.Workspaces, 0) }) t.Run("FilterQuery", func(t *testing.T) { t.Parallel() @@ -772,12 +772,12 @@ func TestWorkspaceFilterManual(t *testing.T) { defer cancel() // single workspace - ws, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ FilterQuery: fmt.Sprintf("template:%s %s/%s", template.Name, workspace.OwnerName, workspace.Name), }) require.NoError(t, err) - require.Len(t, ws, 1) - require.Equal(t, workspace.ID, ws[0].ID) + require.Len(t, res.Workspaces, 1) + require.Equal(t, workspace.ID, res.Workspaces[0].ID) }) } @@ -797,14 +797,14 @@ func TestOffsetLimit(t *testing.T) { // empty finds all workspaces ws, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) - require.Len(t, ws, 3) + require.Len(t, ws.Workspaces, 3) // offset 1 finds 2 workspaces ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ Offset: 1, }) require.NoError(t, err) - require.Len(t, ws, 2) + require.Len(t, ws.Workspaces, 2) // offset 1 limit 1 finds 1 workspace ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ @@ -812,41 +812,14 @@ func TestOffsetLimit(t *testing.T) { Limit: 1, }) require.NoError(t, err) - require.Len(t, ws, 1) + require.Len(t, ws.Workspaces, 1) // offset 3 finds no workspaces ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ Offset: 3, }) require.NoError(t, err) - require.Len(t, ws, 0) -} - -func TestWorkspaceCount(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) - user := coderdtest.CreateFirstUser(t, client) - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - template2 := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template2.ID) - _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template2.ID) - - response, err := client.WorkspaceCount(ctx, codersdk.WorkspaceCountRequest{}) - require.NoError(t, err, "fetch workspace count") - // counts all - require.Equal(t, int(response.Count), 3) - - response2, err2 := client.WorkspaceCount(ctx, codersdk.WorkspaceCountRequest{ - SearchQuery: fmt.Sprintf("template:%s", template.Name), - }) - require.NoError(t, err2, "fetch workspace count") - // counts only those that pass filter - require.Equal(t, int(response2.Count), 1) + require.Len(t, ws.Workspaces, 0) } func TestPostWorkspaceBuild(t *testing.T) { @@ -990,11 +963,11 @@ func TestPostWorkspaceBuild(t *testing.T) { require.Equal(t, workspace.LatestBuild.BuildNumber+1, build.BuildNumber) coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: user.UserID.String(), }) require.NoError(t, err) - require.Len(t, workspaces, 0) + require.Len(t, res.Workspaces, 0) }) } diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 69d287a595ca6..6852159ed8fe8 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -36,11 +36,9 @@ type WorkspacesRequest struct { Pagination } -type WorkspaceCountRequest struct { - SearchQuery string `json:"q,omitempty"` -} -type WorkspaceCountResponse struct { - Count int64 `json:"count"` +type WorkspacesResponse struct { + Workspaces []Workspace `json:"workspaces"` + Count int `json:"count"` } // CreateWorkspaceBuildRequest provides options to update the latest workspace build. @@ -305,51 +303,23 @@ func (f WorkspaceFilter) asRequestOption() RequestOption { } // Workspaces returns all workspaces the authenticated user has access to. -func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) ([]Workspace, error) { +func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) (WorkspacesResponse, error) { page := Pagination{ Offset: filter.Offset, Limit: filter.Limit, } res, err := c.Request(ctx, http.MethodGet, "/api/v2/workspaces", nil, filter.asRequestOption(), page.asRequestOption()) if err != nil { - return nil, err - } - defer res.Body.Close() - - if res.StatusCode != http.StatusOK { - return nil, readBodyAsError(res) - } - - var workspaces []Workspace - return workspaces, json.NewDecoder(res.Body).Decode(&workspaces) -} - -func (c *Client) WorkspaceCount(ctx context.Context, req WorkspaceCountRequest) (WorkspaceCountResponse, error) { - res, err := c.Request(ctx, http.MethodGet, "/api/v2/workspaces/count", nil, func(r *http.Request) { - q := r.URL.Query() - var params []string - if req.SearchQuery != "" { - params = append(params, req.SearchQuery) - } - q.Set("q", strings.Join(params, " ")) - r.URL.RawQuery = q.Encode() - }) - if err != nil { - return WorkspaceCountResponse{}, err + return WorkspacesResponse{}, err } defer res.Body.Close() if res.StatusCode != http.StatusOK { - return WorkspaceCountResponse{}, readBodyAsError(res) - } - - var countRes WorkspaceCountResponse - err = json.NewDecoder(res.Body).Decode(&countRes) - if err != nil { - return WorkspaceCountResponse{}, err + return WorkspacesResponse{}, readBodyAsError(res) } - return countRes, nil + var wres WorkspacesResponse + return wres, json.NewDecoder(res.Body).Decode(&wres) } // WorkspaceByOwnerAndName returns a workspace by the owner's UUID and the workspace's name. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6b8affde3d827..361c44f5108c1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -860,16 +860,6 @@ export interface WorkspaceBuildsRequest extends Pagination { readonly Since: string } -// From codersdk/workspaces.go -export interface WorkspaceCountRequest { - readonly q?: string -} - -// From codersdk/workspaces.go -export interface WorkspaceCountResponse { - readonly count: number -} - // From codersdk/workspaces.go export interface WorkspaceFilter { readonly q?: string @@ -912,6 +902,12 @@ export interface WorkspacesRequest extends Pagination { readonly q?: string } +// From codersdk/workspaces.go +export interface WorkspacesResponse { + readonly workspaces: Workspace[] + readonly count: number +} + // From codersdk/apikey.go export type APIKeyScope = "all" | "application_connect" From 7cf3058983dcd646f346da7d8595240937e16d5c Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 31 Oct 2022 15:07:06 +0000 Subject: [PATCH 2/7] fix authorize test --- coderd/coderdtest/authorize.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 516b96809a049..615cc72a7d485 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -243,8 +243,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { "POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, // Endpoints that use the SQLQuery filter. - "GET:/api/v2/workspaces/": {StatusCode: http.StatusOK, NoAuthorize: true}, - "GET:/api/v2/workspaces/count": {StatusCode: http.StatusOK, NoAuthorize: true}, + "GET:/api/v2/workspaces/": {StatusCode: http.StatusOK, NoAuthorize: true}, } // Routes like proxy routes support all HTTP methods. A helper func to expand From 16569f9bcc5667def5c3f534bd978ff8eb5b340e Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 31 Oct 2022 16:02:38 +0000 Subject: [PATCH 3/7] frontend --- site/src/api/api.ts | 12 +--- site/src/testHelpers/entities.ts | 4 -- .../workspaces/workspacesXService.ts | 60 ++++--------------- 3 files changed, 12 insertions(+), 64 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 3804e83aed2aa..e92efa1a799ef 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -290,17 +290,9 @@ export const getURLWithSearchParams = ( export const getWorkspaces = async ( options: TypesGen.WorkspacesRequest, -): Promise => { +): Promise => { const url = getURLWithSearchParams("/api/v2/workspaces", options) - const response = await axios.get(url) - return response.data -} - -export const getWorkspacesCount = async ( - options: TypesGen.WorkspaceCountRequest, -): Promise => { - const url = getURLWithSearchParams("/api/v2/workspaces/count", options) - const response = await axios.get(url) + const response = await axios.get(url) return response.data } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index f0f51152b963b..07447dadbd18e 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -517,10 +517,6 @@ export const MockWorkspaceRequest: TypesGen.CreateWorkspaceRequest = { template_id: "test-template", } -export const MockWorkspaceCountResponse: TypesGen.WorkspaceCountResponse = { - count: 26, // just over 1 page -} - export const MockUserAgent: Types.UserAgent = { browser: "Chrome 99.0.4844", device: "Other", diff --git a/site/src/xServices/workspaces/workspacesXService.ts b/site/src/xServices/workspaces/workspacesXService.ts index 92dfe7adb01af..e16a8fae2cb54 100644 --- a/site/src/xServices/workspaces/workspacesXService.ts +++ b/site/src/xServices/workspaces/workspacesXService.ts @@ -235,10 +235,7 @@ export const workspacesMachine = events: {} as WorkspacesEvent, services: {} as { getWorkspaces: { - data: TypesGen.Workspace[] - } - getWorkspacesCount: { - data: { count: number } + data: TypesGen.WorkspacesResponse } updateWorkspaceRefs: { data: { @@ -255,39 +252,7 @@ export const workspacesMachine = actions: "triggerUpdateVersion", }, }, - type: "parallel", states: { - count: { - initial: "gettingCount", - states: { - idle: {}, - gettingCount: { - entry: "clearGetCountError", - invoke: { - src: "getWorkspacesCount", - id: "getWorkspacesCount", - onDone: [ - { - target: "idle", - actions: "assignCount", - }, - ], - onError: [ - { - target: "idle", - actions: "assignGetCountError", - }, - ], - }, - }, - }, - on: { - UPDATE_FILTER: { - target: ".gettingCount", - actions: ["assignFilter", "sendResetPage"], - }, - }, - }, workspaces: { initial: "startingPagination", states: { @@ -311,6 +276,9 @@ export const workspacesMachine = { target: "updatingWorkspaceRefs", }, + { + actions: "assignCount", + }, ], onError: [ { @@ -358,10 +326,13 @@ export const workspacesMachine = actions: { assignWorkspaceRefs: assign({ workspaceRefs: (_, event) => - event.data.map((data) => { + event.data.workspaces.map((data) => { return spawn(workspaceItemMachine.withContext({ data }), data.id) }), }), + assignCount: assign({ + count: (_, event) => event.data.count, + }), assignPaginationRef: assign({ paginationRef: (context) => spawn( @@ -403,15 +374,6 @@ export const workspacesMachine = return event.data.refsToKeep.concat(newWorkspaceRefs) }, }), - assignCount: assign({ - count: (_, event) => event.data.count, - }), - assignGetCountError: assign({ - getCountError: (_, event) => event.data, - }), - clearGetCountError: assign({ - getCountError: (_) => undefined, - }), }, services: { getWorkspaces: (context) => { @@ -429,7 +391,7 @@ export const workspacesMachine = updateWorkspaceRefs: (context, event) => { const refsToKeep: WorkspaceItemMachineRef[] = [] context.workspaceRefs?.forEach((ref) => { - const matchingWorkspace = event.data.find( + const matchingWorkspace = event.data.workspaces.find( (workspace) => ref.id === workspace.id, ) if (matchingWorkspace) { @@ -443,7 +405,7 @@ export const workspacesMachine = } }) - const newWorkspaces = event.data.filter( + const newWorkspaces = event.data.workspaces.filter( (workspace) => !context.workspaceRefs?.find((ref) => ref.id === workspace.id), ) @@ -453,8 +415,6 @@ export const workspacesMachine = newWorkspaces, }) }, - getWorkspacesCount: (context) => - API.getWorkspacesCount({ q: context.filter }), }, }, ) From 7a38059c4a26a01dd7a3c01a251ef7ce1b713d1e Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 31 Oct 2022 16:08:53 +0000 Subject: [PATCH 4/7] fix js test --- site/src/testHelpers/handlers.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 599e6d88a00d3..d881449cbcb64 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -140,10 +140,6 @@ export const handlers = [ rest.get("/api/v2/workspaces", async (req, res, ctx) => { return res(ctx.status(200), ctx.json([M.MockWorkspace])) }), - // has to come before the parameterized endpoints - rest.get("/api/v2/workspaces/count", async (req, res, ctx) => { - return res(ctx.status(200), ctx.json(M.MockWorkspaceCountResponse)) - }), rest.get("/api/v2/workspaces/:workspaceId", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockWorkspace)) }), From 79a2a7177c4e8633ba31743e0c02febb9d0abc39 Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 8 Nov 2022 19:46:36 +0000 Subject: [PATCH 5/7] Fix xservice --- .../workspaces/workspacesXService.ts | 109 +++++++++--------- 1 file changed, 52 insertions(+), 57 deletions(-) diff --git a/site/src/xServices/workspaces/workspacesXService.ts b/site/src/xServices/workspaces/workspacesXService.ts index e16a8fae2cb54..f7959dcf57df1 100644 --- a/site/src/xServices/workspaces/workspacesXService.ts +++ b/site/src/xServices/workspaces/workspacesXService.ts @@ -226,7 +226,7 @@ type WorkspacesEvent = | { type: "UPDATE_FILTER"; query?: string } export const workspacesMachine = - /** @xstate-layout N4IgpgJg5mDOIC5QHcD2AnA1rADgQwGM4BlAFz1LADoDUBXAO1KplNIEsGoBhepgYgioG1TgDdUmaqwDqGbPiKxejUgG0ADAF1EoHKljsOw3SAAeiAIyWNVAMwA2AJwAOACyWnAJjsuvGty8AGhAAT0QfWxcAVm9ohwcXAHYkp0sXFwBfTJC0LFxCEnJKGj5mVg4uFQEwdHQMKhwAGwoAMwwAWxYwUjl8xThq9W1TfUNjBlMLBDsnaKovaMtfDQ00jLckhxDwhEiqGO9-OxPZk69s3PkCpTIKalpVfgBVAAUAEQBBABUAUQB9ABiAEkADJ-ABKmh0SBAYyM7BMsOmbm8VA00Q0KRcli2di86TcOwiDioDk8DjcMS8Lm8dg0LgclxAeQUhVgdxKrJucCosHI6EqUFeeCgnAoiIY-GhowMCKRoGmAFocU57J4vG58VSMqliQglR4FmsvF5sX5XMloszuQMOcVqLb2d02JwoH02UpBMJRAwJFIXR6ebAZbD4RMpoglfF5gSGWt8ZYdZZ9ekXFQktEXI58dn3B4LjkWdc7ZzHSXnRU3UG7d6RFRxJJpD0a+y1JYYXo5RHkVHGZYqE43AEYgnnNE7PqaUl0dFTU4NA5Ui41m4bRXbg6qE6lC6ha2vbV6uhGi1SO10F1ZBu4KGu+NJZGDdFAmTacscTSnLn9RkFvS-CTKlNmia0ix3Ip7m3G9YCoOgcAgCUuAPMAITAVpYDrX1-WoeDEMoFC0Iwu84W7R9ewNelSQ0JNyQ0Al4hsE59XOKg3GHId0gcHwkgyMCrn6dky2gwTd2QPAjG+VAiPQOAAAsUMwsx+SgvBWkodAAApMVWABKfgIPtKDDO3CTSCkmT5MUkjw3IxUoznGdVksHw7FA1xZicFi1QcRccVpAleIcF911EyCuRgl4Ph+AFXk+ABxX4bLIhVzAc78DjsawHDc7wdW2MIo2WNVeNNTMaTNLLF1Cz1wrAKKvj+f4ADVfghYhgQAeQAOWSh9UumWYM2cWjVixdxJ0Kg1NXmXzEhcjF3Bo7IiwYVAIDgUxDOEx4mAbCAmjAWV+smCj-HTLK50xZxFg0WZgimxwSrcSkkheuY3GiJJCwE2qjJKXbyh6IUhmO+VTvshBuIORJKQJHx4esKcfOOfwEi-OxUhq4MdrKMGe0hpMYcZQIXMTAkXP1JVoZWNyGSTQkMX44swv+8tWb5AUhRFMUGAlVLbIGvtGSoSxYj8LNPretIqaTWwvG-JwcvYrxfJfH6Wb+4STKrZCYPxuy0oNWlYxiNyTi1fx0inUlYnJJwHY8dwJ3ibHSy3Ey8KQ90byI+AwxSiGjaVJJrCoJYGUCF9s3xFjUTJD7VdcdItjFt2hI9mDTMk6T0Nk2AFP1gOTqfJU3IHb7UmxSI3qxFi7AWKuFbSBImacdPN2Mov73B0uhwbmkXYt-EaJcKn5wWRXuN4r6XNAju6oNoWDQdgdB-NuxLdHqmsvTBXZm4lyMgpJkVqAA */ + /** @xstate-layout N4IgpgJg5mDOIC5QHcD2AnA1rADgQwGM4BlAFz1LADpZz1SBLAOygAU8pmKHUmBiANoAGALqJQOVLAaNe4kAA9EAWgAcANnVUAzAEYATOoCcAdiFHt+obtUAaEAE9Euo6qo2LR0y-3bV+1QBfQPs0LFxCEnJKKhhSRhYAdQxsfCJYPgheamYAN1RMajjk8LS4YTEkEElpWSZ5JQRlAFYjABYdA1UjXVbmtu1teycENqN9KhNm5tVVbXU5nvHm4NCUiPSyCiKweOYoEtTIjKymHKZ8wtjdw43y3UqJKRkeeqrG5X02rT11XSEAiYBl4TMNEG1+lQhEJ1FZtF4hM0TPpdKsQGEjptojs9kl1mUMmB0OgMFQcAAbCgAMwwAFtrqRbgSKvIai85O8VM11EIdNMNEJtBCBkNHODoVQAtChCZ4eYbNo0Ri7rAtjEAK44CDcPGlSIAJTAVJO2SoeQK1E12soTINRtgLKqbLqDRU+n0RioRn6JhMqiRQjaQn96jBCBszSozX0zW0rSMxnUst0ipC6PxxzV1GQeBkABVUIaqeg4AALW3pPgKWjbKh4KmUdAACma0oAlHxlQSs1Qc-nC0aS7Byxn0o6nrVXq6mtzPULevDdEm2kHQWKEB6Jt045ZoUn-uo2krR1FtnwAKqsAAiAEE8wBRAD6ADV7-riABJADyADlx9VnhdTkmksXRJgTBY-GMYNvTDZRek9TRej9XQDFcP0VjTLtM2xC9rzvJ9WBvABxe9-2dKdgOUWUtGjL5Az8QY-TsdcEyjVCTBsIEvB49Rjz1LEz0vW8H0fAAxD8ABkH31cjAMo0APn6Dpvhlf4E3+GNmjgpdtCoMYYVUAZuQ0JMgjRJhUAgOB5GwwSYhreh9nYTgmG4DkJ3ZN5FJULwOiFMw-SsVRrEFMNvS9Hiouikx+MxU8YjiBIDhPeAnXkjzFF8gYdERfwEy8PwUTDQw9KRZoU2hcZNDafRYqw1KeytHUUoEsAizSzygJ8poAjcKxvmMNpZW5VoSosL0iu6UrLGROKVR7PtSALIshxHNrOoAydMqUmYo00ExTCM-pBhK8woRTWFWh5WYgyPBqNqzVkMu8rKmn+WqdGGmV-GDULRRGLQotUXRfVBsx2kRYJgiAA */ createMachine( { tsTypes: {} as import("./workspacesXService.typegen").Typegen1, @@ -251,69 +251,64 @@ export const workspacesMachine = UPDATE_VERSION: { actions: "triggerUpdateVersion", }, + UPDATE_PAGE: { + target: "gettingWorkspaces", + actions: "updateURL", + }, + UPDATE_FILTER: { + actions: ["assignFilter", "sendResetPage"], + }, }, + initial: "startingPagination", states: { - workspaces: { - initial: "startingPagination", - states: { - startingPagination: { - entry: "assignPaginationRef", - always: { - target: "gettingWorkspaces", + startingPagination: { + entry: "assignPaginationRef", + always: { + target: "gettingWorkspaces", + }, + }, + gettingWorkspaces: { + entry: "clearGetWorkspacesError", + invoke: { + src: "getWorkspaces", + id: "getWorkspaces", + onDone: [ + { + target: "waitToRefreshWorkspaces", + cond: "isEmpty", + actions: ["assignWorkspaceRefs", "assignCount"], }, - }, - gettingWorkspaces: { - entry: "clearGetWorkspacesError", - invoke: { - src: "getWorkspaces", - id: "getWorkspaces", - onDone: [ - { - target: "waitToRefreshWorkspaces", - cond: "isEmpty", - actions: "assignWorkspaceRefs", - }, - { - target: "updatingWorkspaceRefs", - }, - { - actions: "assignCount", - }, - ], - onError: [ - { - target: "waitToRefreshWorkspaces", - actions: "assignGetWorkspacesError", - }, - ], + { + target: "updatingWorkspaceRefs", + actions: "assignCount", }, - }, - updatingWorkspaceRefs: { - invoke: { - src: "updateWorkspaceRefs", - id: "updateWorkspaceRefs", - onDone: [ - { - target: "waitToRefreshWorkspaces", - actions: "assignUpdatedWorkspaceRefs", - }, - ], + ], + onError: [ + { + target: "waitToRefreshWorkspaces", + actions: "assignGetWorkspacesError", }, - }, - waitToRefreshWorkspaces: { - after: { - "5000": { - target: "#workspacesState.workspaces.gettingWorkspaces", - actions: [], - internal: false, - }, + ], + }, + }, + updatingWorkspaceRefs: { + invoke: { + src: "updateWorkspaceRefs", + id: "updateWorkspaceRefs", + onDone: [ + { + target: "waitToRefreshWorkspaces", + actions: "assignUpdatedWorkspaceRefs", }, - }, + ], }, - on: { - UPDATE_PAGE: { - target: ".gettingWorkspaces", - actions: "updateURL", + }, + waitToRefreshWorkspaces: { + after: { + "5000": { + target: "gettingWorkspaces", + actions: [], + internal: false, }, }, }, From 0da0bafc9f230ea289f1b0d4c2ae2c42984e0a07 Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 8 Nov 2022 19:48:01 +0000 Subject: [PATCH 6/7] Fix js tests --- site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx | 4 ++-- site/src/testHelpers/entities.ts | 11 +++++++++++ site/src/testHelpers/handlers.ts | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 7ba6ef88ee4a4..40880407d99bf 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -19,7 +19,7 @@ describe("WorkspacesPage", () => { // Given server.use( rest.get("/api/v2/workspaces", async (req, res, ctx) => { - return res(ctx.status(200), ctx.json([])) + return res(ctx.status(200), ctx.json({ workspaces: [], count: 0 })) }), ) @@ -52,6 +52,6 @@ describe("WorkspacesPage", () => { }, { timeout: 2000 }, ) - await screen.findByText(MockWorkspace.name) + await screen.findByText(`${MockWorkspace.name}1`) }) }) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 07447dadbd18e..410e919e51747 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -2,6 +2,7 @@ import { FieldError } from "api/errors" import { everyOneGroup } from "util/groups" import * as Types from "../api/types" import * as TypesGen from "../api/typesGenerated" +import { range } from "lodash" export const MockTemplateDAUResponse: TypesGen.TemplateDAUsResponse = { entries: [ @@ -510,6 +511,16 @@ export const MockPendingWorkspace: TypesGen.Workspace = { }, } +// just over one page of workspaces +export const MockWorkspacesResponse: TypesGen.WorkspacesResponse = { + workspaces: range(1, 27).map((id: number) => ({ + ...MockWorkspace, + id: id.toString(), + name: `${MockWorkspace.name}${id}`, + })), + count: 26, +} + // requests the MockWorkspace export const MockWorkspaceRequest: TypesGen.CreateWorkspaceRequest = { name: "test", diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index d881449cbcb64..2cbe388659851 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -138,7 +138,7 @@ export const handlers = [ // workspaces rest.get("/api/v2/workspaces", async (req, res, ctx) => { - return res(ctx.status(200), ctx.json([M.MockWorkspace])) + return res(ctx.status(200), ctx.json(M.MockWorkspacesResponse)) }), rest.get("/api/v2/workspaces/:workspaceId", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockWorkspace)) From 86dade1216843db898acb0c665ca761038b45e0f Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 8 Nov 2022 20:54:42 +0000 Subject: [PATCH 7/7] Unpack workspaces response in load tests --- loadtest/workspacebuild/run_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/loadtest/workspacebuild/run_test.go b/loadtest/workspacebuild/run_test.go index 438c188fd83e7..ee6e7aad86dfe 100644 --- a/loadtest/workspacebuild/run_test.go +++ b/loadtest/workspacebuild/run_test.go @@ -107,12 +107,13 @@ func Test_Runner(t *testing.T) { go func() { var workspace codersdk.Workspace for { - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: codersdk.Me, }) if !assert.NoError(t, err) { return } + workspaces := res.Workspaces if len(workspaces) == 1 { workspace = workspaces[0] @@ -165,10 +166,11 @@ func Test_Runner(t *testing.T) { require.Contains(t, logsStr, `"agent3" is connected`) // Find the workspace. - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Owner: codersdk.Me, }) require.NoError(t, err) + workspaces := res.Workspaces require.Len(t, workspaces, 1) coderdtest.AwaitWorkspaceBuildJob(t, client, workspaces[0].LatestBuild.ID)