Choosing a group picture replaced your own avatar (GRYT-1182)

.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

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",

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: "<pre>Cannot POST /api/uploads/group-icon</pre>" },
+  );
+  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");

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 {

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<string> {
+  const form = new FormData();
+  form.append("file", file, filename);
+  const response = await fetch(`${base}/api/uploads/group-icon`, {
+    method: "POST",
+    headers: { Authorization: `Bearer ${accessToken}` },
+    body: form,
+  });
+  if (response.status === 404) throw new Error(GROUP_PICTURE_NEEDS_UPDATE);
+
+  const data = (await response.json().catch(() => ({}))) as {
+    fileId?: string;
+    message?: string;
+  };
+  if (!response.ok || !data.fileId) {
+    throw new Error(data.message || "The server would not take that picture");
+  }
+  return data.fileId;
+}