From d2d956edb1f9ec17051f7c7ed01dd2365738151a Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 3 Mar 2026 20:39:19 +0000 Subject: [PATCH] fix: add archived query parameter to chat list endpoint (#22562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Despite the SDK type having an `Archived` field for chats, this data was never fetched from the database — the `GetChatsByOwnerID` query hardcoded `AND archived = false`, and the `convertChat` function never mapped the field. This PR adds an optional `archived` query parameter to `GET /api/experimental/chats`: | Value | Behavior | |-------|----------| | *(not provided)* | Returns all chats (active and archived) | | `archived=false` | Returns only non-archived chats | | `archived=true` | Returns only archived chats | This follows the same pattern used by template versions (`sqlc.narg('archived')` nullable boolean). Also fixes `convertChat` to populate the `Archived` field in API responses, which was never being set despite existing on the SDK type. --- coderd/chats.go | 28 +++++++++++++-- coderd/chats_test.go | 42 +++++++++++++++++------ coderd/database/dbauthz/dbauthz.go | 2 +- coderd/database/dbauthz/dbauthz_test.go | 5 +-- coderd/database/dbmetrics/querymetrics.go | 2 +- coderd/database/dbmock/dbmock.go | 8 ++--- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 14 ++++++-- coderd/database/queries/chats.sql | 5 ++- codersdk/chats.go | 14 ++++++-- site/src/api/typesGenerated.ts | 8 +++++ 11 files changed, 101 insertions(+), 29 deletions(-) diff --git a/coderd/chats.go b/coderd/chats.go index d01684cd77..8a1cb5e1b7 100644 --- a/coderd/chats.go +++ b/coderd/chats.go @@ -171,7 +171,24 @@ func (api *API) listChats(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() apiKey := httpmw.APIKey(r) - chats, err := api.Database.GetChatsByOwnerID(ctx, apiKey.UserID) + params := database.GetChatsByOwnerIDParams{ + OwnerID: apiKey.UserID, + } + if v := r.URL.Query().Get("archived"); v != "" { + b, err := strconv.ParseBool(v) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid query parameter.", + Validations: []codersdk.ValidationError{ + {Field: "archived", Detail: "Must be a valid boolean"}, + }, + }) + return + } + params.Archived = sql.NullBool{Bool: b, Valid: true} + } + + chats, err := api.Database.GetChatsByOwnerID(ctx, params) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to list chats.", @@ -1059,7 +1076,9 @@ func (api *API) storeChatGitRef(ctx context.Context, workspaceID, workspaceOwner } chatsToUpdate = []database.Chat{chat} } else { - chats, err := api.Database.GetChatsByOwnerID(ctx, workspaceOwnerID) + chats, err := api.Database.GetChatsByOwnerID(ctx, database.GetChatsByOwnerIDParams{ + OwnerID: workspaceOwnerID, + }) if err != nil { api.Logger.Warn(ctx, "failed to list chats for git ref storage", slog.F("workspace_id", workspaceID), @@ -1111,7 +1130,9 @@ func (api *API) refreshWorkspaceChatDiffStatuses(ctx context.Context, workspaceI } filtered = []database.Chat{chat} } else { - chats, err := api.Database.GetChatsByOwnerID(ctx, workspaceOwnerID) + chats, err := api.Database.GetChatsByOwnerID(ctx, database.GetChatsByOwnerIDParams{ + OwnerID: workspaceOwnerID, + }) if err != nil { api.Logger.Warn(ctx, "failed to list workspace owner chats for diff refresh", slog.F("workspace_id", workspaceID), @@ -2070,6 +2091,7 @@ func convertChat(c database.Chat, diffStatus *database.ChatDiffStatus) codersdk. LastModelConfigID: c.LastModelConfigID, Title: c.Title, Status: codersdk.ChatStatus(c.Status), + Archived: c.Archived, CreatedAt: c.CreatedAt, UpdatedAt: c.UpdatedAt, } diff --git a/coderd/chats_test.go b/coderd/chats_test.go index 1bc342c704..8498ad2260 100644 --- a/coderd/chats_test.go +++ b/coderd/chats_test.go @@ -18,6 +18,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/externalauth" coderdpubsub "github.com/coder/coder/v2/coderd/pubsub" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/websocket" @@ -322,7 +323,7 @@ func TestListChats(t *testing.T) { }) require.NoError(t, err) - chats, err := client.ListChats(ctx) + chats, err := client.ListChats(ctx, nil) require.NoError(t, err) require.Len(t, chats, 2) @@ -361,7 +362,7 @@ func TestListChats(t *testing.T) { require.Less(t, chatIndexes[firstChatB.ID], chatIndexes[firstChatA.ID]) } - memberChats, err := memberClient.ListChats(ctx) + memberChats, err := memberClient.ListChats(ctx, nil) require.NoError(t, err) require.Len(t, memberChats, 1) require.Equal(t, memberDBChat.ID, memberChats[0].ID) @@ -381,7 +382,7 @@ func TestListChats(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) unauthenticatedClient := codersdk.New(client.URL) - _, err := unauthenticatedClient.ListChats(ctx) + _, err := unauthenticatedClient.ListChats(ctx, nil) requireSDKError(t, err, http.StatusUnauthorized) }) } @@ -1185,18 +1186,35 @@ func TestArchiveChat(t *testing.T) { }) require.NoError(t, err) - chatsBeforeArchive, err := client.ListChats(ctx) + chatsBeforeArchive, err := client.ListChats(ctx, nil) require.NoError(t, err) require.Len(t, chatsBeforeArchive, 2) err = client.ArchiveChat(ctx, chatToArchive.ID) require.NoError(t, err) - // Archived chats should not appear in the list. - chatsAfterArchive, err := client.ListChats(ctx) + // Default (no filter) returns all chats including archived. + allChats, err := client.ListChats(ctx, nil) require.NoError(t, err) - require.Len(t, chatsAfterArchive, 1) - require.Equal(t, chatToKeep.ID, chatsAfterArchive[0].ID) + require.Len(t, allChats, 2) + + // archived=false returns only non-archived chats. + activeChats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{ + Archived: ptr.Ref(false), + }) + require.NoError(t, err) + require.Len(t, activeChats, 1) + require.Equal(t, chatToKeep.ID, activeChats[0].ID) + require.False(t, activeChats[0].Archived) + + // archived=true returns only archived chats. + archivedChats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{ + Archived: ptr.Ref(true), + }) + require.NoError(t, err) + require.Len(t, archivedChats, 1) + require.Equal(t, chatToArchive.ID, archivedChats[0].ID) + require.True(t, archivedChats[0].Archived) }) t.Run("NotFound", func(t *testing.T) { @@ -1252,10 +1270,12 @@ func TestArchiveChat(t *testing.T) { err = client.ArchiveChat(ctx, parentChat.ID) require.NoError(t, err) - // List chats — none of the family should appear. - chats, err := client.ListChats(ctx) + // archived=false should exclude the entire archived family. + activeChats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{ + Archived: ptr.Ref(false), + }) require.NoError(t, err) - for _, c := range chats { + for _, c := range activeChats { require.NotEqual(t, parentChat.ID, c.ID, "parent should not appear") require.NotEqual(t, child1.ID, c.ID, "child1 should not appear") require.NotEqual(t, child2.ID, c.ID, "child2 should not appear") diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index de008bc086..1b7d927eaa 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2540,7 +2540,7 @@ func (q *querier) GetChatQueuedMessages(ctx context.Context, chatID uuid.UUID) ( return q.db.GetChatQueuedMessages(ctx, chatID) } -func (q *querier) GetChatsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.Chat, error) { +func (q *querier) GetChatsByOwnerID(ctx context.Context, ownerID database.GetChatsByOwnerIDParams) ([]database.Chat, error) { return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetChatsByOwnerID)(ctx, ownerID) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index bb3bb1736f..2f038b97fe 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -530,8 +530,9 @@ func (s *MethodTestSuite) TestChats() { s.Run("GetChatsByOwnerID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { c1 := testutil.Fake(s.T(), faker, database.Chat{}) c2 := testutil.Fake(s.T(), faker, database.Chat{}) - dbm.EXPECT().GetChatsByOwnerID(gomock.Any(), c1.OwnerID).Return([]database.Chat{c1, c2}, nil).AnyTimes() - check.Args(c1.OwnerID).Asserts(c1, policy.ActionRead, c2, policy.ActionRead).Returns([]database.Chat{c1, c2}) + params := database.GetChatsByOwnerIDParams{OwnerID: c1.OwnerID} + dbm.EXPECT().GetChatsByOwnerID(gomock.Any(), params).Return([]database.Chat{c1, c2}, nil).AnyTimes() + check.Args(params).Asserts(c1, policy.ActionRead, c2, policy.ActionRead).Returns([]database.Chat{c1, c2}) })) s.Run("GetChatQueuedMessages", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { chat := testutil.Fake(s.T(), faker, database.Chat{}) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 12cb08a108..60e12d7c96 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1087,7 +1087,7 @@ func (m queryMetricsStore) GetChatQueuedMessages(ctx context.Context, chatID uui return r0, r1 } -func (m queryMetricsStore) GetChatsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.Chat, error) { +func (m queryMetricsStore) GetChatsByOwnerID(ctx context.Context, ownerID database.GetChatsByOwnerIDParams) ([]database.Chat, error) { start := time.Now() r0, r1 := m.s.GetChatsByOwnerID(ctx, ownerID) m.queryLatencies.WithLabelValues("GetChatsByOwnerID").Observe(time.Since(start).Seconds()) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 7965b37e01..724d7f2b7b 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1988,18 +1988,18 @@ func (mr *MockStoreMockRecorder) GetChatQueuedMessages(ctx, chatID any) *gomock. } // GetChatsByOwnerID mocks base method. -func (m *MockStore) GetChatsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.Chat, error) { +func (m *MockStore) GetChatsByOwnerID(ctx context.Context, arg database.GetChatsByOwnerIDParams) ([]database.Chat, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetChatsByOwnerID", ctx, ownerID) + ret := m.ctrl.Call(m, "GetChatsByOwnerID", ctx, arg) ret0, _ := ret[0].([]database.Chat) ret1, _ := ret[1].(error) return ret0, ret1 } // GetChatsByOwnerID indicates an expected call of GetChatsByOwnerID. -func (mr *MockStoreMockRecorder) GetChatsByOwnerID(ctx, ownerID any) *gomock.Call { +func (mr *MockStoreMockRecorder) GetChatsByOwnerID(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatsByOwnerID", reflect.TypeOf((*MockStore)(nil).GetChatsByOwnerID), ctx, ownerID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatsByOwnerID", reflect.TypeOf((*MockStore)(nil).GetChatsByOwnerID), ctx, arg) } // GetConnectionLogsOffset mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index e3b9085a74..1512608212 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -228,7 +228,7 @@ type sqlcQuerier interface { GetChatProviderByProvider(ctx context.Context, provider string) (ChatProvider, error) GetChatProviders(ctx context.Context) ([]ChatProvider, error) GetChatQueuedMessages(ctx context.Context, chatID uuid.UUID) ([]ChatQueuedMessage, error) - GetChatsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]Chat, error) + GetChatsByOwnerID(ctx context.Context, arg GetChatsByOwnerIDParams) ([]Chat, error) GetConnectionLogsOffset(ctx context.Context, arg GetConnectionLogsOffsetParams) ([]GetConnectionLogsOffsetRow, error) GetCoordinatorResumeTokenSigningKey(ctx context.Context) (string, error) GetCryptoKeyByFeatureAndSequence(ctx context.Context, arg GetCryptoKeyByFeatureAndSequenceParams) (CryptoKey, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4957f8504b..f75da88cc8 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3347,13 +3347,21 @@ FROM chats WHERE owner_id = $1::uuid - AND archived = false + AND CASE + WHEN $2 :: boolean IS NULL THEN true + ELSE chats.archived = $2 :: boolean + END ORDER BY updated_at DESC ` -func (q *sqlQuerier) GetChatsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]Chat, error) { - rows, err := q.db.QueryContext(ctx, getChatsByOwnerID, ownerID) +type GetChatsByOwnerIDParams struct { + OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` + Archived sql.NullBool `db:"archived" json:"archived"` +} + +func (q *sqlQuerier) GetChatsByOwnerID(ctx context.Context, arg GetChatsByOwnerIDParams) ([]Chat, error) { + rows, err := q.db.QueryContext(ctx, getChatsByOwnerID, arg.OwnerID, arg.Archived) if err != nil { return nil, err } diff --git a/coderd/database/queries/chats.sql b/coderd/database/queries/chats.sql index 71ca871544..6a5748181a 100644 --- a/coderd/database/queries/chats.sql +++ b/coderd/database/queries/chats.sql @@ -110,7 +110,10 @@ FROM chats WHERE owner_id = @owner_id::uuid - AND archived = false + AND CASE + WHEN sqlc.narg('archived') :: boolean IS NULL THEN true + ELSE chats.archived = sqlc.narg('archived') :: boolean + END ORDER BY updated_at DESC; diff --git a/codersdk/chats.go b/codersdk/chats.go index c3edeed6c1..44ca9c4629 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" "time" @@ -501,9 +502,18 @@ type chatStreamEnvelope struct { Data json.RawMessage `json:"data,omitempty"` } +// ListChatsOptions are optional parameters for ListChats. +type ListChatsOptions struct { + Archived *bool +} + // ListChats returns all chats for the authenticated user. -func (c *Client) ListChats(ctx context.Context) ([]Chat, error) { - res, err := c.Request(ctx, http.MethodGet, "/api/experimental/chats", nil) +func (c *Client) ListChats(ctx context.Context, opts *ListChatsOptions) ([]Chat, error) { + qp := url.Values{} + if opts != nil && opts.Archived != nil { + qp.Set("archived", fmt.Sprintf("%t", *opts.Archived)) + } + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/experimental/chats?%s", qp.Encode()), nil) if err != nil { return nil, err } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 2a4639c792..5abc003815 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3216,6 +3216,14 @@ export interface LinkConfig { readonly location?: string; } +// From codersdk/chats.go +/** + * ListChatsOptions are optional parameters for ListChats. + */ +export interface ListChatsOptions { + readonly Archived: boolean | null; +} + // From codersdk/inboxnotification.go export interface ListInboxNotificationsRequest { readonly targets?: string;