fix(multi-dataset): a sheet that failed to extract is not a clean finish

The stage that reports what the file is doing is meant to belong to the whole
file, and it finished at "Extracting the data finished" whatever happened. A
sheet whose parse rejected had already blanked its datasource and dropped
itself from the submission in the per-sheet catch, so the user was told the
file had extracted while a table was silently missing from the submission.

`allSettled` was already the right primitive, but its results said nothing:
the catch swallowed the error, so every promise fulfilled. The catch now names
the sheet on the component, the stage ends at a new `extractionFailed` state
when anything was left out, and the file pane lists the sheets that could not
be extracted next to the stage line.

Two smaller things in the same pane: the busy overlay carried a second
`id="file-loading-stage"`, a duplicate of the stage line above it and resolved
by document order, and the `isFileBusy` comment now names the new idle state.

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

@@ -167,6 +167,17 @@
                 <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>
@@ -205,11 +216,7 @@
                 size="40"
                 class="is-info icon-dc-fill"
               ></clr-icon>
-              <p
-                class="text-center color-gray mt-10"
-                cds-text="section"
-                id="file-loading-stage"
-              >
+              <p class="text-center color-gray mt-10" cds-text="section">
                 {{ fileLoadingState }}...
               </p>
               <p class="text-center color-gray" cds-text="secondary regular">

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

@@ -0,0 +1,120 @@
+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,6 +45,8 @@ 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
    */
@@ -67,11 +69,13 @@ 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` and `extracted` 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`, `extracted` and
+   * `extractionFailed` 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 (
@@ -290,6 +294,7 @@ export class MultiDatasetComponent implements OnInit, AfterViewInit {
 
       if (this.selectedFile) {
         this.fileLoadingState = FileLoadingState.reading
+        this.sheetsNotExtracted = []
 
         this.selectedFile.sizeMB = this.spreadsheetService.bytesToMB(
           this.selectedFile.size
@@ -415,67 +420,90 @@ 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) {
-          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)
+          parsingPromises.push(this.parseDatasetSheet(parsedDataset, workbook))
         }
 
         // 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.
-        Promise.allSettled(parsingPromises).then(() => {
-          this.fileLoadingState = FileLoadingState.extracted
-        })
+        // 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())
       })
     })
   }
 
+  /**
+   * 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