diff --git a/extensions/mattermost/src/channel.test.ts b/extensions/mattermost/src/channel.test.ts index 5d35056cc1..cd60f4fe65 100644 --- a/extensions/mattermost/src/channel.test.ts +++ b/extensions/mattermost/src/channel.test.ts @@ -1,7 +1,13 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk"; import { createReplyPrefixOptions } from "openclaw/plugin-sdk"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; import { mattermostPlugin } from "./channel.js"; +import { resetMattermostReactionBotUserCacheForTests } from "./mattermost/reactions.js"; +import { + createMattermostReactionFetchMock, + createMattermostTestConfig, + withMockedGlobalFetch, +} from "./mattermost/reactions.test-helpers.js"; describe("mattermostPlugin", () => { describe("messaging", () => { @@ -44,6 +50,10 @@ describe("mattermostPlugin", () => { }); describe("messageActions", () => { + beforeEach(() => { + resetMattermostReactionBotUserCacheForTests(); + }); + it("exposes react when mattermost is configured", () => { const cfg: OpenClawConfig = { channels: { @@ -142,41 +152,14 @@ describe("mattermostPlugin", () => { }); it("handles react by calling Mattermost reactions API", async () => { - const cfg: OpenClawConfig = { - channels: { - mattermost: { - enabled: true, - botToken: "test-token", - baseUrl: "https://chat.example.com", - }, - }, - }; - - const fetchImpl = vi.fn(async (url: any, init?: any) => { - if (String(url).endsWith("/api/v4/users/me")) { - return new Response(JSON.stringify({ id: "BOT123" }), { - status: 200, - headers: { "content-type": "application/json" }, - }); - } - if (String(url).endsWith("/api/v4/reactions")) { - expect(init?.method).toBe("POST"); - expect(JSON.parse(init?.body)).toEqual({ - user_id: "BOT123", - post_id: "POST1", - emoji_name: "thumbsup", - }); - return new Response(JSON.stringify({ ok: true }), { - status: 201, - headers: { "content-type": "application/json" }, - }); - } - throw new Error(`unexpected url: ${url}`); + const cfg = createMattermostTestConfig(); + const fetchImpl = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST1", + emojiName: "thumbsup", }); - const prevFetch = globalThis.fetch; - (globalThis as any).fetch = fetchImpl; - try { + const result = await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => { const result = await mattermostPlugin.actions?.handleAction?.({ channel: "mattermost", action: "react", @@ -185,51 +168,22 @@ describe("mattermostPlugin", () => { accountId: "default", } as any); - expect(result?.content).toEqual([ - { type: "text", text: "Reacted with :thumbsup: on POST1" }, - ]); - expect(result?.details).toEqual({}); - } finally { - (globalThis as any).fetch = prevFetch; - } + return result; + }); + + expect(result?.content).toEqual([{ type: "text", text: "Reacted with :thumbsup: on POST1" }]); + expect(result?.details).toEqual({}); }); it("only treats boolean remove flag as removal", async () => { - const cfg: OpenClawConfig = { - channels: { - mattermost: { - enabled: true, - botToken: "test-token", - baseUrl: "https://chat.example.com", - }, - }, - }; - - const fetchImpl = vi.fn(async (url: any, init?: any) => { - if (String(url).endsWith("/api/v4/users/me")) { - return new Response(JSON.stringify({ id: "BOT123" }), { - status: 200, - headers: { "content-type": "application/json" }, - }); - } - if (String(url).endsWith("/api/v4/reactions")) { - expect(init?.method).toBe("POST"); - expect(JSON.parse(init?.body)).toEqual({ - user_id: "BOT123", - post_id: "POST1", - emoji_name: "thumbsup", - }); - return new Response(JSON.stringify({ ok: true }), { - status: 201, - headers: { "content-type": "application/json" }, - }); - } - throw new Error(`unexpected url: ${url}`); + const cfg = createMattermostTestConfig(); + const fetchImpl = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST1", + emojiName: "thumbsup", }); - const prevFetch = globalThis.fetch; - (globalThis as any).fetch = fetchImpl; - try { + const result = await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => { const result = await mattermostPlugin.actions?.handleAction?.({ channel: "mattermost", action: "react", @@ -238,12 +192,10 @@ describe("mattermostPlugin", () => { accountId: "default", } as any); - expect(result?.content).toEqual([ - { type: "text", text: "Reacted with :thumbsup: on POST1" }, - ]); - } finally { - (globalThis as any).fetch = prevFetch; - } + return result; + }); + + expect(result?.content).toEqual([{ type: "text", text: "Reacted with :thumbsup: on POST1" }]); }); }); diff --git a/extensions/mattermost/src/mattermost/reactions.test-helpers.ts b/extensions/mattermost/src/mattermost/reactions.test-helpers.ts new file mode 100644 index 0000000000..3556067167 --- /dev/null +++ b/extensions/mattermost/src/mattermost/reactions.test-helpers.ts @@ -0,0 +1,83 @@ +import type { OpenClawConfig } from "openclaw/plugin-sdk"; +import { expect, vi } from "vitest"; + +export function createMattermostTestConfig(): OpenClawConfig { + return { + channels: { + mattermost: { + enabled: true, + botToken: "test-token", + baseUrl: "https://chat.example.com", + }, + }, + }; +} + +export function createMattermostReactionFetchMock(params: { + postId: string; + emojiName: string; + mode: "add" | "remove" | "both"; + userId?: string; + status?: number; + body?: unknown; +}) { + const userId = params.userId ?? "BOT123"; + const mode = params.mode; + const allowAdd = mode === "add" || mode === "both"; + const allowRemove = mode === "remove" || mode === "both"; + const addStatus = params.status ?? 201; + const removeStatus = params.status ?? 204; + const removePath = `/api/v4/users/${userId}/posts/${params.postId}/reactions/${encodeURIComponent(params.emojiName)}`; + + return vi.fn(async (url: any, init?: any) => { + if (String(url).endsWith("/api/v4/users/me")) { + return new Response(JSON.stringify({ id: userId }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + + if (allowAdd && String(url).endsWith("/api/v4/reactions")) { + expect(init?.method).toBe("POST"); + expect(JSON.parse(init?.body)).toEqual({ + user_id: userId, + post_id: params.postId, + emoji_name: params.emojiName, + }); + + const responseBody = params.body === undefined ? { ok: true } : params.body; + return new Response( + responseBody === null ? null : JSON.stringify(responseBody), + responseBody === null + ? { status: addStatus, headers: { "content-type": "text/plain" } } + : { status: addStatus, headers: { "content-type": "application/json" } }, + ); + } + + if (allowRemove && String(url).endsWith(removePath)) { + expect(init?.method).toBe("DELETE"); + const responseBody = params.body === undefined ? null : params.body; + return new Response( + responseBody === null ? null : JSON.stringify(responseBody), + responseBody === null + ? { status: removeStatus, headers: { "content-type": "text/plain" } } + : { status: removeStatus, headers: { "content-type": "application/json" } }, + ); + } + + throw new Error(`unexpected url: ${url}`); + }); +} + +export async function withMockedGlobalFetch( + fetchImpl: typeof fetch, + run: () => Promise, +): Promise { + const prevFetch = globalThis.fetch; + (globalThis as any).fetch = fetchImpl; + try { + return await run(); + } finally { + (globalThis as any).fetch = prevFetch; + } +} diff --git a/extensions/mattermost/src/mattermost/reactions.test.ts b/extensions/mattermost/src/mattermost/reactions.test.ts index 6ad74048c8..0b07c1b497 100644 --- a/extensions/mattermost/src/mattermost/reactions.test.ts +++ b/extensions/mattermost/src/mattermost/reactions.test.ts @@ -1,45 +1,28 @@ -import type { OpenClawConfig } from "openclaw/plugin-sdk"; -import { describe, expect, it, vi } from "vitest"; -import { addMattermostReaction, removeMattermostReaction } from "./reactions.js"; - -function createCfg(): OpenClawConfig { - return { - channels: { - mattermost: { - enabled: true, - botToken: "test-token", - baseUrl: "https://chat.example.com", - }, - }, - }; -} +import { beforeEach, describe, expect, it } from "vitest"; +import { + addMattermostReaction, + removeMattermostReaction, + resetMattermostReactionBotUserCacheForTests, +} from "./reactions.js"; +import { + createMattermostReactionFetchMock, + createMattermostTestConfig, +} from "./reactions.test-helpers.js"; describe("mattermost reactions", () => { + beforeEach(() => { + resetMattermostReactionBotUserCacheForTests(); + }); + it("adds reactions by calling /users/me then POST /reactions", async () => { - const fetchMock = vi.fn(async (url: any, init?: any) => { - if (String(url).endsWith("/api/v4/users/me")) { - return new Response(JSON.stringify({ id: "BOT123" }), { - status: 200, - headers: { "content-type": "application/json" }, - }); - } - if (String(url).endsWith("/api/v4/reactions")) { - expect(init?.method).toBe("POST"); - expect(JSON.parse(init?.body)).toEqual({ - user_id: "BOT123", - post_id: "POST1", - emoji_name: "thumbsup", - }); - return new Response(JSON.stringify({ ok: true }), { - status: 201, - headers: { "content-type": "application/json" }, - }); - } - throw new Error(`unexpected url: ${url}`); + const fetchMock = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST1", + emojiName: "thumbsup", }); const result = await addMattermostReaction({ - cfg: createCfg(), + cfg: createMattermostTestConfig(), postId: "POST1", emojiName: "thumbsup", fetchImpl: fetchMock as unknown as typeof fetch, @@ -50,24 +33,16 @@ describe("mattermost reactions", () => { }); it("returns a Result error when add reaction API call fails", async () => { - const fetchMock = vi.fn(async (url: any) => { - if (String(url).endsWith("/api/v4/users/me")) { - return new Response(JSON.stringify({ id: "BOT123" }), { - status: 200, - headers: { "content-type": "application/json" }, - }); - } - if (String(url).endsWith("/api/v4/reactions")) { - return new Response(JSON.stringify({ id: "err", message: "boom" }), { - status: 500, - headers: { "content-type": "application/json" }, - }); - } - throw new Error(`unexpected url: ${url}`); + const fetchMock = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST1", + emojiName: "thumbsup", + status: 500, + body: { id: "err", message: "boom" }, }); const result = await addMattermostReaction({ - cfg: createCfg(), + cfg: createMattermostTestConfig(), postId: "POST1", emojiName: "thumbsup", fetchImpl: fetchMock as unknown as typeof fetch, @@ -80,25 +55,14 @@ describe("mattermost reactions", () => { }); it("removes reactions by calling /users/me then DELETE /users/:id/posts/:postId/reactions/:emoji", async () => { - const fetchMock = vi.fn(async (url: any, init?: any) => { - if (String(url).endsWith("/api/v4/users/me")) { - return new Response(JSON.stringify({ id: "BOT123" }), { - status: 200, - headers: { "content-type": "application/json" }, - }); - } - if (String(url).endsWith("/api/v4/users/BOT123/posts/POST1/reactions/thumbsup")) { - expect(init?.method).toBe("DELETE"); - return new Response(null, { - status: 204, - headers: { "content-type": "text/plain" }, - }); - } - throw new Error(`unexpected url: ${url}`); + const fetchMock = createMattermostReactionFetchMock({ + mode: "remove", + postId: "POST1", + emojiName: "thumbsup", }); const result = await removeMattermostReaction({ - cfg: createCfg(), + cfg: createMattermostTestConfig(), postId: "POST1", emojiName: "thumbsup", fetchImpl: fetchMock as unknown as typeof fetch, @@ -107,4 +71,33 @@ describe("mattermost reactions", () => { expect(result).toEqual({ ok: true }); expect(fetchMock).toHaveBeenCalled(); }); + + it("caches the bot user id across reaction mutations", async () => { + const fetchMock = createMattermostReactionFetchMock({ + mode: "both", + postId: "POST1", + emojiName: "thumbsup", + }); + + const cfg = createMattermostTestConfig(); + const addResult = await addMattermostReaction({ + cfg, + postId: "POST1", + emojiName: "thumbsup", + fetchImpl: fetchMock as unknown as typeof fetch, + }); + const removeResult = await removeMattermostReaction({ + cfg, + postId: "POST1", + emojiName: "thumbsup", + fetchImpl: fetchMock as unknown as typeof fetch, + }); + + const usersMeCalls = fetchMock.mock.calls.filter((call) => + String(call[0]).endsWith("/api/v4/users/me"), + ); + expect(addResult).toEqual({ ok: true }); + expect(removeResult).toEqual({ ok: true }); + expect(usersMeCalls).toHaveLength(1); + }); }); diff --git a/extensions/mattermost/src/mattermost/reactions.ts b/extensions/mattermost/src/mattermost/reactions.ts index 03a01c03f2..cc67e63985 100644 --- a/extensions/mattermost/src/mattermost/reactions.ts +++ b/extensions/mattermost/src/mattermost/reactions.ts @@ -3,6 +3,15 @@ import { resolveMattermostAccount } from "./accounts.js"; import { createMattermostClient, fetchMattermostMe, type MattermostClient } from "./client.js"; type Result = { ok: true } | { ok: false; error: string }; +type ReactionParams = { + cfg: OpenClawConfig; + postId: string; + emojiName: string; + accountId?: string | null; + fetchImpl?: typeof fetch; +}; +type ReactionMutation = (client: MattermostClient, params: MutationPayload) => Promise; +type MutationPayload = { userId: string; postId: string; emojiName: string }; const BOT_USER_CACHE_TTL_MS = 10 * 60_000; const botUserIdCache = new Map(); @@ -31,36 +40,10 @@ export async function addMattermostReaction(params: { accountId?: string | null; fetchImpl?: typeof fetch; }): Promise { - const resolved = resolveMattermostAccount({ cfg: params.cfg, accountId: params.accountId }); - const baseUrl = resolved.baseUrl?.trim(); - const botToken = resolved.botToken?.trim(); - if (!baseUrl || !botToken) { - return { ok: false, error: "Mattermost botToken/baseUrl missing." }; - } - - const client = createMattermostClient({ - baseUrl, - botToken, - fetchImpl: params.fetchImpl, + return runMattermostReaction(params, { + action: "add", + mutation: createReaction, }); - - const cacheKey = `${baseUrl}:${botToken}`; - const userId = await resolveBotUserId(client, cacheKey); - if (!userId) { - return { ok: false, error: "Mattermost reactions failed: could not resolve bot user id." }; - } - - try { - await createReaction(client, { - userId, - postId: params.postId, - emojiName: params.emojiName, - }); - } catch (err) { - return { ok: false, error: `Mattermost add reaction failed: ${String(err)}` }; - } - - return { ok: true }; } export async function removeMattermostReaction(params: { @@ -70,6 +53,23 @@ export async function removeMattermostReaction(params: { accountId?: string | null; fetchImpl?: typeof fetch; }): Promise { + return runMattermostReaction(params, { + action: "remove", + mutation: deleteReaction, + }); +} + +export function resetMattermostReactionBotUserCacheForTests(): void { + botUserIdCache.clear(); +} + +async function runMattermostReaction( + params: ReactionParams, + options: { + action: "add" | "remove"; + mutation: ReactionMutation; + }, +): Promise { const resolved = resolveMattermostAccount({ cfg: params.cfg, accountId: params.accountId }); const baseUrl = resolved.baseUrl?.trim(); const botToken = resolved.botToken?.trim(); @@ -90,22 +90,19 @@ export async function removeMattermostReaction(params: { } try { - await deleteReaction(client, { + await options.mutation(client, { userId, postId: params.postId, emojiName: params.emojiName, }); } catch (err) { - return { ok: false, error: `Mattermost remove reaction failed: ${String(err)}` }; + return { ok: false, error: `Mattermost ${options.action} reaction failed: ${String(err)}` }; } return { ok: true }; } -async function createReaction( - client: MattermostClient, - params: { userId: string; postId: string; emojiName: string }, -): Promise { +async function createReaction(client: MattermostClient, params: MutationPayload): Promise { await client.request>("/reactions", { method: "POST", body: JSON.stringify({ @@ -116,10 +113,7 @@ async function createReaction( }); } -async function deleteReaction( - client: MattermostClient, - params: { userId: string; postId: string; emojiName: string }, -): Promise { +async function deleteReaction(client: MattermostClient, params: MutationPayload): Promise { const emoji = encodeURIComponent(params.emojiName); await client.request( `/users/${params.userId}/posts/${params.postId}/reactions/${emoji}`,