From 62cc5169df043e906f0d875f240015b330ed7e05 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Thu, 23 Oct 2025 10:28:22 -0500 Subject: [PATCH 1/4] SF-3610 Adding a latest draft check to formatting button Before, we we only checked if the latest draft contained that book, so older drafts still had the formatting button enabled. Now, we confirm the button is only enabled when the latest revision is selected. --- .../editor-draft/editor-draft.component.html | 6 +- .../editor-draft.component.spec.ts | 83 +++++++++++++++++++ .../editor-draft/editor-draft.component.ts | 8 ++ 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html index bc3449cf12..955bb67dd1 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html @@ -45,10 +45,10 @@
@if (showDraftOptionsButton$ | async) { - diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts index 21a50dab81..1dbe1566ea 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts @@ -559,6 +559,89 @@ describe('EditorDraftComponent', () => { })); }); + describe('canConfigureFormatting', () => { + beforeEach(() => { + when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); + spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops)); + when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftDelta.ops!)); + when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!); + }); + + it('should be true when latest build has draft and selected revision is latest', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile({ + texts: [ + { + bookNum: 1, + chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }] + } + ] + }) + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + expect(component.isSelectedDraftLatest).toBe(true); + expect(component.canConfigureFormatting).toBe(true); + flush(); + })); + + it('should be false when selected revision is not the latest', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile({ + texts: [ + { + bookNum: 1, + chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }] + } + ] + }) + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + expect(component.canConfigureFormatting).toBe(true); + + // Select earlier revision + component.onSelectionChanged({ value: draftHistory[1] } as MatSelectChange); + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + expect(component.isSelectedDraftLatest).toBe(false); + expect(component.canConfigureFormatting).toBe(false); + flush(); + })); + + it('should be false when latest build does not have a draft', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile({ + texts: [ + { + bookNum: 1, + chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: false }] + } + ] + }) + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false)); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + expect(component.isSelectedDraftLatest).toBe(true); + expect(component.canConfigureFormatting).toBe(false); + flush(); + })); + }); + describe('getLocalizedBookChapter', () => { it('should return an empty string if bookNum or chapter is undefined', () => { component.bookNum = undefined; diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts index 4b95d52362..9dc8f7c9a3 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts @@ -150,6 +150,10 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { return this.draftHandlingService.canApplyDraft(this.targetProject, this.bookNum, this.chapter, this.draftDelta.ops); } + get canConfigureFormatting(): boolean { + return this.doesLatestHaveDraft && this.isSelectedDraftLatest; + } + get doesLatestHaveDraft(): boolean { return ( this.targetProject?.texts.find(t => t.bookNum === this.bookNum)?.chapters.find(c => c.number === this.chapter) @@ -157,6 +161,10 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { ); } + get isSelectedDraftLatest(): boolean { + return this.selectedRevision?.timestamp === this._draftRevisions[0].timestamp; + } + set draftRevisions(value: Revision[]) { this._draftRevisions = value; } From a3f22b0ded608ddeabb24db4ec0a68be1d7c4ed1 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Mon, 27 Oct 2025 15:26:26 -0500 Subject: [PATCH 2/4] Added requirement that the latest build be completed This adds a new backend method to return the latest build. The front end verifies that the latest build is completed and not any other state. This was changed to account for canceled builds. In order to communicate with Serval, the latest build has to be complete (and contain the draft being used). --- .../draft-generation.service.spec.ts | 44 ++++++++++++ .../draft-generation.service.ts | 27 ++++++++ .../editor-draft.component.spec.ts | 45 ++++++++++++ .../editor-draft/editor-draft.component.ts | 45 ++++++++++-- .../translate/editor/editor.component.spec.ts | 1 + .../Controllers/MachineApiController.cs | 48 +++++++++++++ src/SIL.XForge.Scripture/Models/MachineApi.cs | 2 + .../Services/IMachineApiService.cs | 6 ++ .../Services/MachineApiService.cs | 68 +++++++++++++++++++ .../Controllers/MachineApiControllerTests.cs | 48 +++++++++++++ .../Services/MachineApiServiceTests.cs | 59 ++++++++++++++++ 11 files changed, 388 insertions(+), 5 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts index 83b52b4b29..5eda633964 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts @@ -183,6 +183,50 @@ describe('DraftGenerationService', () => { })); }); + describe('getLastPreTranslationBuild', () => { + it('should get last pre-translation build and return an observable of BuildDto', fakeAsync(() => { + // SUT + service.getLastPreTranslationBuild(projectId).subscribe(result => { + expect(result).toEqual(buildDto); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/getLastPreTranslationBuild` + ); + expect(req.request.method).toEqual('GET'); + req.flush(buildDto); + tick(); + })); + + it('should return undefined when no build has ever run', fakeAsync(() => { + // SUT + service.getLastPreTranslationBuild(projectId).subscribe(result => { + expect(result).toBeUndefined(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/getLastPreTranslationBuild` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.NoContent, statusText: 'No Content' }); + tick(); + })); + + it('should return undefined if offline', fakeAsync(() => { + testOnlineStatusService.setIsOnline(false); + + // SUT + service.getLastPreTranslationBuild(projectId).subscribe(result => { + expect(result).toBeUndefined(); + }); + tick(); + })); + }); + describe('getBuildHistory', () => { it('should get project builds and return an observable array of BuildDto', fakeAsync(() => { // SUT diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts index 032d1f60a1..a472dfe15f 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts @@ -131,6 +131,33 @@ export class DraftGenerationService { ); } + /** + * Gets the last pre-translation build regardless of state (Completed, Running, Queued, Faulted, or Canceled). + * This is a simpler accessor than getLastCompletedBuild() and can be used when the consumer + * wants the most recent build even if it has not yet completed. + * @param projectId The SF project id for the target translation. + * @returns An observable BuildDto for the last pre-translation build, or undefined if no build has ever run. + */ + getLastPreTranslationBuild(projectId: string): Observable { + if (!this.onlineStatusService.isOnline) { + return of(undefined); + } + return this.httpClient + .get(`translation/engines/project:${projectId}/actions/getLastPreTranslationBuild`) + .pipe( + map(res => res.data), + catchError(err => { + // If project doesn't exist on Serval, return undefined + if (err.status === 403 || err.status === 404) { + return of(undefined); + } + + this.noticeService.showError(this.i18n.translateStatic('draft_generation.temporarily_unavailable')); + return of(undefined); + }) + ); + } + /** Gets the build exactly as Serval returns it */ getRawBuild(buildId: string): Observable { if (!this.onlineStatusService.isOnline) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts index 1dbe1566ea..479fc935de 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts @@ -86,6 +86,11 @@ describe('EditorDraftComponent', () => { buildProgress$.next({ state: BuildStates.Completed } as BuildDto); when(mockActivatedProjectService.projectId$).thenReturn(of('targetProjectId')); when(mockDraftGenerationService.getLastCompletedBuild(anything())).thenReturn(of(undefined)); + const defaultProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; + when(mockActivatedProjectService.projectDoc$).thenReturn(of(defaultProjectDoc)); + when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn( + of({ state: BuildStates.Completed } as BuildDto) + ); fixture = TestBed.createComponent(EditorDraftComponent); component = fixture.componentInstance; @@ -582,8 +587,12 @@ describe('EditorDraftComponent', () => { }) } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc)); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn( + of({ state: BuildStates.Completed } as BuildDto) + ); fixture.detectChanges(); tick(EDITOR_READY_TIMEOUT); @@ -604,8 +613,12 @@ describe('EditorDraftComponent', () => { }) } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc)); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn( + of({ state: BuildStates.Completed } as BuildDto) + ); fixture.detectChanges(); tick(EDITOR_READY_TIMEOUT); expect(component.canConfigureFormatting).toBe(true); @@ -632,11 +645,43 @@ describe('EditorDraftComponent', () => { }) } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false)); + when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc)); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn( + of({ state: BuildStates.Completed } as BuildDto) + ); fixture.detectChanges(); tick(EDITOR_READY_TIMEOUT); + tick(); + expect(component.isSelectedDraftLatest).toBe(true); + expect(component.canConfigureFormatting).toBe(false); + flush(); + })); + + it('should be false when latest build is canceled even if draft exists and selected revision is latest', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile({ + texts: [ + { + bookNum: 1, + chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }] + } + ] + }) + } as SFProjectProfileDoc; + when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn( + of({ state: BuildStates.Canceled } as BuildDto) + ); + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc)); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + expect(component.isSelectedDraftLatest).toBe(true); + expect(component.doesLatestCompletedHaveDraft).toBe(true); expect(component.canConfigureFormatting).toBe(false); flush(); })); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts index 9dc8f7c9a3..106d3f3e02 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts @@ -30,6 +30,7 @@ import { startWith, Subject, switchMap, + take, tap, throttleTime } from 'rxjs'; @@ -47,6 +48,7 @@ import { isString } from '../../../../type-utils'; import { TextDocId } from '../../../core/models/text-doc'; import { Revision } from '../../../core/paratext.service'; import { SFProjectService } from '../../../core/sf-project.service'; +import { BuildDto } from '../../../machine-api/build-dto'; import { BuildStates } from '../../../machine-api/build-states'; import { NoticeComponent } from '../../../shared/notice/notice.component'; import { TextComponent } from '../../../shared/text/text.component'; @@ -121,6 +123,7 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { private draftDelta?: Delta; private targetDelta?: Delta; + private _latestPreTranslationBuild: BuildDto | undefined; constructor( private readonly activatedProjectService: ActivatedProjectService, @@ -151,16 +154,20 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { } get canConfigureFormatting(): boolean { - return this.doesLatestHaveDraft && this.isSelectedDraftLatest; + return this.doesLatestCompletedHaveDraft && this.isSelectedDraftLatest && this.isLatestBuildCompleted; } - get doesLatestHaveDraft(): boolean { + get doesLatestCompletedHaveDraft(): boolean { return ( this.targetProject?.texts.find(t => t.bookNum === this.bookNum)?.chapters.find(c => c.number === this.chapter) ?.hasDraft ?? false ); } + get isLatestBuildCompleted(): boolean { + return this._latestPreTranslationBuild?.state === BuildStates.Completed; + } + get isSelectedDraftLatest(): boolean { return this.selectedRevision?.timestamp === this._draftRevisions[0].timestamp; } @@ -247,9 +254,6 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { // Respond to project changes return this.activatedProjectService.changes$.pipe( filterNullish(), - tap(projectDoc => { - this.targetProject = projectDoc.data; - }), distinctUntilChanged(), map(() => initialTimestamp) ); @@ -307,6 +311,37 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { this.isDraftReady = this.draftCheckState === 'draft-present' || this.draftCheckState === 'draft-legacy'; }); + + combineLatest([ + this.onlineStatusService.onlineStatus$, + this.activatedProjectService.projectDoc$, + this.draftGenerationService.pollBuildProgress(this.textDocId!.projectId), + this.draftText.editorCreated as EventEmitter, + this.inputChanged$.pipe(startWith(undefined)) + ]) + .pipe( + quietTakeUntilDestroyed(this.destroyRef), + filter(([_, projectDoc]) => projectDoc !== undefined), + tap(([_, projectDoc]) => { + this.targetProject = projectDoc!.data; + }), + filter(([isOnline, _]) => { + return isOnline && this.doesLatestCompletedHaveDraft; + }) + ) + .subscribe(() => this.refreshLastPreTranslationBuild()); + } + + private refreshLastPreTranslationBuild(): void { + if (this.projectId == null) { + return; + } + this.draftGenerationService + .getLastPreTranslationBuild(this.projectId) + .pipe(quietTakeUntilDestroyed(this.destroyRef), take(1)) + .subscribe((build: BuildDto | undefined) => { + this._latestPreTranslationBuild = build; + }); } navigateToFormatting(): void { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts index 3427e7fe4d..cacecbd10f 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts @@ -4905,6 +4905,7 @@ class TestEnvironment { when(mockedDraftGenerationService.pollBuildProgress(anything())).thenReturn( of({ state: BuildStates.Completed } as BuildDto) ); + when(mockedDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(of(undefined)); when( mockedDraftGenerationService.getGeneratedDraftDeltaOperations( anything(), diff --git a/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs b/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs index e31b863754..6377601bfc 100644 --- a/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs +++ b/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs @@ -403,6 +403,54 @@ CancellationToken cancellationToken } } + /// + /// Gets the last pre-translation build regardless of state (completed, running, queued, faulted, or canceled). + /// + /// The Scripture Forge project identifier. + /// The cancellation token. + /// The last pre-translation build was found. + /// There are no pre-translation builds. + /// You do not have permission to get the builds for this project. + /// The project does not exist or is not configured on the ML server. + /// The ML server is temporarily unavailable or unresponsive. + [HttpGet(MachineApi.GetLastPreTranslationBuild)] + public async Task> GetLastPreTranslationBuildAsync( + string sfProjectId, + CancellationToken cancellationToken + ) + { + try + { + bool isServalAdmin = _userAccessor.SystemRoles.Contains(SystemRole.ServalAdmin); + ServalBuildDto? build = await _machineApiService.GetLastPreTranslationBuildAsync( + _userAccessor.UserId, + sfProjectId, + isServalAdmin, + cancellationToken + ); + + if (build is null) + { + return NoContent(); + } + + return Ok(build); + } + catch (BrokenCircuitException e) + { + _exceptionHandler.ReportException(e); + return StatusCode(StatusCodes.Status503ServiceUnavailable, MachineApiUnavailable); + } + catch (DataNotFoundException) + { + return NotFound(); + } + catch (ForbiddenException) + { + return Forbid(); + } + } + /// /// Gets all the pre-translations for the specified chapter. /// diff --git a/src/SIL.XForge.Scripture/Models/MachineApi.cs b/src/SIL.XForge.Scripture/Models/MachineApi.cs index b632828277..e64de47173 100644 --- a/src/SIL.XForge.Scripture/Models/MachineApi.cs +++ b/src/SIL.XForge.Scripture/Models/MachineApi.cs @@ -35,6 +35,8 @@ public static class MachineApi "translation/engines/project:{sfProjectId}/actions/preTranslate/{bookNum}_{chapterNum}/usx"; public const string GetLastCompletedPreTranslationBuild = "translation/engines/project:{sfProjectId}/actions/getLastCompletedPreTranslationBuild"; + public const string GetLastPreTranslationBuild = + "translation/engines/project:{sfProjectId}/actions/getLastPreTranslationBuild"; public static string GetBuildHref(string sfProjectId, string buildId) { diff --git a/src/SIL.XForge.Scripture/Services/IMachineApiService.cs b/src/SIL.XForge.Scripture/Services/IMachineApiService.cs index bf81b319f1..4393d928be 100644 --- a/src/SIL.XForge.Scripture/Services/IMachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/IMachineApiService.cs @@ -82,6 +82,12 @@ CancellationToken cancellationToken bool isServalAdmin, CancellationToken cancellationToken ); + Task GetLastPreTranslationBuildAsync( + string curUserId, + string sfProjectId, + bool isServalAdmin, + CancellationToken cancellationToken + ); Task GetPreTranslationAsync( string curUserId, string sfProjectId, diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index f549831bce..b0fbdaf853 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -1177,6 +1177,74 @@ await translationEnginesClient.GetAllBuildsAsync(translationEngineId, cancellati return buildDto; } + /// + /// Gets the last pre-translation build (NMT) for the specified project regardless of its final state. + /// + /// The current user identifier. + /// The Scripture Forge project identifier. + /// If true, the current user is a Serval Administrator. + /// The cancellation token. + /// The last pre-translation , or null if none exist. + /// + /// This differs from in that it does not restrict + /// the result to only completed builds; queued, running, faulted, or canceled builds may be returned if + /// they are the most recent. This method intentionally performs no chapter HasDraft verification or + /// background job enqueueing – it is a simple convenience accessor. + /// + public async Task GetLastPreTranslationBuildAsync( + string curUserId, + string sfProjectId, + bool isServalAdmin, + CancellationToken cancellationToken + ) + { + ServalBuildDto? buildDto = null; + + // Ensure that the user has permission + await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin, cancellationToken); + + // Get the translation engine id for pre-translation builds + string translationEngineId = await GetTranslationIdAsync(sfProjectId, preTranslate: true); + + try + { + // Retrieve all builds and select the most recent by DateFinished if present, otherwise by ObjectId creation time + IList builds = await translationEnginesClient.GetAllBuildsAsync( + translationEngineId, + cancellationToken + ); + + static DateTimeOffset GetSortTimestamp(TranslationBuild b) + { + if (b.DateFinished.HasValue) + { + return b.DateFinished.Value; + } + // Fallback to the MongoDB ObjectId creation time (assumed UTC) + if (ObjectId.TryParse(b.Id, out ObjectId oid)) + { + return new DateTimeOffset(oid.CreationTime, TimeSpan.Zero); + } + return DateTimeOffset.MinValue; + } + + TranslationBuild? lastBuild = builds.OrderByDescending(GetSortTimestamp).FirstOrDefault(); + + if (lastBuild is not null) + { + buildDto = CreateDto(lastBuild); + // Make sure the DTO conforms to the machine-api V2 URLs + buildDto = UpdateDto(buildDto, sfProjectId); + } + } + catch (ServalApiException e) + { + ProcessServalApiException(e); + } + + return buildDto; + } + public async Task GetCurrentBuildAsync( string curUserId, string sfProjectId, diff --git a/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs b/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs index 2e992192dd..7e27f68e78 100644 --- a/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs @@ -794,6 +794,54 @@ await env .GetLastCompletedPreTranslationBuildAsync(User01, Project01, false, CancellationToken.None); } + [Test] + public async Task GetLastPreTranslationBuildAsync_NoBuild() + { + var env = new TestEnvironment(); + env.MachineApiService.GetLastPreTranslationBuildAsync(User01, Project01, false, CancellationToken.None) + .Returns(Task.FromResult(null)); + + ActionResult actual = await env.Controller.GetLastPreTranslationBuildAsync( + Project01, + CancellationToken.None + ); + + Assert.IsInstanceOf(actual.Result); + } + + [Test] + public async Task GetLastPreTranslationBuildAsync_NoProject() + { + var env = new TestEnvironment(); + env.MachineApiService.GetLastPreTranslationBuildAsync(User01, Project01, false, CancellationToken.None) + .Throws(new DataNotFoundException(string.Empty)); + + ActionResult actual = await env.Controller.GetLastPreTranslationBuildAsync( + Project01, + CancellationToken.None + ); + + Assert.IsInstanceOf(actual.Result); + } + + [Test] + public async Task GetLastPreTranslationBuildAsync_Success() + { + var env = new TestEnvironment(); + env.MachineApiService.GetLastPreTranslationBuildAsync(User01, Project01, false, CancellationToken.None) + .Returns(Task.FromResult(new ServalBuildDto())); + + ActionResult actual = await env.Controller.GetLastPreTranslationBuildAsync( + Project01, + CancellationToken.None + ); + + Assert.IsInstanceOf(actual.Result); + await env + .MachineApiService.Received(1) + .GetLastPreTranslationBuildAsync(User01, Project01, false, CancellationToken.None); + } + [Test] public async Task GetPreTranslationAsync_MachineApiDown() { diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index 73181a1e3c..0a83e5517c 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -2046,6 +2046,65 @@ public async Task GetLastCompletedPreTranslationBuildAsync_NullScriptureRange_Su TestEnvironment.AssertCoreBuildProperties(CompletedTranslationBuild, actual); } + [Test] + public async Task GetLastPreTranslationBuildAsync_NoBuilds() + { + var env = new TestEnvironment(); + env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None) + .Returns(Task.FromResult>([])); + + ServalBuildDto? actual = await env.Service.GetLastPreTranslationBuildAsync( + User01, + Project01, + false, + CancellationToken.None + ); + + Assert.IsNull(actual); + } + + [Test] + public async Task GetLastPreTranslationBuildAsync_LatestByDateFinished_Success() + { + var env = new TestEnvironment(); + DateTimeOffset now = DateTimeOffset.UtcNow; + TranslationBuild completedEarlier = new TranslationBuild + { + Url = "https://example.com", + Id = Build01, + Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, + Message = MachineApiService.BuildStateCompleted, + Progress = 100, + Revision = 10, + State = JobState.Completed, + DateFinished = now.AddMinutes(-10), + }; + TranslationBuild completedLater = new TranslationBuild + { + Url = "https://example.com", + Id = Build02, + Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, + Message = MachineApiService.BuildStateCompleted, + Progress = 100, + Revision = 11, + State = JobState.Completed, + DateFinished = now.AddMinutes(-5), + }; + env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None) + .Returns(Task.FromResult>([completedEarlier, completedLater])); + + ServalBuildDto? actual = await env.Service.GetLastPreTranslationBuildAsync( + User01, + Project01, + false, + CancellationToken.None + ); + + Assert.IsNotNull(actual); + Assert.AreEqual(MachineApi.GetBuildHref(Project01, Build02), actual.Href); + Assert.AreEqual(Build02, actual.Id.Split('.')[1]); + } + [Test] public void GetPreTranslationAsync_EngineNotBuilt() { From fb9c1c3427a5759076c06f8396f19265a6fb724d Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Tue, 28 Oct 2025 09:05:35 -0500 Subject: [PATCH 3/4] Formatting button gets removed instead of disabled --- .../editor/editor-draft/editor-draft.component.html | 9 +++------ .../editor/editor-draft/editor-draft.component.ts | 2 +- .../ClientApp/src/assets/i18n/non_checking_en.json | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html index 955bb67dd1..cf6652a221 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html @@ -43,12 +43,9 @@ }
- @if (showDraftOptionsButton$ | async) { - - diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts index 106d3f3e02..9231ee44cd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts @@ -115,7 +115,7 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { this.onlineStatusService.onlineStatus$ ]).pipe(map(([isLoading, isOnline]) => isLoading || !isOnline)); - showDraftOptionsButton$: Observable = this.activatedProjectService.projectId$.pipe( + isFormattingSupportedForLatest$: Observable = this.activatedProjectService.projectId$.pipe( filterNullish(), switchMap(projectId => this.draftGenerationService.getLastCompletedBuild(projectId)), map(build => this.draftOptionsService.areFormattingOptionsSupportedForBuild(build)) 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 f57a7616ff..8737dd276b 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 @@ -410,8 +410,7 @@ "draft_legacy_warning": "We have updated our drafting functionality. You can take advantage of this by [link:generateDraftUrl]generating a new draft[/link].", "error_applying_draft": "Failed to add the draft to the project. Try again later.", "format_draft": "Formatting options", - "format_draft_can": "Customize formatting options for the draft", - "format_draft_cannot": "You can only change formatting for books from the latest draft", + "format_draft_tooltip": "Customize formatting options for the draft", "no_draft_notice": "{{ bookChapterName }} has no draft.", "offline_notice": "Generated drafts are not available offline.", "overwrite": "Adding the draft will overwrite the current chapter. Are you sure you want to continue?", From c5977f1a3bace1ff141b1a7d014231783f0722df Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Mon, 3 Nov 2025 12:38:31 -0600 Subject: [PATCH 4/4] Review comments --- .../draft-generation.service.spec.ts | 17 +++++++++++++++++ .../editor-draft/editor-draft.component.ts | 17 ++++++++--------- .../Services/MachineApiServiceTests.cs | 2 +- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts index 5eda633964..bcff44dc36 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts @@ -225,6 +225,23 @@ describe('DraftGenerationService', () => { }); tick(); })); + + it('should return undefined and show error when server returns 500', fakeAsync(() => { + // SUT + service.getLastPreTranslationBuild(projectId).subscribe(result => { + expect(result).toBeUndefined(); + verify(mockNoticeService.showError(anything())).once(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/getLastPreTranslationBuild` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.InternalServerError, statusText: 'Server Error' }); + tick(); + })); }); describe('getBuildHistory', () => { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts index 9231ee44cd..ce23dcaf9c 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts @@ -327,21 +327,20 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { }), filter(([isOnline, _]) => { return isOnline && this.doesLatestCompletedHaveDraft; + }), + switchMap(() => this.refreshLastPreTranslationBuild()), + tap((build: BuildDto | undefined) => { + this._latestPreTranslationBuild = build; }) ) - .subscribe(() => this.refreshLastPreTranslationBuild()); + .subscribe(); } - private refreshLastPreTranslationBuild(): void { + private refreshLastPreTranslationBuild(): Observable { if (this.projectId == null) { - return; + return of(undefined); } - this.draftGenerationService - .getLastPreTranslationBuild(this.projectId) - .pipe(quietTakeUntilDestroyed(this.destroyRef), take(1)) - .subscribe((build: BuildDto | undefined) => { - this._latestPreTranslationBuild = build; - }); + return this.draftGenerationService.getLastPreTranslationBuild(this.projectId).pipe(take(1)); } navigateToFormatting(): void { diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index 0a83e5517c..b646c9af59 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -2087,7 +2087,7 @@ public async Task GetLastPreTranslationBuildAsync_LatestByDateFinished_Success() Message = MachineApiService.BuildStateCompleted, Progress = 100, Revision = 11, - State = JobState.Completed, + State = JobState.Canceled, DateFinished = now.AddMinutes(-5), }; env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None)