fix: make sheet version restore fully functional (#957)

* fix: make sheet version restore fully functional

Three bugs made it impossible to restore a sheet from version history:

1. `resetSheets` cleared `el.className` (removing `da-sheet`), so the
   second restore's `document.querySelector('.da-sheet')` returned null
   and crashed with "Cannot read properties of null (reading 'jexcel')".
   Fix: remove only the `jexcel_tabs` class — the one jspreadsheet checks
   to decide whether to reuse existing children — instead of wiping all
   classes.

2. The restore handler called `staleCheck.markSynced(verReview.data)`
   with jspreadsheet-format data. `checkForDrift` fetches DA-JSON from
   the server, so these formats never matched, drift was always detected,
   and `onStale({ dirty: false })` silently reloaded the sheet from the
   server — undoing the restore before the user could save.
   Fix: call `staleCheck.markEdited()` instead, preserving the correct
   server-content baseline while marking the sheet dirty so any genuine
   concurrent edit shows a dialog rather than a silent reload.

3. After restore `da-sheet-panes.data` was never updated, so the Preview
   pane still showed the pre-restore content.
   Fix: set `sheetPanes.data = convertSheets(daTitle.sheet)` after
   reinitialising the spreadsheet.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: persist restored sheet version to server

Sheet version restore was only updating the local jspreadsheet UI; the
restored content was never POSTed, so a page refresh re-fetched the old
data from the server.

Extract restoreVersion into utils.js and have it call saveSheets after
re-initializing the sheet — that drives the PUT to /source plus preview
sync and staleCheck markSynced. Add unit tests covering both the PUT
and the preview-pane update.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com>

Author: 3 people

blocks/sheet/sheet.js

@@ -3,7 +3,7 @@ import getPathDetails from '../shared/pathDetails.js';
 import { getNx } from '../../scripts/utils.js';
 import '../edit/da-title/da-title.js';
 import { getData } from './utils/index.js';
-import { staleCheck, showDaDialog } from './utils/utils.js';
+import { staleCheck, showDaDialog, restoreVersion } from './utils/utils.js';
 import { convertSheets } from '../edit/utils/helpers.js';
 
 const { default: getStyle } = await import(`${getNx()}/utils/styles.js`);
@@ -52,10 +52,7 @@ class DaSheetPanes extends LitElement {
     verReview.addEventListener('restore', async () => {
       const daTitle = document.querySelector('da-title');
       const daSheet = document.querySelector('.da-sheet');
-
-      const initSheet = (await import('./utils/index.js')).default;
-      daTitle.sheet = await initSheet(daSheet, verReview.data);
-      staleCheck.markSynced(verReview.data);
+      await restoreVersion(daTitle, daSheet, verReview.data);
       verReview.remove();
     });
 

blocks/sheet/utils/index.js

@@ -16,7 +16,7 @@ function resetSheets(el) {
   if (!el.jexcel) return;
   delete el.jexcel;
   el.innerHTML = '';
-  el.className = '';
+  el.classList.remove('jexcel_tabs');
 }
 
 function finishSetup(el, data) {

blocks/sheet/utils/utils.js

@@ -93,6 +93,12 @@ export const saveSheets = async (sheets) => {
 
 const debouncedSaveSheets = debounce(saveSheets, DEBOUNCE_TIME);
 
+export async function restoreVersion(daTitle, daSheet, versionData) {
+  const initSheet = (await import('./index.js')).default;
+  daTitle.sheet = await initSheet(daSheet, versionData);
+  return saveSheets(daSheet.jexcel);
+}
+
 export function handleSave(jexcel, view) {
   // markEdited must precede the config bail so config edits still mark dirty for stale-detection.
   staleCheck.markEdited();

test/unit/blocks/sheet/index.test.js

@@ -7,6 +7,182 @@ setNx('/test/fixtures/nx', { hostname: 'example.com' });
 
 const sh = await import('../../../../blocks/sheet/utils/index.js');
 
+function mockJspreadsheetTabs(container, data) {
+  // Mirrors real jspreadsheet.tabs: if jexcel_tabs is present it tries to
+  // reuse e.children[0] / e.children[1], which are undefined after innerHTML='',
+  // causing "Cannot read properties of undefined (reading 'appendChild')".
+  if (container.classList.contains('jexcel_tabs')) {
+    const first = container.children[0];
+    first.appendChild(document.createElement('div')); // throws if first is undefined
+  }
+  const innerContainers = data.map(() => '<div class="jexcel_container"></div>').join('');
+  container.innerHTML = `<div></div><div>${innerContainers}</div>`;
+  container.classList.add('jexcel_tabs');
+  container.jexcel = data.map((d) => ({ name: d.sheetName, options: {} }));
+}
+
+describe('init - double restore', () => {
+  let el;
+  let daTitle;
+  let savedJspreadsheet;
+
+  beforeEach(() => {
+    el = document.createElement('div');
+    el.className = 'da-sheet';
+    document.body.appendChild(el);
+
+    daTitle = document.createElement('da-title');
+    daTitle.permissions = [];
+    document.body.appendChild(daTitle);
+
+    savedJspreadsheet = window.jspreadsheet;
+    window.jspreadsheet = { tabs: mockJspreadsheetTabs };
+  });
+
+  afterEach(() => {
+    el.remove();
+    daTitle.remove();
+    window.jspreadsheet = savedJspreadsheet;
+    document.querySelectorAll('da-sheet-tabs').forEach((t) => t.remove());
+  });
+
+  it('preserves da-sheet class after second init so restore handler can re-query it', async () => {
+    const data = [{ sheetName: 'data', minDimensions: [20, 20], data: [['Key'], ['A']], columns: [{ width: '300' }] }];
+
+    await sh.default(el, data);
+    await sh.default(el, data);
+
+    expect(el.classList.contains('da-sheet')).to.be.true;
+  });
+
+  it('document.querySelector .da-sheet returns non-null after second init', async () => {
+    const data = [{ sheetName: 'data', minDimensions: [20, 20], data: [['Key'], ['A']], columns: [{ width: '300' }] }];
+
+    await sh.default(el, data);
+    await sh.default(el, data);
+
+    expect(document.querySelector('.da-sheet')).to.not.be.null;
+  });
+
+  it('does not crash on third init call (page load + two restores)', async () => {
+    const data = [{ sheetName: 'data', minDimensions: [20, 20], data: [['Key'], ['A']], columns: [{ width: '300' }] }];
+
+    // Simulates: page load, then first restore, then second restore
+    await sh.default(el, data);
+    await sh.default(el, data);
+
+    // The restore handler always queries .da-sheet at restore time
+    const daSheet = document.querySelector('.da-sheet');
+    expect(daSheet).to.not.be.null;
+    await sh.default(daSheet, data);
+  });
+});
+
+describe('restoreVersion', () => {
+  let savedFetch;
+  let savedJspreadsheet;
+  let savedHash;
+  let daSheet;
+  let daTitle;
+  let panes;
+
+  beforeEach(() => {
+    savedFetch = window.fetch;
+    savedHash = window.location.hash;
+    savedJspreadsheet = window.jspreadsheet;
+    window.jspreadsheet = {
+      tabs: (container, data) => {
+        const inner = data.map(() => '<div class="jexcel_container"></div>').join('');
+        container.innerHTML = `<div></div><div>${inner}</div>`;
+        container.classList.add('jexcel_tabs');
+        container.jexcel = data.map((d) => ({
+          name: d.sheetName,
+          options: {},
+          getData: () => d.data,
+          getConfig: () => ({ columns: d.columns || [] }),
+        }));
+      },
+    };
+
+    daSheet = document.createElement('div');
+    daSheet.className = 'da-sheet';
+    document.body.appendChild(daSheet);
+
+    // da-sheet-tabs's connectedCallback queries document for a da-title to
+    // read .permissions from — provide a stub. Since the test file does not
+    // import sheet.js (which would register da-title as a lit element),
+    // da-title remains HTMLUnknownElement and skips connectedCallback.
+    daTitle = document.createElement('da-title');
+    daTitle.permissions = ['read', 'write'];
+    document.body.appendChild(daTitle);
+
+    panes = document.createElement('da-sheet-panes');
+    document.body.appendChild(panes);
+  });
+
+  afterEach(() => {
+    window.fetch = savedFetch;
+    window.jspreadsheet = savedJspreadsheet;
+    if (savedHash) window.location.hash = savedHash;
+    daSheet.remove();
+    daTitle.remove();
+    panes.remove();
+    document.querySelectorAll('da-sheet-tabs').forEach((t) => t.remove());
+  });
+
+  it('PUTs restored version data to /source and returns true', async () => {
+    const { restoreVersion } = await import('../../../../blocks/sheet/utils/utils.js');
+
+    const versionData = [{
+      sheetName: 'data',
+      minDimensions: [20, 20],
+      data: [['Key'], ['restored']],
+      columns: [{ width: '300' }],
+    }];
+
+    window.location.hash = '#/o/r/sheet';
+
+    let putUrl;
+    let putBody;
+    window.fetch = async (url, opts) => {
+      if (opts?.method === 'PUT') {
+        putUrl = String(url);
+        putBody = opts.body;
+        return new Response('', { status: 200 });
+      }
+      return new Response('', { status: 200 });
+    };
+
+    const result = await restoreVersion(daTitle, daSheet, versionData);
+
+    expect(result).to.be.true;
+    expect(daTitle.sheet).to.exist;
+    expect(putUrl).to.contain('/source/o/r/sheet.json');
+    expect(putBody).to.be.instanceOf(FormData);
+    const blob = putBody.get('data');
+    const payload = JSON.parse(await blob.text());
+    expect(JSON.stringify(payload)).to.contain('restored');
+  });
+
+  it('Updates da-sheet-panes data so preview reflects restored content', async () => {
+    const { restoreVersion } = await import('../../../../blocks/sheet/utils/utils.js');
+
+    const versionData = [{
+      sheetName: 'data',
+      data: [['Key'], ['fromVersion']],
+      columns: [{ width: '300' }],
+    }];
+
+    window.location.hash = '#/o/r/sheet';
+    window.fetch = async () => new Response('', { status: 200 });
+
+    await restoreVersion(daTitle, daSheet, versionData);
+
+    expect(panes.data).to.exist;
+    expect(JSON.stringify(panes.data)).to.contain('fromVersion');
+  });
+});
+
 describe('Sheets', () => {
   it('Test single sheet getData', async () => {
     const json = `

test/unit/blocks/sheet/utils-utils.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@esm-bundle/chai';
-import { saveSheets, handleSave } from '../../../../blocks/sheet/utils/utils.js';
+import { saveSheets, handleSave, staleCheck } from '../../../../blocks/sheet/utils/utils.js';
 
 describe('sheet/utils utils', () => {
   let savedFetch;
@@ -51,6 +51,47 @@ describe('sheet/utils utils', () => {
     });
   });
 
+  describe('staleCheck after restore', () => {
+    const SOURCE_URL = 'http://example.com/source/org/repo/sheet';
+    const serverJson = { ':type': 'sheet', ':sheetname': 'data', data: [{ key: 'a' }] };
+
+    beforeEach(() => {
+      staleCheck.start({ url: SOURCE_URL, onStale: () => {} });
+      staleCheck.markSynced(serverJson);
+    });
+
+    afterEach(() => {
+      staleCheck.stop();
+    });
+
+    it('markSynced with jspreadsheet-format array causes saveSheets to falsely bail', async () => {
+      // Simulates the old (buggy) restore handler calling markSynced with wrong-format data
+      const jspsheetData = [{ sheetName: 'data', data: [['key'], ['a']], columns: [] }];
+      staleCheck.markSynced(jspsheetData);
+
+      window.location.hash = '#/org/repo/sheet';
+      window.fetch = async () => new Response(JSON.stringify(serverJson), { status: 200 });
+
+      const sheets = [buildSheet('data', [['key'], ['a']])];
+      const result = await saveSheets(sheets);
+      expect(result).to.be.false; // drift falsely detected — save bailed
+    });
+
+    it('Preserves correct baseline so saveSheets proceeds when server is unchanged', async () => {
+      // Restore handler does NOT call markSynced with wrong-format data — baseline stays intact.
+
+      window.location.hash = '#/org/repo/sheet';
+      window.fetch = async (url, opts) => {
+        if (opts?.method === 'PUT') return new Response('', { status: 200 });
+        return new Response(JSON.stringify(serverJson), { status: 200 });
+      };
+
+      const sheets = [buildSheet('data', [['key'], ['a']])];
+      const result = await saveSheets(sheets);
+      expect(result).to.be.true; // no false drift — save went through
+    });
+  });
+
   describe('handleSave', () => {
     it('Skips saving when view is "config"', () => {
       // No fetch call means handleSave didn't dispatch saveSheets