opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: add archived query parameter to chat list endpoint (#22562)

Browse Source 
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.
This commit is contained in:
Danielle Maywood
2026-03-03 20:39:19 +00:00
committed by GitHub
parent 8a2635285b
commit d2d956edb1
11 changed files with 101 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/chats.go
+25 -3
View File
@@ -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,
}
coderd/chats_test.go
+31 -11
View File
@@ -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")
coderd/database/dbauthz/dbauthz.go
+1 -1
View File
@@ -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)
}
coderd/database/dbauthz/dbauthz_test.go
+3 -2
View File
@@ -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{})
coderd/database/dbmetrics/querymetrics.go
+1 -1
View File
@@ -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())
coderd/database/dbmock/dbmock.go
+4 -4
View File
@@ -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.
coderd/database/querier.go
+1 -1
View File
@@ -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)
coderd/database/queries.sql.go
+11 -3
View File
@@ -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
}
coderd/database/queries/chats.sql
+4 -1
View File
@@ -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;