diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62d89986..4d1ac26a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,7 @@ jobs: - run: yarn test:card-byline - run: yarn test:link-preview-retry - run: yarn test:stable-attachment-urls + - run: yarn test:group-picture-upload - run: yarn test:member-invite - run: yarn test:addon-capabilities - run: yarn test:actor-name diff --git a/package.json b/package.json index c4595cfd..a1b90300 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,7 @@ "test:card-byline": "node scripts/check-card-byline.mjs", "test:link-preview-retry": "node scripts/check-link-preview-retry.mjs", "test:stable-attachment-urls": "node --disable-warning=ExperimentalWarning scripts/check-stable-attachment-urls.mjs", + "test:group-picture-upload": "node --disable-warning=ExperimentalWarning scripts/check-group-picture-upload.mjs", "test:examples": "node --test examples/examples.test.mjs", "test:member-invite": "node scripts/check-member-invite.mjs", "test:addon-capabilities": "node scripts/check-addon-capabilities.mjs", diff --git a/scripts/check-group-picture-upload.mjs b/scripts/check-group-picture-upload.mjs new file mode 100644 index 00000000..d396ae45 --- /dev/null +++ b/scripts/check-group-picture-upload.mjs @@ -0,0 +1,80 @@ +/* eslint-env node */ + +// A group's picture went through the avatar upload, which also made it the uploader's +// own avatar. It has its own route now, and an older server without it gets no picture. GRYT-1182. + +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { stripTypeScriptTypes } from "node:module"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = join(dirname(fileURLToPath(import.meta.url)), ".."); +const read = (path) => readFileSync(join(root, path), "utf8"); + +const HELPER = "src/packages/socket/src/utils/uploadGroupPicture.ts"; +const DIALOG = "src/packages/socket/src/components/GroupDialog.tsx"; + +const helperSource = stripTypeScriptTypes(read(HELPER)).replace(/^export /gm, ""); +const load = (fetch) => + new Function("fetch", `${helperSource}\nreturn { uploadGroupPicture, GROUP_PICTURE_NEEDS_UPDATE };`)(fetch); + +function fakeServer(answer) { + const calls = []; + const fetch = async (url, init) => { + calls.push({ url, init }); + const { status, body } = answer(url); + return { + status, + ok: status >= 200 && status < 300, + json: async () => { + if (typeof body === "string") throw new SyntaxError("not json"); + return body; + }, + }; + }; + return { calls, fetch }; +} + +const picture = new Blob([new Uint8Array([1, 2, 3])], { type: "image/png" }); + +/* ── a current server ───────────────────────────────────────────────────── */ +{ + const server = fakeServer(() => ({ status: 201, body: { fileId: "f-1", processing: false } })); + const { uploadGroupPicture } = load(server.fetch); + + assert.equal(await uploadGroupPicture("http://h", "tok", picture, "g.png"), "f-1"); + assert.equal(server.calls.length, 1); + assert.equal(server.calls[0].url, "http://h/api/uploads/group-icon"); + assert.equal(server.calls[0].init.headers.Authorization, "Bearer tok"); + assert.ok(server.calls[0].init.body.get("file"), "the picture was not in the form"); +} + +/* ── a refusal carries the server's message ─────────────────────────────── */ +{ + const server = fakeServer(() => ({ status: 413, body: { error: "file_too_large", message: "Group picture too large. Max 5.0MB." } })); + const { uploadGroupPicture } = load(server.fetch); + await assert.rejects(uploadGroupPicture("http://h", "tok", picture, "g.png"), /Group picture too large/); +} + +/* ── an older server: no route, and no second try at the avatar one ─────── */ +{ + const server = fakeServer((url) => + url.endsWith("/api/uploads/avatar") + ? { status: 201, body: { avatarFileId: "would-be-your-avatar" } } + : { status: 404, body: "
Cannot POST /api/uploads/group-icon" }, + ); + const { uploadGroupPicture, GROUP_PICTURE_NEEDS_UPDATE } = load(server.fetch); + + await assert.rejects(uploadGroupPicture("http://h", "tok", picture, "g.png"), (e) => e.message === GROUP_PICTURE_NEEDS_UPDATE); + assert.deepEqual(server.calls.map((c) => c.url), ["http://h/api/uploads/group-icon"], "it fell back to the avatar route"); +} + +/* ── the dialog uses it ─────────────────────────────────────────────────── */ +{ + const dialog = read(DIALOG); + assert.match(dialog, /await uploadGroupPicture\(/, "GroupDialog no longer uploads through uploadGroupPicture"); + assert.doesNotMatch(dialog, /\/api\/uploads\/avatar/, "GroupDialog talks to the avatar route again"); +} + +console.log("group picture upload: ok"); diff --git a/src/packages/socket/src/components/GroupDialog.tsx b/src/packages/socket/src/components/GroupDialog.tsx index f507e5de..64a0157d 100644 --- a/src/packages/socket/src/components/GroupDialog.tsx +++ b/src/packages/socket/src/components/GroupDialog.tsx @@ -10,6 +10,7 @@ import { } from "@/common"; import { conversationTitle, type DirectConversation } from "../hooks/useDirectMessages"; +import { uploadGroupPicture } from "../utils/uploadGroupPicture"; import { EmojiText } from "./EmojiText"; import type { MemberInfo } from "./MemberSidebar"; @@ -112,23 +113,9 @@ export const GroupDialog = ({ try { const token = getServerAccessToken(serverHost); if (!token) throw new Error("Not signed in to this server"); - const form = new FormData(); - form.append("file", file, file.name || "group.png"); - /* The avatar endpoint, because a group picture is the same job. A second - endpoint is a second place for the limits to drift. */ - const response = await fetch(`${getServerHttpBase(serverHost)}/api/uploads/avatar`, { - method: "POST", - headers: { Authorization: `Bearer ${token}` }, - body: form, - }); - const data = (await response.json().catch(() => ({}))) as { - avatarFileId?: string; - message?: string; - }; - if (!response.ok || !data.avatarFileId) { - throw new Error(data.message || "The server would not take that picture"); - } - setIconFileId(data.avatarFileId); + setIconFileId( + await uploadGroupPicture(getServerHttpBase(serverHost), token, file, file.name || "group.png"), + ); } catch (error) { toast.error(error instanceof Error ? error.message : "Could not upload that"); } finally { diff --git a/src/packages/socket/src/utils/uploadGroupPicture.ts b/src/packages/socket/src/utils/uploadGroupPicture.ts new file mode 100644 index 00000000..ae315137 --- /dev/null +++ b/src/packages/socket/src/utils/uploadGroupPicture.ts @@ -0,0 +1,30 @@ +/** Shown when the server predates `/api/uploads/group-icon`. */ +export const GROUP_PICTURE_NEEDS_UPDATE = + "This server needs an update before groups can have their own picture."; + +/** Never falls back to the avatar route, which made the picture your own avatar + too (GRYT-1182). An older server gets no group picture instead. */ +export async function uploadGroupPicture( + base: string, + accessToken: string, + file: Blob, + filename: string, +): Promise