Skip to content

Commit 05fd04d

Browse files
committed
chore: show server error messages for test Slack alert failures
The test Slack alert toast showed a hardcoded message because the mutation hook discarded the API error body, hiding guidance like the /subscribe-alerts instruction. The hook now throws the server message and the team page shows it in the toast. Adds an MSW test for the propagation and backend coverage for SendTestSlackAlert, with the Slack send URLs made package vars so tests can stub them. closes #4061
1 parent c217820 commit 05fd04d

6 files changed

Lines changed: 280 additions & 6 deletions

File tree

backend/api/handlers/slack.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ import (
3030
// tests can point it at a stub server.
3131
var slackAccessURL = "https://slack.com/api/oauth.v2.access"
3232

33+
// slackPostMessageURL is Slack's message send endpoint. A package var so
34+
// tests can point it at a stub server.
35+
var slackPostMessageURL = "https://slack.com/api/chat.postMessage"
36+
37+
// slackConversationsInfoURL is Slack's channel info endpoint. A package var
38+
// so tests can point it at a stub server.
39+
var slackConversationsInfoURL = "https://slack.com/api/conversations.info"
40+
3341
type TeamSlack struct {
3442
SlackTeamName string `json:"slack_team_name"`
3543
IsActive bool `json:"is_active"`
@@ -563,7 +571,7 @@ func sendTestSlackMessages(botToken string, channelIds []string) error {
563571
// We'll try to fetch channel names via conversations.info and include them
564572
// in the success/failure message. If fetching the name fails, we fall back
565573
// to using the channel ID.
566-
url := "https://slack.com/api/chat.postMessage"
574+
url := slackPostMessageURL
567575

568576
var failed []struct {
569577
ChannelID string
@@ -675,7 +683,7 @@ func sendTestSlackMessages(botToken string, channelIds []string) error {
675683
// channel ID so callers can still show a usable identifier.
676684
func getSlackChannelName(botToken, channelID string) string {
677685
// build URL with query param
678-
infoURL := fmt.Sprintf("https://slack.com/api/conversations.info?channel=%s", url.QueryEscape(channelID))
686+
infoURL := fmt.Sprintf("%s?channel=%s", slackConversationsInfoURL, url.QueryEscape(channelID))
679687
client := &http.Client{Timeout: 10 * time.Second}
680688
req, err := http.NewRequest("GET", infoURL, nil)
681689
if err != nil {

backend/api/handlers/slack_test.go

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,3 +750,224 @@ func TestGetTeamSlackNeedsReauth(t *testing.T) {
750750
})
751751
}
752752
}
753+
754+
// --------------------------------------------------------------------------
755+
// SendTestSlackAlert
756+
// --------------------------------------------------------------------------
757+
758+
// withSlackSendURLs points the message send and channel info endpoints at a
759+
// stub for a test.
760+
func withSlackSendURLs(t *testing.T, postMessageURL, conversationsInfoURL string) {
761+
t.Helper()
762+
origPost, origInfo := slackPostMessageURL, slackConversationsInfoURL
763+
slackPostMessageURL = postMessageURL
764+
slackConversationsInfoURL = conversationsInfoURL
765+
t.Cleanup(func() {
766+
slackPostMessageURL = origPost
767+
slackConversationsInfoURL = origInfo
768+
})
769+
}
770+
771+
// newSlackSendStub stands in for Slack's chat.postMessage and
772+
// conversations.info endpoints and points the handler at it for a test.
773+
// conversations.info always resolves the channel name to "general".
774+
func newSlackSendStub(t *testing.T, postMessage http.HandlerFunc) {
775+
t.Helper()
776+
mux := http.NewServeMux()
777+
mux.HandleFunc("/chat.postMessage", postMessage)
778+
mux.HandleFunc("/conversations.info", func(w http.ResponseWriter, r *http.Request) {
779+
w.Header().Set("Content-Type", "application/json")
780+
_ = json.NewEncoder(w).Encode(map[string]any{
781+
"ok": true,
782+
"channel": map[string]string{"name": "general"},
783+
})
784+
})
785+
srv := httptest.NewServer(mux)
786+
t.Cleanup(srv.Close)
787+
withSlackSendURLs(t, srv.URL+"/chat.postMessage", srv.URL+"/conversations.info")
788+
}
789+
790+
// rejectSends is a chat.postMessage stub for tests where the handler must
791+
// bail out before sending anything.
792+
func rejectSends(t *testing.T) http.HandlerFunc {
793+
return func(w http.ResponseWriter, r *http.Request) {
794+
t.Error("chat.postMessage was called, want no send attempt")
795+
}
796+
}
797+
798+
func seedTeamSlackWithChannels(ctx context.Context, t *testing.T, teamID uuid.UUID, botToken string, channels []string) {
799+
t.Helper()
800+
_, err := th.PgPool.Exec(ctx,
801+
`INSERT INTO team_slack
802+
(team_id, slack_team_id, slack_team_name, bot_token, bot_user_id, channel_ids, scopes, is_active, created_at, updated_at)
803+
VALUES ($1, 'T1', 'Acme Workspace', $2, 'U1', $3, 'chat:write', true, now(), now())`,
804+
teamID, botToken, channels)
805+
if err != nil {
806+
t.Fatalf("seed team_slack with channels: %v", err)
807+
}
808+
}
809+
810+
func newTestAlertContext(userID string, teamID string) (*gin.Context, *httptest.ResponseRecorder) {
811+
c, w := newTestGinContext("POST", "/teams/"+teamID+"/slack/test", nil)
812+
c.Set("userId", userID)
813+
c.Params = gin.Params{{Key: "id", Value: teamID}}
814+
return c, w
815+
}
816+
817+
func TestSendTestSlackAlert(t *testing.T) {
818+
ctx := context.Background()
819+
820+
readError := func(t *testing.T, w *httptest.ResponseRecorder) string {
821+
t.Helper()
822+
var body struct {
823+
Error string `json:"error"`
824+
}
825+
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
826+
t.Fatalf("unmarshal error body %q: %v", w.Body.String(), err)
827+
}
828+
return body.Error
829+
}
830+
831+
t.Run("invalid team id gives 400", func(t *testing.T) {
832+
defer cleanupAll(ctx, t)
833+
834+
c, w := newTestAlertContext(uuid.New().String(), "not-a-uuid")
835+
h.SendTestSlackAlert(c)
836+
837+
if w.Code != http.StatusBadRequest {
838+
t.Fatalf("status = %d, want 400, body: %s", w.Code, w.Body.String())
839+
}
840+
})
841+
842+
// sending a test alert needs ScopeTeamAll, which only owner holds
843+
for _, role := range []string{"admin", "developer", "viewer"} {
844+
t.Run(role+" is forbidden", func(t *testing.T) {
845+
defer cleanupAll(ctx, t)
846+
newSlackSendStub(t, rejectSends(t))
847+
848+
userID, teamID := seedTeamAndMemberWithRole(t, ctx, role)
849+
seedTeamSlackWithChannels(ctx, t, teamID, "xoxb-test", []string{"C1"})
850+
851+
c, w := newTestAlertContext(userID, teamID.String())
852+
h.SendTestSlackAlert(c)
853+
854+
if w.Code != http.StatusForbidden {
855+
t.Fatalf("role %s: status = %d, want 403, body: %s", role, w.Code, w.Body.String())
856+
}
857+
})
858+
}
859+
860+
t.Run("no slack connection gives 500", func(t *testing.T) {
861+
defer cleanupAll(ctx, t)
862+
newSlackSendStub(t, rejectSends(t))
863+
864+
ownerID, teamID := seedTeamAndMemberWithRole(t, ctx, "owner")
865+
866+
c, w := newTestAlertContext(ownerID, teamID.String())
867+
h.SendTestSlackAlert(c)
868+
869+
if w.Code != http.StatusInternalServerError {
870+
t.Fatalf("status = %d, want 500, body: %s", w.Code, w.Body.String())
871+
}
872+
if got := readError(t, w); !strings.Contains(got, "error occurred while querying team slack") {
873+
t.Errorf("error = %q, want it to mention the team slack query", got)
874+
}
875+
})
876+
877+
t.Run("no subscribed channels gives 500 with subscribe guidance", func(t *testing.T) {
878+
defer cleanupAll(ctx, t)
879+
newSlackSendStub(t, rejectSends(t))
880+
881+
ownerID, teamID := seedTeamAndMemberWithRole(t, ctx, "owner")
882+
seedTeamSlackWithChannels(ctx, t, teamID, "xoxb-test", []string{})
883+
884+
c, w := newTestAlertContext(ownerID, teamID.String())
885+
h.SendTestSlackAlert(c)
886+
887+
if w.Code != http.StatusInternalServerError {
888+
t.Fatalf("status = %d, want 500, body: %s", w.Code, w.Body.String())
889+
}
890+
got := readError(t, w)
891+
if !strings.Contains(got, "No registered alert channels found for Workspace Acme Workspace") {
892+
t.Errorf("error = %q, want the workspace named in the no-channels message", got)
893+
}
894+
if !strings.Contains(got, "/subscribe-alerts") {
895+
t.Errorf("error = %q, want the /subscribe-alerts instruction", got)
896+
}
897+
})
898+
899+
t.Run("owner with subscribed channels gets 200 and a message per channel", func(t *testing.T) {
900+
defer cleanupAll(ctx, t)
901+
902+
var gotChannels, gotAuth []string
903+
newSlackSendStub(t, func(w http.ResponseWriter, r *http.Request) {
904+
var body struct {
905+
Channel string `json:"channel"`
906+
Text string `json:"text"`
907+
}
908+
_ = json.NewDecoder(r.Body).Decode(&body)
909+
gotChannels = append(gotChannels, body.Channel)
910+
gotAuth = append(gotAuth, r.Header.Get("Authorization"))
911+
if body.Text == "" {
912+
t.Error("message text is empty")
913+
}
914+
w.Header().Set("Content-Type", "application/json")
915+
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true})
916+
})
917+
918+
ownerID, teamID := seedTeamAndMemberWithRole(t, ctx, "owner")
919+
seedTeamSlackWithChannels(ctx, t, teamID, "xoxb-test", []string{"C1", "C2"})
920+
921+
c, w := newTestAlertContext(ownerID, teamID.String())
922+
h.SendTestSlackAlert(c)
923+
924+
if w.Code != http.StatusOK {
925+
t.Fatalf("status = %d, want 200, body: %s", w.Code, w.Body.String())
926+
}
927+
if len(gotChannels) != 2 || gotChannels[0] != "C1" || gotChannels[1] != "C2" {
928+
t.Errorf("sent to channels %v, want [C1 C2]", gotChannels)
929+
}
930+
for _, auth := range gotAuth {
931+
if auth != "Bearer xoxb-test" {
932+
t.Errorf("Authorization = %q, want the stored bot token as bearer", auth)
933+
}
934+
}
935+
})
936+
937+
t.Run("a failed channel gives 500 naming successes and failures", func(t *testing.T) {
938+
defer cleanupAll(ctx, t)
939+
940+
newSlackSendStub(t, func(w http.ResponseWriter, r *http.Request) {
941+
var body struct {
942+
Channel string `json:"channel"`
943+
}
944+
_ = json.NewDecoder(r.Body).Decode(&body)
945+
w.Header().Set("Content-Type", "application/json")
946+
if body.Channel == "C2" {
947+
_ = json.NewEncoder(w).Encode(map[string]any{"ok": false, "error": "channel_not_found"})
948+
return
949+
}
950+
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true})
951+
})
952+
953+
ownerID, teamID := seedTeamAndMemberWithRole(t, ctx, "owner")
954+
seedTeamSlackWithChannels(ctx, t, teamID, "xoxb-test", []string{"C1", "C2"})
955+
956+
c, w := newTestAlertContext(ownerID, teamID.String())
957+
h.SendTestSlackAlert(c)
958+
959+
if w.Code != http.StatusInternalServerError {
960+
t.Fatalf("status = %d, want 500, body: %s", w.Code, w.Body.String())
961+
}
962+
got := readError(t, w)
963+
if !strings.Contains(got, "Successes") || !strings.Contains(got, "C1") {
964+
t.Errorf("error = %q, want the delivered channel listed under successes", got)
965+
}
966+
if !strings.Contains(got, "Failures") || !strings.Contains(got, "C2") {
967+
t.Errorf("error = %q, want the failed channel listed under failures", got)
968+
}
969+
if !strings.Contains(got, "channel_not_found") {
970+
t.Errorf("error = %q, want the slack error reason included", got)
971+
}
972+
})
973+
}

frontend/dashboard/__tests__/integration/team_msw_test.tsx

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ afterAll(() => server.close());
9898
// --- Store/component imports ---
9999
import TeamOverview from "@/app/[teamId]/team/page";
100100
import { Team } from "@/app/api/api_calls";
101+
import { Toaster } from "@/app/components/toaster";
101102
import { useCreateTeamMutation } from "@/app/query/hooks";
102103
import { queryClient } from "@/app/query/query_client";
103104
import { QueryClientProvider } from "@tanstack/react-query";
@@ -1253,6 +1254,47 @@ describe("Team Page — mutations", () => {
12531254
await new Promise((r) => setTimeout(r, 300));
12541255
expect(postCalled).toBe(false);
12551256
});
1257+
1258+
it("shows the server error message in the toast when the API fails", async () => {
1259+
const serverError =
1260+
"No registered alert channels found for Workspace Acme. Please add Measure app to a channel and use /subscribe-alerts";
1261+
server.use(
1262+
http.post("*/api/teams/:teamId/slack/test", () => {
1263+
return HttpResponse.json({ error: serverError }, { status: 500 });
1264+
}),
1265+
);
1266+
1267+
renderWithProviders(
1268+
<>
1269+
<TeamOverview params={promiseParams({ teamId: "team-001" })} />
1270+
<Toaster />
1271+
</>,
1272+
);
1273+
await waitFor(
1274+
() => {
1275+
expect(screen.getByText("Send Test Alert")).toBeTruthy();
1276+
},
1277+
{ timeout: 5000 },
1278+
);
1279+
1280+
await act(async () => {
1281+
fireEvent.click(screen.getByText("Send Test Alert"));
1282+
});
1283+
1284+
await waitFor(() => {
1285+
expect(screen.getByText("Yes, I'm sure")).toBeTruthy();
1286+
});
1287+
await act(async () => {
1288+
fireEvent.click(screen.getByText("Yes, I'm sure"));
1289+
});
1290+
1291+
await waitFor(
1292+
() => {
1293+
expect(screen.getByText(serverError)).toBeTruthy();
1294+
},
1295+
{ timeout: 5000 },
1296+
);
1297+
});
12561298
});
12571299

12581300
describe("remove Slack connection", () => {

frontend/dashboard/__tests__/pages/team_test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ jest.mock("@/app/query/hooks", () => ({
449449
if (result) {
450450
opts?.onSuccess?.();
451451
} else {
452-
opts?.onError?.();
452+
opts?.onError?.(new Error("Failed to send test Slack alert"));
453453
}
454454
},
455455
isPending: s.testSlackAlertApiStatus === "loading",
@@ -1649,7 +1649,7 @@ describe("Team Page", () => {
16491649

16501650
await waitFor(() => {
16511651
expect(mockToastNegative).toHaveBeenCalledWith(
1652-
"Error sending test Slack alerts",
1652+
"Failed to send test Slack alert",
16531653
);
16541654
});
16551655
});

frontend/dashboard/app/[teamId]/team/page.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,8 @@ export default function TeamOverview(props: {
385385
onSuccess: () => {
386386
toastPositive(`Slack integration test alert sent successfully`);
387387
},
388-
onError: () => {
389-
toastNegative(`Error sending test Slack alerts`);
388+
onError: (error) => {
389+
toastNegative(error.message);
390390
},
391391
},
392392
);

frontend/dashboard/app/query/hooks.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,6 +1641,9 @@ export function useTestSlackAlertMutation() {
16411641
return useMutation({
16421642
mutationFn: async (params: { teamId: string }) => {
16431643
const result = await sendTestSlackAlertFromServer(params.teamId);
1644+
if (result.status === TestSlackAlertApiStatus.Error) {
1645+
throw new Error(result.error ?? "Failed to send test Slack alert");
1646+
}
16441647
if (result.status !== TestSlackAlertApiStatus.Success) {
16451648
throw new Error("Failed to send test Slack alert");
16461649
}

0 commit comments

Comments
 (0)