- Notifications
You must be signed in to change notification settings - Fork926
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
46ca00d015aabb0b0dce69878da74fba23883b03d90961298a814b6589d618d7238b954eef59a0f8904dd921aa0b68c18e3d0545a46103a4f6c036189f0f50c9ed43cff3cde2f735b69f6e953134f290299c7f96d5305466392db99979c53File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -61,6 +61,7 @@ | ||
| "failing_agents": [] | ||
| }, | ||
| "automatic_updates": "never", | ||
| "allow_renames": false, | ||
| "favorite": false | ||
| } | ||
| ] | ||
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Uh oh!
There was an error while loading.Please reload this page.
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Uh oh!
There was an error while loading.Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() { | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 { | ||
| @@ -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() | ||
| @@ -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 | ||
| @@ -7696,7 +7737,15 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. | ||
| w1 := workspaces[i] | ||
| w2 := workspaces[j] | ||
| // 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]) | ||
| @@ -7709,12 +7758,12 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. | ||
| } | ||
| // Order by: usernames | ||
| ifstrings.Compare(preloadedUsers[w1.ID].Username, preloadedUsers[w2.ID].Username) < 0 { | ||
| returntrue | ||
| } | ||
| // Order by: workspace names | ||
| returnstrings.Compare(w1.Name, w2.Name) < 0 | ||
Comment on lines +7761 to +7766 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Fixed a pre-existing sort ordering bug in dbmem. | ||
| }) | ||
| beforePageCount := len(workspaces) | ||
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Uh oh!
There was an error while loading.Please reload this page.
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Uh oh!
There was an error while loading.Please reload this page.
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Uh oh!
There was an error while loading.Please reload this page.
| 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
Uh oh!
There was an error while loading.Please reload this page.