mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
## Problem In the **Edit channel** dialog, flipping visibility (Public <> Private) persisted **immediately on selection**, bypassing the **Save changes** button — while every other field (name, description, temporary, TTL) waited for an explicit save. This surprised users and gave no chance to cancel a flip, e.g. a private->public change that instantly exposes channel history. Reported in the Buzz "Welcome" channel by Kevin Chung. ## Root cause The visibility dropdown was wired to `handleConvertVisibility()`, which called the update mutation on selection. This was intentional at the time (there was even an e2e test named `02 — visibility updates immediately` and an "Updating…" spinner), but it is inconsistent with the rest of the dialog and is the surprising behavior reported. ## Change (defer to Save) - Visibility becomes a **deferred draft** like the other fields: selecting a value updates local `isPrivateDraft` and marks the draft dirty. The change commits via `handleSaveChannelEdits` (which already handled visibility) on **Save**, and is discarded on **Cancel**. - The dialog title now reflects the **pending draft** (`nextVisibility`), so the pending choice is visible before saving. - The edit-dialog reset restores `isPrivateDraft` from server state. - Removed the now-dead `handleConvertVisibility` handler, `isConvertingVisibility` state, the `channelIdRef` race guard it needed, and the unused `isPending`/"Updating…" spinner path in `ChannelPermissionsSettings` (no caller passes `isPending` anymore). ## Tests - Rewrote e2e `02` -> **`visibility defers to Save`**: select -> Save enabled -> title reflects draft -> Save -> persists; toggling back to the original value clears the draft and disables Save. - Extended `09` (cancel discards drafts) to also cover a visibility change. - Repurposed `10`: the stale-update race it guarded is architecturally gone, so it now asserts an **unsaved visibility draft does not leak across a channel switch**. ## Validation - `pnpm typecheck` — clean - `biome check` (changed files) — clean - `pnpm test` — **4497 passed / 0 failed** - `playwright test --project=smoke channel-controls` — **10 passed** Signed-off-by: Kevin Chung <chung@squareup.com> Co-authored-by: Fizz <e3f95089179cc1bcc68d70c334b9bdf670d0470496db90bcdbb20386963432da@buzz.block.builderlab.xyz>
408 lines
14 KiB
TypeScript
408 lines
14 KiB
TypeScript
import { expect, test } from "@playwright/test";
|
|
|
|
import { installMockBridge } from "../helpers/bridge";
|
|
|
|
// `general` seeds the mock identity as owner, so the owner/admin-gated
|
|
// visibility + ephemeral controls are editable. Visibility is a deferred
|
|
// draft: selecting a value only updates local state; it persists on Save.
|
|
async function openManagementSheet(page: import("@playwright/test").Page) {
|
|
await page.goto("/");
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
await page.getByTestId("channel-management-trigger").click();
|
|
await expect(page.getByTestId("channel-management-sheet")).toBeVisible();
|
|
}
|
|
|
|
async function openEditDialog(page: import("@playwright/test").Page) {
|
|
await page.getByTestId("channel-management-edit").click();
|
|
await expect(
|
|
page.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
}),
|
|
).toBeVisible();
|
|
}
|
|
|
|
async function settle(page: import("@playwright/test").Page) {
|
|
await page.evaluate(() =>
|
|
Promise.all(document.getAnimations().map((a) => a.finished)),
|
|
);
|
|
}
|
|
|
|
async function selectTemporaryChannelType(
|
|
page: import("@playwright/test").Page,
|
|
) {
|
|
await page.getByTestId("channel-management-channel-type").click();
|
|
await page.getByLabel("Temporary channel").click();
|
|
}
|
|
|
|
test.describe("channel controls", () => {
|
|
test("01 — lifecycle section: visibility + channel type", async ({
|
|
page,
|
|
}) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit public channel" }),
|
|
).toBeVisible();
|
|
await expect(page.getByText(/Update settings for/)).toHaveCount(0);
|
|
|
|
const lifecycle = page.getByTestId("channel-management-lifecycle");
|
|
await lifecycle.scrollIntoViewIfNeeded();
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions-container"),
|
|
).toBeVisible();
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions"),
|
|
).toHaveAccessibleName("Visibility: Public");
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeDisabled();
|
|
await expect(
|
|
page.getByTestId("channel-management-channel-type"),
|
|
).toBeVisible();
|
|
await expect(
|
|
page.getByTestId("channel-management-channel-type"),
|
|
).toContainText("Ongoing");
|
|
await expect(
|
|
page.getByTestId("channel-management-ephemeral-settings"),
|
|
).toHaveCount(0);
|
|
await expect(
|
|
page
|
|
.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
})
|
|
.getByTestId("channel-management-topic"),
|
|
).toHaveCount(0);
|
|
await expect(
|
|
page
|
|
.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
})
|
|
.getByTestId("channel-management-purpose"),
|
|
).toHaveCount(0);
|
|
await settle(page);
|
|
});
|
|
|
|
test("02 — visibility defers to Save", async ({ page }) => {
|
|
await installMockBridge(page, { updateChannelDelayMs: 500 });
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
const lifecycle = page.getByTestId("channel-management-lifecycle");
|
|
await lifecycle.scrollIntoViewIfNeeded();
|
|
const permissions = page.getByTestId("channel-management-permissions");
|
|
|
|
// Save starts disabled with no pending edits.
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeDisabled();
|
|
|
|
// Selecting a visibility only updates the local draft: the dialog title
|
|
// reflects the pending choice and Save becomes enabled, but nothing is
|
|
// persisted yet (no "Updating…" state).
|
|
await permissions.click();
|
|
await page
|
|
.getByTestId("channel-management-permissions-option-private")
|
|
.click();
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit private channel" }),
|
|
).toBeVisible();
|
|
await expect(permissions).toHaveAccessibleName("Visibility: Private");
|
|
await expect(permissions).not.toHaveAttribute("aria-busy", "true");
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeEnabled();
|
|
// Wait for the dropdown to fully close before reopening it, so the
|
|
// trigger stays mounted/stable for the next interaction.
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions-option-private"),
|
|
).toHaveCount(0);
|
|
|
|
// Toggling back to the original value clears the draft and disables Save.
|
|
await permissions.click();
|
|
await page
|
|
.getByTestId("channel-management-permissions-option-open")
|
|
.click();
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit public channel" }),
|
|
).toBeVisible();
|
|
await expect(permissions).toHaveAccessibleName("Visibility: Public");
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeDisabled();
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions-option-open"),
|
|
).toHaveCount(0);
|
|
|
|
// Choose Private again and commit via Save.
|
|
await permissions.click();
|
|
await page
|
|
.getByTestId("channel-management-permissions-option-private")
|
|
.click();
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeEnabled();
|
|
await page.getByTestId("channel-management-save-changes").click();
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toHaveText("Saving...");
|
|
await expect(
|
|
page.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
}),
|
|
).toHaveCount(0);
|
|
|
|
// Reopen to confirm the visibility change persisted.
|
|
await openEditDialog(page);
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit private channel" }),
|
|
).toBeVisible();
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions"),
|
|
).toHaveAccessibleName("Visibility: Private");
|
|
await settle(page);
|
|
});
|
|
|
|
test("03 — Temporary type reveals expiration presets", async ({ page }) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
const lifecycle = page.getByTestId("channel-management-lifecycle");
|
|
await lifecycle.scrollIntoViewIfNeeded();
|
|
await selectTemporaryChannelType(page);
|
|
|
|
const ephemeralSettings = page.getByTestId(
|
|
"channel-management-ephemeral-settings",
|
|
);
|
|
await expect(ephemeralSettings).toBeVisible();
|
|
const ttl = ephemeralSettings.getByTestId("channel-management-ttl");
|
|
await expect(ttl).toBeVisible();
|
|
await expect(ttl).toHaveAttribute("aria-label", "Expires after");
|
|
await expect(ttl).toContainText("7 days");
|
|
await ttl.click();
|
|
await expect(
|
|
page.getByTestId("channel-management-ttl-option-1800"),
|
|
).toHaveText("30 minutes");
|
|
await expect(
|
|
page.getByTestId("channel-management-ttl-option-2592000"),
|
|
).toHaveText("30 days");
|
|
await page.getByTestId("channel-management-ttl-option-86400").click();
|
|
await expect(ttl).toContainText("1 day");
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeEnabled();
|
|
await page.getByTestId("channel-management-channel-type").click();
|
|
await page.getByLabel("Ongoing channel").click();
|
|
await expect(ephemeralSettings).toHaveCount(0);
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeDisabled();
|
|
});
|
|
|
|
test("04 — changing the timeout preset keeps save enabled", async ({
|
|
page,
|
|
}) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
const lifecycle = page.getByTestId("channel-management-lifecycle");
|
|
await lifecycle.scrollIntoViewIfNeeded();
|
|
await selectTemporaryChannelType(page);
|
|
|
|
const ttl = page.getByTestId("channel-management-ttl");
|
|
await ttl.click();
|
|
await page.getByTestId("channel-management-ttl-option-21600").click();
|
|
await expect(ttl).toContainText("6 hours");
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeEnabled();
|
|
});
|
|
|
|
test("05 — sticky footer pins lifecycle buttons", async ({ page }) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
|
|
const footer = page.getByTestId("channel-management-footer");
|
|
await expect(footer).toBeVisible();
|
|
await expect(page.getByTestId("channel-management-archive")).toBeVisible();
|
|
await settle(page);
|
|
});
|
|
|
|
test("06 — full sheet with new controls", async ({ page }) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
|
|
const sheet = page.getByTestId("channel-management-sheet");
|
|
await expect(sheet).toBeVisible();
|
|
await settle(page);
|
|
});
|
|
|
|
test("07 — saving lifecycle uses unified save", async ({ page }) => {
|
|
await installMockBridge(page, { updateChannelDelayMs: 500 });
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
await selectTemporaryChannelType(page);
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeEnabled();
|
|
|
|
await page.getByTestId("channel-management-save-changes").click();
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toHaveText("Saving...");
|
|
|
|
await expect(
|
|
page.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
}),
|
|
).toHaveCount(0);
|
|
});
|
|
|
|
test("08 — saved ephemeral lifecycle is reflected after reopen", async ({
|
|
page,
|
|
}) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
await page.getByTestId("channel-management-permissions").click();
|
|
await page
|
|
.getByTestId("channel-management-permissions-option-private")
|
|
.click();
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit private channel" }),
|
|
).toBeVisible();
|
|
await selectTemporaryChannelType(page);
|
|
await page.getByTestId("channel-management-save-changes").click();
|
|
await expect(
|
|
page.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
}),
|
|
).toHaveCount(0);
|
|
|
|
await page.getByTestId("auxiliary-panel-close").click();
|
|
await expect(
|
|
page.getByTestId("channel-management-sheet"),
|
|
).not.toBeVisible();
|
|
await page.getByTestId("channel-management-trigger").click();
|
|
await openEditDialog(page);
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit private channel" }),
|
|
).toBeVisible();
|
|
|
|
const lifecycle = page.getByTestId("channel-management-lifecycle");
|
|
await lifecycle.scrollIntoViewIfNeeded();
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions"),
|
|
).toHaveAccessibleName("Visibility: Private");
|
|
await expect(
|
|
page.getByTestId("channel-management-channel-type"),
|
|
).toContainText("Temporary");
|
|
await expect(page.getByTestId("channel-management-ttl")).toContainText(
|
|
"7 days",
|
|
);
|
|
await settle(page);
|
|
});
|
|
|
|
test("09 — cancel discards unsaved channel drafts", async ({ page }) => {
|
|
await installMockBridge(page);
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
await page.getByTestId("channel-management-name").fill("discarded-name");
|
|
await page
|
|
.getByRole("textbox", { name: "Description" })
|
|
.fill("This description should be discarded");
|
|
await page.getByTestId("channel-management-permissions").click();
|
|
await page
|
|
.getByTestId("channel-management-permissions-option-private")
|
|
.click();
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit private channel" }),
|
|
).toBeVisible();
|
|
await selectTemporaryChannelType(page);
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeEnabled();
|
|
|
|
await page.getByRole("button", { name: "Cancel" }).click();
|
|
await expect(
|
|
page.getByRole("dialog", {
|
|
name: /Edit (?:public|private) channel/,
|
|
}),
|
|
).toHaveCount(0);
|
|
|
|
await openEditDialog(page);
|
|
await expect(page.getByTestId("channel-management-name")).toHaveValue(
|
|
"general",
|
|
);
|
|
await expect(
|
|
page.getByRole("textbox", { name: "Description" }),
|
|
).toHaveValue("General discussion for everyone");
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit public channel" }),
|
|
).toBeVisible();
|
|
await expect(
|
|
page.getByTestId("channel-management-permissions"),
|
|
).toHaveAccessibleName("Visibility: Public");
|
|
await expect(
|
|
page.getByTestId("channel-management-channel-type"),
|
|
).toContainText("Ongoing");
|
|
await expect(
|
|
page.getByTestId("channel-management-ephemeral-settings"),
|
|
).toHaveCount(0);
|
|
await expect(
|
|
page.getByTestId("channel-management-save-changes"),
|
|
).toBeDisabled();
|
|
});
|
|
|
|
test("10 — unsaved visibility draft does not leak to a new channel", async ({
|
|
page,
|
|
}) => {
|
|
await installMockBridge(page, { updateChannelDelayMs: 1_500 });
|
|
await openManagementSheet(page);
|
|
await openEditDialog(page);
|
|
|
|
const permissions = page.getByTestId("channel-management-permissions");
|
|
await permissions.click();
|
|
await page
|
|
.getByTestId("channel-management-permissions-option-private")
|
|
.click();
|
|
// Draft only — never saved. The dialog title reflects the pending choice.
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit private channel" }),
|
|
).toBeVisible();
|
|
|
|
const agentsChannelId = await page
|
|
.getByTestId("channel-agents")
|
|
.getAttribute("data-channel-id");
|
|
if (!agentsChannelId) {
|
|
throw new Error("Expected the agents channel id.");
|
|
}
|
|
await page.evaluate((channelId) => {
|
|
const hash = window.location.hash.replace(/^#/, "") || "/";
|
|
const [, query = ""] = hash.split("?");
|
|
const nextHash = `#/channels/${channelId}${query ? `?${query}` : ""}`;
|
|
window.history.pushState(
|
|
{},
|
|
"",
|
|
`${window.location.pathname}${nextHash}`,
|
|
);
|
|
window.dispatchEvent(new HashChangeEvent("hashchange"));
|
|
window.dispatchEvent(new PopStateEvent("popstate"));
|
|
}, agentsChannelId);
|
|
await expect(page.getByTestId("chat-title")).toHaveText("agents");
|
|
|
|
// The unsaved draft must not carry over: the new channel re-syncs from
|
|
// server state and shows its own (public) visibility.
|
|
await expect(
|
|
page.getByRole("dialog", { name: "Edit public channel" }),
|
|
).toBeVisible();
|
|
await expect(permissions).toHaveAccessibleName("Visibility: Public");
|
|
});
|
|
});
|