fix(puzzles): merge anon progress, rehydrate completion, recover orphaned draft (#1650)

app/admin/puzzles/webapp/controller/Builder.controller.js

@@ -6,8 +6,9 @@ sap.ui.define([
   "sap/ui/core/Fragment",
   "sap/tutorials/admin/puzzles/lib/crossword-geometry",
   "sap/tutorials/admin/puzzles/lib/puzzle-io",
-  "sap/tutorials/admin/puzzles/lib/solver-core"
-], function (Controller, JSONModel, MessageToast, MessageBox, Fragment, geom, io, solver) {
+  "sap/tutorials/admin/puzzles/lib/solver-core",
+  "sap/tutorials/admin/puzzles/lib/draft-save"
+], function (Controller, JSONModel, MessageToast, MessageBox, Fragment, geom, io, solver, draftSave) {
   "use strict";
 
   // ──────────────────────────────────────────────────────────────────────────
@@ -523,54 +524,15 @@ sap.ui.define([
           "Accept": "application/json",
           "x-csrf-token": token
         };
-
-        if (editId) {
-          // ── UPDATE: draftEdit → PATCH draft → draftActivate ──────────────
-          return fetch(
-            "/admin/Puzzles(ID=" + editId + ",IsActiveEntity=true)/AdminService.draftEdit",
-            { method: "POST", credentials: "include", headers: headers, body: "{}" }
-          ).then(function (r) {
-            if (!r.ok) { return r.text().then(function (t) { throw new Error("draftEdit HTTP " + r.status + ": " + t); }); }
-            return r.json();
-          }).then(function (draft) {
-            var draftId = draft.ID || editId;
-            return fetch(
-              "/admin/Puzzles(ID=" + draftId + ",IsActiveEntity=false)",
-              { method: "PATCH", credentials: "include", headers: headers, body: JSON.stringify(fields) }
-            ).then(function (r2) {
-              if (!r2.ok) { return r2.text().then(function (t) { throw new Error("PATCH draft HTTP " + r2.status + ": " + t); }); }
-              return draftId;
-            });
-          }).then(function (draftId) {
-            return fetch(
-              "/admin/Puzzles(ID=" + draftId + ",IsActiveEntity=false)/AdminService.draftActivate",
-              { method: "POST", credentials: "include", headers: headers, body: "{}" }
-            ).then(function (r3) {
-              if (!r3.ok) { return r3.text().then(function (t) { throw new Error("draftActivate HTTP " + r3.status + ": " + t); }); }
-              return r3.json();
-            });
-          });
-        }
-
-        // ── CREATE: POST draft → draftActivate ───────────────────────────
-        return fetch("/admin/Puzzles", {
-          method: "POST",
-          credentials: "include",
+        // Draft orchestration (create vs. update, incl. recovery from an
+        // orphaned edit draft that returns 409 DRAFT_ALREADY_EXISTS — issue
+        // #1650 bug 3) lives in the unit-tested lib/draft-save module.
+        return draftSave.performPuzzleSave({
+          fetchFn: fetch,
           headers: headers,
-          body: JSON.stringify(fields)
-        }).then(function (r) {
-          if (!r.ok) { return r.text().then(function (t) { throw new Error("POST draft HTTP " + r.status + ": " + t); }); }
-          return r.json();
-        }).then(function (draft) {
-          return fetch(
-            "/admin/Puzzles(ID=" + draft.ID + ",IsActiveEntity=false)/AdminService.draftActivate",
-            { method: "POST", credentials: "include", headers: headers, body: "{}" }
-          ).then(function (r2) {
-            if (!r2.ok) { return r2.text().then(function (t) { throw new Error("draftActivate HTTP " + r2.status + ": " + t); }); }
-            return r2.json();
-          });
+          editId: editId,
+          fields: fields
         });
-
       }).then(function (active) {
         var savedSlug = (active && active.slug) || slug;
         var savedId = (active && active.ID) || editId;

app/admin/puzzles/webapp/lib/draft-save.js

@@ -0,0 +1,94 @@
+sap.ui.define([], function () {
+  "use strict";
+
+  // ──────────────────────────────────────────────────────────────────────────
+  // draft-save.js
+  //
+  // Orchestrates the CAP draft save flow for a puzzle:
+  //   CREATE: POST /admin/Puzzles              → draftActivate
+  //   UPDATE: draftEdit → PATCH draft           → draftActivate
+  //
+  // Bug fix (issue #1650 bug 3): on UPDATE, `draftEdit` can return
+  //   409 { code: "DRAFT_ALREADY_EXISTS" }
+  // when a prior edit session left an un-activated draft (opened the editor and
+  // navigated away, or a save failed mid-flight). The old controller treated
+  // that as a hard error, so the puzzle could never be re-saved until the stale
+  // draft was cleared. A CAP edit-draft shares the active entity's key, so we
+  // recover by resuming that existing draft — PATCH it with the fresh fields
+  // and activate — instead of failing.
+  //
+  // `fetchFn` is injected so this is unit-testable without a browser.
+  // ──────────────────────────────────────────────────────────────────────────
+
+  function jsonOrThrow(step) {
+    return function (r) {
+      if (!r.ok) {
+        return r.text().then(function (t) {
+          throw new Error(step + " HTTP " + r.status + ": " + t);
+        });
+      }
+      return r.json();
+    };
+  }
+
+  /**
+   * @param {object}   opts
+   * @param {function} opts.fetchFn  fetch implementation (window.fetch)
+   * @param {object}   opts.headers  request headers (incl. x-csrf-token)
+   * @param {string}   [opts.editId] active entity ID when updating; falsy = create
+   * @param {object}   opts.fields   the puzzle fields to persist
+   * @returns {Promise<object>} the activated (active) entity JSON
+   */
+  function performPuzzleSave(opts) {
+    var fetchFn = opts.fetchFn;
+    var headers = opts.headers;
+    var editId = opts.editId;
+    var body = JSON.stringify(opts.fields);
+
+    function patchDraft(draftId) {
+      return fetchFn(
+        "/admin/Puzzles(ID=" + draftId + ",IsActiveEntity=false)",
+        { method: "PATCH", credentials: "include", headers: headers, body: body }
+      ).then(function (r) {
+        if (!r.ok) {
+          return r.text().then(function (t) { throw new Error("PATCH draft HTTP " + r.status + ": " + t); });
+        }
+        return draftId;
+      });
+    }
+
+    function activate(draftId) {
+      return fetchFn(
+        "/admin/Puzzles(ID=" + draftId + ",IsActiveEntity=false)/AdminService.draftActivate",
+        { method: "POST", credentials: "include", headers: headers, body: "{}" }
+      ).then(jsonOrThrow("draftActivate"));
+    }
+
+    if (editId) {
+      return fetchFn(
+        "/admin/Puzzles(ID=" + editId + ",IsActiveEntity=true)/AdminService.draftEdit",
+        { method: "POST", credentials: "include", headers: headers, body: "{}" }
+      ).then(function (r) {
+        if (r.ok) {
+          return r.json().then(function (draft) { return (draft && draft.ID) || editId; });
+        }
+        return r.text().then(function (t) {
+          // Resume an orphaned draft rather than failing the save.
+          if (r.status === 409 && /DRAFT_ALREADY_EXISTS/.test(t)) {
+            return editId; // edit draft shares the active entity's key
+          }
+          throw new Error("draftEdit HTTP " + r.status + ": " + t);
+        });
+      }).then(patchDraft).then(activate);
+    }
+
+    // CREATE: POST a draft, then activate it.
+    return fetchFn("/admin/Puzzles", {
+      method: "POST", credentials: "include", headers: headers, body: body
+    }).then(jsonOrThrow("POST draft")).then(function (draft) {
+      return activate(draft.ID);
+    });
+  }
+
+  return { performPuzzleSave: performPuzzleSave };
+});

hugo-apps/src/puzzle/App.vue

@@ -27,7 +27,7 @@ import {
   postComplete,
   postResetProgress,
 } from './lib/server';
-import { emptyWhiteCells, shouldMigrate } from './lib/progress';
+import { emptyWhiteCells, mergeProgress } from './lib/progress';
 
 const props = defineProps<{ slug: string; apiUrl: string }>();
 
@@ -146,32 +146,51 @@ async function resumeProgress() {
 
   if (authed.value) {
     let serverGrid: string | null = null;
+    let completed = false;
     try {
       const prog = await fetchProgress(props.apiUrl, props.slug);
       serverGrid = prog.filledGrid ?? null;
+      completed = prog.completed === true;
     } catch { /* 401/network — treat as empty */ }
 
-    // Server grid non-empty → server wins
-    if (serverGrid) {
-      let parsed: Record<string, string> = {};
-      try { parsed = JSON.parse(serverGrid) as Record<string, string>; } catch { /* ignore */ }
-      if (Object.values(parsed).some((v: any) => v)) {
-        answers.value = parsed;
-        return;
-      }
-    }
-    // Server empty but local has data → migrate local to server
-    if (shouldMigrate(true, serverGrid, local)) {
-      answers.value = local;
-      try { await postSaveProgress(props.apiUrl, props.slug, JSON.stringify(local)); }
-      catch { /* migration best-effort; local copy remains */ }
-      return;
+    // Merge local ∪ server: the server is authoritative for the cells it holds,
+    // but any answers typed while logged out (present only in localStorage) are
+    // preserved instead of being overwritten (issue #1650 bug 1).
+    const { merged, changed } = mergeProgress(serverGrid, local);
+    answers.value = merged;
+    if (changed) {
+      try { await postSaveProgress(props.apiUrl, props.slug, JSON.stringify(merged)); }
+      catch { /* best-effort; local copy remains */ }
     }
+
+    // Re-hydrate the solved state so the completed banner + Reset button survive
+    // a page reload (issue #1650 bug 2). Painting the grid green is derived from
+    // the (correct) completed grid — no answer key is shipped to the client.
+    if (completed) markSolvedFromServer();
+    return;
   }
-  // Anonymous, or authed with nothing anywhere: use local.
+  // Anonymous: use local only.
   answers.value = local;
 }
 
+/**
+ * Reflect a server-recorded completion in the UI on load: show the solved
+ * banner + Reset button and paint every filled white cell green. A completed
+ * puzzle is fully and correctly filled, so marking filled white cells 'correct'
+ * matches the server's verdict without re-grading or exposing the solution.
+ */
+function markSolvedFromServer() {
+  solved.value = true;
+  const status: Record<string, 'correct' | 'wrong'> = {};
+  for (let r = 0; r < grid.value.length; r++) {
+    for (let c = 0; c < grid.value[r].length; c++) {
+      if (grid.value[r][c]?.black) continue;
+      if (answers.value[`${r},${c}`]) status[`${r},${c}`] = 'correct';
+    }
+  }
+  cellStatus.value = status;
+}
+
 // ── Autosave (debounced) ──────────────────────────────────────────────────────
 let saveTimer: ReturnType<typeof setTimeout> | null = null;
 

hugo-apps/src/puzzle/__tests__/progress.test.ts

@@ -1,15 +1,43 @@
 import { describe, it, expect } from 'vitest';
-import { shouldMigrate, emptyWhiteCells } from '../lib/progress';
+import { mergeProgress, emptyWhiteCells } from '../lib/progress';
 
-describe('shouldMigrate', () => {
-  it('migrates when authed + empty server + non-empty local', () => {
-    expect(shouldMigrate(true, '{}', { '0,0':'C' })).toBe(true);
-    expect(shouldMigrate(true, null, { '0,0':'C' })).toBe(true);
+describe('mergeProgress', () => {
+  it('migrates local when the server grid is empty or null (changed=true)', () => {
+    expect(mergeProgress('{}', { '0,0': 'C' })).toEqual({ merged: { '0,0': 'C' }, changed: true });
+    expect(mergeProgress(null, { '0,0': 'C' })).toEqual({ merged: { '0,0': 'C' }, changed: true });
   });
-  it('does not migrate when server has data, or local empty, or anon', () => {
-    expect(shouldMigrate(true, '{"0,0":"C"}', { '0,1':'A' })).toBe(false);
-    expect(shouldMigrate(true, '{}', {})).toBe(false);
-    expect(shouldMigrate(false, '{}', { '0,0':'C' })).toBe(false);
+
+  it('preserves local-only answers on top of the server grid (issue #1650 bug 1)', () => {
+    // Server has the old answers; local was filled anonymously with an extra word.
+    const server = '{"0,0":"C","0,1":"A"}';
+    const local = { '0,0': 'C', '0,1': 'A', '2,0': 'O', '2,1': 'D', '2,2': 'B', '2,3': 'C' };
+    const { merged, changed } = mergeProgress(server, local);
+    expect(merged).toEqual({ '0,0': 'C', '0,1': 'A', '2,0': 'O', '2,1': 'D', '2,2': 'B', '2,3': 'C' });
+    expect(changed).toBe(true); // local contributed cells the server lacked → persist back
+  });
+
+  it('lets the server win for conflicting cells but still adds local-only cells', () => {
+    const server = '{"0,0":"C"}';
+    const local = { '0,0': 'X', '0,1': 'A' }; // 0,0 conflicts; 0,1 is local-only
+    const { merged, changed } = mergeProgress(server, local);
+    expect(merged).toEqual({ '0,0': 'C', '0,1': 'A' });
+    expect(changed).toBe(true);
+  });
+
+  it('is a no-op when local adds nothing (server wins, changed=false)', () => {
+    expect(mergeProgress('{"0,0":"C"}', {})).toEqual({ merged: { '0,0': 'C' }, changed: false });
+    expect(mergeProgress('{"0,0":"C"}', { '0,0': 'C' })).toEqual({ merged: { '0,0': 'C' }, changed: false });
+    expect(mergeProgress('{}', {})).toEqual({ merged: {}, changed: false });
+  });
+
+  it('drops empty-string cells from both sides', () => {
+    const { merged, changed } = mergeProgress('{"0,0":"C","0,1":""}', { '0,2': '', '0,3': 'D' });
+    expect(merged).toEqual({ '0,0': 'C', '0,3': 'D' });
+    expect(changed).toBe(true);
+  });
+
+  it('tolerates corrupt server JSON by treating it as empty', () => {
+    expect(mergeProgress('not json', { '0,0': 'C' })).toEqual({ merged: { '0,0': 'C' }, changed: true });
   });
 });
 

hugo-apps/src/puzzle/lib/progress.ts

@@ -1,14 +1,51 @@
-export function shouldMigrate(
-  authed: boolean,
+// hugo-apps/src/puzzle/lib/progress.ts
+// Pure helpers for reconciling puzzle progress across localStorage (anonymous)
+// and the server (authenticated), plus grid inspection utilities.
+
+/**
+ * Merge a logged-in user's server grid with locally-stored (possibly anonymous)
+ * answers.
+ *
+ * Rationale (issue #1650 bug 1): the old resume logic did "server wins if the
+ * server grid is non-empty", which silently discarded any answers a user typed
+ * while logged out (those never reach the server, whose write endpoints are
+ * auth-gated). On login the server grid — missing the anonymous additions —
+ * overwrote everything, losing the just-typed words.
+ *
+ * Merge rule:
+ *   - The server is authoritative for every cell it already has a letter in
+ *     (protects cross-device: a fully-solved grid on the server is never
+ *     clobbered by stale local state).
+ *   - Local answers are kept ONLY for cells the server leaves blank, so
+ *     anonymous progress is preserved instead of dropped.
+ *   - Empty-string values on either side are treated as "no answer".
+ *
+ * @returns `{ merged, changed }` — `changed` is true when local contributed at
+ *   least one cell the server lacked, signalling the caller to persist `merged`
+ *   back to the server.
+ */
+export function mergeProgress(
   serverGrid: string | null,
   localGrid: Record<string, string>
-): boolean {
-  if (!authed) return false;
+): { merged: Record<string, string>; changed: boolean } {
   let server: Record<string, string> = {};
-  try { server = serverGrid ? JSON.parse(serverGrid) : {}; } catch { server = {}; }
-  const serverHas = Object.values(server).some(v => v && v.length > 0);
-  if (serverHas) return false;
-  return Object.values(localGrid || {}).some(v => v && v.length > 0);
+  try { server = serverGrid ? (JSON.parse(serverGrid) as Record<string, string>) : {}; }
+  catch { server = {}; }
+
+  const merged: Record<string, string> = {};
+  // Server letters first — authoritative for the cells it holds.
+  for (const [k, v] of Object.entries(server)) {
+    if (v && v.length > 0) merged[k] = v;
+  }
+  // Local-only additions: keep any non-empty local cell the server left blank.
+  let changed = false;
+  for (const [k, v] of Object.entries(localGrid || {})) {
+    if (v && v.length > 0 && !merged[k]) {
+      merged[k] = v;
+      changed = true;
+    }
+  }
+  return { merged, changed };
 }
 
 export function emptyWhiteCells(

hugo-apps/src/puzzle/lib/server.ts

@@ -71,6 +71,8 @@ export interface CheckResult {
 export interface ProgressResult {
   filledGrid: string | null;
   attemptNumber: number;
+  /** True when the caller has already solved this puzzle (issue #1650 bug 2). */
+  completed?: boolean;
 }
 
 export interface CompleteResult {

srv/puzzle-service.cds

@@ -26,7 +26,7 @@ service PuzzleService {
   action saveProgress(slug : String, filledGrid : LargeString) returns Boolean;
 
   @(requires: 'authenticated-user')
-  function getProgress(slug : String) returns { filledGrid : LargeString; attemptNumber : Integer; };
+  function getProgress(slug : String) returns { filledGrid : LargeString; attemptNumber : Integer; completed : Boolean; };
 
   @(requires: 'authenticated-user')
   action complete(slug : String) returns { recorded : Boolean; alreadyComplete : Boolean; };