Skip to content

Commit 6bee180

Browse files
EmyrkEmyrk
authored
fix: Sort workspace by name by created_at (#2214)
* fix: Sort workspace by name by created_at Fix bug where deleting workspaces with the same name returns the oldest deleted workspace
1 parent 953e8c8 commit 6bee180

File tree

10 files changed

+60
-29
lines changed

10 files changed

+60
-29
lines changed

‎cli/create.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func create() *cobra.Command {
4949
workspaceName, err = cliui.Prompt(cmd, cliui.PromptOptions{
5050
Text: "Specify a name for your workspace:",
5151
Validate: func(workspaceName string) error {
52-
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceByOwnerAndNameParams{})
52+
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceOptions{})
5353
if err == nil {
5454
return xerrors.Errorf("A workspace already exists named %q!", workspaceName)
5555
}
@@ -61,7 +61,7 @@ func create() *cobra.Command {
6161
}
6262
}
6363

64-
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceByOwnerAndNameParams{})
64+
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceOptions{})
6565
if err == nil {
6666
return xerrors.Errorf("A workspace already exists named %q!", workspaceName)
6767
}

‎cli/root.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func namedWorkspace(cmd *cobra.Command, client *codersdk.Client, identifier stri
214214
return codersdk.Workspace{}, xerrors.Errorf("invalid workspace name: %q", identifier)
215215
}
216216

217-
return client.WorkspaceByOwnerAndName(cmd.Context(), owner, name, codersdk.WorkspaceByOwnerAndNameParams{})
217+
return client.WorkspaceByOwnerAndName(cmd.Context(), owner, name, codersdk.WorkspaceOptions{})
218218
}
219219

220220
// createConfig consumes the global configuration flag to produce a config root.

‎coderd/database/databasefake/databasefake.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,9 @@ func (q *fakeQuerier) GetWorkspaceByOwnerIDAndName(_ context.Context, arg databa
375375
q.mutex.RLock()
376376
defer q.mutex.RUnlock()
377377

378+
var found *database.Workspace
378379
for _, workspace := range q.workspaces {
380+
workspace := workspace
379381
if workspace.OwnerID != arg.OwnerID {
380382
continue
381383
}
@@ -385,7 +387,14 @@ func (q *fakeQuerier) GetWorkspaceByOwnerIDAndName(_ context.Context, arg databa
385387
if workspace.Deleted != arg.Deleted {
386388
continue
387389
}
388-
return workspace, nil
390+
391+
// Return the most recent workspace with the given name
392+
if found == nil || workspace.CreatedAt.After(found.CreatedAt) {
393+
found = &workspace
394+
}
395+
}
396+
if found != nil {
397+
return *found, nil
389398
}
390399
return database.Workspace{}, sql.ErrNoRows
391400
}

‎coderd/database/queries.sql.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ FROM
7070
WHERE
7171
owner_id = @owner_id
7272
AND deleted = @deleted
73-
AND LOWER("name") = LOWER(@name);
73+
AND LOWER("name") = LOWER(@name)
74+
ORDER BY created_at DESC;
7475

7576
-- name: GetWorkspaceOwnerCountsByTemplateIDs :many
7677
SELECT

‎coderd/workspaces.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,16 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
3535
return
3636
}
3737

38-
// The `deleted` query parameter (which defaults to `false`) MUST match the
39-
// `Deleted` field on the workspace otherwise you will get a 410 Gone.
4038
var (
41-
deletedStr = r.URL.Query().Get("deleted")
39+
deletedStr = r.URL.Query().Get("include_deleted")
4240
showDeleted = false
4341
)
4442
if deletedStr != "" {
4543
var err error
4644
showDeleted, err = strconv.ParseBool(deletedStr)
4745
if err != nil {
4846
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
49-
Message: fmt.Sprintf("Invalid boolean value %q for \"deleted\" query param.", deletedStr),
47+
Message: fmt.Sprintf("Invalid boolean value %q for \"include_deleted\" query param.", deletedStr),
5048
Validations: []httpapi.Error{
5149
{Field: "deleted", Detail: "Must be a valid boolean"},
5250
},

‎coderd/workspaces_test.go

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
258258
t.Run("NotFound", func(t *testing.T) {
259259
t.Parallel()
260260
client := coderdtest.New(t, nil)
261-
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, "something", codersdk.WorkspaceByOwnerAndNameParams{})
261+
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, "something", codersdk.WorkspaceOptions{})
262262
var apiErr *codersdk.Error
263263
require.ErrorAs(t, err, &apiErr)
264264
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
@@ -271,7 +271,7 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
271271
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
272272
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
273273
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
274-
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, workspace.Name, codersdk.WorkspaceByOwnerAndNameParams{})
274+
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, workspace.Name, codersdk.WorkspaceOptions{})
275275
require.NoError(t, err)
276276
})
277277
t.Run("Deleted", func(t *testing.T) {
@@ -294,12 +294,43 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
294294

295295
// Then:
296296
// When we call without includes_deleted, we don't expect to get the workspace back
297-
_, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceByOwnerAndNameParams{})
297+
_, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{})
298298
require.ErrorContains(t, err, "403")
299299

300300
// Then:
301301
// When we call with includes_deleted, we should get the workspace back
302-
workspaceNew, err := client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceByOwnerAndNameParams{IncludeDeleted: true})
302+
workspaceNew, err := client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{IncludeDeleted: true})
303+
require.NoError(t, err)
304+
require.Equal(t, workspace.ID, workspaceNew.ID)
305+
306+
// Given:
307+
// We recreate the workspace with the same name
308+
workspace, err = client.CreateWorkspace(context.Background(), user.OrganizationID, codersdk.CreateWorkspaceRequest{
309+
TemplateID: workspace.TemplateID,
310+
Name: workspace.Name,
311+
AutostartSchedule: workspace.AutostartSchedule,
312+
TTLMillis: workspace.TTLMillis,
313+
})
314+
require.NoError(t, err)
315+
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
316+
317+
// Then:
318+
// We can fetch the most recent workspace
319+
workspaceNew, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{})
320+
require.NoError(t, err)
321+
require.Equal(t, workspace.ID, workspaceNew.ID)
322+
323+
// Given:
324+
// We delete the workspace again
325+
build, err = client.CreateWorkspaceBuild(context.Background(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{
326+
Transition: codersdk.WorkspaceTransitionDelete,
327+
})
328+
require.NoError(t, err, "delete the workspace")
329+
coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
330+
331+
// Then:
332+
// When we fetch the deleted workspace, we get the most recently deleted one
333+
workspaceNew, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{IncludeDeleted: true})
303334
require.NoError(t, err)
304335
require.Equal(t, workspace.ID, workspaceNew.ID)
305336
})

‎codersdk/workspaces.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,16 @@ type CreateWorkspaceBuildRequest struct {
3939
}
4040

4141
type WorkspaceOptions struct {
42-
Deleted bool `json:"deleted,omitempty"`
42+
IncludeDeleted bool `json:"include_deleted,omitempty"`
4343
}
4444

4545
// asRequestOption returns a function that can be used in (*Client).Request.
4646
// It modifies the request query parameters.
4747
func (o WorkspaceOptions) asRequestOption() requestOption {
4848
return func(r *http.Request) {
4949
q := r.URL.Query()
50-
if o.Deleted {
51-
q.Set("deleted", "true")
50+
if o.IncludeDeleted {
51+
q.Set("include_deleted", "true")
5252
}
5353
r.URL.RawQuery = q.Encode()
5454
}
@@ -62,7 +62,7 @@ func (c *Client) Workspace(ctx context.Context, id uuid.UUID) (Workspace, error)
6262
// DeletedWorkspace returns a single workspace that was deleted.
6363
func (c *Client) DeletedWorkspace(ctx context.Context, id uuid.UUID) (Workspace, error) {
6464
o := WorkspaceOptions{
65-
Deleted: true,
65+
IncludeDeleted: true,
6666
}
6767
return c.getWorkspace(ctx, id, o.asRequestOption())
6868
}
@@ -258,12 +258,8 @@ func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) ([]Work
258258
return workspaces, json.NewDecoder(res.Body).Decode(&workspaces)
259259
}
260260

261-
type WorkspaceByOwnerAndNameParams struct {
262-
IncludeDeleted bool `json:"include_deleted,omitempty"`
263-
}
264-
265261
// WorkspaceByOwnerAndName returns a workspace by the owner's UUID and the workspace's name.
266-
func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name string, params WorkspaceByOwnerAndNameParams) (Workspace, error) {
262+
func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name string, params WorkspaceOptions) (Workspace, error) {
267263
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/workspace/%s", owner, name), nil, func(r *http.Request) {
268264
q := r.URL.Query()
269265
q.Set("include_deleted", fmt.Sprintf("%t", params.IncludeDeleted))

‎site/src/api/api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export const getWorkspaces = async (filter?: TypesGen.WorkspaceFilter): Promise<
144144
export const getWorkspaceByOwnerAndName = async (
145145
username = "me",
146146
workspaceName: string,
147-
params?: TypesGen.WorkspaceByOwnerAndNameParams,
147+
params?: TypesGen.WorkspaceOptions,
148148
): Promise<TypesGen.Workspace> => {
149149
const response = await axios.get<TypesGen.Workspace>(`/api/v2/users/${username}/workspace/${workspaceName}`, {
150150
params,

‎site/src/api/typesGenerated.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -464,11 +464,6 @@ export interface WorkspaceBuildsRequest extends Pagination {
464464
readonly WorkspaceID: string
465465
}
466466

467-
// From codersdk/workspaces.go:261:6
468-
export interface WorkspaceByOwnerAndNameParams {
469-
readonly include_deleted?: boolean
470-
}
471-
472467
// From codersdk/workspaces.go:219:6
473468
export interface WorkspaceFilter {
474469
readonly organization_id?: string
@@ -478,7 +473,7 @@ export interface WorkspaceFilter {
478473

479474
// From codersdk/workspaces.go:41:6
480475
export interface WorkspaceOptions {
481-
readonly deleted?: boolean
476+
readonly include_deleted?: boolean
482477
}
483478

484479
// From codersdk/workspaceresources.go:21:6

0 commit comments

Comments
 (0)