move favorite status to workspaces table to avoid sqlc join sadness

coder · johnstcn · Jan 16, 2024 · Jan 17, 2024 · Jan 17, 2024 · Jan 17, 2024
commit 7238b95c0cbb6c18a6f2b7b18a7c28b3b4276158
diff --git a/coderd/audit.go b/coderd/audit.go
@@ -271,11 +271,6 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
 		return str
 	}
 
-	// "Pinning" (favoriting) a workspace is a separate thing.
-	if alog.ResourceType == database.ResourceTypeFavoriteWorkspace {
-		return fmt.Sprintf("{user} pinned workspace %s", alog.ResourceTarget)
-	}
-
 	str += fmt.Sprintf(" %s",
 		codersdk.ResourceType(alog.ResourceType).FriendlyString())
 

diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go
@@ -13,7 +13,6 @@ type Auditable interface {
 		database.TemplateVersion |
 		database.User |
 		database.Workspace |
-		database.FavoriteWorkspace |
 		database.GitSSHKey |
 		database.WorkspaceBuild |
 		database.AuditableGroup |

diff --git a/coderd/audit/request.go b/coderd/audit/request.go
@@ -95,8 +95,6 @@ func ResourceTarget[T Auditable](tgt T) string {
 		return string(typed.ToLoginType)
 	case database.HealthSettings:
 		return "" // no target?
-	case database.FavoriteWorkspace:
-		return typed.WorkspaceID.String()
 	default:
 		panic(fmt.Sprintf("unknown resource %T", tgt))
 	}
@@ -130,8 +128,6 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
 	case database.HealthSettings:
 		// Artificial ID for auditing purposes
 		return typed.ID
-	case database.FavoriteWorkspace:
-		return typed.WorkspaceID
 	default:
 		panic(fmt.Sprintf("unknown resource %T", tgt))
 	}
@@ -163,8 +159,6 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
 		return database.ResourceTypeConvertLogin
 	case database.HealthSettings:
 		return database.ResourceTypeHealthSettings
-	case database.FavoriteWorkspace:
-		return database.ResourceTypeFavoriteWorkspace
 	default:
 		panic(fmt.Sprintf("unknown resource %T", typed))
 	}

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -891,11 +891,11 @@ func (q *querier) DeleteTailnetTunnel(ctx context.Context, arg database.DeleteTa
 	return q.db.DeleteTailnetTunnel(ctx, arg)
 }
 
-func (q *querier) FavoriteWorkspace(ctx context.Context, arg database.FavoriteWorkspaceParams) error {
-	if _, err := q.GetWorkspaceByID(ctx, arg.WorkspaceID); err != nil {
-		return err
+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 q.db.FavoriteWorkspace(ctx, arg)
+	return update(q.log, q.auth, fetch, q.db.FavoriteWorkspace)(ctx, id)
 }
 
 func (q *querier) GetAPIKeyByID(ctx context.Context, id string) (database.APIKey, error) {
@@ -2516,11 +2516,11 @@ func (q *querier) UnarchiveTemplateVersion(ctx context.Context, arg database.Una
 	return q.db.UnarchiveTemplateVersion(ctx, arg)
 }
 
-func (q *querier) UnfavoriteWorkspace(ctx context.Context, arg database.UnfavoriteWorkspaceParams) error {
-	if _, err := q.GetWorkspaceByID(ctx, arg.WorkspaceID); err != nil {
-		return err
+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 q.db.UnfavoriteWorkspace(ctx, arg)
+	return update(q.log, q.auth, fetch, q.db.UnfavoriteWorkspace)(ctx, id)
 }
 
 func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKeyByIDParams) error {

diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -1581,18 +1581,12 @@ func (s *MethodTestSuite) TestWorkspace() {
 	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(database.FavoriteWorkspaceParams{
-			UserID:      u.ID,
-			WorkspaceID: ws.ID,
-		}).Asserts(ws, rbac.ActionRead).Returns()
+		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(database.UnfavoriteWorkspaceParams{
-			UserID:      u.ID,
-			WorkspaceID: ws.ID,
-		}).Asserts(ws, rbac.ActionRead).Returns()
+		check.Args(ws.ID).Asserts(ws, rbac.ActionUpdate).Returns()
 	}))
 }
 

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -71,7 +71,6 @@ func New() database.Store {
 			workspaceBuilds:           make([]database.WorkspaceBuildTable, 0),
 			workspaceApps:             make([]database.WorkspaceApp, 0),
 			workspaces:                make([]database.Workspace, 0),
-			favoriteWorkspaces:        make([]database.FavoriteWorkspace, 0),
 			licenses:                  make([]database.License, 0),
 			workspaceProxies:          make([]database.WorkspaceProxy, 0),
 			locks:                     map[int64]struct{}{},
@@ -155,7 +154,6 @@ type data struct {
 	workspaceResourceMetadata     []database.WorkspaceResourceMetadatum
 	workspaceResources            []database.WorkspaceResource
 	workspaces                    []database.Workspace
-	favoriteWorkspaces            []database.FavoriteWorkspace
 	workspaceProxies              []database.WorkspaceProxy
 	// Locks is a map of lock names. Any keys within the map are currently
 	// locked.
@@ -1317,7 +1315,7 @@ func (*FakeQuerier) DeleteTailnetTunnel(_ context.Context, arg database.DeleteTa
 	return database.DeleteTailnetTunnelRow{}, ErrUnimplemented
 }
 
-func (q *FakeQuerier) FavoriteWorkspace(_ context.Context, arg database.FavoriteWorkspaceParams) error {
+func (q *FakeQuerier) FavoriteWorkspace(_ context.Context, arg uuid.UUID) error {
 	err := validateDatabaseType(arg)
 	if err != nil {
 		return err
@@ -1326,10 +1324,13 @@ func (q *FakeQuerier) FavoriteWorkspace(_ context.Context, arg database.Favorite
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
 
-	for _, upw := range q.favoriteWorkspaces {
-		if arg.UserID == upw.UserID && arg.WorkspaceID == upw.WorkspaceID {
-			return errDuplicateKey
+	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
 }
@@ -6003,7 +6004,7 @@ func (q *FakeQuerier) UnarchiveTemplateVersion(_ context.Context, arg database.U
 	return sql.ErrNoRows
 }
 
-func (q *FakeQuerier) UnfavoriteWorkspace(_ context.Context, arg database.UnfavoriteWorkspaceParams) error {
+func (q *FakeQuerier) UnfavoriteWorkspace(_ context.Context, arg uuid.UUID) error {
 	err := validateDatabaseType(arg)
 	if err != nil {
 		return err
@@ -6012,15 +6013,11 @@ func (q *FakeQuerier) UnfavoriteWorkspace(_ context.Context, arg database.Unfavo
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
 
-	for index, upw := range q.favoriteWorkspaces {
-		if upw.UserID != arg.UserID {
-			continue
-		}
-		if upw.WorkspaceID != arg.WorkspaceID {
+	for i := 0; i < len(q.workspaces); i++ {
+		if q.workspaces[i].ID != arg {
 			continue
 		}
-		q.favoriteWorkspaces[index] = q.favoriteWorkspaces[len(q.apiKeys)-1]
-		q.favoriteWorkspaces = q.favoriteWorkspaces[:len(q.favoriteWorkspaces)-1]
+		q.workspaces[i].FavoriteOf = uuid.NullUUID{}
 		return nil
 	}
 

diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000186_user_favorite_workspaces.down.sql b/coderd/database/migrations/000186_user_favorite_workspaces.down.sql
@@ -1 +1 @@
-DROP TABLE favorite_workspaces;
+ALTER TABLE ONLY workspaces DROP COLUMN favorite_of;
diff --git a/coderd/database/migrations/000186_user_favorite_workspaces.up.sql b/coderd/database/migrations/000186_user_favorite_workspaces.up.sql
@@ -1,7 +1,3 @@
-CREATE TABLE favorite_workspaces (
-	user_id uuid NOT NULL,
-	workspace_id uuid NOT NULL,
-	UNIQUE(user_id, workspace_id)
-);
-
-ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'favorite_workspace';
+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.';
diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go
@@ -213,9 +213,9 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
 	// The name comment is for metric tracking
 	query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", filtered)
 	rows, err := q.db.QueryContext(ctx, query,
-		arg.OwnerID,
 		arg.Deleted,
 		arg.Status,
+		arg.OwnerID,
 		arg.OwnerUsername,
 		arg.TemplateName,
 		pq.Array(arg.TemplateIDs),
@@ -250,10 +250,10 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
 			&i.DormantAt,
 			&i.DeletingAt,
 			&i.AutomaticUpdates,
+			&i.FavoriteOf,
 			&i.TemplateName,
 			&i.TemplateVersionID,
 			&i.TemplateVersionName,
-			&i.Favorite,
 			&i.Count,
 		); err != nil {
 			return nil, err

diff --git a/coderd/database/models.go b/coderd/database/models.go
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		DROP TABLE favorite_workspaces;
		ALTER TABLE ONLY workspaces DROP COLUMN favorite_of;