From aa96cb2045d26d96b8b26298c0916a7685f8eb76 Mon Sep 17 00:00:00 2001 From: douglance <4741454+douglance@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:18:18 -0400 Subject: [PATCH] test(e2e): cover drafts, candidate profiles, and the avatar gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the remaining SIWE e2e coverage. Every new spec replays a saved session via signedInPage(), so the suite still spends exactly five nonces per run regardless of how many tests it grows. Running these found four real defects, all fixed here: - POST /api/drafts/shared/:slug/submitted is behind requireSession. The UI offered the form to anonymous readers and the code comments claimed the route was unauthenticated. It requires a session but not authorship, so the form is now gated on being signed in and says so — the `second` wallet, which did not write the draft, records the submission in the spec. - Editing, publishing and deleting all require status `draft` (409 not_editable otherwise), because a share link has to keep resolving to what reviewers were shown. DraftList offered Delete on published and submitted drafts, where it could only ever 409. A submitted draft now offers none of the three, and the spec asserts that. - /elections was unreachable. next.config.mjs permanently redirects it to /security-council for old bookmarks, so the new page never rendered. Moved to /profile/candidate, beside the other owned surfaces. Adding it to the Security Council tab bar was the alternative, but that component's active-tab logic is a two-tab binary that a third entry would have forced me to rewrite. - e2e/.auth/*.json were tracked in git. They are listed in .gitignore, but that has no effect on already-committed files, so live session cookies were repository content and every run dirtied the tree. The 429 avatar branch is deliberately left uncovered: reaching it means exhausting the real limiter with a key that is a delegate, which would leave that key limited for the rest of the run and make the result order-dependent. The proposal description is located by MDEditor's own `.w-md-editor-text-input` class rather than a data-testid, because ITextAreaProps rejects extra attributes and CreateProposalForm already depends on sibling library classes. Verified: lint clean, tsc clean, 1262 unit tests, and three consecutive full e2e runs at 19 passed / 1 skipped with no reset between them. --- app/{elections => profile/candidate}/page.tsx | 0 components/delegate/RegistrationIntroCard.tsx | 13 +- components/drafts/DraftList.tsx | 24 ++- components/drafts/SharedDraftView.tsx | 21 ++- e2e/avatar-gate.spec.ts | 73 ++++++++ e2e/candidates.spec.ts | 84 +++++++++ e2e/drafts.spec.ts | 163 ++++++++++++++++++ e2e/fixtures/auth.ts | 9 +- e2e/fixtures/proposal-form.ts | 15 ++ e2e/fixtures/session.ts | 10 +- hooks/use-drafts.ts | 7 +- 11 files changed, 394 insertions(+), 25 deletions(-) rename app/{elections => profile/candidate}/page.tsx (100%) create mode 100644 e2e/avatar-gate.spec.ts create mode 100644 e2e/candidates.spec.ts create mode 100644 e2e/drafts.spec.ts create mode 100644 e2e/fixtures/proposal-form.ts diff --git a/app/elections/page.tsx b/app/profile/candidate/page.tsx similarity index 100% rename from app/elections/page.tsx rename to app/profile/candidate/page.tsx diff --git a/components/delegate/RegistrationIntroCard.tsx b/components/delegate/RegistrationIntroCard.tsx index 5a945f0..d2417fe 100644 --- a/components/delegate/RegistrationIntroCard.tsx +++ b/components/delegate/RegistrationIntroCard.tsx @@ -27,12 +27,17 @@ export function RegistrationIntroCard() { - {/* The election route is deliberately not in the main nav — it is only - relevant to the handful of addresses standing in an election, and - this is the page they already come to for their own details. */} + {/* Deliberately not in the main nav — it is only relevant to the + handful of addresses standing in an election, and this is the page + they already come to for their own details. It also cannot live at + /elections — next.config.mjs permanently redirects that to + /security-council for old bookmarks. */}

Standing in a Security Council election?{" "} - + Publish your candidate profile . diff --git a/components/drafts/DraftList.tsx b/components/drafts/DraftList.tsx index b0389b5..e58a4d7 100644 --- a/components/drafts/DraftList.tsx +++ b/components/drafts/DraftList.tsx @@ -205,15 +205,21 @@ function DraftRow({ draft }: { draft: DraftSummary }) { ) : null} - + {/* Editing, publishing and deleting all require status `draft`: + the API answers 409 not_editable once a draft is published, + because the share link has to keep resolving to what reviewers + were shown. So a published draft offers none of them. */} + {isEditable ? ( + + ) : null} )} diff --git a/components/drafts/SharedDraftView.tsx b/components/drafts/SharedDraftView.tsx index 1022980..3941012 100644 --- a/components/drafts/SharedDraftView.tsx +++ b/components/drafts/SharedDraftView.tsx @@ -11,6 +11,7 @@ import { Input } from "@/components/ui/Input"; import { Label } from "@/components/ui/Label"; import { Skeleton } from "@/components/ui/Skeleton"; import { useMarkSubmitted, useSharedDraft } from "@/hooks/use-drafts"; +import { useSiwe } from "@/hooks/use-siwe"; import { getProposalPreviewRehypePlugins, getProposalPreviewRemarkPlugins, @@ -189,11 +190,13 @@ function SubmittedCard({ draft }: { draft: Draft }) { /** * Attaches the transaction that put this draft on chain. * - * Unauthenticated, matching the route: whoever submits a proposal is often not - * its author, and requiring the author to come back and record it would leave - * most drafts permanently marked unsubmitted. + * Reading a shared draft needs no session, but recording a submission does — the + * route is behind `requireSession`. It does not require *authorship*, though, so + * the delegate who actually submitted the proposal can record it without the + * author coming back. */ function MarkSubmittedForm({ slug }: { slug: string }) { + const { isSignedIn } = useSiwe(); const { markSubmitted, isSubmitting, error } = useMarkSubmitted(slug); const [transactionHash, setTransactionHash] = useState(""); const [governorAddress, setGovernorAddress] = useState(""); @@ -231,6 +234,16 @@ function MarkSubmittedForm({ slug }: { slug: string }) { holding this draft can follow it.

+ {!isSignedIn ? ( +

+ Sign in with your wallet to record a submission. You do not need to + be the draft's author. +

+ ) : null} +
{isSubmitting ? "Recording…" : "Mark as submitted"} diff --git a/e2e/avatar-gate.spec.ts b/e2e/avatar-gate.spec.ts new file mode 100644 index 0000000..7847cd7 --- /dev/null +++ b/e2e/avatar-gate.spec.ts @@ -0,0 +1,73 @@ +import { expect, test } from "@playwright/test"; + +import { signedInPage } from "./fixtures/session"; + +// 1x1 transparent PNG — small enough to be well under the 2MB cap, and real PNG +// bytes so it survives the server's content sniffing. +const PNG = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==", + "base64" +); + +// Covers POST /api/me/avatar-intent's 401 and 403 branches, which the happy path +// in profile.spec.ts cannot reach. +// +// The 429 branch is deliberately NOT covered. Reaching it means exhausting the +// real per-address limiter with a key that *is* a delegate, which would leave +// that key rate-limited for the rest of the run and make the suite's result +// depend on test order. It stays covered by the indexer's own unit tests. +test.describe("avatar upload gate", () => { + test("refuses an address with no delegated voting power (403)", async ({ + browser, + }) => { + // The `noVotingPower` key holds ARB but has delegated all of it away, so the + // indexer's delegatedVotesCount is 0. It is the only dev key that can take + // this branch — every other funded key is a delegate. + const page = await signedInPage(browser, "noVotingPower"); + + await page.getByTestId("profile-avatar-input").setInputFiles({ + name: "avatar.png", + mimeType: "image/png", + buffer: PNG, + }); + + await expect( + page.getByText(/Only delegates with voting power can upload an avatar/) + ).toBeVisible(); + // Rejected before storage, so nothing is attached to the profile. + await expect(page.getByTestId("profile-avatar-preview")).toHaveCount(0); + }); + + test("authorizes before inspecting the file at all", async ({ browser }) => { + const page = await signedInPage(browser, "noVotingPower"); + + // A plain text file would be a 400 for a delegate. This key is not one, so + // the delegate check is what surfaces — proving the gate runs first and an + // unauthorized caller cannot make the server read its upload. + const res = await page.request.post("/api/profile/avatar", { + multipart: { + file: { + name: "not-an-image.txt", + mimeType: "text/plain", + buffer: Buffer.from("definitely not a png"), + }, + }, + }); + + expect(res.status()).toBe(403); + }); + + test("requires a session (401)", async ({ browser }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + + const res = await page.request.post("/api/profile/avatar", { + multipart: { + file: { name: "avatar.png", mimeType: "image/png", buffer: PNG }, + }, + }); + + expect(res.status()).toBe(401); + await context.close(); + }); +}); diff --git a/e2e/candidates.spec.ts b/e2e/candidates.spec.ts new file mode 100644 index 0000000..f4c8414 --- /dev/null +++ b/e2e/candidates.spec.ts @@ -0,0 +1,84 @@ +import { expect, test } from "@playwright/test"; + +import { signedInPage } from "./fixtures/session"; +import { DEV_WALLETS } from "./fixtures/wallets"; + +// Covers GET /api/elections, GET/PUT /api/me/candidate-profile/:electionId, and +// the public GET /api/elections/:id/candidate-profiles/:address. +// +// Candidate profiles are per-election, so every write needs an election that has +// actually been indexed. The testnode bakes no election fixture (the indexer only +// ever writes phase CONTENDER_SUBMISSION, so a synthetic completed election would +// misreport itself — see the plan's dropped items), which means these tests skip +// on a bare stack rather than passing vacuously. +test.describe("candidate profiles", () => { + test("shows the empty state when no election is indexed", async ({ + browser, + }) => { + const page = await signedInPage( + browser, + "candidates", + "/profile/candidate" + ); + const res = await page.request.get("/api/governance-indexer/api/elections"); + const { elections } = (await res.json()) as { elections: unknown[] }; + + test.skip( + elections.length > 0, + "an election is indexed, so the empty state is not the case under test" + ); + + await expect( + page.getByText(/No elections have been indexed yet/) + ).toBeVisible(); + }); + + test.describe("with an indexed election", () => { + test("saves a version and publishes it to the contender page", async ({ + browser, + }) => { + const page = await signedInPage( + browser, + "candidates", + "/profile/candidate" + ); + const res = await page.request.get( + "/api/governance-indexer/api/elections" + ); + const { elections } = (await res.json()) as { elections: unknown[] }; + + test.skip( + elections.length === 0, + "needs an election indexed on the local stack" + ); + + const name = `E2E Candidate ${Date.now()}`; + await expect(page.getByTestId("candidate-name")).toBeVisible(); + await page.getByTestId("candidate-name").fill(name); + await page.getByTestId("candidate-country").fill("Portugal"); + // Stored as a bare handle; the contender page turns it into a URL. + await page.getByTestId("candidate-twitter").fill("e2e_candidate"); + await page + .getByTestId("candidate-skills") + .fill("Solidity, Incident response"); + await page.getByTestId("candidate-motivation").fill("Motivated by e2e."); + + await page.getByTestId("candidate-save").click(); + // Writes are append-only, so the toast names the version it minted. + await expect(page.getByText(/Saved as version \d+/)).toBeVisible(); + + // A reload re-reads it from the indexer rather than local state. + await page.reload(); + await expect(page.getByTestId("candidate-name")).toHaveValue(name); + + // And it reaches the public contender page, where it is labelled as + // self-published because this address is not in the candidate registry. + await page.goto( + `/security-council/contender/${DEV_WALLETS.candidates.address}` + ); + await expect(page.getByTestId("candidate-unverified")).toBeVisible(); + await expect(page.getByText(name)).toBeVisible(); + await expect(page.getByText("Incident response")).toBeVisible(); + }); + }); +}); diff --git a/e2e/drafts.spec.ts b/e2e/drafts.spec.ts new file mode 100644 index 0000000..eb6ad2e --- /dev/null +++ b/e2e/drafts.spec.ts @@ -0,0 +1,163 @@ +import { expect, test } from "@playwright/test"; + +import { descriptionInput } from "./fixtures/proposal-form"; +import { signedInPage } from "./fixtures/session"; + +// Covers the eight draft routes end to end: +// POST/GET /api/me/drafts +// GET/PATCH /api/me/drafts/:id +// POST /api/me/drafts/:id/publish +// GET /api/drafts/shared/:slug +// POST /api/drafts/shared/:slug/submitted +// DELETE /api/me/drafts/:id +// +// One long test rather than several, on purpose. Drafts are server state keyed +// on the signing address, so separate tests would either share a draft (making +// them order-dependent) or each create their own and leave rows behind. A single +// lifecycle creates exactly one draft and deletes it at the end. +test("draft lifecycle: save, reopen, publish, share, mark submitted", async ({ + browser, +}) => { + const page = await signedInPage(browser, "drafts", "/proposal/new"); + const marker = `E2E draft ${Date.now()}`; + + // --- save ------------------------------------------------------------------ + // The markdown H1 becomes the derived draft name, which is also what the + // list is asserted on below. + await descriptionInput(page).fill( + `# ${marker}\n\nBody written by the drafts e2e spec.` + ); + + await page.getByTestId("open-save-to-drafts").click(); + await expect(page.getByTestId("draft-title-input")).toHaveValue(marker); + await page.getByTestId("confirm-save-to-drafts").click(); + await expect(page.getByText("Saved to your drafts.")).toBeVisible(); + + // --- list ------------------------------------------------------------------ + await page.goto("/drafts"); + const row = page.getByTestId("draft-title").filter({ hasText: marker }); + await expect(row).toBeVisible(); + + // --- reopen ---------------------------------------------------------------- + // Proves GET /api/me/drafts/:id and that ?draft= seeds the form instead of + // the localStorage autosave. + await page.getByRole("link", { name: "Open in form" }).first().click(); + await expect(descriptionInput(page)).toHaveValue(new RegExp(marker)); + + // --- update ---------------------------------------------------------------- + await page.getByTestId("open-save-to-drafts").click(); + // Reopened from a stored draft, so the dialog keeps its name rather than + // re-deriving one. + await expect(page.getByTestId("draft-title-input")).toHaveValue(marker); + await page.getByTestId("confirm-save-to-drafts").click(); + await expect(page.getByText("Draft updated.")).toBeVisible(); + + // --- publish --------------------------------------------------------------- + await page.goto("/drafts"); + await page.getByTestId("publish-draft").first().click(); + await page.getByTestId("confirm-publish").click(); + await expect(page.getByText(/share link is ready/)).toBeVisible(); + + // The slug is minted by the server; read it off the link rather than guessing. + const shareHref = await page + .locator('a[href^="/drafts/shared/"]') + .first() + .getAttribute("href"); + expect(shareHref).toBeTruthy(); + const slug = shareHref!.split("/").pop()!; + + // --- share (no session) ---------------------------------------------------- + // A fresh context with no session: the slug alone is enough to *read* the + // draft, which is the whole point of publishing. + const anonContext = await browser.newContext(); + const anonPage = await anonContext.newPage(); + await anonPage.goto(`/drafts/shared/${slug}`); + await expect(anonPage.getByTestId("shared-draft-title")).toHaveText(marker); + // Recording a submission is a different matter: that route is behind + // requireSession, so an anonymous reader is told to sign in rather than + // handed a form that would 401. + await expect(anonPage.getByTestId("submit-needs-signin")).toBeVisible(); + await expect(anonPage.getByTestId("mark-submitted")).toBeDisabled(); + await anonContext.close(); + + // --- mark submitted, as someone else --------------------------------------- + // The `second` wallet did not author this draft. The route requires a session + // but not authorship, because the delegate who actually submits a proposal is + // usually not the person who drafted it. + const submitter = await signedInPage( + browser, + "second", + `/drafts/shared/${slug}` + ); + await submitter.getByTestId("draft-tx-hash").fill(`0x${"ab".repeat(32)}`); + await submitter + .getByTestId("draft-governor") + .fill("0x1111111111111111111111111111111111111111"); + await submitter.getByTestId("draft-proposal-id").fill("12345"); + await submitter.getByTestId("mark-submitted").click(); + await expect(submitter.getByText(/marked submitted/)).toBeVisible(); + + // The form is replaced by the record, so the state change is durable. + await submitter.reload(); + // By role, not text: the page subtitle also says "submitted on chain". + await expect( + submitter.getByRole("heading", { name: "Submitted on chain" }) + ).toBeVisible(); + await expect(submitter.getByTestId("mark-submitted")).toHaveCount(0); + + // --- terminal ------------------------------------------------------------- + // `submitted` is the end of the line. Editing, publishing and deleting all + // require status `draft` (409 not_editable otherwise), so the row must offer + // none of them — the share link has to keep resolving to what reviewers saw. + await page.goto("/drafts"); + const submittedRow = page.locator("li", { hasText: marker }); + await expect(submittedRow).toContainText("Submitted"); + await expect(submittedRow.getByTestId("delete-draft")).toHaveCount(0); + await expect(submittedRow.getByTestId("publish-draft")).toHaveCount(0); + await expect( + submittedRow.getByRole("link", { name: "Open in form" }) + ).toHaveCount(0); +}); + +// DELETE has its own draft because a published one can never be deleted, so the +// lifecycle above has nothing left to remove by the time it finishes. +test("deletes an unpublished draft", async ({ browser }) => { + const page = await signedInPage(browser, "drafts", "/proposal/new"); + const marker = `E2E disposable ${Date.now()}`; + + await descriptionInput(page).fill(`# ${marker}\n\nCreated to be deleted.`); + await page.getByTestId("open-save-to-drafts").click(); + await page.getByTestId("confirm-save-to-drafts").click(); + await expect(page.getByText("Saved to your drafts.")).toBeVisible(); + + await page.goto("/drafts"); + const row = page.locator("li", { hasText: marker }); + await expect(row).toBeVisible(); + + await row.getByTestId("delete-draft").click(); + await row.getByTestId("confirm-delete").click(); + await expect(page.getByText("Draft deleted.")).toBeVisible(); + await expect(page.locator("li", { hasText: marker })).toHaveCount(0); +}); + +test("refuses to save a draft with no description", async ({ browser }) => { + const page = await signedInPage(browser, "drafts", "/proposal/new"); + + // The API requires a non-empty description, so the button is disabled with the + // reason rather than letting the request 400. + const save = page.getByTestId("open-save-to-drafts"); + await expect(save).toBeDisabled(); + await expect(save).toHaveAttribute("title", /description/i); +}); + +test("an unknown share slug reports itself rather than 500ing", async ({ + browser, +}) => { + const context = await browser.newContext(); + const page = await context.newPage(); + + await page.goto("/drafts/shared/definitely-not-a-real-slug"); + await expect(page.getByTestId("shared-draft-error")).toBeVisible(); + + await context.close(); +}); diff --git a/e2e/fixtures/auth.ts b/e2e/fixtures/auth.ts index e6114e9..518cd9b 100644 --- a/e2e/fixtures/auth.ts +++ b/e2e/fixtures/auth.ts @@ -22,7 +22,14 @@ export const AUTH_WALLETS = { export type AuthWalletName = keyof typeof AUTH_WALLETS; -/** Storage-state path for a pre-authenticated wallet. Gitignored. */ +/** + * Storage-state path for a pre-authenticated wallet. + * + * Gitignored — and untracked, which is not the same thing: these files were + * committed once before the ignore rule was added, so .gitignore had no effect + * on them and every run showed up as a diff. Live session cookies are not + * repository content. + */ export function authFile(name: AuthWalletName): string { return resolve(__dirname, "..", ".auth", `${name}.json`); } diff --git a/e2e/fixtures/proposal-form.ts b/e2e/fixtures/proposal-form.ts new file mode 100644 index 0000000..02beb24 --- /dev/null +++ b/e2e/fixtures/proposal-form.ts @@ -0,0 +1,15 @@ +import type { Locator, Page } from "@playwright/test"; + +/** + * The proposal description input. + * + * MDEditor renders its own textarea and its prop types reject extra attributes, + * so there is no data-testid to hang this on. `.w-md-editor-text-input` is the + * library's documented class for that element, and CreateProposalForm already + * depends on sibling classes (`.w-md-editor-toolbar`) in its tooltip + * MutationObserver — so this leans on the same contract rather than bending + * production code to suit a test. + */ +export function descriptionInput(page: Page): Locator { + return page.locator("textarea.w-md-editor-text-input"); +} diff --git a/e2e/fixtures/session.ts b/e2e/fixtures/session.ts index d5f91b0..dda7803 100644 --- a/e2e/fixtures/session.ts +++ b/e2e/fixtures/session.ts @@ -3,8 +3,10 @@ import type { Browser, Page } from "@playwright/test"; import { AUTH_WALLETS, type AuthWalletName, authFile } from "./auth"; import type { DevWallet } from "./wallets"; -// The SIWE sign-in controls live on the profile page. -const PROFILE_PATH = "/profile"; +// The SIWE sign-in controls live on the delegate registration page — SiweGate +// renders `siwe-sign-in` there, and the form behind it renders `siwe-address`. +// /profile was retired in favour of this route. +const SIGN_IN_PATH = "/delegates/register"; /** * Inject a dev key so TestWalletProvider auto-connects it as a signing wallet. @@ -27,7 +29,7 @@ export async function useWallet(page: Page, wallet: DevWallet): Promise { */ export async function signIn(page: Page, wallet: DevWallet): Promise { await useWallet(page, wallet); - await page.goto(PROFILE_PATH); + await page.goto(SIGN_IN_PATH); await page.getByTestId("siwe-sign-in").click(); await page .getByTestId("siwe-address") @@ -46,7 +48,7 @@ export async function signIn(page: Page, wallet: DevWallet): Promise { export async function signedInPage( browser: Browser, name: AuthWalletName, - path = PROFILE_PATH + path = SIGN_IN_PATH ): Promise { const context = await browser.newContext({ storageState: authFile(name) }); const page = await context.newPage(); diff --git a/hooks/use-drafts.ts b/hooks/use-drafts.ts index 0fd7ef2..f79526e 100644 --- a/hooks/use-drafts.ts +++ b/hooks/use-drafts.ts @@ -119,9 +119,10 @@ export function useSharedDraft(slug: string) { /** * Records the on-chain submission of a published draft. * - * Also unauthenticated — anyone with the link can attach the transaction that - * submitted it, which is deliberate: the person who submits a draft on chain is - * often not the person who wrote it. + * Requires a session but *not* authorship: the route checks `requireSession` and + * then only that the draft is published, so any signed-in user can attach the + * transaction. That is deliberate — whoever submits a proposal on chain is often + * a delegate with enough voting power rather than the person who drafted it. */ export function useMarkSubmitted(slug: string) { const queryClient = useQueryClient();