From 970b06bd656339223601deee42cba71f33c2728c Mon Sep 17 00:00:00 2001
From: Kobe Attias <kobeattias@gmail.com>
Date: Thu, 3 Sep 2026 10:31:36 -0400
Subject: [PATCH 1/2] RFP specs, comment specs

---
 .env.example                                  |   8 +-
 .github/workflows/smoke.yml                   |   9 +
 components/Comment/CollapsedCommentEditor.tsx |   1 +
 components/Comment/CommentEditor.tsx          |   5 +-
 components/Comment/CommentEmptyState.tsx      |   5 +-
 components/Comment/CommentItem.tsx            |   4 +-
 components/Comment/CommentList.tsx            |   2 +-
 components/Comment/CommentSortAndFilters.tsx  |   8 +-
 .../Comment/components/EditorFooter.tsx       |   7 +-
 components/Feed/FeedItemActions.tsx           |  10 +
 .../components/GrantDescriptionSection.tsx    |   1 +
 .../components/GrantFundingAmountSection.tsx  |   1 +
 playwright.config.ts                          |  18 +-
 smoke/comments.spec.ts                        | 302 ++++++++++++++++++
 smoke/helpers/fixtures.ts                     |  24 +-
 smoke/helpers/notebook.ts                     |  36 +++
 smoke/proposal.spec.ts                        | 101 +++++-
 smoke/rfp.spec.ts                             | 183 +++++++++++
 18 files changed, 698 insertions(+), 27 deletions(-)
 create mode 100644 smoke/comments.spec.ts
 create mode 100644 smoke/helpers/notebook.ts
 create mode 100644 smoke/rfp.spec.ts

diff --git a/.env.example b/.env.example
index 2346067bd..84628a6e3 100755
--- a/.env.example
+++ b/.env.example
@@ -55,6 +55,8 @@ SMOKE_BASE_URL=http://localhost:3000
 SMOKE_USER_EMAIL=
 SMOKE_USER_PASSWORD=
 
-# Content the specs navigate to. Ids only - /grant/<id> and /proposal/<id>
-SMOKE_GRANT_ID=33
-SMOKE_PROPOSAL_ID=5
+# Content the specs navigate to. Post ids only - /grant/<id> and /proposal/<id>.
+# Both must be owned by the smoke account: the title specs edit them, and
+# editing is gated on authorship.
+SMOKE_GRANT_POST_ID=662
+SMOKE_PROPOSAL_POST_ID=663
diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml
index 669360924..257ca34aa 100644
--- a/.github/workflows/smoke.yml
+++ b/.github/workflows/smoke.yml
@@ -65,6 +65,15 @@ jobs:
           SMOKE_BASE_URL: ${{ inputs.base_url || vars.SMOKE_BASE_URL }}
           SMOKE_USER_EMAIL: ${{ secrets.SMOKE_USER_EMAIL }}
           SMOKE_USER_PASSWORD: ${{ secrets.SMOKE_USER_PASSWORD }}
+          # Ids of the fixture content, which is public and so kept in
+          # variables rather than secrets. They are read through requiredEnv,
+          # which throws rather than defaults: unset here, every spec that
+          # navigates to a grant or a proposal fails before its first
+          # assertion. Note they have to be listed even though they exist as
+          # repository variables, since only what is named in this block
+          # reaches the process.
+          SMOKE_GRANT_POST_ID: ${{ vars.SMOKE_GRANT_POST_ID }}
+          SMOKE_PROPOSAL_POST_ID: ${{ vars.SMOKE_PROPOSAL_POST_ID }}
         run: npm run test:smoke
 
       - name: Upload report
diff --git a/components/Comment/CollapsedCommentEditor.tsx b/components/Comment/CollapsedCommentEditor.tsx
index af8bff3f1..5c0c09147 100644
--- a/components/Comment/CollapsedCommentEditor.tsx
+++ b/components/Comment/CollapsedCommentEditor.tsx
@@ -26,6 +26,7 @@ export const CollapsedCommentEditor = ({ commentType, onExpand }: CollapsedComme
       type="button"
       onClick={onExpand}
       className="group w-full flex items-center gap-3 rounded-lg border border-blue-200 bg-blue-50/40 px-4 py-3 text-left transition-colors hover:border-blue-400 hover:bg-blue-50"
+      data-testid="comment-editor-collapsed"
     >
       <Avatar
         src={authorProfile?.profileImage}
diff --git a/components/Comment/CommentEditor.tsx b/components/Comment/CommentEditor.tsx
index e726f829a..2b9555b05 100644
--- a/components/Comment/CommentEditor.tsx
+++ b/components/Comment/CommentEditor.tsx
@@ -293,7 +293,10 @@ export const CommentEditor = ({
   }
 
   const renderEditor = (canReview: boolean, banner: React.ReactNode = null) => (
-    <div className="relative border border-gray-200 rounded-lg overflow-hidden bg-white focus-within:ring-blue-500 focus-within:border-blue-500 transition-all duration-200">
+    <div
+      className="relative border border-gray-200 rounded-lg overflow-hidden bg-white focus-within:ring-blue-500 focus-within:border-blue-500 transition-all duration-200"
+      data-testid="comment-editor"
+    >
       {/* User info header */}
       {showHeader && (
         <EditorHeader
diff --git a/components/Comment/CommentEmptyState.tsx b/components/Comment/CommentEmptyState.tsx
index 0b80a50ae..0149184f7 100644
--- a/components/Comment/CommentEmptyState.tsx
+++ b/components/Comment/CommentEmptyState.tsx
@@ -63,7 +63,10 @@ export const CommentEmptyState = ({
     );
 
   return (
-    <div className="flex flex-col items-center justify-center py-12 text-center">
+    <div
+      className="flex flex-col items-center justify-center py-12 text-center"
+      data-testid="comment-empty-state"
+    >
       <div className="mb-4 rounded-full bg-gray-100 p-3">{icon}</div>
       <h3 className="mb-2 text-lg font-medium text-gray-900">{message}</h3>
       <p className="text-sm text-gray-500">{description}</p>
diff --git a/components/Comment/CommentItem.tsx b/components/Comment/CommentItem.tsx
index 887640550..98e29a038 100644
--- a/components/Comment/CommentItem.tsx
+++ b/components/Comment/CommentItem.tsx
@@ -312,7 +312,7 @@ export const CommentItem = ({
 
         {/* If we're replying, show the reply editor */}
         {isReplying && !readOnly && (
-          <div className="mt-4 border-t pt-4 px-4 pb-4">
+          <div className="mt-4 border-t pt-4 px-4 pb-4" data-testid="comment-reply-editor">
             <h4 className="text-sm font-medium mb-2">Your reply:</h4>
             <CommentEditor
               onSubmit={handleReply}
@@ -328,7 +328,7 @@ export const CommentItem = ({
   };
 
   return (
-    <div className="mt-4" id={`comment-${comment.id}`}>
+    <div className="mt-4" id={`comment-${comment.id}`} data-testid="comment-item">
       {/* Debug information for comment ID */}
       {showDebugInfo && (
         <div className="bg-gray-100 p-2 mb-2 rounded-md border border-gray-300">
diff --git a/components/Comment/CommentList.tsx b/components/Comment/CommentList.tsx
index 2f1ce8500..b4f20a5de 100644
--- a/components/Comment/CommentList.tsx
+++ b/components/Comment/CommentList.tsx
@@ -21,7 +21,7 @@ const CommentList: React.FC<CommentListProps> = ({
   readOnly = false,
 }) => {
   return (
-    <div className="space-y-8">
+    <div className="space-y-8" data-testid={isRootList ? 'comment-list' : undefined}>
       {comments.map((comment) => (
         <CommentItem
           commentType={commentType}
diff --git a/components/Comment/CommentSortAndFilters.tsx b/components/Comment/CommentSortAndFilters.tsx
index ca4c8cd55..de4b52730 100644
--- a/components/Comment/CommentSortAndFilters.tsx
+++ b/components/Comment/CommentSortAndFilters.tsx
@@ -44,7 +44,12 @@ export const CommentSortAndFilters: FC<CommentSortAndFiltersProps> = ({
         <BaseMenu
           align="start"
           trigger={
-            <Button variant="outlined" size="sm" className="flex items-center gap-1">
+            <Button
+              variant="outlined"
+              size="sm"
+              className="flex items-center gap-1"
+              data-testid="comment-sort-trigger"
+            >
               {(() => {
                 const currentOption = sortOptions.find((option) => option.value === sortBy);
                 const Icon = currentOption?.icon || Star;
@@ -62,6 +67,7 @@ export const CommentSortAndFilters: FC<CommentSortAndFiltersProps> = ({
                 key={option.value}
                 onClick={() => handleSortChange(option.value)}
                 className={sortBy === option.value ? 'bg-gray-100' : ''}
+                data-testid={`comment-sort-option-${option.value}`}
               >
                 <div className="flex items-center gap-2">
                   <Icon className="h-4 w-4" />
diff --git a/components/Comment/components/EditorFooter.tsx b/components/Comment/components/EditorFooter.tsx
index cc3dacff7..94e67f890 100644
--- a/components/Comment/components/EditorFooter.tsx
+++ b/components/Comment/components/EditorFooter.tsx
@@ -42,8 +42,10 @@ export const EditorFooter = ({
     <div className="flex flex-col-reverse mobile:!flex-row justify-between items-start mobile:!items-center px-4 py-2 border-t border-gray-200 gap-2 mobile:!gap-0">
       {/* Left section: Draft status and word count */}
       <div className="flex items-center gap-3 text-xs text-gray-500">
-        {saveStatus === 'saved' && lastSaved && <span>Draft saved {formatLastSaved()}</span>}
-        {saveStatus === 'saving' && <span>Saving draft...</span>}
+        {saveStatus === 'saved' && lastSaved && (
+          <span data-testid="comment-draft-status">Draft saved {formatLastSaved()}</span>
+        )}
+        {saveStatus === 'saving' && <span data-testid="comment-draft-status">Saving draft...</span>}
         {showWordCount && (
           <span className={isOverLimit ? 'text-red-600 font-medium' : ''}>
             {wordCount.toLocaleString()} / {wordLimit.toLocaleString()} words
@@ -78,6 +80,7 @@ export const EditorFooter = ({
             size="sm"
             onClick={onSubmit}
             disabled={isSubmitting || !canSubmit}
+            data-testid="comment-editor-submit"
           >
             <span className="flex items-center justify-between w-full gap-3">
               <span>{isSubmitting ? 'Submitting...' : 'Submit'}</span>
diff --git a/components/Feed/FeedItemActions.tsx b/components/Feed/FeedItemActions.tsx
index 76e437e4c..fc7850d19 100644
--- a/components/Feed/FeedItemActions.tsx
+++ b/components/Feed/FeedItemActions.tsx
@@ -100,6 +100,7 @@ interface ActionButtonProps {
   hideIcon?: boolean;
   /** Bare icon and count instead of the pill chrome. */
   flat?: boolean;
+  testId?: string;
 }
 
 // Export ActionButton so it can be used in other components
@@ -116,8 +117,10 @@ export const ActionButton: FC<ActionButtonProps> = ({
   showTooltip = true,
   hideIcon = false,
   flat = false,
+  testId,
 }) => (
   <Button
+    data-testid={testId}
     variant="ghost"
     size="sm"
     className={cn(
@@ -503,9 +506,13 @@ export const FeedItemActions: FC<FeedItemActionsProps> = ({
     )
   ) : null;
 
+  // Note the test id rather than the label: FeedItemComment renders this whole
+  // bar inside an aria-hidden wrapper, which takes every control in it out of
+  // the accessibility tree and so out of reach of a role-based locator.
   const moreMenuTrigger = isFlat ? (
     <button
       type="button"
+      data-testid="feed-item-more-options"
       onMouseDown={(e) => e.stopPropagation()}
       onClick={(e) => e.stopPropagation()}
       className="flex h-6 w-6 items-center justify-center text-gray-500 transition-colors hover:text-gray-800"
@@ -515,6 +522,7 @@ export const FeedItemActions: FC<FeedItemActionsProps> = ({
     </button>
   ) : (
     <Button
+      data-testid="feed-item-more-options"
       onMouseDown={(e) => {
         e.stopPropagation();
       }}
@@ -524,6 +532,7 @@ export const FeedItemActions: FC<FeedItemActionsProps> = ({
       variant="ghost"
       size="sm"
       className="flex h-8 w-8 !p-0 items-center justify-center rounded-full text-gray-700 transition-all hover:bg-white hover:text-gray-900 hover:shadow-sm"
+      aria-label="More options"
     >
       <MoreHorizontal className="h-[18px] w-[18px]" />
     </Button>
@@ -664,6 +673,7 @@ export const FeedItemActions: FC<FeedItemActionsProps> = ({
               showLabel={Boolean(actionLabels?.comment)}
               showTooltip={showTooltips}
               flat={isFlat}
+              testId="feed-item-comment-action"
             />
           )}
           {(onTip || totalAwarded > 0) &&
diff --git a/components/Notebook/PublishingForm/components/GrantDescriptionSection.tsx b/components/Notebook/PublishingForm/components/GrantDescriptionSection.tsx
index 5db002a88..fed8fe58d 100644
--- a/components/Notebook/PublishingForm/components/GrantDescriptionSection.tsx
+++ b/components/Notebook/PublishingForm/components/GrantDescriptionSection.tsx
@@ -14,6 +14,7 @@ export function GrantDescriptionSection() {
       <SectionHeader icon={FileText}>Short Description</SectionHeader>
       <div className="mt-2">
         <Textarea
+          data-testid="grant-description-input"
           {...register('shortDescription')}
           placeholder="Describe what this RFP is for and what you're looking to fund"
           error={errors.shortDescription?.message?.toString()}
diff --git a/components/Notebook/PublishingForm/components/GrantFundingAmountSection.tsx b/components/Notebook/PublishingForm/components/GrantFundingAmountSection.tsx
index 689771c81..d06764daf 100644
--- a/components/Notebook/PublishingForm/components/GrantFundingAmountSection.tsx
+++ b/components/Notebook/PublishingForm/components/GrantFundingAmountSection.tsx
@@ -15,6 +15,7 @@ export function GrantFundingAmountSection() {
       <SectionHeader icon={DollarSign}>Funding Amount</SectionHeader>
       <div className="mt-2">
         <Input
+          data-testid="grant-amount-input"
           {...register('budget')}
           placeholder="10,000"
           type="text"
diff --git a/playwright.config.ts b/playwright.config.ts
index 4559dec9c..fceaaf5c3 100644
--- a/playwright.config.ts
+++ b/playwright.config.ts
@@ -21,6 +21,8 @@ if (!baseURL) {
   );
 }
 
+const isLocalServer = /^https?:\/\/(localhost|127\.0\.0\.1|\[::1\])(:|\/|$)/i.test(baseURL);
+
 export default defineConfig({
   testDir: './smoke',
   // Deployed environments are slower than local, and these tests wait on a
@@ -28,10 +30,18 @@ export default defineConfig({
   timeout: 60_000,
   expect: { timeout: 15_000 },
   retries: process.env.CI ? 1 : 0,
-  // Playwright parallelises by file, and concurrent browsers overwhelm a local
-  // dev server: three at once pushed every test near the 60s timeout. The suite
-  // is small enough that running it serially costs little and stays reliable.
-  workers: 1,
+  // Playwright parallelises by file, which takes the wall clock from the sum of
+  // the files to the longest one — worth a lot here, where the notebook specs
+  // run minutes and the rest run seconds.
+  //
+  // Only off against a local dev server, which compiles routes on demand and
+  // buckles under concurrent browsers: three at once pushed every test near the
+  // 60s timeout. A deployed environment serves those routes prebuilt.
+  //
+  // Files that must not interleave say so themselves, with
+  // `test.describe.configure({ mode: 'serial' })`; nothing here depends on the
+  // worker count for ordering.
+  workers: isLocalServer ? 1 : 3,
   reporter: [['list'], ['html', { open: 'never' }]],
   use: {
     baseURL,
diff --git a/smoke/comments.spec.ts b/smoke/comments.spec.ts
new file mode 100644
index 000000000..6e333b308
--- /dev/null
+++ b/smoke/comments.spec.ts
@@ -0,0 +1,302 @@
+import { randomUUID } from 'node:crypto';
+import { expect, test, type Locator, type Page, type Request } from '@playwright/test';
+import { logIn } from './helpers/auth';
+import { proposalPostId } from './helpers/fixtures';
+
+/**
+ * Comments are exercised against the fixture proposal rather than a paper.
+ * It is content the smoke account owns, so the editor and the Edit action are
+ * reachable without borrowing someone else's post.
+ *
+ * The tradeoff is that a proposal renders its conversation through
+ * FundDocument, which overrides the editor's draft key to
+ * `rh-comments-comment-feed-<id>` instead of the CommentFeed default. Same
+ * behaviour either way — only the key differs — but a regression confined to
+ * the paper/post branch would not be caught here.
+ */
+
+/**
+ * A sort change refetches immediately, so this is capped well below the test
+ * timeout: a dead control then reports in seconds rather than eating the whole
+ * budget, twice over once CI retries.
+ */
+const SORT_CHANGE_TIMEOUT = 15_000;
+
+/**
+ * Comment ordering never reaches the URL, so the request the feed builds is
+ * the only place the chosen sort is observable. Matched on the path suffix
+ * rather than the full path because the document segment depends on how the
+ * content type maps to an API route (`researchhubpost` here, `paper`
+ * elsewhere), and none of that is what this is checking.
+ */
+function commentsRequest(ordering: string) {
+  return (request: Request) => {
+    const url = new URL(request.url());
+    return url.pathname.endsWith('/comments/') && url.searchParams.get('ordering') === ordering;
+  };
+}
+
+/** Matches the create endpoint, which is shared by top-level comments and replies. */
+function isCommentCreate(url: string) {
+  return new URL(url).pathname.endsWith('/comments/create_rh_comment/');
+}
+
+/** The TipTap surface inside a given editor. Typing has to go through the real
+ * contenteditable: `fill` bypasses the input events ProseMirror listens for. */
+function proseMirror(editor: Locator) {
+  return editor.locator('.ProseMirror');
+}
+
+/**
+ * Opens the fixture proposal's conversation.
+ *
+ * The tab strip is client state, not routing — WorkHeader wires it straight to
+ * `setActiveTab` — so clicking a tab swaps the panel without touching the URL,
+ * and a reload would land back on the default tab. The conversation does have
+ * a real route, so resolving the canonical slug and navigating to it gives the
+ * specs an address that survives the reload the draft test depends on.
+ */
+async function openConversation(page: Page) {
+  await page.goto(`/proposal/${proposalPostId()}`);
+  await page.waitForURL(/\/proposal\/\d+\/[^/?#]+/);
+
+  const { pathname } = new URL(page.url());
+  await page.goto(`${pathname.replace(/\/$/, '')}/conversation`);
+}
+
+/**
+ * Drops any draft left in localStorage and reloads so the editor remounts
+ * without it.
+ *
+ * Drafts outlive the test that wrote them: they have no expiry, and the store
+ * is keyed by user and document, so every spec here shares one. Without this,
+ * a draft test that failed before cleaning up would prepend its text to the
+ * comment the next test posts, and the failure would surface in the wrong
+ * place. Clearing on the way in rather than the way out is what makes that
+ * true even when the previous test crashed.
+ */
+async function clearDrafts(page: Page) {
+  await page.evaluate(() => window.localStorage.removeItem('comment_drafts'));
+  await page.reload();
+}
+
+/** Waits for the feed to resolve into either a populated list or the empty state. */
+function settledFeed(page: Page) {
+  return page.getByTestId('comment-list').or(page.getByTestId('comment-empty-state'));
+}
+
+/**
+ * Writes a top-level comment through the editor and returns the new id.
+ *
+ * The id comes from the response rather than from assuming the comment is
+ * first in the list. It is prepended today, but that is a reducer detail, and
+ * an id gives callers a locator that cannot drift onto a neighbour.
+ */
+async function postComment(page: Page, body: string): Promise<number> {
+  await page.getByTestId('comment-editor-collapsed').click();
+
+  // `.first()` distinguishes the feed's own composer, which renders above the
+  // list, from any editor opened inline on a comment further down.
+  const composer = page.getByTestId('comment-editor').first();
+  await proseMirror(composer).click();
+  await proseMirror(composer).pressSequentially(body);
+
+  const posted = page.waitForResponse(
+    (response) => isCommentCreate(response.url()) && response.request().method() === 'POST'
+  );
+  await composer.getByTestId('comment-editor-submit').click();
+  const response = await posted;
+  expect(response.ok()).toBe(true);
+
+  return (await response.json()).id;
+}
+
+/**
+ * Guarantees the conversation holds at least one comment, writing one if it
+ * does not.
+ *
+ * Deployed environments carry no comments at all — every post and paper
+ * sampled on staging returns a count of zero, the fixture proposal included —
+ * so a spec that simply assumed one would fail outright on a fresh
+ * environment. Pinning a hand-seeded document instead would only move the
+ * problem to the next database refresh. Seeding through the UI keeps the suite
+ * self-sufficient, and because comments are never cleaned up it is a no-op on
+ * every run after the first.
+ */
+async function ensureComment(page: Page) {
+  await expect(settledFeed(page)).toBeVisible();
+
+  if (await page.getByTestId('comment-empty-state').isVisible()) {
+    await postComment(page, `Smoke test seed comment, please ignore ${randomUUID().slice(0, 8)}`);
+  }
+}
+
+test('the conversation tab lists comments', async ({ page }) => {
+  // Logged in because an empty conversation has to be seeded before it can be
+  // listed, and the editor is only offered to a signed-in user.
+  test.setTimeout(120_000);
+
+  await logIn(page);
+  await page.goto(`/proposal/${proposalPostId()}`);
+
+  // The tab is the feature under test here, so this clicks it rather than
+  // navigating to /conversation directly. Its accessible name carries the
+  // comment count after the word, hence the prefix match.
+  await page
+    .getByRole('button', { name: /^Conversation/ })
+    .first()
+    .click();
+
+  await ensureComment(page);
+
+  await expect(page.getByTestId('comment-list')).toBeVisible();
+  await expect(page.getByTestId('comment-item').first()).toBeVisible();
+
+  // A failed fetch renders the empty state rather than an error, so the list
+  // being populated is only half the assertion: without this, a broken feed
+  // that happened to leave one stale item mounted would still pass.
+  await expect(page.getByTestId('comment-empty-state')).toHaveCount(0);
+});
+
+test('a comment draft is restored after a reload', async ({ page }) => {
+  test.setTimeout(120_000);
+
+  await logIn(page);
+  await openConversation(page);
+  await clearDrafts(page);
+
+  const draft = `Smoke test draft, please ignore ${randomUUID().slice(0, 8)}`;
+
+  await page.getByTestId('comment-editor-collapsed').click();
+  const editor = page.getByTestId('comment-editor').first();
+  await proseMirror(editor).click();
+  await proseMirror(editor).pressSequentially(draft);
+
+  // The write is debounced by 1.5s, and the footer is the only place the user
+  // is told it happened. Waiting on it rather than on a fixed sleep is also
+  // what stops the reload below racing the save.
+  await expect(page.getByTestId('comment-draft-status')).toHaveText(/Draft saved/);
+
+  await page.reload();
+
+  // Restoring a draft auto-expands the editor, so a user who comes back finds
+  // their text in front of them rather than behind a collapsed prompt. Both
+  // halves matter: the text alone would pass even if it were hidden.
+  await expect(page.getByTestId('comment-editor-collapsed')).toHaveCount(0);
+  await expect(proseMirror(page.getByTestId('comment-editor').first())).toContainText(draft);
+});
+
+test('the conversation can be sorted', async ({ page }) => {
+  test.setTimeout(120_000);
+
+  await logIn(page);
+
+  const initialBest = page.waitForRequest(commentsRequest('BEST'));
+  await openConversation(page);
+  await initialBest;
+
+  // Not just a readability nicety: the feed skips the refetch entirely while
+  // it holds no comments, so changing sort on an empty conversation would
+  // assert nothing and time out somewhere less obvious.
+  await ensureComment(page);
+  await expect(page.getByTestId('comment-item').first()).toBeVisible();
+
+  // The feed defaults to Best, so picking Newest is already a real change of
+  // order and needs no round trip through a second option to mean something.
+  const byNewest = page.waitForRequest(commentsRequest('CREATED_DATE'), {
+    timeout: SORT_CHANGE_TIMEOUT,
+  });
+  await page.getByTestId('comment-sort-trigger').click();
+
+  // The menu is portalled to the body by Radix, so the option cannot be
+  // scoped to the control that opened it.
+  await page.getByTestId('comment-sort-option-CREATED_DATE').click();
+  await byNewest;
+
+  await expect(page.getByTestId('comment-item').first()).toBeVisible();
+});
+
+/**
+ * Posting, editing and replying run as one test against one comment.
+ *
+ * Comments are not cleaned up — `censor` exists, but leaving the thread as a
+ * user would leave it keeps this consistent with `proposal.spec.ts` — so each
+ * independent test would add another comment to the fixture proposal on every
+ * run. Chaining costs isolation and buys a third of the litter, and it is also
+ * the order a real thread happens in.
+ */
+test('a comment can be posted, then edited, then replied to', async ({ page }) => {
+  // Three round trips through Django on top of a login and two navigations.
+  test.setTimeout(180_000);
+
+  await logIn(page);
+  await openConversation(page);
+  await clearDrafts(page);
+  await expect(settledFeed(page)).toBeVisible();
+
+  const body = `Smoke test comment, please ignore ${randomUUID().slice(0, 8)}`;
+  const commentId = await postComment(page, body);
+
+  const comment = page.locator(`#comment-${commentId}`);
+  await expect(comment).toContainText(body);
+
+  // Edit. The action bar is addressed by test id rather than by role:
+  // FeedItemComment renders it inside an aria-hidden wrapper, so none of its
+  // controls exist as far as an accessible-name lookup is concerned. The menu
+  // it opens is portalled to the body, which puts the item back in the tree
+  // but also out of the comment, so that one cannot be scoped to it.
+  const edited = `Smoke test comment, edited, please ignore ${randomUUID().slice(0, 8)}`;
+
+  await comment.getByTestId('feed-item-more-options').first().click();
+  await page.getByRole('menuitem', { name: 'Edit' }).click();
+
+  const editor = comment.getByTestId('comment-editor').first();
+  await expect(proseMirror(editor)).toContainText(body);
+
+  // Selecting all first, rather than appending: a click lands the caret
+  // wherever it happened to hit, which makes the resulting text unpredictable.
+  await proseMirror(editor).click();
+  await page.keyboard.press('ControlOrMeta+a');
+  await proseMirror(editor).pressSequentially(edited);
+
+  const updated = page.waitForResponse(
+    (response) =>
+      new URL(response.url()).pathname.endsWith(`/comments/${commentId}/`) &&
+      response.request().method() === 'PATCH'
+  );
+  await editor.getByTestId('comment-editor-submit').click();
+  expect((await updated).ok()).toBe(true);
+
+  // The edit is applied optimistically and only then overwritten by the
+  // response, so this could pass a moment before the server has agreed.
+  // Awaiting the PATCH above is what makes it an assertion about the saved
+  // comment rather than about the guess the reducer rendered.
+  await expect(comment).toContainText(edited);
+  await expect(comment).not.toContainText(body);
+
+  // Reply.
+  const replyBody = `Smoke test reply, please ignore ${randomUUID().slice(0, 8)}`;
+
+  // Same bar, same reason. `.first()` keeps this on the comment's own control
+  // rather than one belonging to a reply nested underneath it.
+  await comment.getByTestId('feed-item-comment-action').first().click();
+  const replyEditor = comment.getByTestId('comment-reply-editor');
+  await expect(replyEditor).toBeVisible();
+  await proseMirror(replyEditor).click();
+  await proseMirror(replyEditor).pressSequentially(replyBody);
+
+  const replied = page.waitForResponse(
+    (response) => isCommentCreate(response.url()) && response.request().method() === 'POST'
+  );
+  await replyEditor.getByTestId('comment-editor-submit').click();
+  const repliedResponse = await replied;
+  expect(repliedResponse.ok()).toBe(true);
+
+  // Threading is the whole point of a reply, and the nesting below could be
+  // produced by a flat insert that merely rendered in the right place, so the
+  // parent link is asserted on the request as well.
+  expect(repliedResponse.request().postDataJSON()).toMatchObject({ parent_id: commentId });
+
+  const replyId = (await repliedResponse.json()).id;
+  await expect(comment.locator(`#comment-${replyId}`)).toContainText(replyBody);
+});
diff --git a/smoke/helpers/fixtures.ts b/smoke/helpers/fixtures.ts
index 74b565223..fa0654a8b 100644
--- a/smoke/helpers/fixtures.ts
+++ b/smoke/helpers/fixtures.ts
@@ -9,11 +9,27 @@ import { requiredEnv } from './env';
  * browser — the proposal layout streams, so Next inlines the redirect instead
  * of sending a status. Both are transparent to Playwright; only a bare
  * request-level check would notice.
+ *
+ * "Post id" is in the names deliberately. A grant and a proposal each own two
+ * different numbers: the post that carries the document, and the grant or
+ * fundraise record hanging off it. Only the post id appears in a URL, and the
+ * two are easy to confuse when reading a payload — `proposal.spec.ts` asserts
+ * on a `grant_id` that is the record, not the post.
+ *
+ * Neither has to be owned by the smoke account, and no spec writes to either
+ * beyond leaving a comment. What the grant does have to be is OPEN, since
+ * `proposal.spec.ts` reaches its flow through the "Submit Proposal" button
+ * that only an open RFP renders.
+ *
+ * The specs that edit a title deliberately do not use these. Editing means
+ * reopening the notebook note behind the post, which lives in the personal
+ * notebook of whoever drafted it and cannot be shared, so those specs publish
+ * their own content first rather than depending on who owns a fixture.
  */
-export function grantId(): string {
-  return requiredEnv('SMOKE_GRANT_ID');
+export function grantPostId(): string {
+  return requiredEnv('SMOKE_GRANT_POST_ID');
 }
 
-export function proposalId(): string {
-  return requiredEnv('SMOKE_PROPOSAL_ID');
+export function proposalPostId(): string {
+  return requiredEnv('SMOKE_PROPOSAL_POST_ID');
 }
diff --git a/smoke/helpers/notebook.ts b/smoke/helpers/notebook.ts
new file mode 100644
index 000000000..559204ec5
--- /dev/null
+++ b/smoke/helpers/notebook.ts
@@ -0,0 +1,36 @@
+import { expect, type Page } from '@playwright/test';
+
+/** The notebook editor, as `/notebook/<org slug>/<note id>`. */
+const NOTEBOOK_NOTE = /\/notebook\/[^/]+\/\d+/;
+
+/**
+ * Opens the notebook note behind a published grant or proposal, by way of the
+ * Edit action in the work header.
+ *
+ * Neither has an edit route of its own: Edit sends the author back to the note
+ * the work was published from, and every change lands through a republish from
+ * there. Editing the heading in the notebook alone would only move the draft's
+ * title — the published one comes from that round trip.
+ *
+ * The click is retried as a unit because the action renders well before the app
+ * can serve it. `handleEdit` needs the signed-in user's organisation, and
+ * `OrganizationContext` only has one after the session resolves and a further
+ * request comes back — several seconds into a page's life. Until then the click
+ * produces an "Unable to edit" toast and nothing else, and because a grant
+ * offers Edit to any author, contact or moderator without consulting that
+ * state, nothing on the page marks the moment it becomes usable. There is no
+ * readiness to wait on, so this waits on the outcome instead.
+ *
+ * Each attempt reopens the menu: a click on Edit closes it whether or not it
+ * navigated.
+ */
+export async function openNotebookEditor(page: Page) {
+  await expect(async () => {
+    // `.first()` picks the work header's menu over any other on the page; the
+    // header renders above the sidebar, so document order is enough to
+    // distinguish them.
+    await page.getByRole('button', { name: 'More options' }).first().click();
+    await page.getByRole('menuitem', { name: 'Edit' }).click();
+    await page.waitForURL(NOTEBOOK_NOTE, { timeout: 10_000 });
+  }).toPass({ timeout: 120_000 });
+}
diff --git a/smoke/proposal.spec.ts b/smoke/proposal.spec.ts
index 246161953..7bb4ab153 100644
--- a/smoke/proposal.spec.ts
+++ b/smoke/proposal.spec.ts
@@ -1,7 +1,8 @@
 import { randomUUID } from 'node:crypto';
 import { expect, test } from '@playwright/test';
 import { logIn } from './helpers/auth';
-import { grantId } from './helpers/fixtures';
+import { openNotebookEditor } from './helpers/notebook';
+import { grantPostId } from './helpers/fixtures';
 
 /**
  * A 64x64 PNG, inline rather than a committed binary. A cover image is required
@@ -24,6 +25,26 @@ const COVER_IMAGE = Buffer.from(
  */
 const proposalTitle = () => `Smoke test proposal, please ignore ${randomUUID().slice(0, 8)}`;
 
+/**
+ * The edit below retitles the proposal published above rather than a pinned
+ * fixture, and the two run in order for that reason.
+ *
+ * Editing a proposal means reopening the notebook note it was published from,
+ * and a note lives in the personal notebook of whoever wrote it. Transferring
+ * the post to another account leaves the note where it was, and a personal
+ * notebook takes no second member — so a proposal drafted by anyone but the
+ * smoke account is permanently uneditable here, however its post is owned.
+ * Creating one first is the only way to be sure there is something to edit,
+ * and it costs nothing: this file publishes a proposal either way.
+ *
+ * Serial also means the edit is skipped rather than failed when the publish
+ * before it breaks, so one cause reports once.
+ */
+test.describe.configure({ mode: 'serial' });
+
+/** Where the publish leaves off, and what the edit picks up. */
+let proposalUrl: string;
+
 /**
  * Note that this leaves its proposal behind: publishing is not reversible from
  * the outside. The author-facing removal endpoint
@@ -45,7 +66,7 @@ test('a new proposal can be drafted from an RFP and published', async ({ page })
   const title = proposalTitle();
 
   await logIn(page);
-  await page.goto(`/grant/${grantId()}`);
+  await page.goto(`/grant/${grantPostId()}`);
 
   // The entry point only exists while the RFP is open for applications, so a
   // closed or expired fixture fails here rather than somewhere confusing.
@@ -58,8 +79,19 @@ test('a new proposal can be drafted from an RFP and published', async ({ page })
 
   // Two ways to apply: continue an existing draft, or start a new one. Only
   // the second creates a note from the proposal template.
-  await page.getByTestId('apply-draft-new').click();
-  await page.getByTestId('apply-continue').click();
+  //
+  // Retried as a unit because the modal throws the choice away underneath it.
+  // Its reset effect keys on the selected organisation, `OrganizationContext`
+  // resolves that a few seconds after the page loads, and the reset that fires
+  // when it lands clears the selection — leaving Continue disabled with no
+  // radio checked, which is what a user sees if they choose quickly too. The
+  // click on Continue is inside the loop so that a selection wiped between
+  // choosing and confirming is simply made again.
+  await expect(async () => {
+    await page.getByTestId('apply-draft-new').click();
+    await page.getByTestId('apply-continue').click({ timeout: 5_000 });
+    await page.waitForURL(/\/notebook/, { timeout: 10_000 });
+  }).toPass({ timeout: 120_000 });
 
   // /notebook resolves the user's organisation, the org page creates the note
   // from the proposal template, and only then does the URL carry a note id.
@@ -125,7 +157,7 @@ test('a new proposal can be drafted from an RFP and published', async ({ page })
 
   // The RFP link is the whole point of this flow and is invisible in the UI at
   // this stage, so the request is where it has to be asserted. Only that a
-  // grant was attached, not which one: SMOKE_GRANT_ID is the post id the URL
+  // grant was attached, not which one: SMOKE_GRANT_POST_ID is the post id the URL
   // uses, while the payload carries the id of the grant record hanging off it,
   // and the two are different numbers.
   const payload = response.request().postDataJSON();
@@ -140,13 +172,66 @@ test('a new proposal can be drafted from an RFP and published', async ({ page })
   // the page and takes the document out of the accessibility tree with it.
   // Dropping the parameter makes the assertion about the published proposal
   // rather than the modal sitting on top of it.
-  const proposalUrl = new URL(page.url());
-  proposalUrl.searchParams.delete('new');
-  await page.goto(proposalUrl.toString());
+  const canonical = new URL(page.url());
+  canonical.searchParams.delete('new');
+  await page.goto(canonical.toString());
 
   // The title lands on the page twice as an h1: once in the header, from the
   // post record the API returned, and once in the document body, from the
   // editor content. Either proves the publish, but the body one mounts a beat
   // later — matching both is a race that only shows up on a slower run.
   await expect(page.getByRole('heading', { name: title }).first()).toBeVisible();
+
+  proposalUrl = canonical.toString();
+});
+
+test("a published proposal's title can be edited", async ({ page }) => {
+  test.setTimeout(300_000);
+
+  const title = proposalTitle();
+
+  // A fresh browser context per test, so the session from the publish above
+  // does not carry over even though the proposal does.
+  await logIn(page);
+
+  await page.goto(proposalUrl);
+
+  await openNotebookEditor(page);
+
+  // Nothing is filled in: opening an existing proposal repopulates the form
+  // from the published post, so it already satisfies the schema. A validation
+  // error here means the publish above wrote a proposal the form will not
+  // accept back, not that editing is broken.
+  await page.getByTestId('notebook-add-details').click();
+  await page.getByTestId('publishing-form-submit').click();
+
+  // The dialog distinguishes a republish from a first publish, which is the
+  // cheapest available proof the form knows it is editing rather than creating
+  // a second proposal.
+  await expect(page.getByRole('heading', { name: 'Confirm Re-publication' })).toBeVisible();
+
+  const titleField = page.getByTestId('confirm-publish-title');
+  await expect(titleField).toBeVisible();
+  await titleField.fill(title);
+  await page.getByLabel('I have adhered to the ResearchHub posting guidelines').check();
+
+  const republished = page.waitForResponse(
+    (response) =>
+      new URL(response.url()).pathname === '/api/researchhubpost/' &&
+      response.request().method() === 'POST'
+  );
+  await page.getByTestId('confirm-publish-submit').click();
+  const response = await republished;
+
+  expect(response.ok()).toBe(true);
+
+  // `post_id` is what makes this an update. Without it the same endpoint would
+  // happily create a second proposal, and the assertions below would still
+  // pass against it.
+  const payload = response.request().postDataJSON();
+  expect(payload).toMatchObject({ document_type: 'PREREGISTRATION', title });
+  expect(payload.post_id).toBeTruthy();
+
+  await page.waitForURL(/\/proposal\/\d+\//);
+  await expect(page.getByRole('heading', { name: title }).first()).toBeVisible();
 });
diff --git a/smoke/rfp.spec.ts b/smoke/rfp.spec.ts
new file mode 100644
index 000000000..d6e041fd9
--- /dev/null
+++ b/smoke/rfp.spec.ts
@@ -0,0 +1,183 @@
+import { randomUUID } from 'node:crypto';
+import { expect, test, type Page } from '@playwright/test';
+import { logIn } from './helpers/auth';
+import { openNotebookEditor } from './helpers/notebook';
+
+/**
+ * An RFP is a grant: the notebook calls the work type `grant`, the API calls
+ * the document type `GRANT`, the published work's content type is
+ * `funding_request`, and only the UI says "RFP". All four appear below.
+ */
+
+/**
+ * Both titles are randomised so a leftover from a failed run is identifiable,
+ * and kept over 20 characters because the confirm dialog and Django
+ * (MIN_POST_TITLE_LENGTH) both reject anything shorter.
+ */
+const rfpTitle = () => `Smoke test RFP, please ignore ${randomUUID().slice(0, 8)}`;
+
+const isPostUpsert = (url: string) => new URL(url).pathname === '/api/researchhubpost/';
+
+/**
+ * Picks the first topic matching a query. Scoped to its own section on
+ * purpose: unscoped, `option` also matches the currency select in the sidebar,
+ * whose entries carry the same role and can win the race for `.first()`.
+ */
+async function selectFirstTopic(page: Page) {
+  const topics = page.getByTestId('topics-section');
+  await topics.getByPlaceholder('Search topics...').fill('bio');
+
+  const firstTopic = topics.getByRole('option').first();
+  await expect(firstTopic).toBeVisible();
+  const topicName = (await firstTopic.innerText()).trim();
+  await firstTopic.click();
+
+  // The list stays open on select, so it is dismissed first: the remaining
+  // copy of the label is then the chip for the chosen topic, which is what
+  // shows the selection registered rather than silently missing.
+  await page.keyboard.press('Escape');
+  await expect(topics.getByText(topicName, { exact: true })).toBeVisible();
+}
+
+/** Fills in the confirm dialog and publishes, returning the upsert response. */
+async function confirmPublish(page: Page, title: string) {
+  const titleField = page.getByTestId('confirm-publish-title');
+  await expect(titleField).toBeVisible();
+  await titleField.fill(title);
+  await page.getByLabel('I have adhered to the ResearchHub posting guidelines').check();
+
+  const published = page.waitForResponse(
+    (response) => isPostUpsert(response.url()) && response.request().method() === 'POST'
+  );
+  await page.getByTestId('confirm-publish-submit').click();
+  return published;
+}
+
+/**
+ * The edit below retitles the RFP published above rather than a pinned
+ * fixture, and the two run in order for that reason.
+ *
+ * Editing an RFP means reopening the notebook note it was published from, and
+ * a note lives in the personal notebook of whoever wrote it. Transferring the
+ * post to another account leaves the note where it was, and a personal
+ * notebook takes no second member — so an RFP authored by anyone but the smoke
+ * account is permanently uneditable here, however its post is owned. Creating
+ * one first is the only way to be sure there is something to edit, and it
+ * costs nothing: this file publishes an RFP either way.
+ *
+ * Serial also means the edit is skipped rather than failed when the publish
+ * before it breaks, so one cause reports once.
+ */
+test.describe.configure({ mode: 'serial' });
+
+/** Where the publish leaves off, and what the edit picks up. */
+let grantUrl: string;
+
+/**
+ * Note this leaves its RFP behind: publishing is not reversible from the
+ * outside. Each run adds one, recognisable by title and safe to decline in
+ * whichever environment's moderation queue it lands in.
+ */
+test('a new RFP can be drafted in the notebook and published', async ({ page }) => {
+  // A login, a note created from a template, and a publish, each a round trip
+  // through Next.js and Django. The generous budget is mostly for a local run,
+  // where the notebook route is compiled on first visit and that alone can
+  // take the best part of a minute.
+  test.setTimeout(300_000);
+
+  const title = rfpTitle();
+
+  await logIn(page);
+
+  // The route the "New RFP" entry points push to. `/notebook` resolves the
+  // user's organisation and carries the query string over to it, and the org
+  // page is what turns `newGrant` into a note seeded from the grant template.
+  // Driving it by URL rather than through the publish menu keeps this test
+  // about the notebook rather than about which menu happens to link to it.
+  await page.goto('/notebook?newGrant=true&grantSource=template');
+
+  // Only once the note exists does the URL carry an id, so waiting for it is
+  // what confirms the draft was created. Given its own timeout because it is
+  // by far the slowest step, so a hang here reports as itself rather than as
+  // the whole test running out of budget.
+  await page.waitForURL(/\/notebook\/[^/]+\/\d+/, { timeout: 120_000 });
+
+  await page.getByTestId('notebook-add-details').click();
+
+  // Topics, a short description and a funding amount are what the schema
+  // demands of a grant. Contacts are required too but deliberately not filled:
+  // the form seeds them with the current user, and asserting the publish
+  // payload carries one covers that without driving a user search.
+  await selectFirstTopic(page);
+  await page
+    .getByTestId('grant-description-input')
+    .fill('Smoke test RFP. Please ignore — created by the automated smoke suite.');
+  await page.getByTestId('grant-amount-input').fill('10000');
+
+  await page.getByTestId('publishing-form-submit').click();
+
+  const response = await confirmPublish(page, title);
+  expect(response.ok()).toBe(true);
+
+  // The grant fields are invisible on the page at this stage — a pending RFP
+  // does not render as an open one — so the request is where they have to be
+  // asserted. `grant_contacts` in particular is the only evidence that the
+  // form seeded a contact rather than publishing without one.
+  const payload = response.request().postDataJSON();
+  expect(payload).toMatchObject({
+    document_type: 'GRANT',
+    title,
+    grant_amount: 10000,
+    grant_currency: 'USD',
+  });
+  expect(payload.grant_contacts?.length).toBeGreaterThan(0);
+  expect(payload.note_id).toBeTruthy();
+
+  // Publishing redirects to the RFP itself, which is the user-visible
+  // confirmation that it exists and is readable by its author.
+  await page.waitForURL(/\/grant\/\d+\//);
+  await expect(page.getByRole('heading', { name: title }).first()).toBeVisible();
+
+  grantUrl = page.url();
+});
+
+test("a published RFP's title can be edited", async ({ page }) => {
+  test.setTimeout(300_000);
+
+  const title = rfpTitle();
+
+  // A fresh browser context per test, so the session from the publish above
+  // does not carry over even though the grant does.
+  await logIn(page);
+
+  await page.goto(grantUrl);
+
+  await openNotebookEditor(page);
+
+  // Nothing is filled in this time: opening an existing RFP repopulates the
+  // form from the published grant, so it already satisfies the schema. A
+  // validation error here means the publish above wrote a grant the form will
+  // not accept back, not that editing is broken.
+  await page.getByTestId('notebook-add-details').click();
+  await page.getByTestId('publishing-form-submit').click();
+
+  // The dialog distinguishes a republish from a first publish, which is the
+  // cheapest available proof that the form knows it is editing rather than
+  // creating a second RFP.
+  await expect(page.getByRole('heading', { name: 'Confirm Re-publication' })).toBeVisible();
+
+  const response = await confirmPublish(page, title);
+  expect(response.ok()).toBe(true);
+
+  // `post_id` is what makes this an update. Without it the same endpoint would
+  // happily create a new RFP, and the assertions below would still pass.
+  const payload = response.request().postDataJSON();
+  expect(payload).toMatchObject({ document_type: 'GRANT', title });
+  expect(payload.post_id).toBeTruthy();
+
+  // Republishing redirects back to the RFP, rendered from the response rather
+  // than from anything cached in the notebook — so the heading is the title as
+  // the API now has it.
+  await page.waitForURL(/\/grant\/\d+\//);
+  await expect(page.getByRole('heading', { name: title }).first()).toBeVisible();
+});

From 6e43494eb24e18fe4f797157d4307a8fd8a1d6db Mon Sep 17 00:00:00 2001
From: Kobe Attias <kobeattias@gmail.com>
Date: Thu, 3 Sep 2026 10:37:41 -0400
Subject: [PATCH 2/2] Remove superfluous comment

---
 .github/workflows/smoke.yml | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml
index 257ca34aa..9d4c1cacc 100644
--- a/.github/workflows/smoke.yml
+++ b/.github/workflows/smoke.yml
@@ -65,13 +65,7 @@ jobs:
           SMOKE_BASE_URL: ${{ inputs.base_url || vars.SMOKE_BASE_URL }}
           SMOKE_USER_EMAIL: ${{ secrets.SMOKE_USER_EMAIL }}
           SMOKE_USER_PASSWORD: ${{ secrets.SMOKE_USER_PASSWORD }}
-          # Ids of the fixture content, which is public and so kept in
-          # variables rather than secrets. They are read through requiredEnv,
-          # which throws rather than defaults: unset here, every spec that
-          # navigates to a grant or a proposal fails before its first
-          # assertion. Note they have to be listed even though they exist as
-          # repository variables, since only what is named in this block
-          # reaches the process.
+          # Ids of the fixture content, which is public.
           SMOKE_GRANT_POST_ID: ${{ vars.SMOKE_GRANT_POST_ID }}
           SMOKE_PROPOSAL_POST_ID: ${{ vars.SMOKE_PROPOSAL_POST_ID }}
         run: npm run test:smoke