Show why a review comment failed to post (production)

components/review/Discussion.vue

@@ -20,6 +20,7 @@
 import { toast } from 'vue3-toastify';
 import type { ReviewListItem } from '~/services/review';
 import { osmClient } from '~/services/index';
+import { resolveHttpErrorMessage } from '~/services/http';
 
 import type ChatBox from '~/components/ChatBox.vue';
 import type { ChatMessage } from '~/components/ChatBox.vue';
@@ -77,9 +78,27 @@ function noteComments(note: OsmNote): ChatMessage[] {
 async function send(message: string) {
   try {
     if (props.item.isChangeset) {
+      const idsBefore = new Set(messages.value.map(m => m.id));
       await osmClient.postChangesetComment(workspaceId, props.item.id, message);
       // Re-fetch so the newly posted comment (and its server timestamp) appears.
       await refreshChangeset();
+
+      // OSM answers 200 even when it fails to save the comment (it uses
+      // `create`, not `create!`, so a comment whose author fails validation is
+      // dropped silently). Check it actually landed rather than trusting the
+      // status, and keep the user's text if it did not. Look for a new comment
+      // with this text rather than a higher count, which someone else's
+      // comment could also cause. Not by author: OSM display names need not
+      // match the TDEI name we hold (some are the account's email address).
+      const landed = messages.value.some(m =>
+        !idsBefore.has(m.id) && m.text.trim() === message.trim());
+
+      if (!landed) {
+        toast.error('Your comment was not saved. The server accepted the request but did not '
+          + 'record the comment, which usually means a problem with your account. Please '
+          + 'contact your workspace administrator.');
+        return;
+      }
     }
     else if (props.item.isNote) {
       await osmClient.postNoteComment(workspaceId, props.item.id, message);
@@ -92,8 +111,12 @@ async function send(message: string) {
     // user's text.
     chat.value?.clear();
   }
-  catch {
-    toast.error('Failed to post your comment. Please try again.');
+  catch (e: unknown) {
+    // Show why the server refused -- e.g. the changeset is still open, the
+    // hourly comment limit was hit, or the proxy denied workspace access --
+    // since "try again" does not help with any of those.
+    const reason = await resolveHttpErrorMessage(e, 'Please try again.');
+    toast.error(`Failed to post your comment. ${reason}`);
   }
 }
 

test/e2e/review.spec.ts

@@ -495,6 +495,126 @@ test.describe('workspace review', () => {
     await expect(discussion.locator('.message-list')).not.toContainText('This should fail to post.');
   });
 
+  // Opens changeset 4242's discussion. Its comments come from
+  // changeset/4242.json?include_discussion=true, which answers with `comments`
+  // every time, so a test can make a post appear to succeed without landing.
+  async function openChangesetDiscussion(page: Page, comments: object[]) {
+    await seedAuthenticatedSession(page);
+    await stubReviewApis(page);
+    await page.route('**/workspaces/1/changesets/*/adiff', route =>
+      route.fulfill({ json: { actions: [] } })
+    );
+    await page.route('https://ecn.*.tiles.virtualearth.net/**', route => route.abort());
+    await page.route('**/osm/api/0.6/changeset/4242.json**', route =>
+      route.fulfill({ json: { changeset: { ...changesets.changesets[0], comments } } })
+    );
+
+    await page.goto('/workspace/1/review');
+    await sidebar(page).locator('.review-item', { hasText: '#4242' }).click();
+
+    const overlay = page.locator('.review-overlay');
+    await expect(overlay.locator('.review-toolbar')).toContainText('#4242');
+    await overlay.locator('.review-toolbar button:has(.md-chat_bubble_outline)').click();
+
+    const discussion = page.locator('.review-discussion');
+    await expect(discussion).toContainText('Looks good to me.');
+    return discussion;
+  }
+
+  const existingChangesetComment = {
+    id: 1,
+    text: 'Looks good to me.',
+    visible: true,
+    user: 'mapper_jane',
+    uid: 7,
+    date: '2026-06-11T09:00:00.000Z'
+  };
+
+  // @test e2e: if the server refuses a changeset comment, the toast gives the
+  //            server's reason rather than a generic "try again"
+  test('a refused changeset comment shows the server reason', async ({ page }) => {
+    // OSM refuses comments on a changeset that is still open, as plain text.
+    await page.route('**/osm/api/0.6/changeset/4242/comment**', route =>
+      route.fulfill({
+        status: 409,
+        contentType: 'text/plain',
+        body: 'The changeset 4242 was not yet closed at 2026-06-10 10:05:00 UTC'
+      })
+    );
+    const discussion = await openChangesetDiscussion(page, [existingChangesetComment]);
+
+    await discussion.locator('textarea').fill('Please fix the crossing.');
+    await discussion.locator('button[title="Send"]').click();
+
+    const errorToast = page.locator('.Toastify__toast--error');
+    await expect(errorToast).toContainText('was not yet closed');
+    await expect(discussion.locator('textarea')).toHaveValue('Please fix the crossing.');
+  });
+
+  // @test e2e: a posted changeset comment appears and clears the draft, even
+  //            when OSM's author name differs from the signed-in user's name
+  test('a posted changeset comment appears and clears the draft', async ({ page }) => {
+    await page.route('**/osm/api/0.6/changeset/4242/comment**', route =>
+      route.fulfill({ contentType: 'application/xml', body: '<osm version="0.6"/>' })
+    );
+    const discussion = await openChangesetDiscussion(page, [existingChangesetComment]);
+
+    await page.route('**/osm/api/0.6/changeset/4242.json**', route =>
+      route.fulfill({
+        json: {
+          changeset: {
+            ...changesets.changesets[0],
+            comments: [
+              existingChangesetComment,
+              { ...existingChangesetComment, id: 2, text: 'Please fix the crossing.', user: 'tester@example.com', uid: 42 }
+            ]
+          }
+        }
+      })
+    );
+
+    await discussion.locator('textarea').fill('Please fix the crossing.');
+    await discussion.locator('button[title="Send"]').click();
+
+    await expect(discussion.locator('.message-list')).toContainText('Please fix the crossing.');
+    await expect(discussion.locator('textarea')).toHaveValue('');
+    await expect(page.locator('.Toastify__toast--error')).toHaveCount(0);
+  });
+
+  // @test e2e: a changeset comment the server accepts but does not save is
+  //            reported, and the draft is kept
+  test('a changeset comment that does not land is reported', async ({ page }) => {
+    // OSM answers 200 but drops the comment when its author fails validation.
+    // The re-fetched discussion is unchanged.
+    await page.route('**/osm/api/0.6/changeset/4242/comment**', route =>
+      route.fulfill({ contentType: 'application/xml', body: '<osm version="0.6"/>' })
+    );
+    const discussion = await openChangesetDiscussion(page, [existingChangesetComment]);
+
+    // Someone else's comment lands meanwhile, so the count still goes up: only
+    // a comment with the submitted text counts as this one landing.
+    await page.route('**/osm/api/0.6/changeset/4242.json**', route =>
+      route.fulfill({
+        json: {
+          changeset: {
+            ...changesets.changesets[0],
+            comments: [
+              existingChangesetComment,
+              { ...existingChangesetComment, id: 2, text: 'Crossing fixed.', user: 'mapper_joe', uid: 8 }
+            ]
+          }
+        }
+      })
+    );
+
+    await discussion.locator('textarea').fill('Please fix the crossing.');
+    await discussion.locator('button[title="Send"]').click();
+
+    const errorToast = page.locator('.Toastify__toast--error');
+    await expect(errorToast).toContainText('was not saved');
+    await expect(discussion.locator('textarea')).toHaveValue('Please fix the crossing.');
+  });
+
   // @test e2e: validate that all the API calls used on this page match the Swagger spec
   test('new-API calls conform to the OpenAPI spec', async ({ page }) => {
     await seedAuthenticatedSession(page);