From a21270b623b25bf64ebeb486e4949a0812b5116c Mon Sep 17 00:00:00 2001
From: Peter Chapman <peter@conglomo.co.nz>
Date: Mon, 27 Jul 2026 10:06:25 +1200
Subject: [PATCH] SF-3835 Do not allow uploading training data when offline

---
 .../chapter-audio-dialog.component.spec.ts    |   2 +-
 .../chapter-audio-dialog.component.ts         |   2 +-
 .../configure-sources.component.html          |   1 +
 ...ining-data-upload-dialog.component.spec.ts | 121 +++++++++++++++++-
 .../training-data-upload-dialog.component.ts  |  34 +++--
 .../src/assets/i18n/non_checking_en.json      |   3 +-
 .../src/xforge-common/file.service.ts         |  72 ++++++-----
 .../xforge-common/models/project-data-doc.ts  |   2 +-
 8 files changed, 183 insertions(+), 54 deletions(-)

diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.spec.ts
index e750f3614c6..2b29fbd346e 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.spec.ts
+++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.spec.ts
@@ -669,7 +669,7 @@ class TestEnvironment {
       ['1', '0', 'v2']
     ]);
     when(
-      mockedFileService.onlineUploadFileOrFail(
+      mockedFileService.tryOnlineUploadFile(
         FileType.Audio,
         anything(),
         TextAudioDoc.COLLECTION,
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.ts
index a340e61d7df..35546242859 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.ts
+++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/chapter-audio-dialog/chapter-audio-dialog.component.ts
@@ -341,7 +341,7 @@ export class ChapterAudioDialogComponent implements AfterViewInit, OnDestroy {
     if (!this.onlineStatusService.isOnline) return;
 
     this._loadingAudio = true;
-    const audioUrl: string | undefined = await this.fileService.onlineUploadFileOrFail(
+    const audioUrl: string | undefined = await this.fileService.tryOnlineUploadFile(
       FileType.Audio,
       this.data.projectId,
       TextAudioDoc.COLLECTION,
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/configure-sources/configure-sources.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/configure-sources/configure-sources.component.html
index f89ce34e056..88e9ad3ae34 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/configure-sources/configure-sources.component.html
+++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/configure-sources/configure-sources.component.html
@@ -25,6 +25,7 @@ <h2 class="heading-source-side">{{ t("overview_reference") }} {{ preHyphenate(re
           <button
             mat-button
             (click)="trainingSources.push(undefined); $event.preventDefault()"
+            [disabled]="!appOnline"
             class="add-another-project"
           >
             <mat-icon>add</mat-icon> {{ t("add_another_reference_project") }}
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.spec.ts
index 18840b0e8fe..46e4d87cc38 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.spec.ts
+++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.spec.ts
@@ -1,20 +1,22 @@
 import { OverlayContainer } from '@angular/cdk/overlay';
-import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
-import { provideHttpClientTesting } from '@angular/common/http/testing';
+import { HttpErrorResponse, HttpStatusCode } from '@angular/common/http';
 import { NgZone } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { MatDialog, MatDialogRef } from '@angular/material/dialog';
 import { ngfModule } from 'angular-file';
 import { TrainingData } from 'realtime-server/lib/esm/scriptureforge/models/training-data';
-import { anything, mock, when } from 'ts-mockito';
+import { anything, mock, verify, when } from 'ts-mockito';
+import { DialogService } from 'xforge-common/dialog.service';
 import { FileService } from 'xforge-common/file.service';
 import { FileType } from 'xforge-common/models/file-offline-data';
+import { provideTestOnlineStatus } from 'xforge-common/test-online-status-providers';
 import { ChildViewContainerComponent, configureTestingModule, getTestTranslocoModule } from 'xforge-common/test-utils';
 import { UserService } from 'xforge-common/user.service';
 import { TrainingDataDoc } from '../../../core/models/training-data-doc';
 import { TrainingDataFileUpload, TrainingDataUploadDialogComponent } from './training-data-upload-dialog.component';
 import { TrainingDataService } from './training-data.service';
 
+const mockedDialogService = mock(DialogService);
 const mockedFileService = mock(FileService);
 const mockedTrainingDataService = mock(TrainingDataService);
 const mockedUserService = mock(UserService);
@@ -23,8 +25,8 @@ describe('TrainingDataUploadDialogComponent', () => {
   configureTestingModule(() => ({
     imports: [ngfModule, getTestTranslocoModule()],
     providers: [
-      provideHttpClient(withInterceptorsFromDi()),
-      provideHttpClientTesting(),
+      provideTestOnlineStatus(),
+      { provide: DialogService, useMock: mockedDialogService },
       { provide: FileService, useMock: mockedFileService },
       { provide: TrainingDataService, useMock: mockedTrainingDataService },
       { provide: UserService, useMock: mockedUserService }
@@ -40,6 +42,113 @@ describe('TrainingDataUploadDialogComponent', () => {
     overlayContainer.ngOnDestroy();
   });
 
+  it('cannot save if offline', async () => {
+    const env = new TestEnvironment();
+    when(
+      mockedFileService.onlineUploadFile(
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything()
+      )
+    ).thenThrow(new HttpErrorResponse({ status: 0 }));
+    let result: TrainingData = { dataId: '' } as TrainingData;
+    env.dialogRef.afterClosed().subscribe((_result: TrainingData) => {
+      result = _result;
+    });
+    env.component.updateTrainingData(env.trainingDataFile);
+    await env.component.save();
+    await env.wait();
+
+    verify(
+      mockedFileService.onlineUploadFile(
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything()
+      )
+    ).once();
+    expect(result.dataId).toEqual('');
+  });
+
+  it('should show an error message if the file is invalid', async () => {
+    const env = new TestEnvironment();
+    when(
+      mockedFileService.onlineUploadFile(
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything()
+      )
+    ).thenThrow(new HttpErrorResponse({ status: HttpStatusCode.BadRequest }));
+    let result: TrainingData = { dataId: '' } as TrainingData;
+    env.dialogRef.afterClosed().subscribe((_result: TrainingData) => {
+      result = _result;
+    });
+    env.component.updateTrainingData(env.trainingDataFile);
+    await env.component.save();
+    await env.wait();
+
+    verify(
+      mockedFileService.onlineUploadFile(
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything()
+      )
+    ).once();
+    verify(mockedDialogService.message(anything())).once();
+    expect(result.dataId).toEqual('');
+  });
+
+  it('should show an error message for all other errors when uploading', async () => {
+    const env = new TestEnvironment();
+    when(
+      mockedFileService.onlineUploadFile(
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything()
+      )
+    ).thenThrow(new HttpErrorResponse({ status: HttpStatusCode.NotFound }));
+    let result: TrainingData = { dataId: '' } as TrainingData;
+    env.dialogRef.afterClosed().subscribe((_result: TrainingData) => {
+      result = _result;
+    });
+    env.component.updateTrainingData(env.trainingDataFile);
+    await env.component.save();
+    await env.wait();
+
+    verify(
+      mockedFileService.onlineUploadFile(
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything(),
+        anything()
+      )
+    ).once();
+    verify(mockedDialogService.message(anything())).once();
+    expect(result.dataId).toEqual('');
+  });
+
   it('should upload training data and return the object on save', async () => {
     const env = new TestEnvironment();
     let result: TrainingData = { dataId: '' } as TrainingData;
@@ -111,7 +220,7 @@ class TestEnvironment {
 
   constructor(availableTrainingData: TrainingData[] = []) {
     when(
-      mockedFileService.onlineUploadFileOrFail(
+      mockedFileService.onlineUploadFile(
         FileType.TrainingData,
         anything(),
         TrainingDataDoc.COLLECTION,
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.ts
index cac7d4ba3cc..60b5a5a23f3 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.ts
+++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.ts
@@ -1,4 +1,5 @@
 import { NgClass } from '@angular/common';
+import { HttpErrorResponse } from '@angular/common/http';
 import { AfterViewInit, Component, ElementRef, Inject, ViewChild } from '@angular/core';
 import { MatButton } from '@angular/material/button';
 import { MatCheckbox } from '@angular/material/checkbox';
@@ -112,26 +113,33 @@ export class TrainingDataUploadDialogComponent implements AfterViewInit {
   }
 
   async save(): Promise<void> {
-    // We cannot save a file if it has not been uploaded, or if offline
+    // We cannot save a file if it has not been uploaded
     if (!this.hasBeenUploaded) {
       return;
     }
 
     this._isUploading = true;
     const dataId: string = objectId();
-    const fileUrl: string | undefined = await this.fileService.onlineUploadFileOrFail(
-      FileType.TrainingData,
-      this.data.projectId,
-      TrainingDataDoc.COLLECTION,
-      dataId,
-      this.trainingDataFile!.blob!,
-      this.trainingDataFile!.fileName!,
-      true
-    );
-    this._isUploading = false;
-    if (fileUrl == null) {
-      void this.dialogService.message('training_data_upload_dialog.upload_failed');
+    let fileUrl: string;
+    try {
+      fileUrl = await this.fileService.onlineUploadFile(
+        FileType.TrainingData,
+        this.data.projectId,
+        TrainingDataDoc.COLLECTION,
+        dataId,
+        this.trainingDataFile!.blob!,
+        this.trainingDataFile!.fileName!,
+        true
+      );
+    } catch (e) {
+      if (e instanceof HttpErrorResponse && e.status === 400) {
+        void this.dialogService.message('training_data_upload_dialog.invalid_format');
+      } else {
+        void this.dialogService.message('training_data_upload_dialog.upload_failed');
+      }
       return;
+    } finally {
+      this._isUploading = false;
     }
 
     // Create the training_data record
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json
index 5dbceaea35b..32c0dc9e0c2 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json
+++ b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json
@@ -971,11 +971,12 @@
     "drag_and_drop_files": "Drag and drop training data file here",
     "drag_and_drop_or_browse": "OR",
     "filename_already_exists": "A file already exists with this name.",
+    "invalid_format": "Your file needs to have two columns, and every row must have a value in both columns. Please fill in any empty cells and try again.",
     "skip_first_row": "Skip first row of data file",
     "no_training_data_file_uploaded": "No training data file uploaded",
     "no_upload_offline": "Training data cannot be uploaded without connecting to the internet.",
     "save": "Save",
-    "upload_failed": "Your file needs to have two columns, and every row must have a value in both columns. Please fill in any empty cells and try again.",
+    "upload_failed": "The training data file could not be uploaded. Please try again or use a different file.",
     "upload_training_data": "Upload Training Data",
     "uploading": "Uploading..."
   },
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/file.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/file.service.ts
index f053b2bcc9c..07c1b473778 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/file.service.ts
+++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/file.service.ts
@@ -95,7 +95,7 @@ export class FileService {
     filename: string,
     alwaysKeepFileOffline: boolean
   ): Promise<string | undefined> {
-    const onlineUrl: string | undefined = await this.onlineUploadFileOrFail(
+    const onlineUrl: string | undefined = await this.tryOnlineUploadFile(
       fileType,
       projectId,
       dataCollection,
@@ -119,11 +119,33 @@ export class FileService {
     }
   }
 
-  /**
-   * Specifically upload a file only when online and cache the file if specified.
-   * @returns The audio url if the upload was successful, or undefined if otherwise.
-   */
-  async onlineUploadFileOrFail(
+  async onlineUploadFile(
+    fileType: FileType,
+    projectId: string,
+    dataCollection: string,
+    dataId: string,
+    blob: Blob,
+    filename: string,
+    alwaysKeepFileOffline: boolean
+  ): Promise<string> {
+    const formData = new FormData();
+    formData.append('projectId', projectId);
+    formData.append('dataId', dataId);
+    formData.append('file', new File([blob], filename));
+    const response = await lastValueFrom(
+      this.http.post<HttpResponse<string>>(`${COMMAND_API_NAMESPACE}/${PROJECTS_URL}/${fileType}`, formData, {
+        headers: { Accept: 'application/json' },
+        observe: 'response'
+      })
+    );
+    const onlineUrl = response.headers.get('Location')!.replace(`${environment.assets}${fileType}/`, '/');
+    if (alwaysKeepFileOffline) {
+      await this.findOrUpdateCache(fileType, dataCollection, dataId, onlineUrl);
+    }
+    return onlineUrl;
+  }
+
+  async tryOnlineUploadFile(
     fileType: FileType,
     projectId: string,
     dataCollection: string,
@@ -132,17 +154,20 @@ export class FileService {
     filename: string,
     alwaysKeepFileOffline: boolean
   ): Promise<string | undefined> {
-    if (this.onlineStatusService.isOnline) {
-      // Try and upload it online
-      try {
-        const onlineUrl = await this.onlineUploadFile(fileType, projectId, dataId, new File([blob], filename));
-        if (alwaysKeepFileOffline) {
-          await this.findOrUpdateCache(fileType, dataCollection, dataId, onlineUrl);
-        }
-        return onlineUrl;
-      } catch {}
+    try {
+      if (!this.onlineStatusService.isOnline) return undefined;
+      return await this.onlineUploadFile(
+        fileType,
+        projectId,
+        dataCollection,
+        dataId,
+        blob,
+        filename,
+        alwaysKeepFileOffline
+      );
+    } catch {
+      return undefined;
     }
-    return undefined;
   }
 
   /**
@@ -248,21 +273,6 @@ export class FileService {
     return this.commandService.onlineInvoke(PROJECTS_URL, method, { projectId, ownerId, dataId });
   }
 
-  private async onlineUploadFile(fileType: FileType, projectId: string, dataId: string, file: File): Promise<string> {
-    const formData = new FormData();
-    formData.append('projectId', projectId);
-    formData.append('dataId', dataId);
-    formData.append('file', file);
-    const response = await lastValueFrom(
-      this.http.post<HttpResponse<string>>(`${COMMAND_API_NAMESPACE}/${PROJECTS_URL}/${fileType}`, formData, {
-        headers: { Accept: 'application/json' },
-        observe: 'response'
-      })
-    );
-    const path = response.headers.get('Location')!;
-    return path.replace(`${environment.assets}${fileType}/`, '/');
-  }
-
   private async onlineCacheFile(
     fileType: FileType,
     source: string,
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/models/project-data-doc.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/models/project-data-doc.ts
index e36a221f6da..6f1f1e49111 100644
--- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/models/project-data-doc.ts
+++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/models/project-data-doc.ts
@@ -44,7 +44,7 @@ export abstract class ProjectDataDoc<T extends ProjectData = ProjectData> extend
       );
     }
 
-    return await this.realtimeService.fileService.onlineUploadFileOrFail(
+    return await this.realtimeService.fileService.tryOnlineUploadFile(
       fileType,
       this.data.projectRef,
       this.collection,