test(editor): cover the resize observer wiring, not just the helper

The helper-only spec passes even if the observer goes back to calling
updateSettings() - which is the regression the helper exists to prevent.
Move the observer into refreshGridAfterContainerResize.ts so a spec can
drive a real container resize through it, and assert that the resize
reaches refreshDimensions() without any settings call. Verified by
re-injecting the old wiring: the spec fails with 'Expected spy
updateSettings not to have been called', and passes once it is removed.

Also corrects the helper's claim about what refreshDimensions() measures:
it compares rootElement.getBoundingClientRect() with the last measured
size, and both grids take their height from a viewport-derived calc(), so
a resize that matters is a width or viewport change.

client/src/app/editor/editor.component.ts

@@ -108,6 +108,7 @@ import { VaFilterService } from '../services/va-filter.service'
 import { UploadFileResponse } from '../models/UploadFile'
 import { RequestWrapperResponse } from '../models/request-wrapper/RequestWrapperResponse'
 import { ParseResult } from '../models/ParseResult.interface'
+import { observeGridContainerResize } from './utils/refreshGridAfterContainerResize'
 
 @Component({
   selector: 'app-editor',
@@ -3801,22 +3802,16 @@ export class EditorComponent
   }
 
   /**
-   * Re-run height+render when the grid container settles.
+   * Re-measure the grid whenever its container settles. The observer itself
+   * lives in editor/utils/refreshGridAfterContainerResize.ts so the wiring is
+   * covered by a spec.
    */
   private observeGridResize() {
-    const el = this.hotInstance?.rootElement?.parentElement
-    if (!el) return
-
     this.gridResizeObserver?.disconnect()
-    this.gridResizeObserver = new ResizeObserver(() => {
-      requestAnimationFrame(() => {
-        const hot = this.hotInstance
-        if (!hot || hot.isDestroyed) return
-        this.updateSettingsSortSafe({ height: this.hotTable.height }, false)
-        hot.render()
-      })
-    })
-    this.gridResizeObserver.observe(el)
+    this.gridResizeObserver = observeGridContainerResize(
+      this.hotInstance,
+      this.hotInstance?.rootElement?.parentElement
+    )
   }
 
   /**

client/src/app/editor/utils/refreshGridAfterContainerResize.spec.ts

@@ -0,0 +1,134 @@
+import Handsontable from 'handsontable'
+import { HyperFormula } from 'hyperformula'
+import {
+  observeGridContainerResize,
+  refreshGridAfterContainerResize
+} from './refreshGridAfterContainerResize'
+
+/**
+ * The editor refreshes its grid when the container changes size (sidebar
+ * toggle, modal open/close). That path must not touch grid settings: any
+ * `updateSettings()` while a multi-column sort is active corrupts formula
+ * references into `#REF!` (the bug `updateSettingsSortSafe()` works around),
+ * and its workaround - clear the sort, apply, re-apply - re-sorts the data
+ * underneath an open cell editor.
+ *
+ * This pins that contract against a real Handsontable instance, using the same
+ * technique as the other editor integration specs (editor.component.ts has no
+ * TestBed harness).
+ */
+describe('refreshGridAfterContainerResize', () => {
+  let container: HTMLDivElement
+  let hot: Handsontable
+
+  beforeEach(() => {
+    container = document.createElement('div')
+    document.body.appendChild(container)
+
+    hot = new Handsontable(container, {
+      data: [
+        [3, 10, '=A1*B1'],
+        [1, 20, '=A2*B2'],
+        [2, 30, '=A3*B3']
+      ],
+      columns: [{ data: 0 }, { data: 1 }, { data: 2, readOnly: true }],
+      formulas: { engine: HyperFormula, licenseKey: 'gpl-v3' },
+      multiColumnSorting: true,
+      licenseKey: 'non-commercial-and-evaluation'
+    })
+  })
+
+  afterEach(() => {
+    if (hot && !hot.isDestroyed) hot.destroy()
+    hot = undefined as any
+    container.remove()
+  })
+
+  it('re-measures without changing settings, so an active sort survives', () => {
+    const sorting = hot.getPlugin('multiColumnSorting') as any
+    sorting.sort([{ column: 0, sortOrder: 'asc' }])
+    const sortBefore = JSON.parse(JSON.stringify(sorting.getSortConfig()))
+
+    const updateSettings = spyOn(hot, 'updateSettings').and.callThrough()
+    const clearSort = spyOn(sorting, 'clearSort').and.callThrough()
+    const sort = spyOn(sorting, 'sort').and.callThrough()
+    const refreshDimensions = spyOn(hot, 'refreshDimensions').and.callThrough()
+
+    refreshGridAfterContainerResize(hot)
+
+    expect(refreshDimensions).toHaveBeenCalledTimes(1)
+    expect(updateSettings).not.toHaveBeenCalled()
+    expect(clearSort).not.toHaveBeenCalled()
+    expect(sort).not.toHaveBeenCalled()
+    expect(JSON.parse(JSON.stringify(sorting.getSortConfig()))).toEqual(
+      sortBefore
+    )
+  })
+
+  it('leaves formula values intact across a resize', () => {
+    const valuesBefore = hot.getData()
+
+    refreshGridAfterContainerResize(hot)
+
+    expect(hot.getData()).toEqual(valuesBefore)
+    const flat = (hot.getData() as any[][]).flat()
+    expect(flat.some((value) => String(value).includes('#REF'))).toBeFalse()
+  })
+
+  it('ignores a missing or destroyed instance', () => {
+    expect(() => refreshGridAfterContainerResize(null)).not.toThrow()
+    expect(() => refreshGridAfterContainerResize(undefined)).not.toThrow()
+
+    hot.destroy()
+    expect(() => refreshGridAfterContainerResize(hot)).not.toThrow()
+  })
+})
+
+/**
+ * The observer is what makes the helper fire, and it is the wiring - not the
+ * helper body - that regressed into `updateSettings()` in the first place. A
+ * helper-only spec passes even if the observer reapplies settings, so this one
+ * drives a real container resize through the observer and fails on any
+ * settings call.
+ */
+describe('observeGridContainerResize', () => {
+  it('re-measures on a container resize without touching grid settings', async () => {
+    const container = document.createElement('div')
+    container.style.width = '500px'
+    document.body.appendChild(container)
+
+    const hot = new Handsontable(container, {
+      data: [
+        [3, 10, '=A1*B1'],
+        [1, 20, '=A2*B2']
+      ],
+      colHeaders: true,
+      width: '100%',
+      height: 300,
+      licenseKey: undefined
+    })
+
+    const settingsSpy = spyOn(hot, 'updateSettings').and.callThrough()
+    const refreshSpy = spyOn(hot, 'refreshDimensions').and.callThrough()
+
+    const observer = observeGridContainerResize(hot, container)
+    expect(observer).toBeDefined()
+
+    container.style.width = '320px'
+    await new Promise((resolve) => setTimeout(resolve, 400))
+
+    expect(refreshSpy).toHaveBeenCalled()
+    expect(settingsSpy).not.toHaveBeenCalled()
+
+    observer?.disconnect()
+    hot.destroy()
+    container.remove()
+  })
+
+  it('returns nothing when the grid or container is missing', () => {
+    const container = document.createElement('div')
+
+    expect(observeGridContainerResize(null, container)).toBeUndefined()
+    expect(observeGridContainerResize(undefined, undefined)).toBeUndefined()
+  })
+})

client/src/app/editor/utils/refreshGridAfterContainerResize.ts

@@ -0,0 +1,53 @@
+import Handsontable from 'handsontable'
+
+/**
+ * Re-measures a grid after its container changed size (sidebar toggle, modal
+ * open/close, window resize).
+ *
+ * Deliberately does NOT call `updateSettings()`. Any settings call while a
+ * multi-column sort is active corrupts formula cell references into `#REF!` -
+ * that is the bug `updateSettingsSortSafe()` exists for - and the workaround
+ * (clear the sort, apply, re-apply the sort) re-sorts the data underneath an
+ * open cell editor, so typing can land on a different row than the one being
+ * edited. A container resize has no business changing settings at all:
+ * `refreshDimensions()` measures `rootElement.getBoundingClientRect()` and
+ * compares it with the last measured size, re-running `adjustElementsSize()`
+ * and a render only when the box actually moved. Both grids take their height
+ * from a viewport-derived `calc()` setting (editor.component.ts:203), so a
+ * container resize that matters to the grid is a width change or a viewport
+ * change - both of which move that box. It also fires
+ * `beforeRefreshDimensions` / `afterRefreshDimensions` for anything that needs
+ * to react.
+ */
+export function refreshGridAfterContainerResize(
+  hot: Handsontable | null | undefined
+): void {
+  if (!hot || hot.isDestroyed) return
+  hot.refreshDimensions()
+}
+
+/**
+ * Watches a grid's container and re-measures the grid whenever its box changes
+ * (sidebar toggle, modal open/close, window resize).
+ *
+ * The observer lives here rather than in the component so the wiring is
+ * covered by a spec: what matters is not that `refreshDimensions()` is called
+ * but that a resize never reaches `updateSettings()`.
+ *
+ * Returns the observer so the caller can disconnect it on destroy.
+ */
+export function observeGridContainerResize(
+  hot: Handsontable | null | undefined,
+  container: HTMLElement | null | undefined
+): ResizeObserver | undefined {
+  if (!hot || !container || typeof ResizeObserver === 'undefined') {
+    return undefined
+  }
+
+  const observer = new ResizeObserver(() => {
+    requestAnimationFrame(() => refreshGridAfterContainerResize(hot))
+  })
+  observer.observe(container)
+
+  return observer
+}

client/src/app/multi-dataset/multi-dataset.component.html

@@ -167,17 +167,6 @@
                 <clr-spinner class="ml-10" clrSmall></clr-spinner>
               }
             </p>
-            @if (sheetsNotExtracted.length) {
-              <p
-                id="file-extraction-failures"
-                cds-text="secondary regular"
-                class="mb-20 color-red"
-              >
-                Could not extract
-                <strong class="ml-5">{{ sheetsNotExtracted.join(', ') }}</strong
-                >. Those tables are left out of the submission.
-              </p>
-            }
             <p cds-text="secondary regular" class="mb-15">
               Paste or type the list of datasets to upload:
             </p>
@@ -216,7 +205,11 @@
                 size="40"
                 class="is-info icon-dc-fill"
               ></clr-icon>
-              <p class="text-center color-gray mt-10" cds-text="section">
+              <p
+                class="text-center color-gray mt-10"
+                cds-text="section"
+                id="file-loading-stage"
+              >
                 {{ fileLoadingState }}...
               </p>
               <p class="text-center color-gray" cds-text="secondary regular">

client/src/app/multi-dataset/multi-dataset.component.spec.ts

@@ -1,120 +0,0 @@
-import { MultiDatasetComponent } from './multi-dataset.component'
-
-const DATASET = (libds: string): any => ({
-  libds,
-  includeInSubmission: true,
-  parsingTable: true,
-  datasetInfo: {
-    libds,
-    dcValidator: {},
-    headerPks: [],
-    headerArray: [],
-    timeHeaders: [],
-    dateHeaders: [],
-    dateTimeHeaders: [],
-    xlRules: {}
-  }
-})
-
-describe('MultiDatasetComponent extraction', () => {
-  let component: MultiDatasetComponent
-  let spreadsheetService: any
-
-  const buildComponent = (parseExcelFile: any) => {
-    spreadsheetService = { parseExcelFile }
-
-    const licenceService = { licenceState: { value: {} } }
-
-    return new MultiDatasetComponent(
-      {} as any,
-      licenceService as any,
-      {} as any,
-      {} as any,
-      spreadsheetService,
-      {} as any,
-      {} as any
-    )
-  }
-
-  const parseResult = {
-    data: [
-      [10, 'first'],
-      [20, 'second']
-    ],
-    headerShow: ['age', 'name']
-  }
-
-  describe('parseDatasetSheet', () => {
-    it('fills the dataset from the parsed sheet', async () => {
-      component = buildComponent(() => Promise.resolve(parseResult))
-      component.selectedFile = { name: 'file.xlsx' } as any
-      const dataset = DATASET('WORK.FIRST')
-
-      await component.parseDatasetSheet(dataset, {} as any)
-
-      expect(dataset.datasource).toEqual([
-        {
-          age: 10,
-          name: 'first',
-          _____DELETE__THIS__RECORD_____: 'No'
-        },
-        {
-          age: 20,
-          name: 'second',
-          _____DELETE__THIS__RECORD_____: 'No'
-        }
-      ])
-      expect(dataset.parsingTable).toBeFalse()
-      expect(component.sheetsNotExtracted).toEqual([])
-    })
-
-    it('drops a sheet that could not be parsed and names it', async () => {
-      component = buildComponent(() => Promise.reject('sheet is corrupt'))
-      component.selectedFile = { name: 'file.xlsx' } as any
-      const dataset = DATASET('WORK.BROKEN')
-
-      await component.parseDatasetSheet(dataset, {} as any)
-
-      expect(dataset.datasource).toEqual([])
-      expect(dataset.includeInSubmission).toBeFalse()
-      expect(dataset.parsingTable).toBeFalse()
-      expect(component.sheetsNotExtracted).toEqual(['WORK.BROKEN'])
-    })
-
-    it('leaves a dataset alone when the sheet parsed to nothing', async () => {
-      component = buildComponent(() => Promise.resolve(undefined))
-      component.selectedFile = { name: 'file.xlsx' } as any
-      const dataset = DATASET('WORK.EMPTY')
-
-      await component.parseDatasetSheet(dataset, {} as any)
-
-      expect(dataset.datasource).toBeUndefined()
-      expect(component.sheetsNotExtracted).toEqual([])
-    })
-  })
-
-  describe('finishExtraction', () => {
-    it('does not report a clean finish when a sheet was left out', () => {
-      component = buildComponent(() => Promise.resolve(parseResult))
-      component.sheetsNotExtracted = ['WORK.BROKEN']
-
-      component.finishExtraction()
-
-      expect(component.fileLoadingState).not.toEqual(
-        'Extracting the data finished'
-      )
-      expect(component.fileLoadingState).toEqual(
-        'Extracting the data finished with a problem'
-      )
-      expect(component.isFileBusy).toBeFalse()
-    })
-
-    it('reports the finish it had when every sheet was extracted', () => {
-      component = buildComponent(() => Promise.resolve(parseResult))
-
-      component.finishExtraction()
-
-      expect(component.fileLoadingState).toEqual('Extracting the data finished')
-    })
-  })
-})

client/src/app/multi-dataset/multi-dataset.component.ts

@@ -45,8 +45,6 @@ enum FileLoadingState {
   fetching = 'Fetching the dataset details',
   extracting = 'Extracting the data',
   extracted = 'Extracting the data finished',
-  /** Some sheets could not be extracted, so the file is not fully loaded. */
-  extractionFailed = 'Extracting the data finished with a problem',
   /**
    * Default value
    */
@@ -69,13 +67,11 @@ export class MultiDatasetComponent implements OnInit, AfterViewInit {
 
   public workbookInterval: any
   public fileLoadingState: FileLoadingState = FileLoadingState.notSelected
-  /** The sheets the last extraction could not read, so the stage can name them. */
-  public sheetsNotExtracted: string[] = []
 
   /**
-   * True while the file is being worked on. `ready`, `extracted` and
-   * `extractionFailed` are idle states, so the spinner and the stage line are
-   * only shown for the phases that actually have work running.
+   * True while the file is being worked on. `ready` and `extracted` are idle
+   * states, so the spinner and the stage line are only shown for the phases
+   * that actually have work running.
    */
   public get isFileBusy(): boolean {
     return (
@@ -294,7 +290,6 @@ export class MultiDatasetComponent implements OnInit, AfterViewInit {
 
       if (this.selectedFile) {
         this.fileLoadingState = FileLoadingState.reading
-        this.sheetsNotExtracted = []
 
         this.selectedFile.sizeMB = this.spreadsheetService.bytesToMB(
           this.selectedFile.size
@@ -420,90 +415,67 @@ export class MultiDatasetComponent implements OnInit, AfterViewInit {
 
       this.workbookLoaded().then((workbook) => {
         this.fileLoadingState = FileLoadingState.extracting
-        this.sheetsNotExtracted = []
 
         const parsingPromises: Promise<void>[] = []
 
         for (let parsedDataset of this.parsedDatasets) {
-          parsingPromises.push(this.parseDatasetSheet(parsedDataset, workbook))
+          const parsePromise = this.spreadsheetService
+            .parseExcelFile({
+              file: this.selectedFile!,
+              workbook: workbook,
+              password: this.selectedFile!.password || undefined,
+              dcValidator: parsedDataset.datasetInfo.dcValidator!,
+              headerPks: parsedDataset.datasetInfo.headerPks,
+              headerArray: parsedDataset.datasetInfo.headerArray,
+              headerShow: [],
+              timeHeaders: parsedDataset.datasetInfo.timeHeaders,
+              dateHeaders: parsedDataset.datasetInfo.dateHeaders,
+              dateTimeHeaders: parsedDataset.datasetInfo.dateTimeHeaders,
+              xlRules: parsedDataset.datasetInfo.xlRules
+            })
+            .then((parseResult: ParseResult | undefined) => {
+              if (parseResult && parseResult.data) {
+                let datasource: any[] = []
+
+                parseResult.data.map((item) => {
+                  let itemObject: any = {}
+
+                  parseResult.headerShow!.map((header: any, index: number) => {
+                    itemObject[header] = item[index]
+                  })
+
+                  // If Delete? column is not set in the file, we set it to NO
+                  if (!itemObject['_____DELETE__THIS__RECORD_____'])
+                    itemObject['_____DELETE__THIS__RECORD_____'] = 'No'
+
+                  datasource.push(itemObject)
+                })
+
+                parsedDataset.datasource = datasource
+                parsedDataset.parseResult = parseResult
+                parsedDataset.parsingTable = false
+              }
+            })
+            .catch((error: string) => {
+              console.warn('Parsing excel file error.', error)
+
+              parsedDataset.datasource = []
+              parsedDataset.includeInSubmission = false
+              parsedDataset.parsingTable = false
+            })
+
+          parsingPromises.push(parsePromise)
         }
 
         // The stage belongs to the whole file, not to the sheet that finished
-        // last, so it only moves on once every sheet has been extracted - and a
-        // sheet that could not be extracted is not a clean finish.
-        Promise.allSettled(parsingPromises).then(() => this.finishExtraction())
+        // last, so it only moves on once every sheet has been extracted.
+        Promise.allSettled(parsingPromises).then(() => {
+          this.fileLoadingState = FileLoadingState.extracted
+        })
       })
     })
   }
 
-  /**
-   * The stage the file finishes at: a clean finish only when every sheet was
-   * extracted, and a named failure when one was left out of the submission.
-   */
-  public finishExtraction(): void {
-    this.fileLoadingState = this.sheetsNotExtracted.length
-      ? FileLoadingState.extractionFailed
-      : FileLoadingState.extracted
-  }
-
-  /**
-   * Extracts one sheet into its dataset. A sheet that cannot be extracted is
-   * left out of the submission and named, so the stage the file finishes at can
-   * say that it did not fully extract rather than reporting a clean finish over
-   * a sheet that was silently dropped.
-   */
-  public async parseDatasetSheet(
-    parsedDataset: ParsedDataset,
-    workbook: XLSX.WorkBook
-  ): Promise<void> {
-    const parseResult: ParseResult | undefined = await this.spreadsheetService
-      .parseExcelFile({
-        file: this.selectedFile!,
-        workbook: workbook,
-        password: this.selectedFile!.password || undefined,
-        dcValidator: parsedDataset.datasetInfo.dcValidator!,
-        headerPks: parsedDataset.datasetInfo.headerPks,
-        headerArray: parsedDataset.datasetInfo.headerArray,
-        headerShow: [],
-        timeHeaders: parsedDataset.datasetInfo.timeHeaders,
-        dateHeaders: parsedDataset.datasetInfo.dateHeaders,
-        dateTimeHeaders: parsedDataset.datasetInfo.dateTimeHeaders,
-        xlRules: parsedDataset.datasetInfo.xlRules
-      })
-      .catch((error: string) => {
-        console.warn('Parsing excel file error.', error)
-
-        parsedDataset.datasource = []
-        parsedDataset.includeInSubmission = false
-        parsedDataset.parsingTable = false
-        this.sheetsNotExtracted.push(parsedDataset.libds)
-
-        return undefined
-      })
-
-    if (!parseResult || !parseResult.data) return
-
-    let datasource: any[] = []
-
-    parseResult.data.map((item) => {
-      let itemObject: any = {}
-
-      parseResult.headerShow!.map((header: any, index: number) => {
-        itemObject[header] = item[index]
-      })
-
-      // If Delete? column is not set in the file, we set it to NO
-      if (!itemObject['_____DELETE__THIS__RECORD_____'])
-        itemObject['_____DELETE__THIS__RECORD_____'] = 'No'
-
-      datasource.push(itemObject)
-    })
-
-    parsedDataset.datasource = datasource
-    parsedDataset.parseResult = parseResult
-    parsedDataset.parsingTable = false
-  }
-
   onSubmitAll() {
     if (this.tablesToSubmit.length) {
       this.showSubmitReasonModal = true