Skip to content

feat(coderd): add ability to mark workspaces as favorite #11673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
46ca00d
feat(coderd): add user_pinned_workspaces table
johnstcn Jan 16, 2024
015aabb
add unique index
johnstcn Jan 17, 2024
0b0dce6
rename migration
johnstcn Jan 17, 2024
9878da7
add queries to pin/unpin workspace
johnstcn Jan 17, 2024
4fba238
add pinned status to GetWorkspaces/GetAuthorizedWorkspaces queries
johnstcn Jan 17, 2024
83b03d9
add API endpoints and test (currently fails)
johnstcn Jan 18, 2024
0961298
rename to favorite workspace
johnstcn Jan 18, 2024
a814b65
move to top-level test
johnstcn Jan 22, 2024
89d618d
s/favored/favorite/g
johnstcn Jan 22, 2024
7238b95
move favorite status to workspaces table to avoid sqlc join sadness
johnstcn Jan 22, 2024
4eef59a
more wiring
johnstcn Jan 22, 2024
0f8904d
improve test to include sort order of favorites
johnstcn Jan 22, 2024
d921aa0
fix swagger summary
johnstcn Jan 22, 2024
b68c18e
make gen
johnstcn Jan 22, 2024
3d0545a
use dbfake for testing sort order
johnstcn Jan 22, 2024
46103a4
reduce scope of TestWorkspaceFavoriteUnfavorite to not include sortin…
johnstcn Jan 22, 2024
f6c0361
beef up sort order test
johnstcn Jan 22, 2024
89f0f50
rm unnecessary fixture
johnstcn Jan 22, 2024
c9ed43c
fix sort ordering, dbfake still TODO
johnstcn Jan 22, 2024
ff3cde2
remove unnecessary change to CreateFirstUser
johnstcn Jan 23, 2024
f735b69
best-effort fix but testing dbmem sort order is a waste of time
johnstcn Jan 23, 2024
f6e9531
requestor->requester
johnstcn Jan 23, 2024
34f2902
fixup! remove unnecessary change to CreateFirstUser
johnstcn Jan 23, 2024
99c7f96
remove unused resource
johnstcn Jan 23, 2024
d530546
fix sort ordering bug in dmem
johnstcn Jan 23, 2024
6392db9
add test case
johnstcn Jan 23, 2024
9979c53
remove need for conditional ordering
johnstcn Jan 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cli/testdata/coder_list_--output_json.golden
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"failing_agents": []
},
"automatic_updates": "never",
"allow_renames": false
"allow_renames": false,
"favorite": false
}
]
59 changes: 59 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 55 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,8 @@ func New(options *Options) *API {
r.Get("/watch", api.watchWorkspace)
r.Put("/extend", api.putExtendWorkspace)
r.Put("/dormant", api.putWorkspaceDormant)
r.Put("/favorite", api.putFavoriteWorkspace)
r.Delete("/favorite", api.deleteFavoriteWorkspace)
r.Put("/autoupdates", api.putWorkspaceAutoupdates)
r.Get("/resolve-autostart", api.resolveAutostart)
})
Expand Down
14 changes: 14 additions & 0 deletions coderd/database/dbauthz/dbauthz.go
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,13 @@ func (q *querier) DeleteTailnetTunnel(ctx context.Context, arg database.DeleteTa
return q.db.DeleteTailnetTunnel(ctx, arg)
}

func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error {
fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) {
return q.db.GetWorkspaceByID(ctx, id)
}
return update(q.log, q.auth, fetch, q.db.FavoriteWorkspace)(ctx, id)
}

func (q *querier) GetAPIKeyByID(ctx context.Context, id string) (database.APIKey, error) {
return fetch(q.log, q.auth, q.db.GetAPIKeyByID)(ctx, id)
}
Expand Down Expand Up @@ -2509,6 +2516,13 @@ func (q *querier) UnarchiveTemplateVersion(ctx context.Context, arg database.Una
return q.db.UnarchiveTemplateVersion(ctx, arg)
}

func (q *querier) UnfavoriteWorkspace(ctx context.Context, id uuid.UUID) error {
fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) {
return q.db.GetWorkspaceByID(ctx, id)
}
return update(q.log, q.auth, fetch, q.db.UnfavoriteWorkspace)(ctx, id)
}

func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKeyByIDParams) error {
fetch := func(ctx context.Context, arg database.UpdateAPIKeyByIDParams) (database.APIKey, error) {
return q.db.GetAPIKeyByID(ctx, arg.ID)
Expand Down
10 changes: 10 additions & 0 deletions coderd/database/dbauthz/dbauthz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1578,6 +1578,16 @@ func (s *MethodTestSuite) TestWorkspace() {
WorkspaceID: ws.ID,
}).Asserts(ws, rbac.ActionUpdate).Returns()
}))
s.Run("FavoriteWorkspace", s.Subtest(func(db database.Store, check *expects) {
u := dbgen.User(s.T(), db, database.User{})
ws := dbgen.Workspace(s.T(), db, database.Workspace{OwnerID: u.ID})
check.Args(ws.ID).Asserts(ws, rbac.ActionUpdate).Returns()
}))
s.Run("UnfavoriteWorkspace", s.Subtest(func(db database.Store, check *expects) {
u := dbgen.User(s.T(), db, database.User{})
ws := dbgen.Workspace(s.T(), db, database.Workspace{OwnerID: u.ID})
check.Args(ws.ID).Asserts(ws, rbac.ActionUpdate).Returns()
}))
Comment on lines +1581 to +1590
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self-review: you need to be able to update a workspace in order to favorite it.

}

func (s *MethodTestSuite) TestExtraMethods() {
Expand Down
57 changes: 53 additions & 4 deletions coderd/database/dbmem/dbmem.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac
DeletingAt: w.DeletingAt,
Count: count,
AutomaticUpdates: w.AutomaticUpdates,
FavoriteOf: w.FavoriteOf,
}

for _, t := range q.templates {
Expand Down Expand Up @@ -1315,6 +1316,26 @@ func (*FakeQuerier) DeleteTailnetTunnel(_ context.Context, arg database.DeleteTa
return database.DeleteTailnetTunnelRow{}, ErrUnimplemented
}

func (q *FakeQuerier) FavoriteWorkspace(_ context.Context, arg uuid.UUID) error {
err := validateDatabaseType(arg)
if err != nil {
return err
}

q.mutex.Lock()
defer q.mutex.Unlock()

for i := 0; i < len(q.workspaces); i++ {
if q.workspaces[i].ID != arg {
continue
}
q.workspaces[i].FavoriteOf.Valid = true
q.workspaces[i].FavoriteOf.UUID = q.workspaces[i].OwnerID
return nil
}
return nil
}

func (q *FakeQuerier) GetAPIKeyByID(_ context.Context, id string) (database.APIKey, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()
Expand Down Expand Up @@ -5984,6 +6005,26 @@ func (q *FakeQuerier) UnarchiveTemplateVersion(_ context.Context, arg database.U
return sql.ErrNoRows
}

func (q *FakeQuerier) UnfavoriteWorkspace(_ context.Context, arg uuid.UUID) error {
err := validateDatabaseType(arg)
if err != nil {
return err
}

q.mutex.Lock()
defer q.mutex.Unlock()

for i := 0; i < len(q.workspaces); i++ {
if q.workspaces[i].ID != arg {
continue
}
q.workspaces[i].FavoriteOf = uuid.NullUUID{}
return nil
}

return nil
}

func (q *FakeQuerier) UpdateAPIKeyByID(_ context.Context, arg database.UpdateAPIKeyByIDParams) error {
if err := validateDatabaseType(arg); err != nil {
return err
Expand Down Expand Up @@ -7696,7 +7737,15 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
w1 := workspaces[i]
w2 := workspaces[j]

// Order by: running first
// Order by: favorite first
if w1.FavoriteOf == arg.OrderByFavorite && w2.FavoriteOf != arg.OrderByFavorite {
return true
}
if w1.FavoriteOf != arg.OrderByFavorite && w2.FavoriteOf == arg.OrderByFavorite {
return false
}

// Order by: running
w1IsRunning := isRunning(preloadedWorkspaceBuilds[w1.ID], preloadedProvisionerJobs[w1.ID])
w2IsRunning := isRunning(preloadedWorkspaceBuilds[w2.ID], preloadedProvisionerJobs[w2.ID])

Expand All @@ -7709,12 +7758,12 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
}

// Order by: usernames
if w1.ID != w2.ID {
return sort.StringsAreSorted([]string{preloadedUsers[w1.ID].Username, preloadedUsers[w2.ID].Username})
if strings.Compare(preloadedUsers[w1.ID].Username, preloadedUsers[w2.ID].Username) < 0 {
return true
}

// Order by: workspace names
return sort.StringsAreSorted([]string{w1.Name, w2.Name})
return strings.Compare(w1.Name, w2.Name) < 0
Comment on lines +7761 to +7766
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a pre-existing sort ordering bug in dbmem.

})

beforePageCount := len(workspaces)
Expand Down
14 changes: 14 additions & 0 deletions coderd/database/dbmetrics/dbmetrics.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions coderd/database/dbmock/dbmock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE ONLY workspaces DROP COLUMN favorite_of;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE ONLY workspaces
ADD COLUMN favorite_of uuid DEFAULT NULL;
COMMENT ON COLUMN workspaces.favorite_of IS 'FavoriteOf contains the UUID of the workspace owner if the workspace has been favorited.';
Comment on lines +1 to +3
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self-review: just storing a boolean value here doesn't provide enough context to the query to be able to know how to sort favorites.

Copy link
Member

@Emyrk Emyrk Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there is any benefit to using a zero uuid here instead of null? It just makes you not need to use uuid.NullUUID{...}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't a workspace have the owner uuid already in the table? If so can't we just use a boolean + that? When I hear "favorite of" I kind of expect the data type to be an array and favorable by many.

If we don't have owner uuid in there, should we just add it vs. the current workaround?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally wary of making that conditional on owner_id, but now it's looking like it makes more sense in its current shape.

1 change: 1 addition & 0 deletions coderd/database/modelmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ func ConvertWorkspaceRows(rows []GetWorkspacesRow) []Workspace {
DormantAt: r.DormantAt,
DeletingAt: r.DeletingAt,
AutomaticUpdates: r.AutomaticUpdates,
FavoriteOf: r.FavoriteOf,
}
}

Expand Down
Loading