mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
refactor: remove /diff-status endpoint, include diff_status in chat payload (#23082)
The `/chats/{chat}/diff-status` endpoint was redundant because:
- The `Chat` type already has a `DiffStatus` field
- Listing chats already resolves and returns `diff_status`
- The `getChat` endpoint was the only one not resolving it (passing
`nil`)
## Changes
**Backend:**
- `getChat` now calls `resolveChatDiffStatus` and includes the result in
the response
- Removed `getChatDiffStatus` handler, route (`GET /diff-status`), and
SDK method
- Tests updated to use `GetChat` instead of `GetChatDiffStatus`
**Frontend:**
- `AgentDetail.tsx`: uses `chatQuery.data?.diff_status` instead of
separate query
- `RemoteDiffPanel.tsx`: accepts `diffStatus` as a prop instead of
fetching internally
- `AgentsPage.tsx`: `diff_status_change` events now invalidate the chat
query
- Removed `chatDiffStatus` query, `chatDiffStatusKey`, and
`getChatDiffStatus` API method
This commit is contained in:
+10
-21
@@ -553,7 +553,16 @@ func (api *API) chatCostUsers(rw http.ResponseWriter, r *http.Request) {
|
||||
func (api *API) getChat(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
chat := httpmw.ChatParam(r)
|
||||
httpapi.Write(ctx, rw, http.StatusOK, convertChat(chat, nil))
|
||||
|
||||
diffStatus, err := api.resolveChatDiffStatus(ctx, chat)
|
||||
if err != nil {
|
||||
// Log but don't fail - diff status is supplementary.
|
||||
api.Logger.Error(ctx, "failed to resolve chat diff status",
|
||||
slog.F("chat_id", chat.ID),
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
httpapi.Write(ctx, rw, http.StatusOK, convertChat(chat, diffStatus))
|
||||
}
|
||||
|
||||
// EXPERIMENTAL: this endpoint is experimental and is subject to change.
|
||||
@@ -1299,26 +1308,6 @@ func (api *API) interruptChat(rw http.ResponseWriter, r *http.Request) {
|
||||
httpapi.Write(ctx, rw, http.StatusOK, convertChat(chat, nil))
|
||||
}
|
||||
|
||||
// EXPERIMENTAL: this endpoint is experimental and is subject to change.
|
||||
//
|
||||
//nolint:revive // HTTP handler writes to ResponseWriter.
|
||||
func (api *API) getChatDiffStatus(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
chat := httpmw.ChatParam(r)
|
||||
chatID := chat.ID
|
||||
|
||||
status, err := api.resolveChatDiffStatus(ctx, chat)
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Failed to get chat diff status.",
|
||||
Detail: err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
httpapi.Write(ctx, rw, http.StatusOK, convertChatDiffStatus(chatID, status))
|
||||
}
|
||||
|
||||
// EXPERIMENTAL: this endpoint is experimental and is subject to change.
|
||||
//
|
||||
//nolint:revive // HTTP handler writes to ResponseWriter.
|
||||
|
||||
+15
-16
@@ -2752,17 +2752,10 @@ func TestGetChatDiffStatus(t *testing.T) {
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
noCachedStatus, err := client.GetChatDiffStatus(ctx, noCachedStatusChat.ID)
|
||||
noCachedChat, err := client.GetChat(ctx, noCachedStatusChat.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, noCachedStatusChat.ID, noCachedStatus.ChatID)
|
||||
require.Nil(t, noCachedStatus.URL)
|
||||
require.Nil(t, noCachedStatus.PullRequestState)
|
||||
require.False(t, noCachedStatus.ChangesRequested)
|
||||
require.Zero(t, noCachedStatus.Additions)
|
||||
require.Zero(t, noCachedStatus.Deletions)
|
||||
require.Zero(t, noCachedStatus.ChangedFiles)
|
||||
require.Nil(t, noCachedStatus.RefreshedAt)
|
||||
require.Nil(t, noCachedStatus.StaleAt)
|
||||
require.Equal(t, noCachedStatusChat.ID, noCachedChat.ID)
|
||||
require.Nil(t, noCachedChat.DiffStatus)
|
||||
|
||||
cachedStatusChat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{
|
||||
OwnerID: user.UserID,
|
||||
@@ -2804,8 +2797,11 @@ func TestGetChatDiffStatus(t *testing.T) {
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
cachedStatus, err := client.GetChatDiffStatus(ctx, cachedStatusChat.ID)
|
||||
cachedChat, err := client.GetChat(ctx, cachedStatusChat.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, cachedStatusChat.ID, cachedChat.ID)
|
||||
require.NotNil(t, cachedChat.DiffStatus)
|
||||
cachedStatus := cachedChat.DiffStatus
|
||||
require.Equal(t, cachedStatusChat.ID, cachedStatus.ChatID)
|
||||
require.NotNil(t, cachedStatus.URL)
|
||||
require.Equal(t, "https://github.com/coder/coder/tree/feature/diff-status", *cachedStatus.URL)
|
||||
@@ -2840,11 +2836,11 @@ func TestGetChatDiffStatus(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
otherClient, _ := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
|
||||
_, err = otherClient.GetChatDiffStatus(ctx, createdChat.ID)
|
||||
_, err = otherClient.GetChat(ctx, createdChat.ID)
|
||||
requireSDKError(t, err, http.StatusNotFound)
|
||||
})
|
||||
|
||||
// Integration test: exercises the full HTTP handler refresh
|
||||
// Integration test: exercises the full GetChat handler refresh
|
||||
// path with a real DB, dbauthz, a mock GitHub API, and an
|
||||
// external-auth-linked user. Verifies that a stale chat diff
|
||||
// status is refreshed end-to-end via the gitsync worker's
|
||||
@@ -2943,8 +2939,9 @@ func TestGetChatDiffStatus(t *testing.T) {
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Call the HTTP endpoint. This exercises the full code
|
||||
// path: resolveChatDiffStatus -> RefreshChat (with
|
||||
// Call GetChat which now resolves diff status inline.
|
||||
// This exercises the full code path:
|
||||
// resolveChatDiffStatus -> RefreshChat (with
|
||||
// AsSystemRestricted) -> Refresher.Refresh ->
|
||||
// resolveChatGitAccessToken (GetExternalAuthLink with
|
||||
// AsSystemRestricted) -> FetchPullRequestStatus (mock).
|
||||
@@ -2953,8 +2950,10 @@ func TestGetChatDiffStatus(t *testing.T) {
|
||||
// would fail under the chatd RBAC context (missing
|
||||
// ActionReadPersonal), causing ErrNoTokenAvailable and a
|
||||
// refresh failure that silently returns stale data.
|
||||
status, err := client.GetChatDiffStatus(ctx, chat.ID)
|
||||
result, err := client.GetChat(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, result.DiffStatus)
|
||||
status := result.DiffStatus
|
||||
|
||||
// The mock GitHub API returned PR #42 with 25 additions,
|
||||
// 7 deletions, 4 changed files, state "open".
|
||||
|
||||
@@ -1187,7 +1187,6 @@ func New(options *Options) *API {
|
||||
r.Patch("/messages/{message}", api.patchChatMessage)
|
||||
r.Get("/stream", api.streamChat)
|
||||
r.Post("/interrupt", api.interruptChat)
|
||||
r.Get("/diff-status", api.getChatDiffStatus)
|
||||
r.Get("/diff", api.getChatDiffContents)
|
||||
r.Route("/queue/{queuedMessage}", func(r chi.Router) {
|
||||
r.Delete("/", api.deleteChatQueuedMessage)
|
||||
|
||||
Reference in New Issue
Block a user