SF-3750 Improve import from spreadsheet error messages (#3769)

Author: pmachapman

src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html

@@ -196,6 +196,10 @@ <h2 mat-dialog-title class="dialog-icon-title">
           <div>{{ t("cannot_convert_offline") }}</div>
         }
 
+        @case ("invalid_spreadsheet") {
+          <div [innerHTML]="i18n.translateAndInsertTags('import_questions_dialog.invalid_spreadsheet')"></div>
+        }
+
         @case ("missing_header_row") {
           <div [innerHTML]="i18n.translateAndInsertTags('import_questions_dialog.missing_header_row')"></div>
         }
@@ -327,7 +331,13 @@ <h2 mat-dialog-title class="dialog-icon-title">
           {{ t("back") }}
         </button>
       }
-      @if (status === "no_questions" || status === "update_transcelerator" || status === "missing_header_row") {
+      @if (
+        status === "no_questions" ||
+        status === "update_transcelerator" ||
+        status === "invalid_spreadsheet" ||
+        status === "missing_header_row" ||
+        status === "offline_conversion"
+      ) {
         <button mat-flat-button mat-dialog-close color="primary">
           {{ t("close") }}
         </button>

src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts

@@ -394,6 +394,33 @@ describe('ImportQuestionsDialogComponent', () => {
     env.click(env.backButton);
   }));
 
+  it('does not import from a CSV file with unknown columns', fakeAsync(() => {
+    const env = new TestEnvironment();
+
+    const genQuestions = [['Genesis 1:1', 'Question for Genesis 1:1']];
+    const matQuestions = Array.from(Array(100), (_, i) => [`MAT 1:${i + 1}`, `Question for Matthew 1:${i + 1}`]);
+
+    env.selectFileWithContents([['Col1', 'Col2'], ...genQuestions, ...matQuestions]);
+
+    expect(env.component.errorState).toBe('missing_header_row');
+  }));
+
+  it('does not import from an empty CSV file', fakeAsync(() => {
+    const env = new TestEnvironment();
+
+    env.selectFileWithContents([]);
+
+    expect(env.component.errorState).toBe('missing_header_row');
+  }));
+
+  it('does not import from an Excel 97-2003 file', fakeAsync(() => {
+    const env = new TestEnvironment();
+
+    env.selectFileWithContents(undefined, 'test.xls');
+
+    expect(env.component.errorState).toBe('invalid_spreadsheet');
+  }));
+
   it('it informs the user about invalid rows in the CSV file and skips them', fakeAsync(() => {
     const env = new TestEnvironment();
 
@@ -942,8 +969,8 @@ class TestEnvironment {
     tick();
   }
 
-  selectFileWithContents(contents: string[][], filename = 'filename.csv'): void {
-    when(mockedCsvService.parse(anything())).thenResolve(contents);
+  selectFileWithContents(contents: string[][] | undefined, filename = 'filename.csv'): void {
+    when(mockedCsvService.parse(anything())).thenResolve(contents ?? []);
     when(mockedCsvService.convert(anything())).thenResolve(contents);
     this.component.fileSelected({ name: filename } as File);
     tick();

src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts

@@ -90,6 +90,7 @@ interface DialogListItem {
 type DialogErrorState =
   | 'update_transcelerator'
   | 'file_import_errors'
+  | 'invalid_spreadsheet'
   | 'missing_header_row'
   | 'offline_conversion'
   | 'paratext_tag_load_error';
@@ -688,13 +689,22 @@ export class ImportQuestionsDialogComponent implements OnDestroy {
         return;
       }
 
-      result = await this.csvService.convert(file);
+      const convertedData: string[][] | undefined = await this.csvService.convert(file);
+      if (convertedData == null) {
+        this.questionSource = 'csv_file';
+        this.errorState = 'invalid_spreadsheet';
+        return;
+      }
+
+      result = convertedData;
     } else {
       result = await this.csvService.parse(file);
     }
 
-    const referenceColumn = result[0].findIndex(value => /^\s*References?\s*$/i.test(value));
-    const questionColumn = result[0].findIndex(value => /^\s*Questions?\s*$/i.test(value));
+    const referenceColumn: number =
+      result.length === 0 ? -1 : result[0].findIndex(value => /^\s*References?\s*$/i.test(value));
+    const questionColumn: number =
+      result.length === 0 ? -1 : result[0].findIndex(value => /^\s*Questions?\s*$/i.test(value));
 
     if (referenceColumn === -1 || questionColumn === -1) {
       this.questionSource = 'csv_file';

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json

@@ -582,6 +582,7 @@
     "import_questions": "Import Questions",
     "import": "Import",
     "invalid_csv_rows_will_be_skipped": "These rows in the CSV file were invalid and will be skipped.",
+    "invalid_spreadsheet": "The file is missing text in the {{ boldStart }}Reference{{ boldEnd }} or {{ boldStart }}Question{{ boldEnd }} columns. Please check and update the file.",
     "learn_more": "Learn more",
     "loading": "Loading...",
     "missing_header_row": "The file you imported needs to have a header row with columns titled {{ boldStart }}Reference{{ boldEnd }} and {{ boldStart }}Question{{ boldEnd }}.",

src/SIL.XForge.Scripture/ClientApp/src/xforge-common/csv-service.service.ts

@@ -1,4 +1,4 @@
-import { HttpClient } from '@angular/common/http';
+import { HttpClient, HttpErrorResponse } from '@angular/common/http';
 import { Injectable } from '@angular/core';
 import Papa from 'papaparse';
 import { lastValueFrom } from 'rxjs';
@@ -10,22 +10,30 @@ import { COMMAND_API_NAMESPACE, PROJECTS_URL } from './url-constants';
 export class CsvService {
   constructor(private readonly http: HttpClient) {}
 
-  async convert(file: File): Promise<string[][]> {
-    const formData = new FormData();
-    formData.append('file', file);
-    const response = await lastValueFrom(
-      this.http.post(`${COMMAND_API_NAMESPACE}/${PROJECTS_URL}/convert-to-csv`, formData, {
-        headers: { Accept: 'text/csv' },
-        observe: 'response',
-        responseType: 'text'
-      })
-    );
+  async convert(file: File): Promise<string[][] | undefined> {
+    try {
+      const formData = new FormData();
+      formData.append('file', file);
+      const response = await lastValueFrom(
+        this.http.post(`${COMMAND_API_NAMESPACE}/${PROJECTS_URL}/convert-to-csv`, formData, {
+          headers: { Accept: 'text/csv' },
+          observe: 'response',
+          responseType: 'text'
+        })
+      );
 
-    if (!response.ok || response.body == null) {
-      throw new Error('Your spreadsheet could not be converted');
-    }
+      if (!response.ok || response.body == null) {
+        throw new Error('Your spreadsheet could not be converted');
+      }
 
-    return this.parse(new File([response.body], 'converted.csv', { type: 'text/csv' }));
+      return this.parse(new File([response.body], 'converted.csv', { type: 'text/csv' }));
+    } catch (e) {
+      if (e instanceof HttpErrorResponse && e.status === 400) {
+        return undefined;
+      }
+
+      throw e;
+    }
   }
 
   parse(file: File): Promise<string[][]> {