Make Share the leading filled action and explain rejected shares

client/e2e/learning-path-feedback.e2e.js

@@ -190,15 +190,23 @@ test("only concept headings drag; summaries show details without reordering", as
   );
 });
 
-test("share action stays compact and the share link opens a new tab", async ({
+test("share leads as a compact filled action and its link opens a new tab", async ({
   page,
 }) => {
   await openGenerated(page);
   await page.route("**/v1/shares", (route) =>
     route.fulfill({ json: { share_id: "abcdefghijklmnopqrstuv" } }),
   );
   const button = page.getByRole("button", { name: "Share path", exact: true });
-  expect((await button.boundingBox()).width).toBeLessThan(200);
+  const copy = page.getByRole("button", { name: "Copy path", exact: true });
+  await expect(button).toHaveClass(/primary-button/);
+  await expect(copy).toHaveClass(/secondary-button/);
+  const shareBox = await button.boundingBox();
+  expect(shareBox.width).toBeLessThan(200);
+  expect(shareBox.x).toBeLessThan((await copy.boundingBox()).x);
+  await expect(button).toHaveAccessibleDescription(
+    "Anyone with the link can view a copy of this path.",
+  );
   await button.click();
   const link = page.getByRole("link", { name: /Open share link/ });
   await expect(link).toHaveAttribute("target", "_blank");
@@ -211,6 +219,28 @@ test("share action stays compact and the share link opens a new tab", async ({
   await popup.close();
 });
 
+test("a rejected share explains itself instead of showing raw errors", async ({
+  page,
+}) => {
+  await openGenerated(page);
+  await page.route("**/v1/shares", (route) =>
+    route.fulfill({
+      status: 422,
+      json: {
+        detail: [{ loc: ["body", "graph"], msg: "Invalid", type: "value_error" }],
+      },
+    }),
+  );
+  await page.getByRole("button", { name: "Share path", exact: true }).click();
+  await expect(
+    page.getByText("This path can't be shared. Use Copy to save it instead."),
+  ).toBeVisible();
+  await expect(page.getByText("[object Object]")).toHaveCount(0);
+  await expect(
+    page.getByRole("button", { name: "Share path", exact: true }),
+  ).toBeEnabled();
+});
+
 for (const width of [320, 375, 390]) {
   test(`nav and path controls each fit one row on a ${width}px phone`, async ({
     page,
@@ -243,5 +273,29 @@ for (const width of [320, 375, 390]) {
     expect(
       await page.evaluate(() => document.documentElement.scrollWidth),
     ).toBe(width);
+
+    // The row still fits while the share link is being created.
+    let release;
+    const held = new Promise((resolve) => {
+      release = resolve;
+    });
+    await page.route("**/v1/shares", async (route) => {
+      await held;
+      await route.fulfill({ json: { share_id: "abcdefghijklmnopqrstuv" } });
+    });
+    await page.getByRole("button", { name: "Share path", exact: true }).click();
+    await expect(
+      page.getByRole("button", { name: "Creating link...", exact: true }),
+    ).toBeDisabled();
+    expect(
+      await rowCount(".view-toggle, .copy-action, .share-action"),
+    ).toBeLessThan(4);
+    expect(
+      await page.evaluate(() => document.documentElement.scrollWidth),
+    ).toBe(width);
+    release();
+    await expect(
+      page.getByRole("link", { name: /Open share link/ }),
+    ).toBeVisible();
   });
 }

client/src/pages/LearningPath/LearningPath.jsx

@@ -30,7 +30,10 @@ import { apiUrl } from "../../config/api";
 import { findPublicTopicByAlias } from "../../data/publicTopics";
 import Seo from "../../seo/Seo";
 import NetworkGraph from "./NetworkGraph";
-import { createShareSnapshot } from "../../util/shareSnapshot";
+import {
+  createShareSnapshot,
+  shareErrorMessage,
+} from "../../util/shareSnapshot";
 
 const LEVEL_ORDER = ["Beginner", "Intermediate", "Advanced"];
 const MODERATION_DETAIL_HINTS = ["content policy", "moderation", "flagged"];
@@ -172,8 +175,10 @@ export default function LearningPath() {
         ),
       });
       const body = await response.json().catch(() => ({}));
-      if (!response.ok || !body.share_id)
-        throw new Error(body.detail || "Unable to create share.");
+      if (!response.ok || !body.share_id) {
+        enqueueSnackbar(shareErrorMessage(response.status, body));
+        return;
+      }
       const createdUrl = `${window.location.origin}/share/${body.share_id}`;
       setShareUrl(createdUrl);
       if (navigator.clipboard?.writeText) {
@@ -188,8 +193,10 @@ export default function LearningPath() {
       } else {
         enqueueSnackbar("Share created. Copy the link shown below.");
       }
-    } catch (error) {
-      enqueueSnackbar(error.message || "Unable to create share.");
+    } catch {
+      enqueueSnackbar(
+        "Couldn't reach the server. Check your connection and try again.",
+      );
     } finally {
       setIsSharing(false);
     }
@@ -331,28 +338,21 @@ export default function LearningPath() {
               role="group"
               aria-label="Learning path actions"
             >
-              <Button
-                className="copy-action"
-                aria-label="Copy path"
-                // One inline span keeps the space that a flex button drops.
-                label={
-                  <span>
-                    Copy<span className="action-noun"> path</span>
-                  </span>
-                }
-                isSecondary
-                onClick={() =>
-                  copyToClipboard(lp, conceptDetails, enqueueSnackbar)
-                }
-              />
+              {/* Share leads as the filled button. As a secondary button
+                  after Copy, far fewer people shared. */}
               <Button
                 className="share-action"
-                isSecondary
                 aria-describedby="share-visibility"
-                aria-label={isSharing ? undefined : "Share path"}
+                aria-label={isSharing ? "Creating link..." : "Share path"}
+                aria-busy={isSharing}
                 label={
                   isSharing ? (
-                    "Creating link..."
+                    // Phones keep the short label so the row still fits while
+                    // the link is created.
+                    <span>
+                      <span className="action-busy">Creating link...</span>
+                      <span className="action-busy-short">Share</span>
+                    </span>
                   ) : (
                     <span>
                       Share<span className="action-noun"> path</span>
@@ -362,6 +362,20 @@ export default function LearningPath() {
                 disabled={isSharing}
                 onClick={shareLearningPath}
               />
+              <Button
+                className="copy-action"
+                aria-label="Copy path"
+                // One inline span keeps the space that a flex button drops.
+                label={
+                  <span>
+                    Copy<span className="action-noun"> path</span>
+                  </span>
+                }
+                isSecondary
+                onClick={() =>
+                  copyToClipboard(lp, conceptDetails, enqueueSnackbar)
+                }
+              />
             </div>
           </div>
           <p id="share-visibility" className="share-hint">

client/src/pages/LearningPath/LearningPath.scss

@@ -266,6 +266,9 @@
       margin: 0;
       padding: 10px 16px;
     }
+    .action-busy-short {
+      display: none;
+    }
     // Phones keep the view toggle and both actions on one row.
     @media (max-width: 480px) {
       flex-wrap: nowrap;
@@ -283,9 +286,13 @@
         padding: 9px 12px;
         white-space: nowrap;
       }
-      .action-noun {
+      .action-noun,
+      .action-busy {
         display: none;
       }
+      .action-busy-short {
+        display: inline;
+      }
     }
   }
   .share-hint {

client/src/util/shareSnapshot.js

@@ -7,3 +7,18 @@ export function createShareSnapshot({ topic, levels, conceptDetails, graph, defa
     default_view: defaultView,
   };
 }
+
+// The share endpoint rejects content with 400 (moderation), 413 or 422. FastAPI
+// sends 422 details as a list and slowapi sends 429s as { error }, so only
+// other string details are shown as they are.
+export function shareErrorMessage(status, body) {
+  if (status === 400 || status === 413 || status === 422) {
+    return "This path can't be shared. Use Copy to save it instead.";
+  }
+  if (status === 429) {
+    return "Too many share links in a short time. Wait a minute and try again.";
+  }
+  return typeof body?.detail === "string" && body.detail
+    ? body.detail
+    : "Unable to create share.";
+}

client/src/util/shareSnapshot.test.js

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { createShareSnapshot } from "./shareSnapshot";
+import { createShareSnapshot, shareErrorMessage } from "./shareSnapshot";
 
 describe("share snapshots", () => {
   it("preserves reordered levels and the selected default view exactly", () => {
@@ -16,3 +16,30 @@ describe("share snapshots", () => {
     expect(snapshot.concept_details.First.summary).toBe("One");
   });
 });
+
+describe("share error messages", () => {
+  it("explains content rejections instead of showing their details", () => {
+    const detail = [{ loc: ["body", "graph"], type: "string_too_long" }];
+    const message = shareErrorMessage(422, { detail });
+    expect(message).toBe("This path can't be shared. Use Copy to save it instead.");
+    expect(shareErrorMessage(413, { detail: "Share snapshot is too large." })).toBe(
+      message,
+    );
+    const moderation =
+      "User input does not complies with OpenAI's content policy. https://example.invalid";
+    expect(shareErrorMessage(400, { detail: moderation })).toBe(message);
+  });
+
+  it("explains rate limits, which have no detail field", () => {
+    expect(shareErrorMessage(429, { error: "Rate limit exceeded: 5 per 1 minute" })).toBe(
+      "Too many share links in a short time. Wait a minute and try again.",
+    );
+  });
+
+  it("shows string details and falls back otherwise", () => {
+    const detail = "Share storage is temporarily unavailable.";
+    expect(shareErrorMessage(503, { detail })).toBe(detail);
+    expect(shareErrorMessage(502, {})).toBe("Unable to create share.");
+    expect(shareErrorMessage(201, { detail: "" })).toBe("Unable to create share.");
+  });
+});