From 812ecd946e5ba17c68dbddc0418deb4f96785c01 Mon Sep 17 00:00:00 2001 From: Nathaniel Paulus Date: Fri, 11 Apr 2025 14:53:04 -0400 Subject: [PATCH 01/11] WIP Add draft history TODO: - Refresh history list when draft starts or completes - Allow downloading historical drafts [how?] - Allow previewing historical drafts [how?] - Add draft configuration info - Show who created the draft [maybe?] - Show draft failures --- .../draft-generation.component.html | 2 + .../draft-generation.component.ts | 4 +- .../draft-generation.service.ts | 6 ++ .../_draft-history-entry-theme.scss | 15 ++++ .../draft-history-entry.component.html | 39 ++++++++++ .../draft-history-entry.component.scss | 60 +++++++++++++++ .../draft-history-entry.component.spec.ts | 22 ++++++ .../draft-history-entry.component.ts | 76 +++++++++++++++++++ .../draft-history-list.component.html | 11 +++ .../draft-history-list.component.scss | 5 ++ .../draft-history-list.component.ts | 58 ++++++++++++++ .../draft-history.component.spec.ts | 22 ++++++ .../ClientApp/src/material-styles.scss | 3 + 13 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.scss create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html index 7d0e157c888..91020417e94 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html @@ -384,3 +384,5 @@

{{ t("draft_finishing_header") }}

} } + + diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts index 3dba549aee4..f2fa38dc797 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts @@ -38,6 +38,7 @@ import { DraftGenerationStepsResult } from './draft-generation-steps/draft-generation-steps.component'; import { DraftGenerationService } from './draft-generation.service'; +import { DraftHistoryListComponent } from './draft-history-list/draft-history-list.component'; import { DraftInformationComponent } from './draft-information/draft-information.component'; import { DraftPreviewBooksComponent } from './draft-preview-books/draft-preview-books.component'; import { DraftSource, DraftSourcesService } from './draft-sources.service'; @@ -60,7 +61,8 @@ import { SupportedBackTranslationLanguagesDialogComponent } from './supported-ba DraftGenerationStepsComponent, DraftInformationComponent, ServalProjectComponent, - DraftPreviewBooksComponent + DraftPreviewBooksComponent, + DraftHistoryListComponent ] }) export class DraftGenerationComponent extends DataLoadingComponent implements OnInit { 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 a14d3639071..5a11ae3ea2c 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 @@ -78,6 +78,12 @@ export class DraftGenerationService { ); } + getBuildHistory(sfProjectId: string): Observable { + return this.httpClient + .get(`translation/builds/project:${sfProjectId}?pretranslate=true`) + .pipe(map(res => res.data)); + } + /** * Gets the last completed pre-translation build. * @param projectId The SF project id for the target translation. diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss new file mode 100644 index 00000000000..9b1c4adf136 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss @@ -0,0 +1,15 @@ +@use '@angular/material' as mat; + +@mixin color($theme) { + $is-dark: mat.get-theme-type($theme) == dark; + + --draft-history-entry-heading-background-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 20, 95))}; + --draft-history-entry-heading-background-hover-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 30, 90))}; + --draft-history-entry-details-background-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 10, 98))}; +} + +@mixin theme($theme) { + @if mat.theme-has($theme, color) { + @include color($theme); + } +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html new file mode 100644 index 00000000000..9ca2fc9de17 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html @@ -0,0 +1,39 @@ +@if (entry != null) { +
+
+ {{ i18n.enumerateList(bookNames) }} + {{ formatDate(entry.additionalInfo.dateFinished) }} +
+ + + {{ getStatus(entry.state).icons }} + {{ getStatus(entry.state).text }} + + @if (!forceDetailsOpen) { + expand_{{ detailsOpen ? "less" : "more" }} + } +
+ @if (detailsOpen) { +
+

Click a book below to preview the draft and add it to your project.

+

+ @for (bookId of bookIds; track bookId) { + {{ i18n.localizeBook(bookId) }} + } +

+ +

+ + expand_{{ trainingDataOpen ? "less" : "more" }} + {{ trainingDataOpen ? "Hide" : "Show" }} model training data + +

+ @if (trainingDataOpen) { +

+ The language model was training on this configuration: + [TODO] +

+ } +
+ } +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss new file mode 100644 index 00000000000..a50f974d861 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss @@ -0,0 +1,60 @@ +:host { + background-color: var(--draft-history-entry-details-background-color); + border-radius: 8px; + > * { + padding: 8px 16px; + border-radius: 8px; + } +} + +.draft-entry-heading { + background-color: var(--draft-history-entry-heading-background-color); + display: flex; + align-items: center; + gap: 1em; + &.expandable { + cursor: pointer; + } + &.expandable:hover { + background-color: var(--draft-history-entry-heading-background-hover-color); + } +} + +.drafted-books { + font-size: 1.1em; +} + +.spacer { + flex-grow: 1; +} + +.entry-description { + display: flex; + flex-direction: column; + gap: 4px; +} + +.date { + font-size: 0.9em; + color: #666; +} + +.status { + display: flex; + align-items: center; + gap: 4px; +} + +.training-data-link { + display: flex; + align-items: center; +} + +strong { + font-weight: 500; +} + +.book-buttons { + display: flex; + gap: 4px; +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts new file mode 100644 index 00000000000..a91b273dab6 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts @@ -0,0 +1,22 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; + +import { DraftHistoryEntryComponent } from './draft-history-entry.component'; + +describe('DraftHistoryEntryComponent', () => { + let component: DraftHistoryEntryComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [DraftHistoryEntryComponent] + }).compileComponents(); + + fixture = TestBed.createComponent(DraftHistoryEntryComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts new file mode 100644 index 00000000000..ae06f19e02c --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -0,0 +1,76 @@ +import { Component, Input } from '@angular/core'; +import { MatButtonModule } from '@angular/material/button'; +import { MatIconModule } from '@angular/material/icon'; +import { I18nService } from '../../../../../xforge-common/i18n.service'; +import { BuildStates } from '../../../../machine-api/build-states'; + +interface ScriptureRange { + projectId: string; + scriptureRange: string; +} + +export interface DraftHistoryEntry { + state: BuildStates; + id: string; + percentCompleted: number; + additionalInfo: { + translationScriptureRanges: ScriptureRange[]; + dateFinished: string; + }; +} + +const STATUS_INFO: Record = { + ACTIVE: { icons: 'hourglass_empty', text: 'Running', color: 'grey' }, + COMPLETED: { icons: 'check_circle', text: 'Completed', color: 'green' }, + FAULTED: { icons: 'error', text: 'Failed', color: 'red' }, + CANCELED: { icons: 'cancel', text: 'Cancelled', color: 'grey' }, + QUEUED: { icons: 'hourglass_empty', text: 'pending', color: 'grey' }, + PENDING: { icons: 'hourglass_empty', text: 'pending', color: 'grey' }, + FINISHING: { icons: 'hourglass_empty', text: 'pending', color: 'grey' } +}; + +@Component({ + selector: 'app-draft-history-entry', + standalone: true, + imports: [MatIconModule, MatButtonModule], + templateUrl: './draft-history-entry.component.html', + styleUrl: './draft-history-entry.component.scss' +}) +export class DraftHistoryEntryComponent { + @Input() entry?: DraftHistoryEntry; + private _forceDetailsOpen = false; + @Input() set forceDetailsOpen(value: boolean) { + this._forceDetailsOpen = value; + if (value) this.detailsOpen = true; + } + get forceDetailsOpen(): boolean { + return this._forceDetailsOpen; + } + detailsOpen = false; + trainingDataOpen = false; + + constructor(readonly i18n: I18nService) {} + + get bookIds(): string[] { + if (this.entry == null) return []; + return [ + ...new Set(this.entry?.additionalInfo.translationScriptureRanges.flatMap(r => r.scriptureRange.split(';'))) + ]; + } + + get bookNames(): string[] { + return this.bookIds.map(id => this.i18n.localizeBook(id)); + } + + formatDate(date: string): string { + return this.i18n.formatDate(new Date(date)); + } + + getStatus(state: BuildStates): { icons: string; text: string; color: string } { + return STATUS_INFO[state]; + } + + headerClicked(): void { + if (!this.forceDetailsOpen) this.detailsOpen = !this.detailsOpen; + } +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html new file mode 100644 index 00000000000..b9285197ab3 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html @@ -0,0 +1,11 @@ +@if (latestBuild != null) { +

{{ lastCompletedBuildMessage }}

+ +} + +@if (historicalBuilds.length > 0) { +

Previously generated drafts

+ @for (entry of historicalBuilds; track entry.id) { + + } +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.scss new file mode 100644 index 00000000000..0c483fc1d64 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.scss @@ -0,0 +1,5 @@ +:host { + display: flex; + flex-direction: column; + gap: 8px; +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts new file mode 100644 index 00000000000..2960f85fba9 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts @@ -0,0 +1,58 @@ +import { Component } from '@angular/core'; +import { MatIconModule } from '@angular/material/icon'; +import { take } from 'rxjs'; +import { ActivatedProjectService } from '../../../../xforge-common/activated-project.service'; +import { filterNullish } from '../../../../xforge-common/util/rxjs-util'; +import { activeBuildStates } from '../draft-generation'; +import { DraftGenerationService } from '../draft-generation.service'; +import { DraftHistoryEntry, DraftHistoryEntryComponent } from './draft-history-entry/draft-history-entry.component'; + +@Component({ + selector: 'app-draft-history-list', + standalone: true, + imports: [MatIconModule, DraftHistoryEntryComponent], + templateUrl: './draft-history-list.component.html', + styleUrl: './draft-history-list.component.scss' +}) +export class DraftHistoryListComponent { + history?: DraftHistoryEntry[]; + + constructor( + private readonly activatedProject: ActivatedProjectService, + private readonly draftGenerationService: DraftGenerationService + ) { + this.activatedProject.projectId$.pipe(filterNullish(), take(1)).subscribe(projectId => { + this.draftGenerationService.getBuildHistory(projectId).subscribe(result => { + this.history = result.reverse(); + }); + }); + } + + get nonActiveBuilds(): DraftHistoryEntry[] { + return this.history?.filter(entry => !activeBuildStates.includes(entry.state)) || []; + } + + get latestBuild(): DraftHistoryEntry | undefined { + return this.isBuildActive ? undefined : this.nonActiveBuilds[0]; + } + + get lastCompletedBuildMessage(): string { + if (this.latestBuild == null) return ''; + const entry = this.latestBuild; + return ( + { + COMPLETED: 'Your draft is ready', + CANCELED: 'Your draft was cancelled', + FAULTED: 'The draft failed' + }[entry.state] || entry.state + ); + } + + get historicalBuilds(): DraftHistoryEntry[] { + return this.latestBuild == null ? this.nonActiveBuilds : this.nonActiveBuilds.slice(1); + } + + get isBuildActive(): boolean { + return this.history?.some(entry => activeBuildStates.includes(entry.state)) ?? false; + } +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts new file mode 100644 index 00000000000..5065c096531 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts @@ -0,0 +1,22 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; + +import { DraftHistoryListComponent } from './draft-history.component'; + +describe('DraftHistoryComponent', () => { + let component: DraftHistoryListComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [DraftHistoryListComponent] + }).compileComponents(); + + fixture = TestBed.createComponent(DraftHistoryListComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/material-styles.scss b/src/SIL.XForge.Scripture/ClientApp/src/material-styles.scss index a07c715e4eb..b8564f34a94 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/material-styles.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/material-styles.scss @@ -13,6 +13,8 @@ sf-draft-generation-steps; @use 'src/app/translate/draft-generation/confirm-sources/confirm-sources-theme' as sf-confirm-sources; @use 'src/app/translate/draft-generation/draft-sources/draft-sources-theme' as sf-draft-sources; +@use 'src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry-theme' as + sf-draft-history-entry; @use 'src/app/translate/editor/editor-theme' as sf-editor; @use 'src/app/translate/editor/note-dialog/note-dialog-theme' as sf-note-dialog; @use 'src/app/translate/editor/editor-draft/editor-draft-theme' as sf-editor-draft; @@ -37,6 +39,7 @@ @include sf-checking-questions.theme($theme); @include sf-confirm-sources.theme($theme); @include sf-draft-generation-steps.theme($theme); + @include sf-draft-history-entry.theme($theme); @include sf-draft-sources.theme($theme); @include sf-editor.theme($theme); @include sf-editor-draft.theme($theme); From d742de5b070c785cc6b892680d6efdb940386a5f Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Wed, 16 Apr 2025 11:51:44 +1200 Subject: [PATCH 02/11] Clean up and fix tests --- .../src/app/machine-api/build-dto.ts | 6 +- .../draft-generation.component.spec.ts | 2 + .../draft-generation.component.ts | 12 +-- .../draft-generation.service.spec.ts | 28 +++++++ .../draft-generation.service.ts | 31 ++++++-- .../draft-history-entry.component.html | 78 ++++++++++--------- .../draft-history-entry.component.spec.ts | 15 ++-- .../draft-history-entry.component.ts | 33 +++----- .../draft-history-list.component.html | 20 ++--- .../draft-history-list.component.spec.ts | 37 +++++++++ .../draft-history-list.component.ts | 47 ++++++----- .../draft-history.component.spec.ts | 22 ------ .../src/assets/i18n/non_checking_en.json | 13 ++++ 13 files changed, 216 insertions(+), 128 deletions(-) create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts delete mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts index c506b2c9edf..ec7b20fde79 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts @@ -1,3 +1,5 @@ +import { ProjectScriptureRange } from 'realtime-server/lib/esm/scriptureforge/models/translate-config'; +import { BuildStates } from './build-states'; import { ResourceDto } from './resource-dto'; export interface BuildDto extends ResourceDto { @@ -5,7 +7,7 @@ export interface BuildDto extends ResourceDto { engine: ResourceDto; percentCompleted: number; message: string; - state: string; + state: BuildStates; queueDepth: number; additionalInfo?: ServalBuildAdditionalInfo; } @@ -16,5 +18,7 @@ export interface ServalBuildAdditionalInfo { dateFinished?: string; parallelCorporaIds?: string[]; step: number; + trainingScriptureRanges: ProjectScriptureRange[]; translationEngineId: string; + translationScriptureRanges: ProjectScriptureRange[]; } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts index 667c8615080..8cd1ef6e25b 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts @@ -120,6 +120,7 @@ describe('DraftGenerationComponent', () => { mockDraftGenerationService = jasmine.createSpyObj([ 'startBuildOrGetActiveBuild', 'cancelBuild', + 'getBuildHistory', 'getBuildProgress', 'pollBuildProgress', 'getGeneratedDraftUsfm', @@ -130,6 +131,7 @@ describe('DraftGenerationComponent', () => { mockUserService = jasmine.createSpyObj(['getCurrentUser']); mockDraftGenerationService.startBuildOrGetActiveBuild.and.returnValue(this.startedOrActiveBuild$); + mockDraftGenerationService.getBuildHistory.and.returnValue(of([buildDto])); mockDraftGenerationService.getBuildProgress.and.returnValue(of(buildDto)); mockDraftGenerationService.pollBuildProgress.and.returnValue(of(buildDto)); mockDraftGenerationService.getLastCompletedBuild.and.returnValue(of(buildDto)); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts index f2fa38dc797..4daef7b62b2 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts @@ -380,7 +380,7 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On } isDraftInProgress(job?: BuildDto): boolean { - return activeBuildStates.includes(job?.state as BuildStates); + return job != null && activeBuildStates.includes(job.state); } isSyncing(): boolean { @@ -388,23 +388,23 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On } isDraftQueued(job?: BuildDto): boolean { - return [BuildStates.Queued, BuildStates.Pending].includes(job?.state as BuildStates); + return job != null && [BuildStates.Queued, BuildStates.Pending].includes(job.state); } isDraftActive(job?: BuildDto): boolean { - return (job?.state as BuildStates) === BuildStates.Active; + return job?.state === BuildStates.Active; } isDraftFinishing(job?: BuildDto): boolean { - return (job?.state as BuildStates) === BuildStates.Finishing; + return job?.state === BuildStates.Finishing; } isDraftComplete(job?: BuildDto): boolean { - return (job?.state as BuildStates) === BuildStates.Completed; + return job?.state === BuildStates.Completed; } isDraftFaulted(job?: BuildDto): boolean { - return (job?.state as BuildStates) === BuildStates.Faulted; + return job?.state === BuildStates.Faulted; } isServalAdmin(): boolean { 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 8de46b1537a..421de42e5fc 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 @@ -141,6 +141,34 @@ describe('DraftGenerationService', () => { })); }); + describe('getBuildHistory', () => { + it('should get project builds and return an observable array of BuildDto', fakeAsync(() => { + // SUT + service.getBuildHistory(projectId).subscribe(result => { + expect(result).toEqual([buildDto]); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/builds/project:${projectId}?pretranslate=true` + ); + expect(req.request.method).toEqual('GET'); + req.flush([buildDto]); + tick(); + })); + + it('should return undefined if offline', fakeAsync(() => { + testOnlineStatusService.setIsOnline(false); + + // SUT + service.getBuildHistory(projectId).subscribe(result => { + expect(result).toBeUndefined(); + }); + tick(); + })); + }); + describe('getBuildProgress', () => { it('should get build progress and return an observable 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 5a11ae3ea2c..a11de259f57 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 @@ -12,7 +12,6 @@ import { NoticeService } from 'xforge-common/notice.service'; import { OnlineStatusService } from 'xforge-common/online-status.service'; import { SFProjectProfileDoc } from '../../core/models/sf-project-profile-doc'; import { BuildDto } from '../../machine-api/build-dto'; -import { BuildStates } from '../../machine-api/build-states'; import { HttpClient } from '../../machine-api/http-client'; import { getBookFileNameDigits } from '../../shared/utils'; import { @@ -47,7 +46,7 @@ export class DraftGenerationService { pollBuildProgress(projectId: string): Observable { return timer(0, this.options.pollRate).pipe( switchMap(() => this.getBuildProgress(projectId)), - takeWhile(job => activeBuildStates.includes(job?.state as BuildStates), true), + takeWhile(job => job != null && activeBuildStates.includes(job.state), true), distinct(job => `${job?.state}${job?.queueDepth}${job?.percentCompleted}`), shareReplay({ bufferSize: 1, refCount: true }) ); @@ -78,10 +77,28 @@ export class DraftGenerationService { ); } - getBuildHistory(sfProjectId: string): Observable { - return this.httpClient - .get(`translation/builds/project:${sfProjectId}?pretranslate=true`) - .pipe(map(res => res.data)); + /** + * Gets pre-translation builds for specified project. + * @param projectId The SF project id for the target translation. + * @returns An observable array of BuildDto objects, describing + * the state and progress of past and present builds. + */ + getBuildHistory(projectId: string): Observable { + if (!this.onlineStatusService.isOnline) { + return of(undefined); + } + return this.httpClient.get(`translation/builds/project:${projectId}?pretranslate=true`).pipe( + map(res => res.data), + catchError(err => { + // If no build has ever been started, return undefined + if (err.status === 403 || err.status === 404) { + return of(undefined); + } + + this.noticeService.showError(translate('draft_generation.temporarily_unavailable')); + return of(undefined); + }) + ); } /** @@ -119,7 +136,7 @@ export class DraftGenerationService { return this.getBuildProgress(buildConfig.projectId).pipe( switchMap((job: BuildDto | undefined) => { // If existing build is currently active, return polling observable - if (activeBuildStates.includes(job?.state as BuildStates)) { + if (job != null && activeBuildStates.includes(job.state)) { return this.pollBuildProgress(buildConfig.projectId); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html index 9ca2fc9de17..4cc8900bf9e 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html @@ -1,39 +1,45 @@ -@if (entry != null) { -
-
- {{ i18n.enumerateList(bookNames) }} - {{ formatDate(entry.additionalInfo.dateFinished) }} -
- - - {{ getStatus(entry.state).icons }} - {{ getStatus(entry.state).text }} - - @if (!forceDetailsOpen) { - expand_{{ detailsOpen ? "less" : "more" }} - } -
- @if (detailsOpen) { -
-

Click a book below to preview the draft and add it to your project.

-

- @for (bookId of bookIds; track bookId) { - {{ i18n.localizeBook(bookId) }} - } -

- -

- - expand_{{ trainingDataOpen ? "less" : "more" }} - {{ trainingDataOpen ? "Hide" : "Show" }} model training data - -

- @if (trainingDataOpen) { -

- The language model was training on this configuration: - [TODO] -

+ + @if (entry != null) { +
+
+ {{ i18n.enumerateList(bookNames) }} + {{ formatDate(entry.additionalInfo?.dateFinished) }} +
+ + + {{ getStatus(entry.state).icons }} + {{ getStatus(entry.state).text }} + + @if (!forceDetailsOpen) { + expand_{{ detailsOpen ? "less" : "more" }} }
+ @if (detailsOpen) { +
+

{{ t("click_book_to_preview") }}

+

+ @for (bookId of bookIds; track bookId) { + {{ i18n.localizeBook(bookId) }} + } +

+ +

+ + expand_{{ trainingDataOpen ? "less" : "more" }} + {{ t(trainingDataOpen ? "hide_model_training_data" : "show_model_training_data") }} + +

+ @if (trainingDataOpen) { +

+ {{ t("training_data_configuration") }} + [TODO] +

+ } +
+ } } -} + diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts index a91b273dab6..c3dfe3fd8c4 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts @@ -1,16 +1,21 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; - +import { mock } from 'ts-mockito'; +import { I18nService } from 'xforge-common/i18n.service'; +import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; import { DraftHistoryEntryComponent } from './draft-history-entry.component'; +const mockedI18nService = mock(I18nService); + describe('DraftHistoryEntryComponent', () => { let component: DraftHistoryEntryComponent; let fixture: ComponentFixture; - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [DraftHistoryEntryComponent] - }).compileComponents(); + configureTestingModule(() => ({ + imports: [TestTranslocoModule], + providers: [{ provide: I18nService, useMock: mockedI18nService }] + })); + beforeEach(() => { fixture = TestBed.createComponent(DraftHistoryEntryComponent); component = fixture.componentInstance; fixture.detectChanges(); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts index ae06f19e02c..d02fdf28fe9 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -1,24 +1,11 @@ import { Component, Input } from '@angular/core'; import { MatButtonModule } from '@angular/material/button'; import { MatIconModule } from '@angular/material/icon'; -import { I18nService } from '../../../../../xforge-common/i18n.service'; +import { TranslocoModule } from '@ngneat/transloco'; +import { I18nService } from 'xforge-common/i18n.service'; +import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; -interface ScriptureRange { - projectId: string; - scriptureRange: string; -} - -export interface DraftHistoryEntry { - state: BuildStates; - id: string; - percentCompleted: number; - additionalInfo: { - translationScriptureRanges: ScriptureRange[]; - dateFinished: string; - }; -} - const STATUS_INFO: Record = { ACTIVE: { icons: 'hourglass_empty', text: 'Running', color: 'grey' }, COMPLETED: { icons: 'check_circle', text: 'Completed', color: 'green' }, @@ -32,12 +19,12 @@ const STATUS_INFO: Record r.scriptureRange.split(';'))) - ]; + if (this.entry?.additionalInfo == null) return []; + return [...new Set(this.entry.additionalInfo.translationScriptureRanges.flatMap(r => r.scriptureRange.split(';')))]; } get bookNames(): string[] { return this.bookIds.map(id => this.i18n.localizeBook(id)); } - formatDate(date: string): string { - return this.i18n.formatDate(new Date(date)); + formatDate(date?: string): string { + return date == null ? '' : this.i18n.formatDate(new Date(date)); } getStatus(state: BuildStates): { icons: string; text: string; color: string } { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html index b9285197ab3..cabc298a6e2 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html @@ -1,11 +1,13 @@ -@if (latestBuild != null) { -

{{ lastCompletedBuildMessage }}

- -} + + @if (latestBuild != null) { +

{{ lastCompletedBuildMessage }}

+ + } -@if (historicalBuilds.length > 0) { -

Previously generated drafts

- @for (entry of historicalBuilds; track entry.id) { - + @if (historicalBuilds.length > 0) { +

{{ t("previously_generated_drafts") }}

+ @for (entry of historicalBuilds; track entry.id) { + + } } -} + diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts new file mode 100644 index 00000000000..aee260d870b --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts @@ -0,0 +1,37 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { BehaviorSubject } from 'rxjs'; +import { mock, when } from 'ts-mockito'; +import { ActivatedProjectService } from 'xforge-common/activated-project.service'; +import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; +import { DraftGenerationService } from '../draft-generation.service'; +import { DraftHistoryListComponent } from './draft-history-list.component'; + +const mockActivatedProjectService: ActivatedProjectService = mock(ActivatedProjectService); +const mockDraftGenerationService: DraftGenerationService = mock(DraftGenerationService); + +describe('DraftHistoryListComponent', () => { + let component: DraftHistoryListComponent; + let fixture: ComponentFixture; + + configureTestingModule(() => ({ + imports: [TestTranslocoModule], + providers: [ + { provide: ActivatedProjectService, useMock: mockActivatedProjectService }, + { provide: DraftGenerationService, useMock: mockDraftGenerationService } + ] + })); + + beforeEach(() => { + const mockProjectId = 'project01'; + const mockProjectId$ = new BehaviorSubject(mockProjectId); + when(mockActivatedProjectService.projectId$).thenReturn(mockProjectId$); + + fixture = TestBed.createComponent(DraftHistoryListComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts index 2960f85fba9..3b8d731729c 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts @@ -1,54 +1,65 @@ import { Component } from '@angular/core'; import { MatIconModule } from '@angular/material/icon'; +import { TranslocoModule, TranslocoService } from '@ngneat/transloco'; import { take } from 'rxjs'; -import { ActivatedProjectService } from '../../../../xforge-common/activated-project.service'; -import { filterNullish } from '../../../../xforge-common/util/rxjs-util'; +import { ActivatedProjectService } from 'xforge-common/activated-project.service'; +import { filterNullish } from 'xforge-common/util/rxjs-util'; +import { BuildDto } from '../../../machine-api/build-dto'; +import { BuildStates } from '../../../machine-api/build-states'; import { activeBuildStates } from '../draft-generation'; import { DraftGenerationService } from '../draft-generation.service'; -import { DraftHistoryEntry, DraftHistoryEntryComponent } from './draft-history-entry/draft-history-entry.component'; +import { DraftHistoryEntryComponent } from './draft-history-entry/draft-history-entry.component'; @Component({ selector: 'app-draft-history-list', standalone: true, - imports: [MatIconModule, DraftHistoryEntryComponent], + imports: [MatIconModule, DraftHistoryEntryComponent, TranslocoModule], templateUrl: './draft-history-list.component.html', styleUrl: './draft-history-list.component.scss' }) export class DraftHistoryListComponent { - history?: DraftHistoryEntry[]; + history?: BuildDto[]; constructor( private readonly activatedProject: ActivatedProjectService, - private readonly draftGenerationService: DraftGenerationService + private readonly draftGenerationService: DraftGenerationService, + private readonly transloco: TranslocoService ) { this.activatedProject.projectId$.pipe(filterNullish(), take(1)).subscribe(projectId => { this.draftGenerationService.getBuildHistory(projectId).subscribe(result => { - this.history = result.reverse(); + this.history = result?.reverse() ?? []; }); }); } - get nonActiveBuilds(): DraftHistoryEntry[] { + get nonActiveBuilds(): BuildDto[] { return this.history?.filter(entry => !activeBuildStates.includes(entry.state)) || []; } - get latestBuild(): DraftHistoryEntry | undefined { + get latestBuild(): BuildDto | undefined { return this.isBuildActive ? undefined : this.nonActiveBuilds[0]; } get lastCompletedBuildMessage(): string { if (this.latestBuild == null) return ''; - const entry = this.latestBuild; - return ( - { - COMPLETED: 'Your draft is ready', - CANCELED: 'Your draft was cancelled', - FAULTED: 'The draft failed' - }[entry.state] || entry.state - ); + const state = this.latestBuild.state.toString(); + switch (state) { + case BuildStates.Canceled: + return this.transloco.translate('draft_history_list.draft_canceled'); + case BuildStates.Completed: + return this.transloco.translate('draft_history_list.draft_completed'); + case BuildStates.Faulted: + return this.transloco.translate('draft_history_list.draft_faulted'); + case BuildStates.Active: + case BuildStates.Finishing: + case BuildStates.Pending: + case BuildStates.Queued: + default: + return state.charAt(0).toUpperCase() + state.slice(1).toLowerCase(); + } } - get historicalBuilds(): DraftHistoryEntry[] { + get historicalBuilds(): BuildDto[] { return this.latestBuild == null ? this.nonActiveBuilds : this.nonActiveBuilds.slice(1); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts deleted file mode 100644 index 5065c096531..00000000000 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history.component.spec.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing'; - -import { DraftHistoryListComponent } from './draft-history.component'; - -describe('DraftHistoryComponent', () => { - let component: DraftHistoryListComponent; - let fixture: ComponentFixture; - - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [DraftHistoryListComponent] - }).compileComponents(); - - fixture = TestBed.createComponent(DraftHistoryListComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - }); - - it('should create', () => { - expect(component).toBeTruthy(); - }); -}); 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 d0417904490..0a9fe85270e 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 @@ -261,6 +261,19 @@ "use_echo_warning": "Enabling this will echo your source text instead of translating it.", "use_echo": "Use Echo Translation Engine" }, + "draft_history_entry": { + "click_book_to_preview": "Click a book below to preview the draft and add it to your project.", + "download_zip": "Download draft as ZIP file", + "hide_model_training_data": "Hide model training data", + "show_model_training_data": "Show model training data", + "training_data_configuration": "The language model was trained on this configuration" + }, + "draft_history_list": { + "draft_canceled": "Your draft was canceled", + "draft_completed": "Your draft is ready", + "drafT_faulted": "The draft failed", + "previously_generated_drafts": "Previously generated drafts" + }, "draft_preview_books": { "add_to_project": "Add to project", "add_to_different_project": "Add to a different project", From 6f5a6a99fef1d830acff52f0f56210027de477f2 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Wed, 16 Apr 2025 14:00:28 +1200 Subject: [PATCH 03/11] Update get builds API --- .../src/app/machine-api/build-dto.ts | 2 ++ .../draft-history-entry.component.spec.ts | 16 +++++++-- .../draft-history-entry.component.ts | 36 +++++++++++++++++-- .../Models/ServalBuildAdditionalInfo.cs | 2 ++ .../Services/MachineApiService.cs | 6 +++- .../Services/MachineApiServiceTests.cs | 5 +++ 6 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts index ec7b20fde79..571b8635d71 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts @@ -16,9 +16,11 @@ export interface ServalBuildAdditionalInfo { buildId: string; corporaIds?: string[]; dateFinished?: string; + dateRequested?: string; parallelCorporaIds?: string[]; step: number; trainingScriptureRanges: ProjectScriptureRange[]; translationEngineId: string; translationScriptureRanges: ProjectScriptureRange[]; + requestedByUserId?: string; } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts index c3dfe3fd8c4..5e1fbc1fdd8 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts @@ -1,10 +1,15 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { mock } from 'ts-mockito'; +import { UserProfile } from 'realtime-server/lib/esm/common/models/user'; +import { createTestUserProfile } from 'realtime-server/lib/esm/common/models/user-test-data'; +import { anything, mock, when } from 'ts-mockito'; import { I18nService } from 'xforge-common/i18n.service'; +import { UserProfileDoc } from 'xforge-common/models/user-profile-doc'; import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; +import { UserService } from 'xforge-common/user.service'; import { DraftHistoryEntryComponent } from './draft-history-entry.component'; const mockedI18nService = mock(I18nService); +const mockedUserService = mock(UserService); describe('DraftHistoryEntryComponent', () => { let component: DraftHistoryEntryComponent; @@ -12,10 +17,17 @@ describe('DraftHistoryEntryComponent', () => { configureTestingModule(() => ({ imports: [TestTranslocoModule], - providers: [{ provide: I18nService, useMock: mockedI18nService }] + providers: [ + { provide: I18nService, useMock: mockedI18nService }, + { provide: UserService, useMock: mockedUserService } + ] })); beforeEach(() => { + const user: UserProfile = createTestUserProfile(); + const userDoc = { id: 'sf-user-id', data: user } as UserProfileDoc; + when(mockedUserService.getProfile(anything())).thenResolve(userDoc); + fixture = TestBed.createComponent(DraftHistoryEntryComponent); component = fixture.componentInstance; fixture.detectChanges(); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts index d02fdf28fe9..1dc635cfbcd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -3,6 +3,7 @@ import { MatButtonModule } from '@angular/material/button'; import { MatIconModule } from '@angular/material/icon'; import { TranslocoModule } from '@ngneat/transloco'; import { I18nService } from 'xforge-common/i18n.service'; +import { UserService } from 'xforge-common/user.service'; import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; @@ -24,7 +25,24 @@ const STATUS_INFO: Record { + if (user.data != null) { + this._buildRequestedByUserName = user.data.displayName; + } + }); + } + } + get entry(): BuildDto | undefined { + return this._entry; + } + private _forceDetailsOpen = false; @Input() set forceDetailsOpen(value: boolean) { this._forceDetailsOpen = value; @@ -33,10 +51,24 @@ export class DraftHistoryEntryComponent { get forceDetailsOpen(): boolean { return this._forceDetailsOpen; } + + private _buildRequestedByUserName: string | undefined; + get buildRequestedByUserName(): string | undefined { + return this._buildRequestedByUserName; + } + + get buildRequestedByDate(): string { + if (this._entry?.additionalInfo?.dateRequested == null) return ''; + return this.i18n.formatDate(new Date(this._entry?.additionalInfo?.dateRequested)); + } + detailsOpen = false; trainingDataOpen = false; - constructor(readonly i18n: I18nService) {} + constructor( + readonly i18n: I18nService, + private readonly userService: UserService + ) {} get bookIds(): string[] { if (this.entry?.additionalInfo == null) return []; diff --git a/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs b/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs index 6e6653b24ba..2d50e3c6c3f 100644 --- a/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs +++ b/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs @@ -8,7 +8,9 @@ public class ServalBuildAdditionalInfo public string BuildId { get; init; } = string.Empty; public IEnumerable? CorporaIds { get; init; } public DateTimeOffset? DateFinished { get; init; } + public DateTimeOffset? DateRequested { get; set; } public IEnumerable? ParallelCorporaIds { get; init; } + public string? RequestedByUserId { get; set; } public int Step { get; init; } public HashSet TrainingScriptureRanges { get; init; } = []; public string TranslationEngineId { get; init; } = string.Empty; diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index 7776273f5a5..dd9b6ace65b 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -362,7 +362,7 @@ [EnumeratorCancellation] CancellationToken cancellationToken EventMetric eventMetric = eventMetrics.Results.LastOrDefault(e => e.EventType == nameof(StartPreTranslationBuildAsync) ); - if (eventMetric is not null) + if (preTranslate && eventMetric is not null) { queuedState = UpdateDto(queuedState, eventMetric); } @@ -1615,6 +1615,10 @@ private static ServalBuildDto UpdateDto(ServalBuildDto buildDto, EventMetric eve // Ensure that there is the Serval additional data buildDto.AdditionalInfo ??= new ServalBuildAdditionalInfo(); + // Set the user who requested the build and when they did so + buildDto.AdditionalInfo.DateRequested = new DateTimeOffset(eventMetric.TimeStamp, TimeSpan.Zero); + buildDto.AdditionalInfo.RequestedByUserId = eventMetric.UserId; + // Retrieve the training and translation books from the build config, as the build from Serval // will not have project information. diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index a193a045b2c..e07d2c0e10b 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -627,6 +627,7 @@ public async Task GetBuildsAsync_QueuedStateWithEventMetric() var env = new TestEnvironment(); const string trainingScriptureRange = "GEN;EXO"; const string translationScriptureRange = "LEV;NUM"; + DateTime requestedDateTime = DateTime.UtcNow; await env.QueueBuildAsync(preTranslate: true); env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None) .Returns(Task.FromResult>([])); @@ -658,6 +659,8 @@ public async Task GetBuildsAsync_QueuedStateWithEventMetric() }, ProjectId = Project01, Scope = EventScope.Drafting, + TimeStamp = requestedDateTime, + UserId = User01, }, ], UnpagedCount = 1, @@ -683,6 +686,8 @@ ServalBuildDto build in env.Service.GetBuildsAsync( Assert.AreEqual(1, builds.Count); Assert.AreEqual(MachineApiService.BuildStateQueued, builds[0].State); Assert.AreEqual(Project01, builds[0].Id); + Assert.AreEqual(new DateTimeOffset(requestedDateTime, TimeSpan.Zero), builds[0].AdditionalInfo?.DateRequested); + Assert.AreEqual(User01, builds[0].AdditionalInfo?.RequestedByUserId); Assert.IsEmpty(builds[0].AdditionalInfo?.TrainingScriptureRanges.First().ProjectId); Assert.AreEqual( trainingScriptureRange, From 22223fd82b09d5d0ba7a322bebda578e5b58a680 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Thu, 17 Apr 2025 08:23:13 +1200 Subject: [PATCH 04/11] Record when pre-translations are downloaded to SF --- .../event-metrics-log.component.ts | 7 +- .../src/app/machine-api/build-dto.ts | 1 + .../draft-download-button.component.html | 26 ++++ .../draft-download-button.component.scss | 0 .../draft-download-button.component.spec.ts | 129 ++++++++++++++++++ .../draft-download-button.component.ts | 54 ++++++++ .../draft-generation.component.html | 23 +--- .../draft-generation.component.spec.ts | 85 +----------- .../draft-generation.component.ts | 29 +--- .../draft-generation.service.spec.ts | 62 ++++++++- .../draft-generation.service.ts | 54 +++++--- .../draft-history-entry.component.html | 18 +-- .../draft-history-entry.component.ts | 9 +- .../draft-history-list.component.html | 6 +- .../Controllers/MachineApiController.cs | 2 +- .../Controllers/SFProjectsRpcController.cs | 2 +- .../Models/ServalBuildAdditionalInfo.cs | 1 + .../Services/IMachineApiService.cs | 3 +- .../Services/MachineApiService.cs | 84 +++++++++--- .../Services/MachineApiServiceTests.cs | 10 +- 20 files changed, 415 insertions(+), 190 deletions(-) create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.scss create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.spec.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.ts index 3c91bf95b83..2ae8eb4fa8f 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.ts @@ -159,15 +159,18 @@ export class EventMetricsLogComponent extends DataLoadingComponent implements On // I have not localized at this time because these strings are likely to change based on feedback. // When this feature is mature, these should be localized to help Project Administrators. const eventTypeMap: { [key: string]: string } = { + BuildProjectAsync: 'Start draft generation on Serval', CancelPreTranslationBuildAsync: 'Cancel draft generation', CancelSyncAsync: 'Cancel synchronization with Paratext', + RetrievePreTranslationStatusAsync: 'Save drafts to Scripture Forge', SetDraftAppliedAsync: "Updated the chapter's draft applied status", SetIsValidAsync: 'Marked chapter as valid/invalid', SetPreTranslateAsync: 'Set drafting as enabled/disabled for the project', SetServalConfigAsync: 'Manually update drafting configuration for the project', StartBuildAsync: 'Begin training translation suggestions', - StartPreTranslationBuildAsync: 'Start draft generation', - SyncAsync: 'Start synchronization with Paratext' + StartPreTranslationBuildAsync: 'Request draft generation', + SyncAsync: 'Start synchronization with Paratext', + UpdateSettingsAsync: 'Update Scripture Forge settings' }; // Allow specific cases based on payload values diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts index 571b8635d71..c5c71a71d47 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts @@ -17,6 +17,7 @@ export interface ServalBuildAdditionalInfo { corporaIds?: string[]; dateFinished?: string; dateRequested?: string; + dateGenerated?: string; parallelCorporaIds?: string[]; step: number; trainingScriptureRanges: ProjectScriptureRange[]; diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html new file mode 100644 index 00000000000..217dc2e7dda --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html @@ -0,0 +1,26 @@ + + + diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.scss new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.spec.ts new file mode 100644 index 00000000000..c8ed50f868a --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.spec.ts @@ -0,0 +1,129 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; +import { of, throwError } from 'rxjs'; +import { anything, mock, verify, when } from 'ts-mockito'; +import { ActivatedProjectService } from 'xforge-common/activated-project.service'; +import { NoticeService } from 'xforge-common/notice.service'; +import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; +import { SFProjectProfileDoc } from '../../../core/models/sf-project-profile-doc'; +import { DraftGenerationService } from '../draft-generation.service'; +import { DraftDownloadButtonComponent } from './draft-download-button.component'; + +const mockActivatedProjectService: ActivatedProjectService = mock(ActivatedProjectService); +const mockDraftGenerationService: DraftGenerationService = mock(DraftGenerationService); +const mockNoticeService: NoticeService = mock(NoticeService); + +describe('DraftDownloadButtonComponent', () => { + configureTestingModule(() => ({ + imports: [TestTranslocoModule], + providers: [ + { provide: ActivatedProjectService, useMock: mockActivatedProjectService }, + { provide: DraftGenerationService, useMock: mockDraftGenerationService }, + { provide: NoticeService, useMock: mockNoticeService } + ] + })); + + it('should create', () => { + const env = new TestEnvironment(); + expect(env.component).toBeTruthy(); + }); + + describe('downloadProgress', () => { + it('should show number between 0 and 100', () => { + const env = new TestEnvironment(); + env.component.downloadBooksProgress = 4; + env.component.downloadBooksTotal = 8; + expect(env.component.downloadProgress).toBe(50); + }); + + it('should not divide by zero', () => { + const env = new TestEnvironment(); + env.component.downloadBooksProgress = 4; + env.component.downloadBooksTotal = 0; + expect(env.component.downloadProgress).toBe(0); + }); + }); + + describe('download draft button', () => { + it('button should start the download', () => { + const env = new TestEnvironment(); + spyOn(env.component, 'downloadDraft').and.stub(); + env.fixture.detectChanges(); + + env.downloadButton!.click(); + expect(env.component.downloadDraft).toHaveBeenCalled(); + }); + + it('spinner should display while the download is in progress', () => { + const env = new TestEnvironment(); + env.component.downloadBooksProgress = 2; + env.component.downloadBooksTotal = 4; + env.fixture.detectChanges(); + + expect(env.downloadSpinner).not.toBeNull(); + }); + + it('spinner should not display while no download is in progress', () => { + const env = new TestEnvironment(); + env.component.downloadBooksProgress = 0; + env.component.downloadBooksTotal = 0; + env.fixture.detectChanges(); + + expect(env.downloadSpinner).toBeNull(); + }); + }); + + describe('downloadDraft', () => { + it('should display an error if one occurs', () => { + const env = new TestEnvironment(); + when(mockDraftGenerationService.downloadGeneratedDraftZip(anything(), anything())).thenReturn( + throwError(() => new Error()) + ); + + env.component.downloadDraft(); + expect(env.component.downloadBooksProgress).toBe(0); + expect(env.component.downloadBooksTotal).toBe(0); + verify(mockNoticeService.showError(anything())).once(); + }); + + it('should emit draft progress', () => { + const env = new TestEnvironment(); + when(mockDraftGenerationService.downloadGeneratedDraftZip(anything(), anything())).thenReturn( + of({ + current: 1, + total: 2 + }) + ); + + env.component.downloadDraft(); + expect(env.component.downloadBooksProgress).toBe(1); + expect(env.component.downloadBooksTotal).toBe(2); + }); + }); + + class TestEnvironment { + component: DraftDownloadButtonComponent; + fixture: ComponentFixture; + + constructor() { + const mockProjectDoc = { id: 'project01', data: createTestProjectProfile() } as SFProjectProfileDoc; + when(mockActivatedProjectService.projectDoc).thenReturn(mockProjectDoc); + + this.fixture = TestBed.createComponent(DraftDownloadButtonComponent); + this.component = this.fixture.componentInstance; + this.fixture.detectChanges(); + } + + get downloadButton(): HTMLElement | null { + return this.getElementByTestId('download-button'); + } + + get downloadSpinner(): HTMLElement | null { + return this.getElementByTestId('download-spinner'); + } + + getElementByTestId(testId: string): HTMLElement | null { + return this.fixture.nativeElement.querySelector(`[data-test-id="${testId}"]`); + } + } +}); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts new file mode 100644 index 00000000000..98a5e74780d --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts @@ -0,0 +1,54 @@ +import { CommonModule } from '@angular/common'; +import { Component, Input } from '@angular/core'; +import { TranslocoModule } from '@ngneat/transloco'; +import { Subscription } from 'rxjs'; +import { ActivatedProjectService } from 'xforge-common/activated-project.service'; +import { NoticeService } from 'xforge-common/notice.service'; +import { UICommonModule } from 'xforge-common/ui-common.module'; +import { BuildDto } from '../../../machine-api/build-dto'; +import { DraftZipProgress } from '../draft-generation'; +import { DraftGenerationService } from '../draft-generation.service'; + +@Component({ + selector: 'app-draft-download-button', + templateUrl: './draft-download-button.component.html', + styleUrls: ['./draft-download-button.component.scss'], + standalone: true, + imports: [CommonModule, TranslocoModule, UICommonModule] +}) +export class DraftDownloadButtonComponent { + /** + * Tracks how many books have been downloaded for the zip file. + */ + downloadBooksProgress: number = 0; + downloadBooksTotal: number = 0; + + zipSubscription?: Subscription; + + @Input() build: BuildDto | undefined; + @Input() flat: boolean = false; + + constructor( + private readonly activatedProject: ActivatedProjectService, + private readonly draftGenerationService: DraftGenerationService, + private readonly noticeService: NoticeService + ) {} + + get downloadProgress(): number { + if (this.downloadBooksTotal === 0) return 0; + return (this.downloadBooksProgress / this.downloadBooksTotal) * 100; + } + + downloadDraft(): void { + this.zipSubscription?.unsubscribe(); + this.zipSubscription = this.draftGenerationService + .downloadGeneratedDraftZip(this.activatedProject.projectDoc, this.build) + .subscribe({ + next: (draftZipProgress: DraftZipProgress) => { + this.downloadBooksProgress = draftZipProgress.current; + this.downloadBooksTotal = draftZipProgress.total; + }, + error: (error: Error) => this.noticeService.showError(error.message) + }); + } +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html index 91020417e94..38594c77c92 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html @@ -278,28 +278,7 @@

{{ t("draft_finishing_header") }}

@if (hasDraftBooksAvailable) { - + } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts index 8cd1ef6e25b..25a5602feda 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts @@ -214,11 +214,7 @@ describe('DraftGenerationComponent', () => { } get downloadButton(): HTMLElement | null { - return this.getElementByTestId('download-button'); - } - - get downloadSpinner(): HTMLElement | null { - return this.getElementByTestId('download-spinner'); + return (this.fixture.nativeElement as HTMLElement).querySelector('app-draft-download-button'); } get improvedDraftGenerationNotice(): HTMLElement | null { @@ -298,7 +294,7 @@ describe('DraftGenerationComponent', () => { describe('Improved draft generation notice', () => { // Do not run this test after April 1 2025 - const shouldRunTest = new Date() < new Date('2025-04-01'); + const shouldRunTest = new Date() < new Date('2025-05-31'); (shouldRunTest ? it : xit)( 'should show notice if back translation or pre-translate approved', fakeAsync(() => { @@ -1399,22 +1395,6 @@ describe('DraftGenerationComponent', () => { }); }); - describe('downloadProgress', () => { - it('should show number between 0 and 100', () => { - const env = new TestEnvironment(); - env.component.downloadBooksProgress = 4; - env.component.downloadBooksTotal = 8; - expect(env.component.downloadProgress).toBe(50); - }); - - it('should not divide by zero', () => { - const env = new TestEnvironment(); - env.component.downloadBooksProgress = 4; - env.component.downloadBooksTotal = 0; - expect(env.component.downloadProgress).toBe(0); - }); - }); - describe('download draft button', () => { it('button should display if there are draft books available', () => { const env = new TestEnvironment(); @@ -1512,66 +1492,5 @@ describe('DraftGenerationComponent', () => { // Verify the button is visible expect(env.downloadButton).not.toBeNull(); })); - - it('button should start the download', () => { - const env = new TestEnvironment(); - spyOn(env.component, 'downloadDraft').and.stub(); - env.component.draftJob = { ...buildDto, state: BuildStates.Faulted }; - env.component.hasDraftBooksAvailable = true; - env.fixture.detectChanges(); - - env.downloadButton!.click(); - expect(env.component.downloadDraft).toHaveBeenCalled(); - }); - - it('button should not display if there are no draft books available', () => { - const env = new TestEnvironment(); - env.component.draftJob = { ...buildDto, state: BuildStates.Faulted }; - env.component.hasDraftBooksAvailable = false; - env.fixture.detectChanges(); - - expect(env.downloadButton).toBeNull(); - }); - - it('spinner should display while the download is in progress', () => { - const env = new TestEnvironment(); - env.component.draftJob = { ...buildDto, state: BuildStates.Faulted }; - env.component.hasDraftBooksAvailable = true; - env.component.downloadBooksProgress = 2; - env.component.downloadBooksTotal = 4; - env.fixture.detectChanges(); - - expect(env.downloadSpinner).not.toBeNull(); - }); - - it('spinner should not display while no download is in progress', () => { - const env = new TestEnvironment(); - env.component.draftJob = { ...buildDto, state: BuildStates.Faulted }; - env.component.hasDraftBooksAvailable = true; - env.component.downloadBooksProgress = 0; - env.component.downloadBooksTotal = 0; - env.fixture.detectChanges(); - - expect(env.downloadSpinner).toBeNull(); - }); - }); - - describe('downloadDraft', () => { - it('should display an error if one occurs', () => { - const env = new TestEnvironment(); - mockDraftGenerationService.downloadGeneratedDraftZip.and.returnValue(throwError(() => new Error())); - - env.component.downloadDraft(); - expect(mockNoticeService.showError).toHaveBeenCalledTimes(1); - }); - - it('should emit draft progress', () => { - const env = new TestEnvironment(); - mockDraftGenerationService.downloadGeneratedDraftZip.and.returnValue(of({ current: 1, total: 2 })); - - env.component.downloadDraft(); - expect(env.component.downloadBooksProgress).toBe(1); - expect(env.component.downloadBooksTotal).toBe(2); - }); }); }); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts index 4daef7b62b2..56a0dfa2a05 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts @@ -32,7 +32,8 @@ import { ServalProjectComponent } from '../../serval-administration/serval-proje import { SharedModule } from '../../shared/shared.module'; import { WorkingAnimatedIndicatorComponent } from '../../shared/working-animated-indicator/working-animated-indicator.component'; import { NllbLanguageService } from '../nllb-language.service'; -import { activeBuildStates, BuildConfig, DraftZipProgress } from './draft-generation'; +import { DraftDownloadButtonComponent } from './draft-download-button/draft-download-button.component'; +import { activeBuildStates, BuildConfig } from './draft-generation'; import { DraftGenerationStepsComponent, DraftGenerationStepsResult @@ -61,6 +62,7 @@ import { SupportedBackTranslationLanguagesDialogComponent } from './supported-ba DraftGenerationStepsComponent, DraftInformationComponent, ServalProjectComponent, + DraftDownloadButtonComponent, DraftPreviewBooksComponent, DraftHistoryListComponent ] @@ -86,7 +88,6 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On additionalTrainingSource?: DraftSource; jobSubscription?: Subscription; - zipSubscription?: Subscription; isOnline = true; currentPage: 'initial' | 'steps' = 'initial'; @@ -110,12 +111,6 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On */ hasDraftBooksAvailable = false; - /** - * Tracks how many books have been downloaded for the zip file. - */ - downloadBooksProgress: number = 0; - downloadBooksTotal: number = 0; - isPreTranslationApproved = false; signupFormUrl?: string; @@ -162,11 +157,6 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On super(noticeService); } - get downloadProgress(): number { - if (this.downloadBooksTotal === 0) return 0; - return (this.downloadBooksProgress / this.downloadBooksTotal) * 100; - } - get hasAnyCompletedBuild(): boolean { return this.lastCompletedBuild != null; } @@ -314,19 +304,6 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On this.currentPage = 'steps'; } - downloadDraft(): void { - this.zipSubscription?.unsubscribe(); - this.zipSubscription = this.draftGenerationService - .downloadGeneratedDraftZip(this.activatedProject.projectDoc, this.lastCompletedBuild) - .subscribe({ - next: (draftZipProgress: DraftZipProgress) => { - this.downloadBooksProgress = draftZipProgress.current; - this.downloadBooksTotal = draftZipProgress.total; - }, - error: (error: Error) => this.noticeService.showError(error.message) - }); - } - async cancel(): Promise { const { dialogRef, result } = this.dialogService.openGenericDialog({ title: this.i18n.translate('draft_generation.dialog_confirm_draft_cancellation_title'), 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 421de42e5fc..e5619b78f5f 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 @@ -460,13 +460,13 @@ describe('DraftGenerationService', () => { }); describe('getGeneratedDraftUsfm', () => { - it('should get the pre-translation USFM for the specified book/chapter and return an observable', fakeAsync(() => { + it('should get USFM for the specified book/chapter without a timestamp and return an observable', fakeAsync(() => { const book = 43; const chapter = 3; const usfm = '\\id Test USFM \\c 1 \\v 1 Test'; // SUT - service.getGeneratedDraftUsfm(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftUsfm(projectId, book, chapter, undefined).subscribe(result => { expect(result).toEqual(usfm); }); tick(); @@ -480,12 +480,33 @@ describe('DraftGenerationService', () => { tick(); })); + it('should get USFM for the specified book/chapter with a timestamp and return an observable', fakeAsync(() => { + const book = 43; + const chapter = 3; + const usfm = '\\id Test USFM \\c 1 \\v 1 Test'; + const date = new Date(); + + // SUT + service.getGeneratedDraftUsfm(projectId, book, chapter, date).subscribe(result => { + expect(result).toEqual(usfm); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/usfm?timestamp=${date.toISOString()}` + ); + expect(req.request.method).toEqual('GET'); + req.flush(usfm); + tick(); + })); + it('should return undefined for a 404 error', fakeAsync(() => { const book = 43; const chapter = 3; // SUT - service.getGeneratedDraftUsfm(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftUsfm(projectId, book, chapter, undefined).subscribe(result => { expect(result).toBeUndefined(); }); tick(); @@ -505,7 +526,7 @@ describe('DraftGenerationService', () => { testOnlineStatusService.setIsOnline(false); // SUT - service.getGeneratedDraftUsfm(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftUsfm(projectId, book, chapter, undefined).subscribe(result => { expect(result).toBeUndefined(); }); tick(); @@ -587,7 +608,7 @@ describe('DraftGenerationService', () => { }); }); - it('should create a zip file containing all of the books with drafts', fakeAsync(() => { + it('should create a zip file containing all of the books with drafts without a generated date', fakeAsync(() => { const projectDoc: SFProjectProfileDoc = { id: projectId, data: createTestProjectProfile({ @@ -631,5 +652,36 @@ describe('DraftGenerationService', () => { req2jn.flush(usfm); tick(); })); + + it('should create a zip file containing all of the books with drafts at the generated date', fakeAsync(() => { + const projectDoc: SFProjectProfileDoc = { + id: projectId, + data: createTestProjectProfile({ + texts: [{ bookNum: 62, chapters: [{ number: 1, hasDraft: true }] }] + }) + } as SFProjectProfileDoc; + const lastCompletedBuild: BuildDto = { + additionalInfo: { + dateFinished: '2024-08-27T00:00:00.000+00:00', + dateGenerated: '2024-08-27T01:02:03.004+00:00' + } + } as BuildDto; + + service.downloadGeneratedDraftZip(projectDoc, lastCompletedBuild).subscribe({ + complete: () => { + expect(saveAs).toHaveBeenCalled(); + } + }); + tick(); + + // Setup the HTTP request for 1 John + const usfm = '\\id Test USFM \\c 1 \\v 1 Test'; + const req1jn = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/62_0/usfm?timestamp=2024-08-27T01:02:03.004Z` + ); + expect(req1jn.request.method).toEqual('GET'); + req1jn.flush(usfm); + tick(); + })); }); }); 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 a11de259f57..a4bb3b48110 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 @@ -228,25 +228,33 @@ export class DraftGenerationService { } /** - * Gets the pre-translation USFM for the specified book/chapter using the last completed build. + * Gets the pre-translation USFM for the specified book/chapter. * @param projectId The SF project id for the target translation. * @param book The book number. * @param chapter The chapter number. Specify 0 to return all chapters in the book. + * @param timestamp The timestamp to download the draft at. If undefined, the latest draft will be downloaded. * @returns An observable string of USFM data, or undefined if no pre-translations exist. */ - getGeneratedDraftUsfm(projectId: string, book: number, chapter: number): Observable { + getGeneratedDraftUsfm( + projectId: string, + book: number, + chapter: number, + timestamp: Date | undefined + ): Observable { if (!this.onlineStatusService.isOnline) { return of(undefined); } - return this.httpClient - .get(`translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/usfm`) - .pipe( - map(res => res.data), - catchError(() => { - // If no USFM could be retrieved, return undefined - return of(undefined); - }) - ); + let url = `translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/usfm`; + if (timestamp != null) { + url += `?timestamp=${timestamp.toISOString()}`; + } + return this.httpClient.get(url).pipe( + map(res => res.data), + catchError(() => { + // If no USFM could be retrieved, return undefined + return of(undefined); + }) + ); } /** @@ -279,17 +287,25 @@ export class DraftGenerationService { const zipProgress: DraftZipProgress = { current: 0, total: books.length }; observer.next(zipProgress); + // Get the date the draft was generated and written to Scripture Forge + let dateGenerated: Date | undefined = undefined; + if (lastCompletedBuild?.additionalInfo?.dateGenerated != null) { + dateGenerated = new Date(lastCompletedBuild.additionalInfo.dateGenerated); + } + // Create the promises to download each book's USFM for (const bookNum of books) { - const usfmFile = firstValueFrom(this.getGeneratedDraftUsfm(projectDoc.id, bookNum, 0)).then(usfm => { - if (usfm != null) { - const fileName: string = - getBookFileNameDigits(bookNum) + Canon.bookNumberToId(bookNum) + projectShortName + '.SFM'; - zip.file(fileName, usfm); - zipProgress.current++; - observer.next(zipProgress); + const usfmFile = firstValueFrom(this.getGeneratedDraftUsfm(projectDoc.id, bookNum, 0, dateGenerated)).then( + usfm => { + if (usfm != null) { + const fileName: string = + getBookFileNameDigits(bookNum) + Canon.bookNumberToId(bookNum) + projectShortName + '.SFM'; + zip.file(fileName, usfm); + zipProgress.current++; + observer.next(zipProgress); + } } - }); + ); usfmFiles.push(usfmFile); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html index 4cc8900bf9e..19fca9d69d2 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html @@ -16,16 +16,18 @@
@if (detailsOpen) {
-

{{ t("click_book_to_preview") }}

-

- @for (bookId of bookIds; track bookId) { - {{ i18n.localizeBook(bookId) }} - } -

- + +

{{ t("click_book_to_preview") }}

+

+ @for (bookId of bookIds; track bookId) { + {{ i18n.localizeBook(bookId) }} + } +

+ +

diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts index 1dc635cfbcd..b3b12a08356 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -1,3 +1,4 @@ +import { CommonModule } from '@angular/common'; import { Component, Input } from '@angular/core'; import { MatButtonModule } from '@angular/material/button'; import { MatIconModule } from '@angular/material/icon'; @@ -6,6 +7,7 @@ import { I18nService } from 'xforge-common/i18n.service'; import { UserService } from 'xforge-common/user.service'; import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; +import { DraftDownloadButtonComponent } from '../../draft-download-button/draft-download-button.component'; const STATUS_INFO: Record = { ACTIVE: { icons: 'hourglass_empty', text: 'Running', color: 'grey' }, @@ -20,7 +22,7 @@ const STATUS_INFO: Record @if (latestBuild != null) {

{{ lastCompletedBuildMessage }}

- + } @if (historicalBuilds.length > 0) { diff --git a/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs b/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs index 3811800565f..c99155fb1c8 100644 --- a/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs +++ b/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs @@ -284,7 +284,7 @@ CancellationToken cancellationToken cancellationToken ); - // A null means no build is running + // A null means no build has run if (build is null) { return NoContent(); diff --git a/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs b/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs index a3e61ee3a96..db30e20a55d 100644 --- a/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs +++ b/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs @@ -708,7 +708,7 @@ public IRpcMethodResult RetrievePreTranslationStatus(string projectId) try { // Run the background job - backgroundJobClient.Enqueue(r => + backgroundJobClient.Enqueue(r => r.RetrievePreTranslationStatusAsync(projectId, CancellationToken.None) ); return Ok(); diff --git a/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs b/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs index 2d50e3c6c3f..8d6173655de 100644 --- a/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs +++ b/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs @@ -9,6 +9,7 @@ public class ServalBuildAdditionalInfo public IEnumerable? CorporaIds { get; init; } public DateTimeOffset? DateFinished { get; init; } public DateTimeOffset? DateRequested { get; set; } + public DateTimeOffset? DateGenerated { get; set; } public IEnumerable? ParallelCorporaIds { get; init; } public string? RequestedByUserId { get; set; } public int Step { get; init; } diff --git a/src/SIL.XForge.Scripture/Services/IMachineApiService.cs b/src/SIL.XForge.Scripture/Services/IMachineApiService.cs index 89cc88f4b1f..9850e48708a 100644 --- a/src/SIL.XForge.Scripture/Services/IMachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/IMachineApiService.cs @@ -119,7 +119,8 @@ CancellationToken cancellationToken Task IsLanguageSupportedAsync(string languageCode, CancellationToken cancellationToken); [Mutex] - Task RetrievePreTranslationStatusAsync(string sfProjectId, CancellationToken cancellationToken); + [LogEventMetric(EventScope.Drafting, projectId: nameof(sfProjectId), captureReturnValue: true)] + Task RetrievePreTranslationStatusAsync(string sfProjectId, CancellationToken cancellationToken); [LogEventMetric(EventScope.Drafting, nameof(curUserId), nameof(sfProjectId))] Task StartBuildAsync(string curUserId, string sfProjectId, CancellationToken cancellationToken); diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index dd9b6ace65b..01c6c20137f 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -119,7 +119,7 @@ await projectSecrets.UpdateAsync( } // Get the translation engine id - string translationEngineId = await GetTranslationIdAsync(sfProjectId, preTranslate: true); + string translationEngineId = GetTranslationId(projectSecret, preTranslate: true); try { @@ -209,7 +209,7 @@ public async Task ExecuteWebhookAsync(string json, string signature) var arguments = new Dictionary { { "buildId", delivery.Payload.Build.Id }, - { "buildStart", delivery.Payload.BuildState }, + { "buildState", delivery.Payload.BuildState }, { "event", delivery.Event }, { "translationEngineId", delivery.Payload.Engine.Id }, }; @@ -306,7 +306,12 @@ [EnumeratorCancellation] CancellationToken cancellationToken eventMetrics = await eventMetricService.GetEventMetricsAsync( sfProjectId, scopes: [EventScope.Drafting], - eventTypes: [nameof(MachineProjectService.BuildProjectAsync), nameof(StartPreTranslationBuildAsync)] + eventTypes: + [ + nameof(MachineProjectService.BuildProjectAsync), + nameof(RetrievePreTranslationStatusAsync), + nameof(StartPreTranslationBuildAsync), + ] ); } @@ -315,8 +320,17 @@ [EnumeratorCancellation] CancellationToken cancellationToken { ServalBuildDto buildDto = CreateDto(translationBuild); - // If we have event metrics, add the scripture ranges to the DTO + // See if we have event metrics for downloading the pre-translation USFM to Scripture Forge EventMetric eventMetric = eventMetrics.Results.FirstOrDefault(e => + e.Result == translationBuild.Id && e.EventType == nameof(RetrievePreTranslationStatusAsync) + ); + if (eventMetric is not null) + { + buildDto.AdditionalInfo!.DateGenerated = new DateTimeOffset(eventMetric.TimeStamp, TimeSpan.Zero); + } + + // If we have event metrics for sending the build to Serval, add the scripture ranges to the DTO + eventMetric = eventMetrics.Results.FirstOrDefault(e => e.Result == translationBuild.Id && e.EventType == nameof(MachineProjectService.BuildProjectAsync) ); if (eventMetric is not null) @@ -337,7 +351,7 @@ [EnumeratorCancellation] CancellationToken cancellationToken #pragma warning restore CS0612 // Type or member is obsolete if (!string.IsNullOrWhiteSpace(scriptureRange)) { - buildDto.AdditionalInfo?.TranslationScriptureRanges.Add( + buildDto.AdditionalInfo!.TranslationScriptureRanges.Add( new ProjectScriptureRange { ProjectId = sfProjectId, ScriptureRange = scriptureRange } ); } @@ -998,7 +1012,17 @@ public async Task IsLanguageSupportedAsync(string languageCode, Can }; } - public async Task RetrievePreTranslationStatusAsync(string sfProjectId, CancellationToken cancellationToken) + /// + /// Retrieves the pre-translation status and text from Serval. + /// + /// The Scripture Forge project identifier. + /// The cancellation token. + /// The id of the build the pre-translations were retrieved for. + /// We return the build id so it can be logged in event metrics. + public async Task RetrievePreTranslationStatusAsync( + string sfProjectId, + CancellationToken cancellationToken + ) { try { @@ -1017,6 +1041,14 @@ public async Task RetrievePreTranslationStatusAsync(string sfProjectId, Cancella // False means another process is running this function. if (projectSecret.ServalData?.PreTranslationsRetrieved ?? true) { + // Get the last completed build + string translationEngineId = GetTranslationId(projectSecret, preTranslate: true); + TranslationBuild? translationBuild = ( + await translationEnginesClient.GetAllBuildsAsync(translationEngineId, cancellationToken) + ) + .Where(b => b.State == JobState.Completed) + .MaxBy(b => b.DateFinished); + // Set the retrieved flag as in progress await projectSecrets.UpdateAsync( sfProjectId, @@ -1034,6 +1066,9 @@ await projectSecrets.UpdateAsync( sfProjectId, u => u.Set(p => p.ServalData.PreTranslationsRetrieved, true) ); + + // Return the build id + return translationBuild?.Id; } } catch (TaskCanceledException e) when (e.InnerException is not TimeoutException) @@ -1055,6 +1090,8 @@ await projectSecrets.UpdateAsync( // Ensure that the retrieved flag is cleared await projectSecrets.UpdateAsync(sfProjectId, u => u.Unset(p => p.ServalData.PreTranslationsRetrieved)); } + + return null; } public async Task StartBuildAsync(string curUserId, string sfProjectId, CancellationToken cancellationToken) @@ -1352,7 +1389,7 @@ public async Task TranslateNAsync( CancellationToken cancellationToken ) { - IEnumerable translationResults = Array.Empty(); + IEnumerable translationResults = []; // Ensure that the user has permission await EnsureProjectPermissionAsync(curUserId, sfProjectId); @@ -1540,6 +1577,26 @@ private static string GetHighestRankedUserId(SFProject project) .Key; } + private static string GetTranslationId( + SFProjectSecret projectSecret, + bool preTranslate, + bool returnEmptyStringIfMissing = false + ) + { + // Ensure we have a translation engine ID + string? translationEngineId = preTranslate + ? projectSecret.ServalData?.PreTranslationEngineId + : projectSecret.ServalData?.TranslationEngineId; + if (string.IsNullOrWhiteSpace(translationEngineId)) + { + return returnEmptyStringIfMissing + ? string.Empty + : throw new DataNotFoundException("The translation engine is not configured"); + } + + return translationEngineId; + } + /// /// This method maps Serval API exceptions to the exceptions that Machine.js understands. /// @@ -1714,17 +1771,6 @@ private async Task GetTranslationIdAsync( : throw new DataNotFoundException("The project secret is missing"); } - // Ensure we have a translation engine ID - string? translationEngineId = preTranslate - ? projectSecret.ServalData?.PreTranslationEngineId - : projectSecret.ServalData?.TranslationEngineId; - if (string.IsNullOrWhiteSpace(translationEngineId)) - { - return returnEmptyStringIfMissing - ? string.Empty - : throw new DataNotFoundException("The translation engine is not configured"); - } - - return translationEngineId; + return GetTranslationId(projectSecret, preTranslate, returnEmptyStringIfMissing); } } diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index e07d2c0e10b..515841acdef 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -780,7 +780,7 @@ ServalBuildDto build in env.Service.GetBuildsAsync( } [Test] - public async Task GetBuildsAsync_SuccessWithEventMetric() + public async Task GetBuildsAsync_SuccessWithEventMetrics() { // Set up test environment var env = new TestEnvironment(); @@ -830,6 +830,14 @@ public async Task GetBuildsAsync_SuccessWithEventMetric() Result = new BsonString(Build01), Scope = EventScope.Drafting, }, + new EventMetric + { + EventType = nameof(MachineApiService.RetrievePreTranslationStatusAsync), + Payload = { { "sfProjectId", Project01 } }, + ProjectId = Project01, + Result = new BsonString(Build01), + Scope = EventScope.Drafting, + }, ], UnpagedCount = 1, } From 961705ac8f33b979dc684acd1624b387af319422 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Tue, 22 Apr 2025 13:58:05 +1200 Subject: [PATCH 05/11] Support for drafts with different books and timestamps --- .../draft-download-button.component.ts | 6 +- .../draft-generation.component.html | 2 +- .../draft-generation.service.spec.ts | 251 +++++++++++++++++- .../draft-generation.service.ts | 73 +++-- .../draft-handling.service.spec.ts | 71 +++-- .../draft-handling.service.ts | 18 +- .../draft-history-entry.component.html | 10 +- .../draft-history-entry.component.scss | 4 + .../draft-history-entry.component.spec.ts | 135 +++++++++- .../draft-history-entry.component.ts | 36 +-- .../draft-history-list.component.spec.ts | 128 +++++++-- .../draft-history-list.component.ts | 17 +- .../draft-preview-books.component.html | 6 +- .../draft-preview-books.component.spec.ts | 75 ++++-- .../draft-preview-books.component.ts | 50 +++- .../editor-draft/editor-draft.component.html | 21 ++ .../editor-draft/editor-draft.component.scss | 13 +- .../editor-draft.component.spec.ts | 86 +++++- .../editor-draft/editor-draft.component.ts | 92 ++++++- .../translate/editor/editor.component.html | 1 + .../translate/editor/editor.component.spec.ts | 20 +- .../app/translate/editor/editor.component.ts | 12 +- .../src/assets/i18n/non_checking_en.json | 8 +- 23 files changed, 968 insertions(+), 167 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts index 98a5e74780d..7df60069d50 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.ts @@ -1,10 +1,12 @@ import { CommonModule } from '@angular/common'; import { Component, Input } from '@angular/core'; +import { MatButtonModule } from '@angular/material/button'; +import { MatIconModule } from '@angular/material/icon'; +import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { TranslocoModule } from '@ngneat/transloco'; import { Subscription } from 'rxjs'; import { ActivatedProjectService } from 'xforge-common/activated-project.service'; import { NoticeService } from 'xforge-common/notice.service'; -import { UICommonModule } from 'xforge-common/ui-common.module'; import { BuildDto } from '../../../machine-api/build-dto'; import { DraftZipProgress } from '../draft-generation'; import { DraftGenerationService } from '../draft-generation.service'; @@ -14,7 +16,7 @@ import { DraftGenerationService } from '../draft-generation.service'; templateUrl: './draft-download-button.component.html', styleUrls: ['./draft-download-button.component.scss'], standalone: true, - imports: [CommonModule, TranslocoModule, UICommonModule] + imports: [CommonModule, MatButtonModule, MatIconModule, MatProgressSpinnerModule, TranslocoModule] }) export class DraftDownloadButtonComponent { /** diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html index 38594c77c92..95bc03cae23 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html @@ -278,7 +278,7 @@

{{ t("draft_finishing_header") }}

@if (hasDraftBooksAvailable) { - + } 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 e5619b78f5f..2a93ccb0363 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 @@ -7,11 +7,14 @@ import JSZip from 'jszip'; import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; import { of } from 'rxjs'; import { first } from 'rxjs/operators'; +import { anything, mock, verify } from 'ts-mockito'; +import { NoticeService } from 'xforge-common/notice.service'; import { OnlineStatusService } from 'xforge-common/online-status.service'; import { TestOnlineStatusModule } from 'xforge-common/test-online-status.module'; import { TestOnlineStatusService } from 'xforge-common/test-online-status.service'; import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; import { SFProjectProfileDoc } from '../../core/models/sf-project-profile-doc'; +import { TextDocSource } from '../../core/models/text-doc'; import { BuildDto } from '../../machine-api/build-dto'; import { BuildStates } from '../../machine-api/build-states'; import { MACHINE_API_BASE_URL } from '../../machine-api/http-client'; @@ -21,11 +24,15 @@ import { DraftGenerationService } from './draft-generation.service'; describe('DraftGenerationService', () => { let service: DraftGenerationService; let httpTestingController: HttpTestingController; + const mockNoticeService = mock(NoticeService); let testOnlineStatusService: TestOnlineStatusService; configureTestingModule(() => ({ imports: [TestOnlineStatusModule.forRoot(), TestTranslocoModule], - providers: [{ provide: OnlineStatusService, useClass: TestOnlineStatusService }] + providers: [ + { provide: NoticeService, useMock: mockNoticeService }, + { provide: OnlineStatusService, useClass: TestOnlineStatusService } + ] })); const projectId = 'testProjectId'; @@ -158,6 +165,40 @@ describe('DraftGenerationService', () => { tick(); })); + it('should return undefined for a 401 error', fakeAsync(() => { + // SUT + service.getBuildHistory(projectId).subscribe(result => { + expect(result).toEqual(undefined); + verify(mockNoticeService.showError(anything())).once(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/builds/project:${projectId}?pretranslate=true` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.Unauthorized, statusText: 'Unauthorized' }); + tick(); + })); + + it('should return undefined for a 404 error', fakeAsync(() => { + // SUT + service.getBuildHistory(projectId).subscribe(result => { + expect(result).toEqual(undefined); + verify(mockNoticeService.showError(anything())).never(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/builds/project:${projectId}?pretranslate=true` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.NotFound, statusText: 'Not Found' }); + tick(); + })); + it('should return undefined if offline', fakeAsync(() => { testOnlineStatusService.setIsOnline(false); @@ -372,7 +413,7 @@ describe('DraftGenerationService', () => { }; // SUT - service.getGeneratedDraftDeltaOperations(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, undefined).subscribe(result => { expect(result).toEqual(ops); }); tick(); @@ -386,12 +427,63 @@ describe('DraftGenerationService', () => { tick(); })); + it('should get the pre-translation ops at the specified time and return an observable', fakeAsync(() => { + const book = 43; + const chapter = 3; + const timestamp = new Date(); + const ops = [ + { + insert: { + chapter: { + number: '1', + style: 'c' + } + } + }, + { + insert: { + verse: { + number: '1', + style: 'v' + } + } + }, + { + insert: 'Verse 1 Contents', + attributes: { + segment: 'verse_1_1' + } + } + ]; + const preTranslationDeltaData = { + id: `${projectId}:${Canon.bookNumberToId(book)}:${chapter}:target`, + version: 0, + data: { + ops + } + }; + + // SUT + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, timestamp).subscribe(result => { + expect(result).toEqual(ops); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/delta?timestamp=${timestamp.toISOString()}` + ); + expect(req.request.method).toEqual('GET'); + req.flush(preTranslationDeltaData); + tick(); + })); + it('should return an empty array for missing data', fakeAsync(() => { const book = 43; const chapter = 3; // SUT - service.getGeneratedDraftDeltaOperations(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, undefined).subscribe(result => { expect(result).toEqual([]); }); tick(); @@ -405,13 +497,34 @@ describe('DraftGenerationService', () => { tick(); })); + it('should return an empty array for a 401 error', fakeAsync(() => { + const book = 43; + const chapter = 3; + + // SUT + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, undefined).subscribe(result => { + expect(result).toEqual([]); + verify(mockNoticeService.showError(anything())).once(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/delta` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.Unauthorized, statusText: 'Unauthorized' }); + tick(); + })); + it('should return an empty array for a 404 error', fakeAsync(() => { const book = 43; const chapter = 3; // SUT - service.getGeneratedDraftDeltaOperations(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, undefined).subscribe(result => { expect(result).toEqual([]); + verify(mockNoticeService.showError(anything())).never(); }); tick(); @@ -429,7 +542,7 @@ describe('DraftGenerationService', () => { const chapter = 3; // SUT - service.getGeneratedDraftDeltaOperations(projectId, book, chapter).subscribe({ + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, undefined).subscribe({ error: (err: HttpErrorResponse) => { expect(err.status).toEqual(405); expect(err.statusText).toEqual('Not Allowed'); @@ -452,10 +565,102 @@ describe('DraftGenerationService', () => { testOnlineStatusService.setIsOnline(false); // SUT - service.getGeneratedDraftDeltaOperations(projectId, book, chapter).subscribe(result => { + service.getGeneratedDraftDeltaOperations(projectId, book, chapter, undefined).subscribe(result => { + expect(result).toEqual([]); + }); + tick(); + })); + }); + + describe('getGeneratedDraftHistory', () => { + it('should get the draft history for the specified book/chapter and return an observable', fakeAsync(() => { + const book = 43; + const chapter = 3; + const revisions = [{ source: 'Draft' as TextDocSource, timestamp: new Date().toISOString() }]; + // SUT + service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { + expect(result).toEqual(revisions); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/history` + ); + expect(req.request.method).toEqual('GET'); + req.flush(revisions); + tick(); + })); + + it('should return undefined for missing data', fakeAsync(() => { + const book = 43; + const chapter = 3; + + // SUT + service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { expect(result).toEqual([]); }); tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/history` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null); + tick(); + })); + + it('should return undefined for a 401 error', fakeAsync(() => { + const book = 43; + const chapter = 3; + + // SUT + service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { + expect(result).toEqual(undefined); + verify(mockNoticeService.showError(anything())).once(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/history` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.Unauthorized, statusText: 'Unauthorized' }); + tick(); + })); + + it('should return undefined for a 404 error', fakeAsync(() => { + const book = 43; + const chapter = 3; + + // SUT + service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { + expect(result).toEqual(undefined); + verify(mockNoticeService.showError(anything())).never(); + }); + tick(); + + // Setup the HTTP request + const req = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/history` + ); + expect(req.request.method).toEqual('GET'); + req.flush(null, { status: HttpStatusCode.NotFound, statusText: 'Not Found' }); + tick(); + })); + + it('should return undefined if offline', fakeAsync(() => { + const book = 43; + const chapter = 3; + testOnlineStatusService.setIsOnline(false); + + // SUT + service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { + expect(result).toEqual(undefined); + }); + tick(); })); }); @@ -653,7 +858,7 @@ describe('DraftGenerationService', () => { tick(); })); - it('should create a zip file containing all of the books with drafts at the generated date', fakeAsync(() => { + it('should create a zip file containing all of the books with drafts at the generated date if the build scripture range is missing', fakeAsync(() => { const projectDoc: SFProjectProfileDoc = { id: projectId, data: createTestProjectProfile({ @@ -683,5 +888,37 @@ describe('DraftGenerationService', () => { req1jn.flush(usfm); tick(); })); + + it('should create a zip file containing all of the books with drafts at the generated date using the build scripture range', fakeAsync(() => { + const projectDoc: SFProjectProfileDoc = { + id: projectId, + data: createTestProjectProfile({ + texts: [] + }) + } as SFProjectProfileDoc; + const lastCompletedBuild: BuildDto = { + additionalInfo: { + dateFinished: '2024-08-27T00:00:00.000+00:00', + dateGenerated: '2024-08-27T01:02:03.004+00:00', + translationScriptureRanges: [{ projectId, scriptureRange: '1JN' }] + } + } as BuildDto; + + service.downloadGeneratedDraftZip(projectDoc, lastCompletedBuild).subscribe({ + complete: () => { + expect(saveAs).toHaveBeenCalled(); + } + }); + tick(); + + // Setup the HTTP request for 1 John + const usfm = '\\id Test USFM \\c 1 \\v 1 Test'; + const req1jn = httpTestingController.expectOne( + `${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/pretranslate/62_0/usfm?timestamp=2024-08-27T01:02:03.004Z` + ); + expect(req1jn.request.method).toEqual('GET'); + req1jn.flush(usfm); + tick(); + })); }); }); 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 a4bb3b48110..cce3a147197 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 @@ -11,9 +11,10 @@ import { Snapshot } from 'xforge-common/models/snapshot'; import { NoticeService } from 'xforge-common/notice.service'; import { OnlineStatusService } from 'xforge-common/online-status.service'; import { SFProjectProfileDoc } from '../../core/models/sf-project-profile-doc'; +import { Revision } from '../../core/paratext.service'; import { BuildDto } from '../../machine-api/build-dto'; import { HttpClient } from '../../machine-api/http-client'; -import { getBookFileNameDigits } from '../../shared/utils'; +import { booksFromScriptureRange, getBookFileNameDigits } from '../../shared/utils'; import { activeBuildStates, BuildConfig, @@ -199,30 +200,65 @@ export class DraftGenerationService { * @param projectId The SF project id for the target translation. * @param book The book number. * @param chapter The chapter number. + * @param timestamp The timestamp to download the draft at. If undefined, the latest draft will be downloaded. * @returns An array of delta operations or an empty array at if no pre-translations exist. * The 405 error that occurs when there is no USFM support is thrown to the caller. */ - getGeneratedDraftDeltaOperations(projectId: string, book: number, chapter: number): Observable { + getGeneratedDraftDeltaOperations( + projectId: string, + book: number, + chapter: number, + timestamp?: Date + ): Observable { if (!this.onlineStatusService.isOnline) { return of([]); } + let url = `translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/delta`; + if (timestamp != null) { + url += `?timestamp=${timestamp.toISOString()}`; + } + return this.httpClient.get | undefined>(url).pipe( + map(res => res.data?.data.ops ?? []), + catchError(err => { + // If no pre-translations exist, return empty array + if (err.status === 403 || err.status === 404 || err.status === 409) { + return of([]); + } else if (err.status === 405) { + // Rethrow a 405 so the frontend can use getGeneratedDraft() + return throwError(() => err); + } + + this.noticeService.showError(translate('draft_generation.temporarily_unavailable')); + return of([]); + }) + ); + } + + /** + * Gets the draft revisions saved in Scripture Forge for the specified book/chapter. + * @param projectId The SF project id for the target translation. + * @param book The book number. + * @param chapter The chapter number. + * @returns The Draft revisions, or undefined if an issue occurred retrieving the revisions. + */ + getGeneratedDraftHistory(projectId: string, book: number, chapter: number): Observable { + if (!this.onlineStatusService.isOnline) { + return of(undefined); + } return this.httpClient .get< - Snapshot | undefined - >(`translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/delta`) + Revision[] | undefined + >(`translation/engines/project:${projectId}/actions/pretranslate/${book}_${chapter}/history`) .pipe( - map(res => res.data?.data.ops ?? []), + map(res => res?.data ?? []), catchError(err => { - // If no pre-translations exist, return empty array + // If no pre-translations exist, return undefined if (err.status === 403 || err.status === 404 || err.status === 409) { - return of([]); - } else if (err.status === 405) { - // Rethrow a 405 so the frontend can use getGeneratedDraft() - return throwError(() => err); + return of(undefined); } this.noticeService.showError(translate('draft_generation.temporarily_unavailable')); - return of([]); + return of(undefined); }) ); } @@ -239,7 +275,7 @@ export class DraftGenerationService { projectId: string, book: number, chapter: number, - timestamp: Date | undefined + timestamp?: Date ): Observable { if (!this.onlineStatusService.isOnline) { return of(undefined); @@ -277,13 +313,12 @@ export class DraftGenerationService { const projectShortName: string = projectDoc.data.shortName; const usfmFiles: Promise[] = []; - // Build the list of book numbers - const books: number[] = projectDoc.data.texts.reduce((acc, text) => { - if (text.chapters.some(c => c.hasDraft)) { - acc.push(text.bookNum); - } - return acc; - }, []); + // Build the list of book numbers, first checking the build, then the project document if that is null + const books: number[] = + lastCompletedBuild?.additionalInfo?.translationScriptureRanges?.flatMap(range => + booksFromScriptureRange(range.scriptureRange) + ) ?? projectDoc.data.texts.filter(text => text.chapters.some(c => c.hasDraft)).map(text => text.bookNum); + const zipProgress: DraftZipProgress = { current: 0, total: books.length }; observer.next(zipProgress); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.spec.ts index dfbb93a7b09..48fe31b7f18 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.spec.ts @@ -69,10 +69,12 @@ describe('DraftHandlingService', () => { const textDocId = new TextDocId('project01', 1, 1); const draftOps: DeltaOperation[] = [{ insert: 'In the beginning', attributes: { segment: 'verse_1_1' } }]; when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(of(draftOps)); - service.getDraft(textDocId, { isDraftLegacy: false }).subscribe(draftData => expect(draftData).toEqual(draftOps)); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).once(); + service + .getDraft(textDocId, { isDraftLegacy: false, timestamp: undefined }) + .subscribe(draftData => expect(draftData).toEqual(draftOps)); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).once(); verify(mockedDraftGenerationService.getGeneratedDraft('project01', 1, 1)).never(); }); @@ -84,11 +86,13 @@ describe('DraftHandlingService', () => { verse_150_3: 'Praise him with the sound of the trumpet: ' }; when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(throwError(() => ({ status: 405 }))); when(mockedDraftGenerationService.getGeneratedDraft(anything(), anything(), anything())).thenReturn(of(draft)); - service.getDraft(textDocId, { isDraftLegacy: false }).subscribe(draftData => expect(draftData).toEqual(draft)); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).once(); + service + .getDraft(textDocId, { isDraftLegacy: false, timestamp: undefined }) + .subscribe(draftData => expect(draftData).toEqual(draft)); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).once(); verify(mockedDraftGenerationService.getGeneratedDraft('project01', 1, 1)).once(); }); }); @@ -388,12 +392,17 @@ describe('DraftHandlingService', () => { { insert: 'In the beginning', attributes: { segment: 'verse_1_1' } } ]; when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(of(draft)); when(mockedTextDocService.canEdit(anything(), 1, 1)).thenReturn(true); - const result: boolean = await service.getAndApplyDraftAsync(mockedSFProject.data!, textDocId, textDocId); + const result: boolean = await service.getAndApplyDraftAsync( + mockedSFProject.data!, + textDocId, + textDocId, + undefined + ); expect(result).toBe(true); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).once(); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).once(); verify(mockedTextDocService.overwrite(textDocId, anything(), 'Draft')).once(); verify( mockedProjectService.onlineSetDraftApplied( @@ -413,12 +422,17 @@ describe('DraftHandlingService', () => { const textDocId = new TextDocId('project01', 1, 1); const draft: DeltaOperation[] = [{ insert: 'In the beginning', attributes: { segment: 'verse_1_1' } }]; when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(of(draft)); when(mockedTextDocService.canEdit(anything(), 1, 1)).thenReturn(false); - const result: boolean = await service.getAndApplyDraftAsync(mockedSFProject.data!, textDocId, textDocId); + const result: boolean = await service.getAndApplyDraftAsync( + mockedSFProject.data!, + textDocId, + textDocId, + undefined + ); expect(result).toBe(false); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).never(); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).never(); verify(mockedTextDocService.overwrite(textDocId, anything(), 'Draft')).never(); }); @@ -426,13 +440,18 @@ describe('DraftHandlingService', () => { const textDocId = new TextDocId('project01', 1, 1); const draft: DraftSegmentMap = { verse_1_1: 'In the beginning' }; when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(throwError(() => ({ status: 405 }))); when(mockedDraftGenerationService.getGeneratedDraft(anything(), anything(), anything())).thenReturn(of(draft)); when(mockedTextDocService.canEdit(anything(), 1, 1)).thenReturn(true); - const result: boolean = await service.getAndApplyDraftAsync(mockedSFProject.data!, textDocId, textDocId); + const result: boolean = await service.getAndApplyDraftAsync( + mockedSFProject.data!, + textDocId, + textDocId, + undefined + ); expect(result).toBe(false); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).once(); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).once(); verify(mockedDraftGenerationService.getGeneratedDraft('project01', 1, 1)).once(); verify(mockedTextDocService.overwrite(textDocId, anything(), 'Draft')).never(); }); @@ -444,15 +463,20 @@ describe('DraftHandlingService', () => { { insert: 'In the beginning', attributes: { segment: 'verse_1_1' } } ]; when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(of(draft)); when(mockedTextDocService.canEdit(anything(), 1, 1)).thenReturn(true); when( mockedProjectService.onlineSetDraftApplied(anything(), anything(), anything(), anything(), anything()) ).thenReturn(Promise.reject(new Error('Failed'))); - const result: boolean = await service.getAndApplyDraftAsync(mockedSFProject.data!, textDocId, textDocId); + const result: boolean = await service.getAndApplyDraftAsync( + mockedSFProject.data!, + textDocId, + textDocId, + undefined + ); expect(result).toBe(false); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).once(); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).once(); verify(mockedErrorReportingService.silentError(anything(), anything())).once(); verify(mockedTextDocService.overwrite(textDocId, anything(), anything())).never(); verify( @@ -470,11 +494,16 @@ describe('DraftHandlingService', () => { const textDocId = new TextDocId('project01', 1, 1); when(mockedTextDocService.canEdit(anything(), 1, 1)).thenReturn(true); when( - mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything()) + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) ).thenReturn(throwError(() => ({ message: 'Getting draft failed', status: 404 }))); - const result: boolean = await service.getAndApplyDraftAsync(mockedSFProject.data!, textDocId, textDocId); + const result: boolean = await service.getAndApplyDraftAsync( + mockedSFProject.data!, + textDocId, + textDocId, + undefined + ); expect(result).toBe(false); - verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1)).once(); + verify(mockedDraftGenerationService.getGeneratedDraftDeltaOperations('project01', 1, 1, undefined)).once(); verify(mockedErrorReportingService.silentError(anything(), anything())).once(); verify( mockedProjectService.onlineSetDraftApplied( diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts index d15e98d07d8..d0a05d2a1a5 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts @@ -149,19 +149,20 @@ export class DraftHandlingService { */ getDraft( textDocId: TextDocId, - { isDraftLegacy }: { isDraftLegacy: boolean } + { isDraftLegacy, timestamp }: { isDraftLegacy: boolean; timestamp?: Date } ): Observable { return isDraftLegacy ? // Fetch legacy draft this.draftGenerationService.getGeneratedDraft(textDocId.projectId, textDocId.bookNum, textDocId.chapterNum) - : // Fetch draft in USFM format (fallback to legacy) + : // Fetch draft in Delta format (fallback to legacy) this.draftGenerationService - .getGeneratedDraftDeltaOperations(textDocId.projectId, textDocId.bookNum, textDocId.chapterNum) + .getGeneratedDraftDeltaOperations(textDocId.projectId, textDocId.bookNum, textDocId.chapterNum, timestamp) .pipe( catchError(err => { - // If the corpus does not support USFM - if (err.status === 405) { - return this.getDraft(textDocId, { isDraftLegacy: true }); + // If the corpus does not support USFM, use the legacy format. + // The legacy format does not support a timestamp + if (err.status === 405 && timestamp == null) { + return this.getDraft(textDocId, { isDraftLegacy: true, timestamp }); } return throwError(() => err); @@ -216,14 +217,15 @@ export class DraftHandlingService { async getAndApplyDraftAsync( project: SFProjectProfile, draftTextDocId: TextDocId, - targetTextDocId: TextDocId + targetTextDocId: TextDocId, + timestamp?: Date ): Promise { if (!this.textDocService.canEdit(project, draftTextDocId.bookNum, draftTextDocId.chapterNum)) { return false; } return await new Promise(resolve => { - this.getDraft(draftTextDocId, { isDraftLegacy: false }).subscribe({ + this.getDraft(draftTextDocId, { isDraftLegacy: false, timestamp }).subscribe({ next: async draft => { let ops: DeltaOperation[] = []; if (this.isDraftSegmentMap(draft)) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html index 19fca9d69d2..ac50b2adc46 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html @@ -8,10 +8,10 @@ {{ getStatus(entry.state).icons }} - {{ getStatus(entry.state).text }} + {{ t(getStatus(entry.state).text) }} @if (!forceDetailsOpen) { - expand_{{ detailsOpen ? "less" : "more" }} + expand_{{ detailsOpen ? "less" : "more" }} }
@if (detailsOpen) { @@ -19,11 +19,9 @@

{{ t("click_book_to_preview") }}

- @for (bookId of bookIds; track bookId) { - {{ i18n.localizeBook(bookId) }} - } +

- +

{ })); beforeEach(() => { - const user: UserProfile = createTestUserProfile(); - const userDoc = { id: 'sf-user-id', data: user } as UserProfileDoc; - when(mockedUserService.getProfile(anything())).thenResolve(userDoc); - fixture = TestBed.createComponent(DraftHistoryEntryComponent); component = fixture.componentInstance; fixture.detectChanges(); @@ -36,4 +33,130 @@ describe('DraftHistoryEntryComponent', () => { it('should create', () => { expect(component).toBeTruthy(); }); + + it('forceDetailsOpen should set detailsOpen', () => { + expect(component.detailsOpen).toBe(false); + component.forceDetailsOpen = true; + expect(component.detailsOpen).toBe(true); + expect(component.forceDetailsOpen).toBe(true); + component.forceDetailsOpen = false; + expect(component.detailsOpen).toBe(true); + expect(component.forceDetailsOpen).toBe(false); + }); + + it('headerClicked should toggle detailsOpen', () => { + expect(component.detailsOpen).toBe(false); + component.headerClicked(); + expect(component.detailsOpen).toBe(true); + component.headerClicked(); + expect(component.detailsOpen).toBe(false); + }); + + describe('entry', () => { + it('should handle undefined values', () => { + component.entry = undefined; + expect(component.bookNames).toEqual([]); + expect(component.buildRequestedByUserName).toBeUndefined(); + expect(component.buildRequestedByDate).toBe(''); + expect(component.canDownloadBuild).toBe(false); + expect(component.entry).toBeUndefined(); + }); + + it('should handle builds with additional info', fakeAsync(() => { + when(mockedI18nService.formatDate(anything())).thenReturn('formatted-date'); + when(mockedI18nService.localizeBook('GEN')).thenReturn('localized-book'); + const userDoc = { + id: 'sf-user-id', + data: createTestUserProfile({ displayName: 'user-display-name' }) + } as UserProfileDoc; + when(mockedUserService.getProfile('sf-user-id')).thenResolve(userDoc); + const entry = { + additionalInfo: { + dateGenerated: new Date().toISOString(), + dateRequested: new Date().toISOString(), + requestedByUserId: 'sf-user-id', + translationScriptureRanges: [{ projectId: 'project01', scriptureRange: 'GEN' }] + } + } as BuildDto; + + // SUT + component.entry = entry; + tick(); + fixture.detectChanges(); + + expect(component.bookNames).toEqual(['localized-book']); + expect(component.buildRequestedByUserName).toBe('user-display-name'); + expect(component.buildRequestedByDate).toBe('formatted-date'); + expect(component.canDownloadBuild).toBe(true); + expect(component.entry).toBe(entry); + })); + + it('should handle builds with additional info referencing a deleted user', fakeAsync(() => { + when(mockedI18nService.formatDate(anything())).thenReturn('formatted-date'); + when(mockedI18nService.localizeBook('GEN')).thenReturn('localized-book'); + const userDoc = { id: 'sf-user-id', data: undefined } as UserProfileDoc; + when(mockedUserService.getProfile(anything())).thenResolve(userDoc); + const entry = { + additionalInfo: { + dateGenerated: new Date().toISOString(), + dateRequested: new Date().toISOString(), + requestedByUserId: 'sf-user-id', + translationScriptureRanges: [{ projectId: 'project01', scriptureRange: 'GEN' }] + } + } as BuildDto; + + // SUT + component.entry = entry; + tick(); + fixture.detectChanges(); + + expect(component.bookNames).toEqual(['localized-book']); + expect(component.buildRequestedByUserName).toBeUndefined(); + expect(component.buildRequestedByDate).toBe('formatted-date'); + expect(component.canDownloadBuild).toBe(true); + expect(component.entry).toBe(entry); + })); + + it('should handle builds with incomplete additional info', () => { + const entry = { additionalInfo: {} } as BuildDto; + component.entry = entry; + expect(component.bookNames).toEqual([]); + expect(component.buildRequestedByUserName).toBeUndefined(); + expect(component.buildRequestedByDate).toBe(''); + expect(component.canDownloadBuild).toBe(false); + expect(component.entry).toBe(entry); + }); + + it('should handle builds without additional info', () => { + const entry = {} as BuildDto; + component.entry = entry; + expect(component.bookNames).toEqual([]); + expect(component.buildRequestedByUserName).toBeUndefined(); + expect(component.buildRequestedByDate).toBe(''); + expect(component.canDownloadBuild).toBe(false); + expect(component.entry).toBe(entry); + }); + }); + + describe('formatDate', () => { + it('should handle undefined values', () => { + expect(component.formatDate(undefined)).toBe(''); + }); + + it('should handle date values', () => { + const date = new Date(); + when(mockedI18nService.formatDate(anything())).thenReturn('formatted-date'); + expect(component.formatDate(date.toISOString())).toBe('formatted-date'); + }); + }); + + describe('getStatus', () => { + it('should handle known build state strings', () => { + expect(component.getStatus(BuildStates.Active)).not.toBeUndefined(); + }); + + it('should handle unknown build state strings', () => { + expect(component.getStatus('unknown build state' as BuildStates)).not.toBeUndefined(); + }); + }); }); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts index b3b12a08356..632399b871e 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -1,28 +1,28 @@ import { CommonModule } from '@angular/common'; import { Component, Input } from '@angular/core'; -import { MatButtonModule } from '@angular/material/button'; -import { MatIconModule } from '@angular/material/icon'; import { TranslocoModule } from '@ngneat/transloco'; import { I18nService } from 'xforge-common/i18n.service'; +import { UICommonModule } from 'xforge-common/ui-common.module'; import { UserService } from 'xforge-common/user.service'; import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; import { DraftDownloadButtonComponent } from '../../draft-download-button/draft-download-button.component'; +import { DraftPreviewBooksComponent } from '../../draft-preview-books/draft-preview-books.component'; const STATUS_INFO: Record = { - ACTIVE: { icons: 'hourglass_empty', text: 'Running', color: 'grey' }, - COMPLETED: { icons: 'check_circle', text: 'Completed', color: 'green' }, - FAULTED: { icons: 'error', text: 'Failed', color: 'red' }, - CANCELED: { icons: 'cancel', text: 'Cancelled', color: 'grey' }, - QUEUED: { icons: 'hourglass_empty', text: 'pending', color: 'grey' }, - PENDING: { icons: 'hourglass_empty', text: 'pending', color: 'grey' }, - FINISHING: { icons: 'hourglass_empty', text: 'pending', color: 'grey' } + ACTIVE: { icons: 'hourglass_empty', text: 'draft_active', color: 'grey' }, + COMPLETED: { icons: 'check_circle', text: 'draft_completed', color: 'green' }, + FAULTED: { icons: 'error', text: 'draft_faulted', color: 'red' }, + CANCELED: { icons: 'cancel', text: 'draft_canceled', color: 'grey' }, + QUEUED: { icons: 'hourglass_empty', text: 'draft_pending', color: 'grey' }, + PENDING: { icons: 'hourglass_empty', text: 'draft_pending', color: 'grey' }, + FINISHING: { icons: 'hourglass_empty', text: 'draft_pending', color: 'grey' } }; @Component({ selector: 'app-draft-history-entry', standalone: true, - imports: [CommonModule, MatButtonModule, MatIconModule, TranslocoModule, DraftDownloadButtonComponent], + imports: [CommonModule, DraftDownloadButtonComponent, DraftPreviewBooksComponent, TranslocoModule, UICommonModule], templateUrl: './draft-history-entry.component.html', styleUrl: './draft-history-entry.component.scss' }) @@ -77,13 +77,15 @@ export class DraftHistoryEntryComponent { private readonly userService: UserService ) {} - get bookIds(): string[] { - if (this.entry?.additionalInfo == null) return []; - return [...new Set(this.entry.additionalInfo.translationScriptureRanges.flatMap(r => r.scriptureRange.split(';')))]; - } - get bookNames(): string[] { - return this.bookIds.map(id => this.i18n.localizeBook(id)); + if (this.entry?.additionalInfo?.translationScriptureRanges == null) return []; + return [ + ...new Set( + this.entry.additionalInfo.translationScriptureRanges.flatMap(r => + r.scriptureRange.split(';').map(id => this.i18n.localizeBook(id)) + ) + ) + ]; } formatDate(date?: string): string { @@ -91,7 +93,7 @@ export class DraftHistoryEntryComponent { } getStatus(state: BuildStates): { icons: string; text: string; color: string } { - return STATUS_INFO[state]; + return STATUS_INFO[state] ?? { icons: 'help_outline', text: 'draft_unknown', color: 'grey' }; } headerClicked(): void { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts index aee260d870b..5210fe0a287 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts @@ -1,37 +1,127 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { BehaviorSubject } from 'rxjs'; +import { ComponentFixture, fakeAsync, TestBed } from '@angular/core/testing'; +import { BehaviorSubject, of } from 'rxjs'; import { mock, when } from 'ts-mockito'; import { ActivatedProjectService } from 'xforge-common/activated-project.service'; +import { I18nService } from 'xforge-common/i18n.service'; +import { TestRealtimeModule } from 'xforge-common/test-realtime.module'; import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; +import { UserService } from 'xforge-common/user.service'; +import { SF_TYPE_REGISTRY } from '../../../core/models/sf-type-registry'; +import { SFProjectService } from '../../../core/sf-project.service'; +import { BuildDto } from '../../../machine-api/build-dto'; +import { BuildStates } from '../../../machine-api/build-states'; import { DraftGenerationService } from '../draft-generation.service'; import { DraftHistoryListComponent } from './draft-history-list.component'; -const mockActivatedProjectService: ActivatedProjectService = mock(ActivatedProjectService); -const mockDraftGenerationService: DraftGenerationService = mock(DraftGenerationService); +const mockedActivatedProjectService = mock(ActivatedProjectService); +const mockedDraftGenerationService = mock(DraftGenerationService); +const mockedI18nService = mock(I18nService); +const mockedSFProjectService = mock(SFProjectService); +const mockedUserService = mock(UserService); describe('DraftHistoryListComponent', () => { - let component: DraftHistoryListComponent; - let fixture: ComponentFixture; - configureTestingModule(() => ({ - imports: [TestTranslocoModule], + imports: [TestTranslocoModule, TestRealtimeModule.forRoot(SF_TYPE_REGISTRY)], providers: [ - { provide: ActivatedProjectService, useMock: mockActivatedProjectService }, - { provide: DraftGenerationService, useMock: mockDraftGenerationService } + { provide: ActivatedProjectService, useMock: mockedActivatedProjectService }, + { provide: DraftGenerationService, useMock: mockedDraftGenerationService }, + { provide: I18nService, useMock: mockedI18nService }, + { provide: SFProjectService, useMock: mockedSFProjectService }, + { provide: UserService, useMock: mockedUserService } ] })); - beforeEach(() => { - const mockProjectId = 'project01'; - const mockProjectId$ = new BehaviorSubject(mockProjectId); - when(mockActivatedProjectService.projectId$).thenReturn(mockProjectId$); + it('should handle a missing build history', () => { + const env = new TestEnvironment(undefined); + expect(env.component.history).toEqual([]); + expect(env.component.historicalBuilds).toEqual([]); + expect(env.component.isBuildActive).toBe(false); + expect(env.component.latestBuild).toBeUndefined(); + expect(env.component.lastCompletedBuildMessage).toBe(''); + expect(env.component.nonActiveBuilds).toEqual([]); + }); - fixture = TestBed.createComponent(DraftHistoryListComponent); - component = fixture.componentInstance; - fixture.detectChanges(); + it('should handle an empty build history', () => { + const env = new TestEnvironment([]); + expect(env.component.history).toEqual([]); + expect(env.component.historicalBuilds).toEqual([]); + expect(env.component.isBuildActive).toBe(false); + expect(env.component.latestBuild).toBeUndefined(); + expect(env.component.lastCompletedBuildMessage).toBe(''); + expect(env.component.nonActiveBuilds).toEqual([]); }); - it('should create', () => { - expect(component).toBeTruthy(); + it('should handle completed and active builds', fakeAsync(() => { + const activeBuild = { state: BuildStates.Active } as BuildDto; + const completedBuild = { state: BuildStates.Completed } as BuildDto; + const env = new TestEnvironment([completedBuild, activeBuild]); + expect(env.component.history).toEqual([activeBuild, completedBuild]); + expect(env.component.historicalBuilds).toEqual([completedBuild]); + expect(env.component.isBuildActive).toBe(true); + expect(env.component.latestBuild).toBeUndefined(); + expect(env.component.lastCompletedBuildMessage).toBe(''); + expect(env.component.nonActiveBuilds).toEqual([completedBuild]); + })); + + it('should handle just one active build', () => { + const buildHistory = [{ state: BuildStates.Active } as BuildDto]; + const env = new TestEnvironment(buildHistory); + expect(env.component.history).toEqual(buildHistory); + expect(env.component.historicalBuilds).toEqual([]); + expect(env.component.isBuildActive).toBe(true); + expect(env.component.latestBuild).toBeUndefined(); + expect(env.component.lastCompletedBuildMessage).toBe(''); + expect(env.component.nonActiveBuilds).toEqual([]); }); + + it('should handle just one canceled build', fakeAsync(() => { + const build = { state: BuildStates.Canceled } as BuildDto; + const buildHistory = [build]; + const env = new TestEnvironment(buildHistory); + expect(env.component.history).toEqual(buildHistory); + expect(env.component.historicalBuilds).toEqual([]); + expect(env.component.isBuildActive).toBe(false); + expect(env.component.latestBuild).toBe(build); + expect(env.component.lastCompletedBuildMessage).not.toBe(''); + expect(env.component.nonActiveBuilds).toEqual(buildHistory); + })); + + it('should handle just one completed build', fakeAsync(() => { + const build = { state: BuildStates.Completed } as BuildDto; + const buildHistory = [build]; + const env = new TestEnvironment(buildHistory); + expect(env.component.history).toEqual(buildHistory); + expect(env.component.historicalBuilds).toEqual([]); + expect(env.component.isBuildActive).toBe(false); + expect(env.component.latestBuild).toBe(build); + expect(env.component.lastCompletedBuildMessage).not.toBe(''); + expect(env.component.nonActiveBuilds).toEqual(buildHistory); + })); + + it('should handle just one faulted build', fakeAsync(() => { + const build = { state: BuildStates.Faulted } as BuildDto; + const buildHistory = [build]; + const env = new TestEnvironment(buildHistory); + expect(env.component.history).toEqual(buildHistory); + expect(env.component.historicalBuilds).toEqual([]); + expect(env.component.isBuildActive).toBe(false); + expect(env.component.latestBuild).toBe(build); + expect(env.component.lastCompletedBuildMessage).not.toBe(''); + expect(env.component.nonActiveBuilds).toEqual(buildHistory); + })); + + class TestEnvironment { + component: DraftHistoryListComponent; + fixture: ComponentFixture; + + constructor(buildHistory: BuildDto[] | undefined) { + when(mockedActivatedProjectService.projectId$).thenReturn(of('project01')); + when(mockedActivatedProjectService.changes$).thenReturn(of(undefined)); + when(mockedDraftGenerationService.getBuildHistory('project01')).thenReturn(new BehaviorSubject(buildHistory)); + + this.fixture = TestBed.createComponent(DraftHistoryListComponent); + this.component = this.fixture.componentInstance; + this.fixture.detectChanges(); + } + } }); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts index 3b8d731729c..bd7fc9883d9 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts @@ -18,7 +18,7 @@ import { DraftHistoryEntryComponent } from './draft-history-entry/draft-history- styleUrl: './draft-history-list.component.scss' }) export class DraftHistoryListComponent { - history?: BuildDto[]; + history: BuildDto[] = []; constructor( private readonly activatedProject: ActivatedProjectService, @@ -33,7 +33,7 @@ export class DraftHistoryListComponent { } get nonActiveBuilds(): BuildDto[] { - return this.history?.filter(entry => !activeBuildStates.includes(entry.state)) || []; + return this.history.filter(entry => !activeBuildStates.includes(entry.state)) || []; } get latestBuild(): BuildDto | undefined { @@ -41,21 +41,16 @@ export class DraftHistoryListComponent { } get lastCompletedBuildMessage(): string { - if (this.latestBuild == null) return ''; - const state = this.latestBuild.state.toString(); - switch (state) { + switch (this.latestBuild?.state) { case BuildStates.Canceled: return this.transloco.translate('draft_history_list.draft_canceled'); case BuildStates.Completed: return this.transloco.translate('draft_history_list.draft_completed'); case BuildStates.Faulted: return this.transloco.translate('draft_history_list.draft_faulted'); - case BuildStates.Active: - case BuildStates.Finishing: - case BuildStates.Pending: - case BuildStates.Queued: default: - return state.charAt(0).toUpperCase() + state.slice(1).toLowerCase(); + // The latest build must abe a build that has finished + return ''; } } @@ -64,6 +59,6 @@ export class DraftHistoryListComponent { } get isBuildActive(): boolean { - return this.history?.some(entry => activeBuildStates.includes(entry.state)) ?? false; + return this.history.some(entry => activeBuildStates.includes(entry.state)) ?? false; } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.html index 67bf98f4ce2..f4ffb75c60f 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.html @@ -1,6 +1,10 @@ @for (book of booksWithDrafts$ | async; track book.bookNumber) { - + {{ bookNumberToName(book.bookNumber) }} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.spec.ts index 120f5e9a957..ebdfa09041c 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.spec.ts @@ -19,6 +19,7 @@ import { UserService } from 'xforge-common/user.service'; import { SFProjectProfileDoc } from '../../../core/models/sf-project-profile-doc'; import { SFProjectService } from '../../../core/sf-project.service'; import { TextDocService } from '../../../core/text-doc.service'; +import { BuildDto } from '../../../machine-api/build-dto'; import { DraftApplyDialogComponent } from '../draft-apply-dialog/draft-apply-dialog.component'; import { DraftApplyProgress } from '../draft-apply-progress-dialog/draft-apply-progress-dialog.component'; import { DraftHandlingService } from '../draft-handling.service'; @@ -56,7 +57,14 @@ describe('DraftPreviewBooks', () => { env.progressSubscription?.unsubscribe(); }); - it('should show books', fakeAsync(() => { + it('should show books for a build', fakeAsync(() => { + env = new TestEnvironment({ + additionalInfo: { translationScriptureRanges: [{ projectId: 'project01', scriptureRange: 'LEV' }] } + } as BuildDto); + expect(env.draftBookCount()).toEqual(1); + })); + + it('should show books for a project if no build', fakeAsync(() => { env = new TestEnvironment(); expect(env.draftBookCount()).toEqual(3); })); @@ -70,7 +78,7 @@ describe('DraftPreviewBooks', () => { const [url, extras] = capture(mockedRouter.navigate).first(); expect(url).toEqual(['/projects', 'project01', 'translate', 'GEN', '1']); expect(extras).toEqual({ - queryParams: { 'draft-active': true } + queryParams: { 'draft-active': true, 'draft-timestamp': undefined } }); })); @@ -97,14 +105,14 @@ describe('DraftPreviewBooks', () => { tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).never(); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).never(); })); it('notifies user if applying a draft failed due to an error', fakeAsync(() => { env = new TestEnvironment(); const bookWithDraft: BookWithDraft = env.booksWithDrafts[0]; setupDialog('project01'); - when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())) + when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())) .thenReject(new Error('Draft error')) .thenResolve(true) .thenResolve(false); @@ -115,7 +123,7 @@ describe('DraftPreviewBooks', () => { expect(env.draftApplyProgress!.chaptersApplied).toEqual([2]); expect(env.draftApplyProgress!.completed).toBe(true); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times(3); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times(3); verify(mockedErrorReportingService.silentError(anything(), anything())).once(); })); @@ -123,7 +131,9 @@ describe('DraftPreviewBooks', () => { env = new TestEnvironment(); const bookWithDraft: BookWithDraft = env.booksWithDrafts[0]; setupDialog('project01'); - when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).thenResolve(true); + when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).thenResolve( + true + ); expect(env.getBookButtonAtIndex(0).querySelector('.book-more')).toBeTruthy(); env.component.chooseProjectToAddDraft(bookWithDraft); tick(); @@ -131,20 +141,42 @@ describe('DraftPreviewBooks', () => { expect(env.draftApplyProgress!.chaptersApplied).toEqual([1, 2, 3]); expect(env.draftApplyProgress!.completed).toBe(true); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times(3); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times(3); })); it('can apply chapters with drafts and skips chapters without drafts', fakeAsync(() => { env = new TestEnvironment(); const bookWithDraft: BookWithDraft = env.booksWithDrafts[1]; setupDialog('project01'); - when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).thenResolve(true); + when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).thenResolve( + true + ); + expect(env.getBookButtonAtIndex(1).querySelector('.book-more')).toBeTruthy(); + env.component.chooseProjectToAddDraft(bookWithDraft); + tick(); + env.fixture.detectChanges(); + verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times(1); + })); + + it('can apply a historic draft', fakeAsync(() => { + env = new TestEnvironment({ + additionalInfo: { + translationScriptureRanges: [{ projectId: 'project01', scriptureRange: 'GEN;EXO;LEV' }], + dateGenerated: '2024-08-27T01:02:03.004+00:00' + } + } as BuildDto); + const bookWithDraft: BookWithDraft = env.booksWithDrafts[1]; + setupDialog('project01'); + when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).thenResolve( + true + ); expect(env.getBookButtonAtIndex(1).querySelector('.book-more')).toBeTruthy(); env.component.chooseProjectToAddDraft(bookWithDraft); tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times(1); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times(1); })); it('can open dialog with the current project', fakeAsync(() => { @@ -160,7 +192,7 @@ describe('DraftPreviewBooks', () => { tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times( + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times( env.booksWithDrafts[0].chaptersWithDrafts.length ); verify(mockedProjectService.onlineAddChapters('project01', anything(), anything())).never(); @@ -178,7 +210,7 @@ describe('DraftPreviewBooks', () => { tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times( + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times( env.booksWithDrafts[0].chaptersWithDrafts.length ); verify(mockedProjectService.onlineAddChapters('otherProject', anything(), anything())).never(); @@ -206,7 +238,7 @@ describe('DraftPreviewBooks', () => { tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).never(); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).never(); expect().nothing(); })); @@ -237,7 +269,7 @@ describe('DraftPreviewBooks', () => { verify(mockedProjectService.onlineAddChapters(projectEmptyBook, anything(), anything())).once(); // needs to create 2 texts verify(mockedTextService.createTextDoc(anything())).twice(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times( + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times( env.booksWithDrafts[0].chaptersWithDrafts.length ); })); @@ -246,13 +278,15 @@ describe('DraftPreviewBooks', () => { env = new TestEnvironment(); const bookWithDraft: BookWithDraft = env.booksWithDrafts[0]; setupDialog('project01'); - when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).thenResolve(false); + when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).thenResolve( + false + ); expect(env.getBookButtonAtIndex(0).querySelector('.book-more')).toBeTruthy(); env.component.chooseProjectToAddDraft(bookWithDraft); tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times(3); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times(3); })); it('can track progress of chapters applied', fakeAsync(() => { @@ -263,7 +297,7 @@ describe('DraftPreviewBooks', () => { const promise: Promise = new Promise(resolve => { resolveSubject$.pipe(filter(value => value)).subscribe(() => resolve(true)); }); - when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())) + when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())) .thenReturn(Promise.resolve(true)) .thenReturn(promise); expect(env.getBookButtonAtIndex(0).querySelector('.book-more')).toBeTruthy(); @@ -271,7 +305,7 @@ describe('DraftPreviewBooks', () => { tick(); env.fixture.detectChanges(); verify(mockedDialogService.openMatDialog(DraftApplyDialogComponent, anything())).once(); - verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).times(3); + verify(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything())).times(3); expect(env.component.numChaptersApplied).toEqual(1); resolveSubject$.next(true); resolveSubject$.complete(); @@ -342,16 +376,19 @@ class TestEnvironment { { bookNumber: 3, canEdit: false, chaptersWithDrafts: [1, 2], draftApplied: false } ]; - constructor() { + constructor(build: BuildDto | undefined = undefined) { when(mockedActivatedProjectService.changes$).thenReturn(of(this.mockProjectDoc)); when(mockedActivatedProjectService.projectDoc).thenReturn(this.mockProjectDoc); when(mockedI18nService.localizeBook(1)).thenReturn('Genesis'); - when(mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything())).thenResolve(); + when( + mockedDraftHandlingService.getAndApplyDraftAsync(anything(), anything(), anything(), anything()) + ).thenResolve(); when(mockedActivatedProjectService.projectId).thenReturn('project01'); when(mockedUserService.currentUserId).thenReturn('user01'); when(mockedProjectService.getProfile(anything())).thenResolve(this.mockProjectDoc); this.fixture = TestBed.createComponent(DraftPreviewBooksComponent); this.component = this.fixture.componentInstance; + this.component.build = build; this.loader = TestbedHarnessEnvironment.loader(this.fixture); this.component.draftApplyProgress$.subscribe(progress => (this.draftApplyProgress = progress)); tick(); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts index f10ed0b8408..0cb3b59a371 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts @@ -1,5 +1,5 @@ import { CommonModule } from '@angular/common'; -import { Component } from '@angular/core'; +import { Component, Input } from '@angular/core'; import { MatDialogRef } from '@angular/material/dialog'; import { Router, RouterModule } from '@angular/router'; import { TranslocoModule } from '@ngneat/transloco'; @@ -19,6 +19,8 @@ import { SFProjectProfileDoc } from '../../../core/models/sf-project-profile-doc import { TextDocId } from '../../../core/models/text-doc'; import { SFProjectService } from '../../../core/sf-project.service'; import { TextDocService } from '../../../core/text-doc.service'; +import { BuildDto } from '../../../machine-api/build-dto'; +import { booksFromScriptureRange } from '../../../shared/utils'; import { DraftApplyDialogComponent, DraftApplyDialogConfig as DraftApplyDialogData, @@ -45,6 +47,8 @@ export interface BookWithDraft { imports: [CommonModule, UICommonModule, RouterModule, TranslocoModule] }) export class DraftPreviewBooksComponent { + @Input() build: BuildDto | undefined; + booksWithDrafts$: Observable = this.activatedProjectService.changes$.pipe( filterNullish(), tap(p => (this.projectParatextId = p.data?.paratextId)), @@ -52,15 +56,33 @@ export class DraftPreviewBooksComponent { if (projectDoc?.data == null) { return []; } - const draftBooks = projectDoc.data.texts - .map(text => ({ - bookNumber: text.bookNum, - canEdit: text.permissions[this.userService.currentUserId] === TextInfoPermission.Write, - chaptersWithDrafts: text.chapters.filter(chapter => chapter.hasDraft).map(chapter => chapter.number), - draftApplied: text.chapters.filter(chapter => chapter.hasDraft).every(chapter => chapter.draftApplied) - })) - .sort((a, b) => a.bookNumber - b.bookNumber) - .filter(book => book.chaptersWithDrafts.length > 0) as BookWithDraft[]; + let draftBooks: BookWithDraft[]; + if (this.build == null) { + draftBooks = projectDoc.data.texts + .map(text => ({ + bookNumber: text.bookNum, + canEdit: text.permissions[this.userService.currentUserId] === TextInfoPermission.Write, + chaptersWithDrafts: text.chapters.filter(chapter => chapter.hasDraft).map(chapter => chapter.number), + draftApplied: text.chapters.filter(chapter => chapter.hasDraft).every(chapter => chapter.draftApplied) + })) + .sort((a, b) => a.bookNumber - b.bookNumber) + .filter(book => book.chaptersWithDrafts.length > 0) as BookWithDraft[]; + } else { + draftBooks = this.build.additionalInfo?.translationScriptureRanges + .flatMap(range => booksFromScriptureRange(range.scriptureRange)) + .map(bookNum => { + const text: TextInfo | undefined = projectDoc.data?.texts.find(t => t.bookNum === bookNum); + return { + bookNumber: bookNum, + canEdit: text?.permissions?.[this.userService.currentUserId] === TextInfoPermission.Write, + chaptersWithDrafts: text?.chapters?.map(ch => ch.number) ?? [], + draftApplied: text?.chapters?.filter(ch => ch.hasDraft).every(ch => ch.draftApplied) ?? false + }; + }) + // Do not filter chapters with drafts, as the book or chapters may have been removed. + // We still want to display these books to the user, but disabled so they cannot interact with them. + .sort((a, b) => a.bookNumber - b.bookNumber) as BookWithDraft[]; + } return draftBooks; }) ); @@ -175,7 +197,7 @@ export class DraftPreviewBooksComponent { navigate(book: BookWithDraft): void { this.router.navigate(this.linkForBookAndChapter(book.bookNumber, book.chaptersWithDrafts[0]), { - queryParams: { 'draft-active': true } + queryParams: { 'draft-active': true, 'draft-timestamp': this.build?.additionalInfo?.dateGenerated } }); } @@ -191,8 +213,12 @@ export class DraftPreviewBooksComponent { draftTextDocId: TextDocId, targetTextDocId: TextDocId ): Promise { + let timestamp: Date | undefined = undefined; + if (this.build?.additionalInfo?.dateGenerated != null) { + timestamp = new Date(this.build.additionalInfo.dateGenerated); + } return await this.draftHandlingService - .getAndApplyDraftAsync(project, draftTextDocId, targetTextDocId) + .getAndApplyDraftAsync(project, draftTextDocId, targetTextDocId, timestamp) .then(result => { this.updateProgress(result ? targetTextDocId.chapterNum : undefined); return result; 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 07895ba427e..c22aab4e25d 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 @@ -18,6 +18,27 @@ @if (isDraftReady) {

+ + + + {{ selectedRevision ?? "" | revisionFormat }} + + @for (r of draftRevisions; track r) { + +
+ {{ r | revisionFormat }} +
+ + } + +
@if (userAppliedDraft) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.scss index 36d5f9e3c76..bdc11dedd19 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.scss @@ -1,3 +1,5 @@ +@use 'src/styles'; + :host { display: flex; flex-direction: column; @@ -18,14 +20,17 @@ app-notice { background: var(--sf-tab-toolbar-background-color); z-index: 1; display: flex; //fill vertical space + padding: 0.5em; + gap: 4px; + flex-wrap: wrap; + align-items: center; + border-bottom: 1px solid var(--sf-tab-group-border-color); } .apply-draft-button-container { flex-grow: 1; display: flex; justify-content: flex-end; - padding: 0.5em; - border-bottom: 1px solid var(--sf-tab-group-border-color); } .draft-indicator { @@ -38,3 +43,7 @@ app-notice { align-items: center; margin: 0 1em; } + +.draft-select { + @include styles.dense_mat_select(); +} 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 602a4c7917f..8d37cdf367c 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 @@ -1,5 +1,7 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; import { MatProgressBarModule } from '@angular/material/progress-bar'; +import { MatSelectChange, MatSelectModule } from '@angular/material/select'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { cloneDeep } from 'lodash-es'; import { TranslocoMarkupModule } from 'ngx-transloco-markup'; import { Delta } from 'quill'; @@ -19,11 +21,13 @@ import { TestRealtimeModule } from 'xforge-common/test-realtime.module'; import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; import { SFProjectProfileDoc } from '../../../core/models/sf-project-profile-doc'; import { SF_TYPE_REGISTRY } from '../../../core/models/sf-type-registry'; +import { Revision } from '../../../core/paratext.service'; import { SharedModule } from '../../../shared/shared.module'; import { EDITOR_READY_TIMEOUT } from '../../../shared/text/text.component'; import { DraftSegmentMap } from '../../draft-generation/draft-generation'; import { DraftGenerationService } from '../../draft-generation/draft-generation.service'; import { DraftHandlingService } from '../../draft-generation/draft-handling.service'; +import { HistoryRevisionFormatPipe } from '../editor-history/history-chooser/history-revision-format.pipe'; import { EditorDraftComponent } from './editor-draft.component'; const mockDraftGenerationService = mock(DraftGenerationService); @@ -40,9 +44,11 @@ describe('EditorDraftComponent', () => { let testOnlineStatus: TestOnlineStatusService; configureTestingModule(() => ({ - declarations: [EditorDraftComponent], + declarations: [EditorDraftComponent, HistoryRevisionFormatPipe], imports: [ MatProgressBarModule, + MatSelectModule, + NoopAnimationsModule, SharedModule.forRoot(), TestOnlineStatusModule.forRoot(), TestRealtimeModule.forRoot(SF_TYPE_REGISTRY), @@ -88,6 +94,9 @@ describe('EditorDraftComponent', () => { when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftMap)); when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!); when(mockDraftHandlingService.isDraftSegmentMap(anything())).thenReturn(true); @@ -112,6 +121,9 @@ describe('EditorDraftComponent', () => { fixture.detectChanges(); expect(component.draftCheckState).toEqual('draft-legacy'); expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); })); it('should return ops and update the editor', fakeAsync(() => { @@ -119,6 +131,32 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); + when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(draftDelta.ops!))); + when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!); + when(mockDraftHandlingService.isDraftSegmentMap(anything())).thenReturn(false); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + verify(mockDraftHandlingService.getDraft(anything(), anything())).once(); + verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).once(); + expect(component.draftCheckState).toEqual('draft-present'); + expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); + })); + + it('should return ops and update the editor when no revision', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile() + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(undefined) + ); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(draftDelta.ops!))); @@ -134,12 +172,44 @@ describe('EditorDraftComponent', () => { expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); })); + it('should return ops and update the editor when the selected revision changes', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile() + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); + when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(draftDelta.ops!))); + when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!); + when(mockDraftHandlingService.isDraftSegmentMap(anything())).thenReturn(false); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + // SUT + component.onSelectionChanged({ value: draftHistory[1] } as MatSelectChange); + + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + verify(mockDraftHandlingService.getDraft(anything(), anything())).twice(); + verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).twice(); + expect(component.draftCheckState).toEqual('draft-present'); + expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); + })); + describe('applyDraft', () => { it('should show a prompt when applying if the target has content', fakeAsync(() => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); when(mockDialogService.confirm(anything(), anything())).thenResolve(true); spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops)); @@ -163,6 +233,9 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); spyOn(component, 'getTargetOps').and.returnValue(of([])); when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftDelta.ops!)); @@ -191,6 +264,9 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); when(mockDialogService.confirm(anything(), anything())).thenResolve(true); when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftDelta.ops!)); @@ -215,6 +291,9 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); when(mockDialogService.confirm(anything(), anything())).thenResolve(true); when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftDelta.ops!)); @@ -305,6 +384,11 @@ const draftDelta = new Delta([ } ]); +const draftHistory: Revision[] = [ + { source: 'Draft', timestamp: '2025-03-25T01:02:03Z' }, + { source: 'Draft', timestamp: '2025-03-22T03:02:01Z' } +]; + const targetDelta = new Delta([ { attributes: { 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 f7f400b9d03..fb5e4843317 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 @@ -1,17 +1,21 @@ import { AfterViewInit, Component, DestroyRef, EventEmitter, Input, OnChanges, ViewChild } from '@angular/core'; +import { MatSelectChange } from '@angular/material/select'; import { translate } from '@ngneat/transloco'; import { Delta } from 'quill'; import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project'; import { DeltaOperation } from 'rich-text'; import { asyncScheduler, + BehaviorSubject, combineLatest, distinctUntilChanged, EMPTY, filter, from, map, + merge, Observable, + observeOn, startWith, Subject, switchMap, @@ -29,6 +33,7 @@ import { OnlineStatusService } from 'xforge-common/online-status.service'; import { filterNullish, quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; 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 { TextComponent } from '../../../shared/text/text.component'; import { DraftGenerationService } from '../../draft-generation/draft-generation.service'; @@ -44,11 +49,14 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { @Input() chapter?: number; @Input() isRightToLeft!: boolean; @Input() fontSize?: string; + @Input() timestamp?: Date; @ViewChild(TextComponent) draftText!: TextComponent; inputChanged$ = new Subject(); draftCheckState: 'draft-unknown' | 'draft-present' | 'draft-legacy' | 'draft-empty' = 'draft-unknown'; + draftRevisions: Revision[] = []; + selectedRevision: Revision | undefined; bookChapterName = ''; generateDraftUrl?: string; targetProject?: SFProjectProfile; @@ -57,6 +65,18 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { isDraftApplied = false; userAppliedDraft = false; + private selectedRevisionSubject = new BehaviorSubject(undefined); + private selectedRevision$ = this.selectedRevisionSubject.asObservable(); + + // 'asyncScheduler' prevents ExpressionChangedAfterItHasBeenCheckedError + private loading$ = new BehaviorSubject(false); + isLoading$: Observable = this.loading$.pipe(observeOn(asyncScheduler)); + + isSelectDisabled$: Observable = combineLatest([ + this.isLoading$, + this.onlineStatusService.onlineStatus$ + ]).pipe(map(([isLoading, isOnline]) => isLoading || !isOnline)); + private draftDelta?: Delta; private targetDelta?: Delta; @@ -88,6 +108,11 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { this.populateDraftTextInit(); } + async onSelectionChanged(e: MatSelectChange): Promise { + this.selectedRevision = e.value; + this.selectedRevisionSubject.next(this.selectedRevision); + } + populateDraftTextInit(): void { combineLatest([ this.onlineStatusService.onlineStatus$, @@ -98,9 +123,38 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { quietTakeUntilDestroyed(this.destroyRef), filter(([isOnline]) => isOnline), tap(() => this.setInitialState()), - switchMap(() => this.draftExists()), - switchMap((draftExists: boolean) => { - if (!draftExists) { + switchMap(() => + combineLatest([ + merge( + this.draftGenerationService + .getGeneratedDraftHistory( + this.textDocId!.projectId, + this.textDocId!.bookNum, + this.textDocId!.chapterNum + ) + .pipe( + map(revisions => { + if (revisions != null) { + this.draftRevisions = revisions; + const date = this.timestamp ?? new Date(); + // Don't emit this.selectedRevision$, as the merge will handle this + this.selectedRevision = this.findClosestRevision(date, this.draftRevisions); + return date; + } else { + return undefined; + } + }) + ), + this.selectedRevision$.pipe( + filter((rev): rev is { timestamp: string } => rev != null), + map(revision => new Date(revision.timestamp)) + ) + ), + this.draftExists() + ]) + ), + switchMap(([timestamp, draftExists]) => { + if (!draftExists && timestamp == null) { this.draftCheckState = 'draft-empty'; return EMPTY; } @@ -111,13 +165,14 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { tap(projectDoc => { this.targetProject = projectDoc.data; }), - distinctUntilChanged() + distinctUntilChanged(), + map(() => timestamp) ); }), - switchMap(() => + switchMap((timestamp: Date | undefined) => combineLatest([ this.getTargetOps(), - this.draftHandlingService.getDraft(this.textDocId!, { isDraftLegacy: false }) + this.draftHandlingService.getDraft(this.textDocId!, { isDraftLegacy: false, timestamp }) ]) ), tap(([_, draft]) => { @@ -201,6 +256,31 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { return this.draftGenerationService.draftExists(this.projectId!, this.bookNum!, this.chapter!); } + findClosestRevision(date: Date, revisions: Revision[]): Revision { + const targetTime: number = date.getTime(); + const oneHour: number = 60 * 60 * 1000; + + let closestBefore: Revision | null = null; + let closestAfter: Revision | null = null; + + for (const rev of revisions) { + const revTime = new Date(rev.timestamp).getTime(); + if (revTime <= targetTime) { + closestBefore = rev; + } else { + closestAfter = rev; + break; + } + } + + // If there is no revision after, or it's too far in the future, prefer the one before + if (closestAfter == null || new Date(closestAfter.timestamp).getTime() - targetTime > oneHour) { + return closestBefore!; + } + + return closestAfter; + } + private hasContent(delta?: DeltaOperation[]): boolean { const hasContent = delta?.some(op => { if (op.insert == null || op.attributes?.segment == null) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.html index eff94cb37d3..42f38fc167b 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.html @@ -277,6 +277,7 @@ [chapter]="chapter" [isRightToLeft]="isTargetRightToLeft" [fontSize]="fontSize" + [timestamp]="draftTimestamp" > } 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 e0189b25517..9326701a2b2 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 @@ -105,6 +105,7 @@ import { BiblicalTermsComponent } from '../biblical-terms/biblical-terms.compone import { DraftGenerationService } from '../draft-generation/draft-generation.service'; import { TrainingProgressComponent } from '../training-progress/training-progress.component'; import { EditorDraftComponent } from './editor-draft/editor-draft.component'; +import { HistoryRevisionFormatPipe } from './editor-history/history-chooser/history-revision-format.pipe'; import { EditorComponent, UPDATE_SUGGESTIONS_TIMEOUT } from './editor.component'; import { LynxInsightStateService } from './lynx/insights/lynx-insight-state.service'; import { LynxInsightsModule } from './lynx/insights/lynx-insights.module'; @@ -155,7 +156,13 @@ class MockConsole { describe('EditorComponent', () => { configureTestingModule(() => ({ - declarations: [EditorComponent, SuggestionsComponent, TrainingProgressComponent, EditorDraftComponent], + declarations: [ + EditorComponent, + SuggestionsComponent, + TrainingProgressComponent, + EditorDraftComponent, + HistoryRevisionFormatPipe + ], imports: [ BiblicalTermsComponent, CopyrightBannerComponent, @@ -4039,7 +4046,9 @@ describe('EditorComponent', () => { it('should select the draft tab if url query param is set', fakeAsync(() => { const env = new TestEnvironment(); - when(mockedActivatedRoute.snapshot).thenReturn({ queryParams: { 'draft-active': 'true' } } as any); + when(mockedActivatedRoute.snapshot).thenReturn({ + queryParams: { 'draft-active': 'true', 'draft-timestamp': new Date().toISOString() } + } as any); when(mockedPermissionsService.canAccessDrafts(anything(), anything())).thenReturn(true); env.wait(); env.routeWithParams({ projectId: 'project01', bookId: 'LUK', chapter: '1' }); @@ -4484,9 +4493,10 @@ class TestEnvironment { }); when(mockedDraftGenerationService.getLastCompletedBuild(anything())).thenReturn(of({} as any)); when(mockedDraftGenerationService.getGeneratedDraft(anything(), anything(), anything())).thenReturn(of({})); - when(mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything())).thenReturn( - of([]) - ); + when( + mockedDraftGenerationService.getGeneratedDraftDeltaOperations(anything(), anything(), anything(), anything()) + ).thenReturn(of([])); + when(mockedDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn(of([])); when(mockedDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockedPermissionsService.isUserOnProject(anything())).thenResolve(true); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts index 23fcf40eebb..19ff3fd6bfc 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts @@ -218,6 +218,7 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy, mobileNoteControl: UntypedFormControl = new UntypedFormControl(''); multiCursorViewers: MultiCursorViewer[] = []; target: TextComponent | undefined; + draftTimestamp?: Date; showInsights = false; @ViewChild('source') source?: TextComponent; @@ -1443,11 +1444,16 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy, this.tabState.getFirstTabOfTypeIndex('draft'); const urlDraftActive: boolean = this.activatedRoute.snapshot.queryParams['draft-active'] === 'true'; + if (this.activatedRoute.snapshot.queryParams['draft-timestamp'] != null) { + this.draftTimestamp = new Date(this.activatedRoute.snapshot.queryParams['draft-timestamp']); + } else { + this.draftTimestamp = undefined; + } const canViewDrafts: boolean = this.permissionsService.canAccessDrafts( this.projectDoc, this.userService.currentUserId ); - if (hasDraft && (!draftApplied || urlDraftActive) && canViewDrafts) { + if (((hasDraft && !draftApplied) || urlDraftActive) && canViewDrafts) { // URL may indicate to select the 'draft' tab (such as when coming from generate draft page) const groupIdToAddTo: EditorTabGroupType = this.showSource ? 'source' : 'target'; @@ -1470,7 +1476,7 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy, } if (urlDraftActive) { - // Remove 'draft-active' query string from url when another tab from group is selected + // Remove 'draft-active' and 'draft-timestamp' query string from url when another tab from group is selected this.tabState.tabs$ .pipe( filter(tabs => tabs.some(tab => tab.groupId === groupIdToAddTo && tab.type !== 'draft' && tab.isSelected)), @@ -1478,7 +1484,7 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy, ) .subscribe(() => { this.router.navigate([], { - queryParams: { 'draft-active': null }, + queryParams: { 'draft-active': null, 'draft-timestamp': null }, queryParamsHandling: 'merge', replaceUrl: true }); 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 0a9fe85270e..44b40827076 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 @@ -264,6 +264,12 @@ "draft_history_entry": { "click_book_to_preview": "Click a book below to preview the draft and add it to your project.", "download_zip": "Download draft as ZIP file", + "draft_active": "Running", + "draft_canceled": "Canceled", + "draft_completed": "Completed", + "draft_faulted": "Failed", + "draft_pending": "Pending", + "draft_unknown": "Unknown", "hide_model_training_data": "Hide model training data", "show_model_training_data": "Show model training data", "training_data_configuration": "The language model was trained on this configuration" @@ -271,7 +277,7 @@ "draft_history_list": { "draft_canceled": "Your draft was canceled", "draft_completed": "Your draft is ready", - "drafT_faulted": "The draft failed", + "draft_faulted": "The draft failed", "previously_generated_drafts": "Previously generated drafts" }, "draft_preview_books": { From 80e60446e4d075340be6832ec1b1d3a7b9847d47 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Tue, 29 Apr 2025 11:29:11 +1200 Subject: [PATCH 06/11] Fix interceptor for BuildProjectAsync --- src/SIL.XForge.Scripture/Services/MachineApiService.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index 01c6c20137f..3d41b4d7d24 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -1321,7 +1321,9 @@ await projectDoc.SubmitJson0OpAsync(op => } // Run the training after the sync has completed - jobId = backgroundJobClient.ContinueJobWith( + // NOTE: This must be MachineProjectService, not IMachineProjectService + // so that the interceptor functions for BuildProjectAsync(). + jobId = backgroundJobClient.ContinueJobWith( jobId, r => r.BuildProjectForBackgroundJobAsync(curUserId, buildConfig, true, CancellationToken.None) ); From 0cb59a83713c8ea421ccb78701809d7d8fd38296 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Thu, 24 Apr 2025 14:43:26 +1200 Subject: [PATCH 07/11] Display training data --- .../draft-history-entry.component.html | 55 ++++++++---- .../draft-history-entry.component.spec.ts | 38 +++++++- .../draft-history-entry.component.ts | 89 ++++++++++++++++++- 3 files changed, 161 insertions(+), 21 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html index ac50b2adc46..4d9b538c716 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html @@ -10,11 +10,11 @@ {{ getStatus(entry.state).icons }} {{ t(getStatus(entry.state).text) }} - @if (!forceDetailsOpen) { + @if (!forceDetailsOpen && hasDetails) { expand_{{ detailsOpen ? "less" : "more" }} }
- @if (detailsOpen) { + @if (detailsOpen && hasDetails) {

{{ t("click_book_to_preview") }}

@@ -23,22 +23,43 @@

-

- - expand_{{ trainingDataOpen ? "less" : "more" }} - {{ t(trainingDataOpen ? "hide_model_training_data" : "show_model_training_data") }} - -

- @if (trainingDataOpen) { -

- {{ t("training_data_configuration") }} - [TODO] + +

+ + expand_{{ trainingDataOpen ? "less" : "more" }} + {{ t(trainingDataOpen ? "hide_model_training_data" : "show_model_training_data") }} +

- } + @if (trainingDataOpen) { +

+ {{ t("training_data_configuration") }} +

+ + + + + + + + + + + + + + + +
{{ i18n.enumerateList(element.bookNames) }} + {{ i18n.getLanguageDisplayName(sourceLanguage) }} + {{ element.source }} + {{ i18n.getLanguageDisplayName(targetLanguage) }} + {{ element.target }}
+ } +
} } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts index 7942efc44f0..d2ce72328bf 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts @@ -1,15 +1,19 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; import { createTestUserProfile } from 'realtime-server/lib/esm/common/models/user-test-data'; +import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; import { anything, mock, when } from 'ts-mockito'; import { I18nService } from 'xforge-common/i18n.service'; import { UserProfileDoc } from 'xforge-common/models/user-profile-doc'; import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; import { UserService } from 'xforge-common/user.service'; +import { SFProjectProfileDoc } from '../../../../core/models/sf-project-profile-doc'; +import { SFProjectService } from '../../../../core/sf-project.service'; import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; import { DraftHistoryEntryComponent } from './draft-history-entry.component'; const mockedI18nService = mock(I18nService); +const mockedSFProjectService = mock(SFProjectService); const mockedUserService = mock(UserService); describe('DraftHistoryEntryComponent', () => { @@ -20,6 +24,7 @@ describe('DraftHistoryEntryComponent', () => { imports: [TestTranslocoModule], providers: [ { provide: I18nService, useMock: mockedI18nService }, + { provide: SFProjectService, useMock: mockedSFProjectService }, { provide: UserService, useMock: mockedUserService } ] })); @@ -59,22 +64,38 @@ describe('DraftHistoryEntryComponent', () => { expect(component.buildRequestedByUserName).toBeUndefined(); expect(component.buildRequestedByDate).toBe(''); expect(component.canDownloadBuild).toBe(false); + expect(component.hasDetails).toBe(false); expect(component.entry).toBeUndefined(); }); it('should handle builds with additional info', fakeAsync(() => { when(mockedI18nService.formatDate(anything())).thenReturn('formatted-date'); - when(mockedI18nService.localizeBook('GEN')).thenReturn('localized-book'); + when(mockedI18nService.localizeBook('GEN')).thenReturn('Genesis'); + when(mockedI18nService.localizeBook('EXO')).thenReturn('Exodus'); const userDoc = { id: 'sf-user-id', data: createTestUserProfile({ displayName: 'user-display-name' }) } as UserProfileDoc; when(mockedUserService.getProfile('sf-user-id')).thenResolve(userDoc); + const targetProjectDoc = { + id: 'project01', + data: createTestProjectProfile({ shortName: 'tar', writingSystem: { tag: 'en' } }) + } as SFProjectProfileDoc; + when(mockedSFProjectService.getProfile('project01')).thenResolve(targetProjectDoc); + const sourceProjectDoc = { + id: 'project02', + data: createTestProjectProfile({ shortName: 'src', writingSystem: { tag: 'fr' } }) + } as SFProjectProfileDoc; + when(mockedSFProjectService.getProfile('project02')).thenResolve(sourceProjectDoc); const entry = { + engine: { + id: 'project01' + }, additionalInfo: { dateGenerated: new Date().toISOString(), dateRequested: new Date().toISOString(), requestedByUserId: 'sf-user-id', + trainingScriptureRanges: [{ projectId: 'project02', scriptureRange: 'EXO' }], translationScriptureRanges: [{ projectId: 'project01', scriptureRange: 'GEN' }] } } as BuildDto; @@ -84,11 +105,21 @@ describe('DraftHistoryEntryComponent', () => { tick(); fixture.detectChanges(); - expect(component.bookNames).toEqual(['localized-book']); + expect(component.bookNames).toEqual(['Genesis']); expect(component.buildRequestedByUserName).toBe('user-display-name'); expect(component.buildRequestedByDate).toBe('formatted-date'); expect(component.canDownloadBuild).toBe(true); + expect(component.hasDetails).toBe(true); expect(component.entry).toBe(entry); + expect(component.sourceLanguage).toBe('fr'); + expect(component.targetLanguage).toBe('en'); + expect(component.trainingData).toEqual([ + { + bookNames: ['Exodus'], + source: 'src', + target: 'tar' + } + ]); })); it('should handle builds with additional info referencing a deleted user', fakeAsync(() => { @@ -114,6 +145,7 @@ describe('DraftHistoryEntryComponent', () => { expect(component.buildRequestedByUserName).toBeUndefined(); expect(component.buildRequestedByDate).toBe('formatted-date'); expect(component.canDownloadBuild).toBe(true); + expect(component.hasDetails).toBe(true); expect(component.entry).toBe(entry); })); @@ -124,6 +156,7 @@ describe('DraftHistoryEntryComponent', () => { expect(component.buildRequestedByUserName).toBeUndefined(); expect(component.buildRequestedByDate).toBe(''); expect(component.canDownloadBuild).toBe(false); + expect(component.hasDetails).toBe(false); expect(component.entry).toBe(entry); }); @@ -134,6 +167,7 @@ describe('DraftHistoryEntryComponent', () => { expect(component.buildRequestedByUserName).toBeUndefined(); expect(component.buildRequestedByDate).toBe(''); expect(component.canDownloadBuild).toBe(false); + expect(component.hasDetails).toBe(false); expect(component.entry).toBe(entry); }); }); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts index 632399b871e..d72b16c8d9a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -1,9 +1,12 @@ import { CommonModule } from '@angular/common'; import { Component, Input } from '@angular/core'; +import { MatIconModule } from '@angular/material/icon'; +import { MatTableModule } from '@angular/material/table'; import { TranslocoModule } from '@ngneat/transloco'; import { I18nService } from 'xforge-common/i18n.service'; -import { UICommonModule } from 'xforge-common/ui-common.module'; import { UserService } from 'xforge-common/user.service'; +import { SFProjectProfileDoc } from '../../../../core/models/sf-project-profile-doc'; +import { SFProjectService } from '../../../../core/sf-project.service'; import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; import { DraftDownloadButtonComponent } from '../../draft-download-button/draft-download-button.component'; @@ -19,10 +22,23 @@ const STATUS_INFO: Record { + // The engine ID is the target project ID + let target: SFProjectProfileDoc | undefined = undefined; + if (this.entry?.engine.id != null) { + target = await this.projectService.getProfile(this.entry?.engine.id); + } + + // Get the target language, if it is not already set + this._targetLanguage ??= target?.data?.writingSystem.tag; + + // Get the source project + const source = await this.projectService.getProfile(r.projectId); + + // Get the source language, if it is not already set + this._sourceLanguage ??= source?.data?.writingSystem.tag; + + // Return the data for this training range + return { + bookNames: r.scriptureRange.split(';').map(id => this.i18n.localizeBook(id)), + source: source?.data?.shortName ?? '', + target: target?.data?.shortName ?? '' + } as TrainingDataRow; + }) + ).then(trainingData => { + // Set the training data for the table + this._trainingData = trainingData; + + // If we can only show training data, expand the training data + if (!this.canDownloadBuild && this.hasTrainingData) { + this.trainingDataOpen = true; + } + }); } get entry(): BuildDto | undefined { return this._entry; @@ -67,6 +124,33 @@ export class DraftHistoryEntryComponent { return this.i18n.formatDate(new Date(this._entry?.additionalInfo?.dateRequested)); } + get columnsToDisplay(): string[] { + return ['bookNames', 'source', 'target']; + } + + get hasDetails(): boolean { + return this.hasTrainingData || this.canDownloadBuild; + } + + get hasTrainingData(): boolean { + return this._trainingData.length > 0; + } + + private _sourceLanguage?: string = undefined; + get sourceLanguage(): string { + return this._sourceLanguage ?? ''; + } + + private _targetLanguage?: string = undefined; + get targetLanguage(): string { + return this._targetLanguage ?? ''; + } + + private _trainingData: TrainingDataRow[] = []; + get trainingData(): TrainingDataRow[] { + return this._trainingData; + } + @Input() canDownloadBuild = false; detailsOpen = false; @@ -74,6 +158,7 @@ export class DraftHistoryEntryComponent { constructor( readonly i18n: I18nService, + private readonly projectService: SFProjectService, private readonly userService: UserService ) {} From 250dbec7f94a4495a9a450d234d3f67b729ceddc Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Tue, 29 Apr 2025 12:03:31 +1200 Subject: [PATCH 08/11] Added feature flag to show/hide the new draft history UI --- scripts/db_tools/parse-version.ts | 3 ++- .../draft-generation/draft-generation.component.html | 8 +++++--- .../draft-generation/draft-generation.component.spec.ts | 9 ++++++++- .../draft-generation/draft-generation.component.ts | 2 ++ .../xforge-common/feature-flags/feature-flag.service.ts | 7 +++++++ 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/scripts/db_tools/parse-version.ts b/scripts/db_tools/parse-version.ts index 0ac7ad89a6c..6fc347558a6 100644 --- a/scripts/db_tools/parse-version.ts +++ b/scripts/db_tools/parse-version.ts @@ -39,7 +39,8 @@ class ParseVersion { 'Allow mixing in an additional training source', 'Updated Learning Rate For Serval', 'Dark Mode', - 'Enable Lynx insights' + 'Enable Lynx insights', + 'Preview new draft history interface' ]; constructor() { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html index 95bc03cae23..1e6600c28e3 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html @@ -261,7 +261,7 @@

{{ t("draft_finishing_header") }}

} - @if (isDraftComplete(draftJob) || hasAnyCompletedBuild) { + @if ((isDraftComplete(draftJob) || hasAnyCompletedBuild) && !featureFlags.newDraftHistory.enabled) {
@@ -333,6 +333,10 @@

{{ t("draft_finishing_header") }}

}
+ @if (featureFlags.newDraftHistory.enabled) { + + } + @if (canShowAdditionalInfo(draftJob)) {
@@ -363,5 +367,3 @@

{{ t("draft_finishing_header") }}

} } - - diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts index 25a5602feda..c3a3ec54511 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.spec.ts @@ -13,6 +13,7 @@ import { anything, instance, mock, verify, when } from 'ts-mockito'; import { ActivatedProjectService } from 'xforge-common/activated-project.service'; import { AuthService } from 'xforge-common/auth.service'; import { DialogService } from 'xforge-common/dialog.service'; +import { createTestFeatureFlag, FeatureFlagService } from 'xforge-common/feature-flags/feature-flag.service'; import { RealtimeQuery } from 'xforge-common/models/realtime-query'; import { UserDoc } from 'xforge-common/models/user-doc'; import { NoticeService } from 'xforge-common/notice.service'; @@ -51,6 +52,7 @@ describe('DraftGenerationComponent', () => { let mockPreTranslationSignupUrlService: jasmine.SpyObj; let mockTrainingDataService: jasmine.SpyObj; let mockProgressService: jasmine.SpyObj; + let mockFeatureFlagService: jasmine.SpyObj; const buildDto: BuildDto = { id: 'testId', @@ -98,7 +100,8 @@ describe('DraftGenerationComponent', () => { { provide: OnlineStatusService, useClass: TestOnlineStatusService }, { provide: PreTranslationSignupUrlService, useValue: mockPreTranslationSignupUrlService }, { provide: TrainingDataService, useValue: mockTrainingDataService }, - { provide: ProgressService, useValue: mockProgressService } + { provide: ProgressService, useValue: mockProgressService }, + { provide: FeatureFlagService, useValue: mockFeatureFlagService } ] }); @@ -156,6 +159,9 @@ describe('DraftGenerationComponent', () => { when(mockTrainingDataQuery.remoteDocChanges$).thenReturn(of()); mockTrainingDataService = jasmine.createSpyObj(['queryTrainingDataAsync']); mockTrainingDataService.queryTrainingDataAsync.and.returnValue(Promise.resolve(instance(mockTrainingDataQuery))); + mockFeatureFlagService = jasmine.createSpyObj({ + newDraftHistory: createTestFeatureFlag(false) + }); } static initProject(currentUserId: string, preTranslate: boolean = true): void { @@ -1416,6 +1422,7 @@ describe('DraftGenerationComponent', () => { it('button should display if there is a completed build while a build is queued', () => { const env = new TestEnvironment(); + env.setup(); env.component.draftJob = { ...buildDto, state: BuildStates.Queued }; env.component.lastCompletedBuild = { ...buildDto, state: BuildStates.Completed }; env.component.hasDraftBooksAvailable = true; diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts index 56a0dfa2a05..dd81d4f2616 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.ts @@ -17,6 +17,7 @@ import { AuthService } from 'xforge-common/auth.service'; import { DataLoadingComponent } from 'xforge-common/data-loading-component'; import { DialogService } from 'xforge-common/dialog.service'; import { ExternalUrlService } from 'xforge-common/external-url.service'; +import { FeatureFlagService } from 'xforge-common/feature-flags/feature-flag.service'; import { I18nService } from 'xforge-common/i18n.service'; import { NoticeService } from 'xforge-common/notice.service'; import { OnlineStatusService } from 'xforge-common/online-status.service'; @@ -152,6 +153,7 @@ export class DraftGenerationComponent extends DataLoadingComponent implements On private readonly preTranslationSignupUrlService: PreTranslationSignupUrlService, protected readonly noticeService: NoticeService, protected readonly urlService: ExternalUrlService, + protected readonly featureFlags: FeatureFlagService, private destroyRef: DestroyRef ) { super(noticeService); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/feature-flags/feature-flag.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/feature-flags/feature-flag.service.ts index 9e32fada11b..27d822998ca 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/feature-flags/feature-flag.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/feature-flags/feature-flag.service.ts @@ -326,6 +326,13 @@ export class FeatureFlagService { this.featureFlagStore ); + readonly newDraftHistory: ObservableFeatureFlag = new FeatureFlagFromStorage( + 'NewDraftHistory', + 'Preview new draft history interface', + 17, + this.featureFlagStore + ); + get featureFlags(): FeatureFlag[] { return Object.values(this).filter(value => value instanceof FeatureFlagFromStorage); } From f160b191bba7f1b2464357264733caf6ea6bfe71 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Wed, 30 Apr 2025 07:59:38 +1200 Subject: [PATCH 09/11] Improve test coverage --- .../draft-history-entry.component.spec.ts | 47 +++++++++++++++ .../editor-draft.component.spec.ts | 57 +++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts index d2ce72328bf..9d17db769d6 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts @@ -109,6 +109,7 @@ describe('DraftHistoryEntryComponent', () => { expect(component.buildRequestedByUserName).toBe('user-display-name'); expect(component.buildRequestedByDate).toBe('formatted-date'); expect(component.canDownloadBuild).toBe(true); + expect(component.columnsToDisplay).toEqual(['bookNames', 'source', 'target']); expect(component.hasDetails).toBe(true); expect(component.entry).toBe(entry); expect(component.sourceLanguage).toBe('fr'); @@ -120,6 +121,52 @@ describe('DraftHistoryEntryComponent', () => { target: 'tar' } ]); + expect(component.trainingDataOpen).toBe(false); + })); + + it('should handle builds where the draft cannot be downloaded yet', fakeAsync(() => { + when(mockedI18nService.localizeBook('GEN')).thenReturn('Genesis'); + when(mockedI18nService.localizeBook('EXO')).thenReturn('Exodus'); + const targetProjectDoc = { + id: 'project01' + } as SFProjectProfileDoc; + when(mockedSFProjectService.getProfile('project01')).thenResolve(targetProjectDoc); + const sourceProjectDoc = { + id: 'project02' + } as SFProjectProfileDoc; + when(mockedSFProjectService.getProfile('project02')).thenResolve(sourceProjectDoc); + const entry = { + engine: { + id: 'project01' + }, + additionalInfo: { + trainingScriptureRanges: [{ projectId: 'project02', scriptureRange: 'EXO' }], + translationScriptureRanges: [{ projectId: 'project01', scriptureRange: 'GEN' }] + } + } as BuildDto; + + // SUT + component.entry = entry; + tick(); + fixture.detectChanges(); + + expect(component.bookNames).toEqual(['Genesis']); + expect(component.buildRequestedByUserName).toBeUndefined(); + expect(component.buildRequestedByDate).toBe(''); + expect(component.canDownloadBuild).toBe(false); + expect(component.columnsToDisplay).toEqual(['bookNames', 'source', 'target']); + expect(component.hasDetails).toBe(true); + expect(component.entry).toBe(entry); + expect(component.sourceLanguage).toBe(''); + expect(component.targetLanguage).toBe(''); + expect(component.trainingData).toEqual([ + { + bookNames: ['Exodus'], + source: '', + target: '' + } + ]); + expect(component.trainingDataOpen).toBe(true); })); it('should handle builds with additional info referencing a deleted user', fakeAsync(() => { 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 8d37cdf367c..c62135f3a14 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 @@ -149,6 +149,63 @@ describe('EditorDraftComponent', () => { expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); })); + it('should support a timestamp earlier than the oldest draft', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile() + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); + when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(draftDelta.ops!))); + when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!); + when(mockDraftHandlingService.isDraftSegmentMap(anything())).thenReturn(false); + + // Set the date to a time before the earliest draft + fixture.componentInstance.timestamp = new Date('2024-03-22T03:02:01Z'); + + // SUT + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + verify(mockDraftHandlingService.getDraft(anything(), anything())).once(); + verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).once(); + expect(component.draftCheckState).toEqual('draft-present'); + expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); + })); + + it('should support a timestamp close to the oldest draft', fakeAsync(() => { + const testProjectDoc: SFProjectProfileDoc = { + data: createTestProjectProfile() + } as SFProjectProfileDoc; + when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( + of(draftHistory) + ); + when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); + spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); + when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(draftDelta.ops!))); + when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!); + when(mockDraftHandlingService.isDraftSegmentMap(anything())).thenReturn(false); + + // Set the date to a time just before the earliest draft + // This will account for the delay in storing the draft + const timestamp = new Date(draftHistory[0].timestamp); + timestamp.setMinutes(timestamp.getMinutes() - 10); + fixture.componentInstance.timestamp = timestamp; + + // SUT + fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + + verify(mockDraftHandlingService.getDraft(anything(), anything())).once(); + verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).once(); + expect(component.draftCheckState).toEqual('draft-present'); + expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); + })); + it('should return ops and update the editor when no revision', fakeAsync(() => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() From a9d733692d1de1022cf6c76144edf3f8c3405ed4 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Mon, 5 May 2025 11:08:47 +1200 Subject: [PATCH 10/11] Remove IAsyncEnumerable to allow correct HTTP status codes --- .../Controllers/MachineApiController.cs | 8 +- .../Services/IMachineApiService.cs | 4 +- .../Services/MachineApiService.cs | 49 +-- .../Controllers/MachineApiControllerTests.cs | 50 ++- .../Services/MachineApiServiceTests.cs | 296 ++++++------------ 5 files changed, 152 insertions(+), 255 deletions(-) diff --git a/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs b/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs index c99155fb1c8..b136b8fbf34 100644 --- a/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs +++ b/src/SIL.XForge.Scripture/Controllers/MachineApiController.cs @@ -185,7 +185,7 @@ CancellationToken cancellationToken /// The project does not exist or is not configured on the ML server. /// The ML server is temporarily unavailable or unresponsive. [HttpGet(MachineApi.GetBuilds)] - public ActionResult> GetBuildsAsync( + public async Task>> GetBuildsAsync( string sfProjectId, [FromQuery] bool preTranslate, CancellationToken cancellationToken @@ -195,7 +195,7 @@ CancellationToken cancellationToken { bool isServalAdmin = _userAccessor.SystemRoles.Contains(SystemRole.ServalAdmin); return Ok( - _machineApiService.GetBuildsAsync( + await _machineApiService.GetBuildsAsync( _userAccessor.UserId, sfProjectId, preTranslate, @@ -438,7 +438,7 @@ CancellationToken cancellationToken /// The user does not have permission to access the draft. /// The draft does not exist. [HttpGet(MachineApi.GetPreTranslationHistory)] - public ActionResult> GetPreTranslationRevisionsAsync( + public async Task>> GetPreTranslationRevisionsAsync( string sfProjectId, int bookNum, int chapterNum, @@ -449,7 +449,7 @@ CancellationToken cancellationToken { bool isServalAdmin = _userAccessor.SystemRoles.Contains(SystemRole.ServalAdmin); return Ok( - _machineApiService.GetPreTranslationRevisionsAsync( + await _machineApiService.GetPreTranslationRevisionsAsync( _userAccessor.UserId, sfProjectId, bookNum, diff --git a/src/SIL.XForge.Scripture/Services/IMachineApiService.cs b/src/SIL.XForge.Scripture/Services/IMachineApiService.cs index 9850e48708a..6ac8beaf134 100644 --- a/src/SIL.XForge.Scripture/Services/IMachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/IMachineApiService.cs @@ -30,7 +30,7 @@ CancellationToken cancellationToken bool isServalAdmin, CancellationToken cancellationToken ); - public IAsyncEnumerable GetBuildsAsync( + Task> GetBuildsAsync( string curUserId, string sfProjectId, bool preTranslate, @@ -75,7 +75,7 @@ Task> GetPreTranslationDeltaAsync( DateTime timestamp, CancellationToken cancellationToken ); - IAsyncEnumerable GetPreTranslationRevisionsAsync( + Task> GetPreTranslationRevisionsAsync( string curUserId, string sfProjectId, int bookNum, diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index 3d41b4d7d24..0affa6f9fcb 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.CompilerServices; using System.Security.Cryptography; using System.Text; using System.Threading; @@ -271,14 +270,17 @@ CancellationToken cancellationToken return buildDto; } - public async IAsyncEnumerable GetBuildsAsync( + public async Task> GetBuildsAsync( string curUserId, string sfProjectId, bool preTranslate, bool isServalAdmin, - [EnumeratorCancellation] CancellationToken cancellationToken + CancellationToken cancellationToken ) { + // Set up the list of builds to be returned + List builds = []; + // Ensure that the user has permission await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin); @@ -359,7 +361,7 @@ [EnumeratorCancellation] CancellationToken cancellationToken } // Make sure the DTO conforms to the machine-api URLs - yield return UpdateDto(buildDto, sfProjectId); + builds.Add(UpdateDto(buildDto, sfProjectId)); } // See if any builds are queued at our end @@ -381,8 +383,10 @@ [EnumeratorCancellation] CancellationToken cancellationToken queuedState = UpdateDto(queuedState, eventMetric); } - yield return queuedState; + builds.Add(queuedState); } + + return builds; } public async Task GetLastCompletedPreTranslationBuildAsync( @@ -783,15 +787,18 @@ CancellationToken cancellationToken /// /// The user does not have permission to access the Serval/Machine API. /// - public async IAsyncEnumerable GetPreTranslationRevisionsAsync( + public async Task> GetPreTranslationRevisionsAsync( string curUserId, string sfProjectId, int bookNum, int chapterNum, bool isServalAdmin, - [EnumeratorCancellation] CancellationToken cancellationToken + CancellationToken cancellationToken ) { + // Set up the list of revisions to be returned + List revisions = []; + // Ensure that the user has permission await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin); @@ -810,11 +817,13 @@ [EnumeratorCancellation] CancellationToken cancellationToken ); if (build is not null) { - yield return new DocumentRevision - { - Source = OpSource.Draft, - Timestamp = build.AdditionalInfo?.DateFinished?.UtcDateTime ?? DateTime.UtcNow, - }; + revisions.Add( + new DocumentRevision + { + Source = OpSource.Draft, + Timestamp = build.AdditionalInfo?.DateFinished?.UtcDateTime ?? DateTime.UtcNow, + } + ); } } else @@ -829,14 +838,18 @@ [EnumeratorCancellation] CancellationToken cancellationToken break; } - yield return new DocumentRevision - { - Source = op.Metadata.Source ?? OpSource.Draft, - Timestamp = op.Metadata.Timestamp, - UserId = op.Metadata.UserId, - }; + revisions.Add( + new DocumentRevision + { + Source = op.Metadata.Source ?? OpSource.Draft, + Timestamp = op.Metadata.Timestamp, + UserId = op.Metadata.UserId, + } + ); } } + + return revisions; } public async Task GetPreTranslationUsfmAsync( diff --git a/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs b/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs index de796386b98..5a6a72da62f 100644 --- a/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Controllers/MachineApiControllerTests.cs @@ -518,7 +518,7 @@ await env } [Test] - public void GetBuildsAsync_MachineApiDown() + public async Task GetBuildsAsync_MachineApiDown() { // Set up test environment var env = new TestEnvironment(); @@ -532,7 +532,7 @@ public void GetBuildsAsync_MachineApiDown() .Throws(new BrokenCircuitException()); // SUT - ActionResult> actual = env.Controller.GetBuildsAsync( + ActionResult> actual = await env.Controller.GetBuildsAsync( Project01, preTranslate: true, CancellationToken.None @@ -544,7 +544,7 @@ public void GetBuildsAsync_MachineApiDown() } [Test] - public void GetBuildsAsync_NoPermission() + public async Task GetBuildsAsync_NoPermission() { // Set up test environment var env = new TestEnvironment(); @@ -558,7 +558,7 @@ public void GetBuildsAsync_NoPermission() .Throws(new ForbiddenException()); // SUT - ActionResult> actual = env.Controller.GetBuildsAsync( + ActionResult> actual = await env.Controller.GetBuildsAsync( Project01, preTranslate: true, CancellationToken.None @@ -568,7 +568,7 @@ public void GetBuildsAsync_NoPermission() } [Test] - public void GetBuildsAsync_NoProject() + public async Task GetBuildsAsync_NoProject() { // Set up test environment var env = new TestEnvironment(); @@ -582,7 +582,7 @@ public void GetBuildsAsync_NoProject() .Throws(new DataNotFoundException(string.Empty)); // SUT - ActionResult> actual = env.Controller.GetBuildsAsync( + ActionResult> actual = await env.Controller.GetBuildsAsync( Project01, preTranslate: true, CancellationToken.None @@ -603,10 +603,10 @@ public async Task GetBuildsAsync_Success() isServalAdmin: false, CancellationToken.None ) - .Returns(env.ServalBuilds()); + .Returns(Task.FromResult>([env.TestBuild])); // SUT - ActionResult> actual = env.Controller.GetBuildsAsync( + ActionResult> actual = await env.Controller.GetBuildsAsync( Project01, preTranslate: true, CancellationToken.None @@ -614,8 +614,7 @@ public async Task GetBuildsAsync_Success() Assert.IsInstanceOf(actual.Result); bool buildsExist = false; - var builds = (IAsyncEnumerable)((OkObjectResult)actual.Result!).Value!; - await foreach (ServalBuildDto build in builds) + foreach (ServalBuildDto build in (IReadOnlyList)((OkObjectResult)actual.Result!).Value!) { buildsExist = true; Assert.AreEqual(env.TestBuild, build); @@ -1064,7 +1063,7 @@ public async Task GetPreTranslationDeltaAsync_Success() } [Test] - public void GetPreTranslationRevisionsAsync_MachineApiDown() + public async Task GetPreTranslationRevisionsAsync_MachineApiDown() { // Set up test environment var env = new TestEnvironment(); @@ -1072,7 +1071,7 @@ public void GetPreTranslationRevisionsAsync_MachineApiDown() .Throws(new BrokenCircuitException()); // SUT - ActionResult> actual = env.Controller.GetPreTranslationRevisionsAsync( + ActionResult> actual = await env.Controller.GetPreTranslationRevisionsAsync( Project01, 40, 1, @@ -1085,7 +1084,7 @@ public void GetPreTranslationRevisionsAsync_MachineApiDown() } [Test] - public void GetPreTranslationRevisionsAsync_NoPermission() + public async Task GetPreTranslationRevisionsAsync_NoPermission() { // Set up test environment var env = new TestEnvironment(); @@ -1093,7 +1092,7 @@ public void GetPreTranslationRevisionsAsync_NoPermission() .Throws(new ForbiddenException()); // SUT - ActionResult> actual = env.Controller.GetPreTranslationRevisionsAsync( + ActionResult> actual = await env.Controller.GetPreTranslationRevisionsAsync( Project01, 40, 1, @@ -1104,7 +1103,7 @@ public void GetPreTranslationRevisionsAsync_NoPermission() } [Test] - public void GetPreTranslationRevisionsAsync_NoProject() + public async Task GetPreTranslationRevisionsAsync_NoProject() { // Set up test environment var env = new TestEnvironment(); @@ -1112,7 +1111,7 @@ public void GetPreTranslationRevisionsAsync_NoProject() .Throws(new DataNotFoundException(string.Empty)); // SUT - ActionResult> actual = env.Controller.GetPreTranslationRevisionsAsync( + ActionResult> actual = await env.Controller.GetPreTranslationRevisionsAsync( Project01, 40, 1, @@ -1128,10 +1127,10 @@ public async Task GetPreTranslationRevisionsAsync_Success() // Set up test environment var env = new TestEnvironment(); env.MachineApiService.GetPreTranslationRevisionsAsync(User01, Project01, 40, 1, false, CancellationToken.None) - .Returns(env.RevisionHistory()); + .Returns(Task.FromResult>([env.TestRevision])); // SUT - ActionResult> actual = env.Controller.GetPreTranslationRevisionsAsync( + ActionResult> actual = await env.Controller.GetPreTranslationRevisionsAsync( Project01, 40, 1, @@ -1140,8 +1139,7 @@ public async Task GetPreTranslationRevisionsAsync_Success() Assert.IsInstanceOf(actual.Result); bool revisionsExist = false; - var revisions = (IAsyncEnumerable)((OkObjectResult)actual.Result!).Value!; - await foreach (DocumentRevision revision in revisions) + foreach (DocumentRevision revision in (IReadOnlyList)((OkObjectResult)actual.Result!).Value!) { revisionsExist = true; Assert.AreEqual(env.TestRevision, revision); @@ -2252,17 +2250,5 @@ public TestEnvironment() public IExceptionHandler ExceptionHandler { get; } public IMachineApiService MachineApiService { get; } public IUserAccessor UserAccessor { get; } - - public async IAsyncEnumerable RevisionHistory() - { - yield return TestRevision; - await Task.CompletedTask; - } - - public async IAsyncEnumerable ServalBuilds() - { - yield return TestBuild; - await Task.CompletedTask; - } } } diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index 515841acdef..c474f1d3178 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -533,18 +533,16 @@ public void GetBuildsAsync_NoPermission() var env = new TestEnvironment(); // SUT - Assert.ThrowsAsync(async () => - { - await foreach ( - ServalBuildDto _ in env.Service.GetBuildsAsync( + Assert.ThrowsAsync( + () => + env.Service.GetBuildsAsync( User02, Project01, preTranslate: false, isServalAdmin: false, CancellationToken.None ) - ) { } - }); + ); } [Test] @@ -554,18 +552,16 @@ public void GetBuildsAsync_NoProject() var env = new TestEnvironment(); // SUT - Assert.ThrowsAsync(async () => - { - await foreach ( - ServalBuildDto _ in env.Service.GetBuildsAsync( + Assert.ThrowsAsync( + () => + env.Service.GetBuildsAsync( User01, "invalid_project_id", preTranslate: false, isServalAdmin: false, CancellationToken.None ) - ) { } - }); + ); } [Test] @@ -575,18 +571,16 @@ public void GetBuildsAsync_NoTranslationEngine() var env = new TestEnvironment(); // SUT - Assert.ThrowsAsync(async () => - { - await foreach ( - ServalBuildDto _ in env.Service.GetBuildsAsync( + Assert.ThrowsAsync( + () => + env.Service.GetBuildsAsync( User01, Project03, preTranslate: false, isServalAdmin: false, CancellationToken.None ) - ) { } - }); + ); } [Test] @@ -599,21 +593,15 @@ public async Task GetBuildsAsync_QueuedState() .Returns(Task.FromResult>([])); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(QueryResults.Empty)); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); Assert.AreEqual(MachineApiService.BuildStateQueued, builds[0].State); @@ -667,21 +655,15 @@ public async Task GetBuildsAsync_QueuedStateWithEventMetric() } ) ); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); Assert.AreEqual(MachineApiService.BuildStateQueued, builds[0].State); @@ -708,21 +690,15 @@ public async Task GetBuildsAsync_ServalAdminDoesNotNeedPermission() env.ConfigureTranslationBuild(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(QueryResults.Empty)); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); } @@ -736,18 +712,16 @@ public void GetBuildsAsync_ServalApiException() .Throws(ServalApiExceptions.NotFound); // SUT - Assert.ThrowsAsync(async () => - { - await foreach ( - ServalBuildDto _ in env.Service.GetBuildsAsync( + Assert.ThrowsAsync( + () => + env.Service.GetBuildsAsync( User01, Project01, preTranslate: false, isServalAdmin: false, CancellationToken.None ) - ) { } - }); + ); } [Test] @@ -758,21 +732,15 @@ public async Task GetBuildsAsync_Success() TranslationBuild translationBuild = env.ConfigureTranslationBuild(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(QueryResults.Empty)); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); TestEnvironment.AssertCoreBuildProperties(translationBuild, builds[0]); @@ -843,21 +811,15 @@ public async Task GetBuildsAsync_SuccessWithEventMetrics() } ) ); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); TestEnvironment.AssertCoreBuildProperties(translationBuild, builds[0]); @@ -879,21 +841,15 @@ public async Task GetBuildsAsync_SuccessWithFallbackToLegacyBuild() env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(QueryResults.Empty)); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); TestEnvironment.AssertCoreBuildProperties(translationBuild, builds[0]); @@ -918,21 +874,15 @@ public async Task GetBuildsAsync_SuccessWithFallbackToPreTranslateBuild() env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(QueryResults.Empty)); - List builds = []; // SUT - await foreach ( - ServalBuildDto build in env.Service.GetBuildsAsync( - User02, - Project01, - preTranslate: true, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - builds.Add(build); - } + IReadOnlyList builds = await env.Service.GetBuildsAsync( + User02, + Project01, + preTranslate: true, + isServalAdmin: true, + CancellationToken.None + ); Assert.AreEqual(1, builds.Count); TestEnvironment.AssertCoreBuildProperties(translationBuild, builds[0]); @@ -1665,35 +1615,6 @@ public async Task GetPreTranslationDeltaAsync_Success() Assert.AreEqual(TextData.GetTextDocId(Project01, "MAT", 1), actual.Id); } - [Test] - public async Task GetPreTranslationRevisionsAsync_CancelEnumeration() - { - // Set up test environment - var env = new TestEnvironment(); - string textDocumentId = TextDocument.GetDocId(Project01, 40, 1, TextDocument.Draft); - env.SetupTextDocument(textDocumentId, 40, alreadyExists: true); - using var cancellationTokenSource = new CancellationTokenSource(); - List revisions = []; - - // SUT - await foreach ( - DocumentRevision revision in env.Service.GetPreTranslationRevisionsAsync( - User01, - Project01, - 40, - 1, - isServalAdmin: false, - cancellationTokenSource.Token - ) - ) - { - await cancellationTokenSource.CancelAsync(); - revisions.Add(revision); - } - - Assert.AreEqual(1, revisions.Count); - } - [Test] public async Task GetPreTranslationRevisionsAsync_NoOps() { @@ -1735,19 +1656,14 @@ public async Task GetPreTranslationRevisionsAsync_NoOps() List revisions = []; // SUT - await foreach ( - DocumentRevision revision in env.Service.GetPreTranslationRevisionsAsync( - User01, - Project01, - 40, - 1, - isServalAdmin: false, - CancellationToken.None - ) - ) - { - revisions.Add(revision); - } + IReadOnlyList revisions = await env.Service.GetPreTranslationRevisionsAsync( + User01, + Project01, + 40, + 1, + isServalAdmin: false, + CancellationToken.None + ); Assert.AreEqual(1, revisions.Count); Assert.AreEqual(revisions[0].Source, OpSource.Draft); @@ -1760,22 +1676,16 @@ public async Task GetPreTranslationRevisionsAsync_NoOpsOrBuildOnServal() { // Set up test environment var env = new TestEnvironment(); - List revisions = []; // SUT - await foreach ( - DocumentRevision revision in env.Service.GetPreTranslationRevisionsAsync( - User01, - Project01, - 40, - 1, - isServalAdmin: false, - CancellationToken.None - ) - ) - { - revisions.Add(revision); - } + IReadOnlyList revisions = await env.Service.GetPreTranslationRevisionsAsync( + User01, + Project01, + 40, + 1, + isServalAdmin: false, + CancellationToken.None + ); Assert.IsEmpty(revisions); } @@ -1787,22 +1697,16 @@ public async Task GetPreTranslationRevisionsAsync_ServalAdminDoesNotNeedPermissi var env = new TestEnvironment(); string textDocumentId = TextDocument.GetDocId(Project01, 40, 1, TextDocument.Draft); env.SetupTextDocument(textDocumentId, 40, alreadyExists: true); - List revisions = []; // SUT - await foreach ( - DocumentRevision revision in env.Service.GetPreTranslationRevisionsAsync( - User02, - Project01, - 40, - 1, - isServalAdmin: true, - CancellationToken.None - ) - ) - { - revisions.Add(revision); - } + IReadOnlyList revisions = await env.Service.GetPreTranslationRevisionsAsync( + User01, + Project01, + 40, + 1, + isServalAdmin: false, + CancellationToken.None + ); Assert.AreEqual(2, revisions.Count); } @@ -1814,22 +1718,16 @@ public async Task GetPreTranslationRevisionsAsync_Success() var env = new TestEnvironment(); string textDocumentId = TextDocument.GetDocId(Project01, 40, 1, TextDocument.Draft); env.SetupTextDocument(textDocumentId, 40, alreadyExists: true); - List revisions = []; // SUT - await foreach ( - DocumentRevision revision in env.Service.GetPreTranslationRevisionsAsync( - User01, - Project01, - 40, - 1, - isServalAdmin: false, - CancellationToken.None - ) - ) - { - revisions.Add(revision); - } + IReadOnlyList revisions = await env.Service.GetPreTranslationRevisionsAsync( + User01, + Project01, + 40, + 1, + isServalAdmin: false, + CancellationToken.None + ); Assert.AreEqual(2, revisions.Count); } From 934a1987721720e5609837a1426e2edf7b7e3776 Mon Sep 17 00:00:00 2001 From: Peter Chapman Date: Mon, 5 May 2025 11:06:54 +1200 Subject: [PATCH 11/11] Code review updates --- .../src/app/machine-api/build-dto.ts | 2 +- .../draft-download-button.component.html | 71 ++++++++++------ .../draft-generation.component.html | 7 +- .../draft-generation.service.spec.ts | 10 +-- .../draft-generation.service.ts | 14 +++- .../draft-handling.service.ts | 2 +- .../_draft-history-entry-theme.scss | 6 +- .../draft-history-entry.component.html | 82 +++++++++++-------- .../draft-history-entry.component.scss | 56 ++++++------- .../draft-history-entry.component.spec.ts | 43 ++++------ .../draft-history-entry.component.ts | 63 ++++++-------- .../draft-history-list.component.html | 8 +- .../draft-history-list.component.spec.ts | 5 +- .../draft-history-list.component.ts | 20 +++-- .../draft-preview-books.component.ts | 1 + .../editor-draft/editor-draft.component.html | 44 +++++----- .../editor-draft/editor-draft.component.ts | 4 +- .../src/assets/i18n/non_checking_en.json | 10 +-- .../Models/ServalBuildAdditionalInfo.cs | 2 +- .../Services/MachineApiService.cs | 49 +++++++++-- .../Services/MachineApiServiceTests.cs | 58 ++++++++++--- 21 files changed, 324 insertions(+), 233 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts index c5c71a71d47..ed9e940d461 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts @@ -16,8 +16,8 @@ export interface ServalBuildAdditionalInfo { buildId: string; corporaIds?: string[]; dateFinished?: string; - dateRequested?: string; dateGenerated?: string; + dateRequested?: string; parallelCorporaIds?: string[]; step: number; trainingScriptureRanges: ProjectScriptureRange[]; diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html index 217dc2e7dda..92e11414f9d 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html @@ -1,26 +1,49 @@ - + @if (flat) { + + } @else { + + } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html index 1e6600c28e3..26b81ffd61a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html @@ -278,7 +278,7 @@

{{ t("draft_finishing_header") }}

@if (hasDraftBooksAvailable) { - + } @@ -334,7 +334,10 @@

{{ t("draft_finishing_header") }}

@if (featureFlags.newDraftHistory.enabled) { - + + @if (isServalAdmin()) { +

Further information

+ } } @if (canShowAdditionalInfo(draftJob)) { 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 2a93ccb0363..144e4f6f523 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 @@ -168,7 +168,7 @@ describe('DraftGenerationService', () => { it('should return undefined for a 401 error', fakeAsync(() => { // SUT service.getBuildHistory(projectId).subscribe(result => { - expect(result).toEqual(undefined); + expect(result).toBeUndefined(); verify(mockNoticeService.showError(anything())).once(); }); tick(); @@ -185,7 +185,7 @@ describe('DraftGenerationService', () => { it('should return undefined for a 404 error', fakeAsync(() => { // SUT service.getBuildHistory(projectId).subscribe(result => { - expect(result).toEqual(undefined); + expect(result).toBeUndefined(); verify(mockNoticeService.showError(anything())).never(); }); tick(); @@ -617,7 +617,7 @@ describe('DraftGenerationService', () => { // SUT service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { - expect(result).toEqual(undefined); + expect(result).toBeUndefined(); verify(mockNoticeService.showError(anything())).once(); }); tick(); @@ -637,7 +637,7 @@ describe('DraftGenerationService', () => { // SUT service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { - expect(result).toEqual(undefined); + expect(result).toBeUndefined(); verify(mockNoticeService.showError(anything())).never(); }); tick(); @@ -658,7 +658,7 @@ describe('DraftGenerationService', () => { // SUT service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => { - expect(result).toEqual(undefined); + expect(result).toBeUndefined(); }); tick(); })); 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 cce3a147197..6c6a7e2bd39 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 @@ -314,12 +314,20 @@ export class DraftGenerationService { const usfmFiles: Promise[] = []; // Build the list of book numbers, first checking the build, then the project document if that is null - const books: number[] = + let books = new Set( lastCompletedBuild?.additionalInfo?.translationScriptureRanges?.flatMap(range => booksFromScriptureRange(range.scriptureRange) - ) ?? projectDoc.data.texts.filter(text => text.chapters.some(c => c.hasDraft)).map(text => text.bookNum); + ) + ); + + // If no books were found in the build, use the project document + if (books.size === 0) { + books = new Set( + projectDoc.data.texts.filter(text => text.chapters.some(c => c.hasDraft)).map(text => text.bookNum) + ); + } - const zipProgress: DraftZipProgress = { current: 0, total: books.length }; + const zipProgress: DraftZipProgress = { current: 0, total: books.size }; observer.next(zipProgress); // Get the date the draft was generated and written to Scripture Forge diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts index d0a05d2a1a5..28eb4e3b8b2 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts @@ -162,7 +162,7 @@ export class DraftHandlingService { // If the corpus does not support USFM, use the legacy format. // The legacy format does not support a timestamp if (err.status === 405 && timestamp == null) { - return this.getDraft(textDocId, { isDraftLegacy: true, timestamp }); + return this.getDraft(textDocId, { isDraftLegacy: true, timestamp: undefined }); } return throwError(() => err); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss index 9b1c4adf136..1eb045595ba 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss @@ -3,9 +3,9 @@ @mixin color($theme) { $is-dark: mat.get-theme-type($theme) == dark; - --draft-history-entry-heading-background-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 20, 95))}; - --draft-history-entry-heading-background-hover-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 30, 90))}; - --draft-history-entry-details-background-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 10, 98))}; + --draft-history-entry-red-color: #{if($is-dark, red, darkRed)}; + --draft-history-entry-green-color: #{if($is-dark, lightGreen, darkGreen)}; + --draft-history-entry-grey-color: lightGrey; } @mixin theme($theme) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html index 4d9b538c716..d544cad91f6 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html @@ -1,44 +1,54 @@ @if (entry != null) { -
-
- {{ i18n.enumerateList(bookNames) }} - {{ formatDate(entry.additionalInfo?.dateFinished) }} -
- - - {{ getStatus(entry.state).icons }} - {{ t(getStatus(entry.state).text) }} - - @if (!forceDetailsOpen && hasDetails) { - expand_{{ detailsOpen ? "less" : "more" }} - } -
- @if (detailsOpen && hasDetails) { + + + + {{ i18n.enumerateList(bookNames) }} + {{ formatDate(entry.additionalInfo?.dateFinished) }} + + + + {{ getStatus(entry.state).icons }} + {{ t(getStatus(entry.state).text) }} + + +
- + @if (canDownloadBuild) {

{{ t("click_book_to_preview") }}

- -

- - - -

- - expand_{{ trainingDataOpen ? "less" : "more" }} - {{ t(trainingDataOpen ? "hide_model_training_data" : "show_model_training_data") }} - +

- @if (trainingDataOpen) { -

- {{ t("training_data_configuration") }} + + } + @if (hasTrainingConfiguration) { + @if (canDownloadBuild) { +

+ + expand_{{ trainingConfigurationOpen ? "less" : "more" }} + {{ + t( + trainingConfigurationOpen + ? "hide_model_training_configuration" + : "show_model_training_configuration" + ) + }} + +

+ } + @if (trainingConfigurationOpen) { +

+ {{ t("training_model_description") }}

- +
@@ -59,8 +69,8 @@
{{ i18n.enumerateList(element.bookNames) }}
} - + }
- } + } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss index c2b2d012c6c..e37f1fa3058 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss @@ -1,37 +1,24 @@ -:host { - background-color: var(--draft-history-entry-details-background-color); - border-radius: 8px; - > * { - padding: 8px 16px; - border-radius: 8px; - } +mat-expansion-panel-header { + padding: 8px 16px; } -.draft-entry-heading { - background-color: var(--draft-history-entry-heading-background-color); +mat-panel-title { display: flex; - align-items: center; - gap: 1em; - &.expandable { - cursor: pointer; - } - &.expandable:hover { - background-color: var(--draft-history-entry-heading-background-hover-color); - } -} - -.drafted-books { - font-size: 1.1em; + align-items: flex-start; + flex-direction: column; + flex-grow: 1; } -.spacer { - flex-grow: 1; +mat-panel-description { + flex-grow: 0; } -.entry-description { - display: flex; - flex-direction: column; - gap: 4px; +.drafted-books { + color: var( + --mat-expansion-header-text-color, + var(--mat-app-on-surface) + ); /* Keep the text color, even when disabled */ + font-size: 1.1em; } .date { @@ -49,7 +36,7 @@ flex-shrink: 0; } -.training-data-link { +.training-configuration-link { display: flex; align-items: center; } @@ -62,3 +49,16 @@ strong { display: flex; gap: 4px; } + +/* Color classes for the status icon */ +.grey { + color: var(--draft-history-entry-grey-color); +} + +.green { + color: var(--draft-history-entry-green-color); +} + +.red { + color: var(--draft-history-entry-red-color); +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts index 9d17db769d6..8d2a5094a68 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts @@ -1,17 +1,22 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { createTestUserProfile } from 'realtime-server/lib/esm/common/models/user-test-data'; import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; import { anything, mock, when } from 'ts-mockito'; import { I18nService } from 'xforge-common/i18n.service'; import { UserProfileDoc } from 'xforge-common/models/user-profile-doc'; +import { TestRealtimeModule } from 'xforge-common/test-realtime.module'; import { configureTestingModule, TestTranslocoModule } from 'xforge-common/test-utils'; import { UserService } from 'xforge-common/user.service'; import { SFProjectProfileDoc } from '../../../../core/models/sf-project-profile-doc'; +import { SF_TYPE_REGISTRY } from '../../../../core/models/sf-type-registry'; import { SFProjectService } from '../../../../core/sf-project.service'; import { BuildDto } from '../../../../machine-api/build-dto'; import { BuildStates } from '../../../../machine-api/build-states'; +import { DraftGenerationService } from '../../draft-generation.service'; import { DraftHistoryEntryComponent } from './draft-history-entry.component'; +const mockedDraftGenerationService = mock(DraftGenerationService); const mockedI18nService = mock(I18nService); const mockedSFProjectService = mock(SFProjectService); const mockedUserService = mock(UserService); @@ -21,8 +26,9 @@ describe('DraftHistoryEntryComponent', () => { let fixture: ComponentFixture; configureTestingModule(() => ({ - imports: [TestTranslocoModule], + imports: [NoopAnimationsModule, TestTranslocoModule, TestRealtimeModule.forRoot(SF_TYPE_REGISTRY)], providers: [ + { provide: DraftGenerationService, useMock: mockedDraftGenerationService }, { provide: I18nService, useMock: mockedI18nService }, { provide: SFProjectService, useMock: mockedSFProjectService }, { provide: UserService, useMock: mockedUserService } @@ -39,24 +45,6 @@ describe('DraftHistoryEntryComponent', () => { expect(component).toBeTruthy(); }); - it('forceDetailsOpen should set detailsOpen', () => { - expect(component.detailsOpen).toBe(false); - component.forceDetailsOpen = true; - expect(component.detailsOpen).toBe(true); - expect(component.forceDetailsOpen).toBe(true); - component.forceDetailsOpen = false; - expect(component.detailsOpen).toBe(true); - expect(component.forceDetailsOpen).toBe(false); - }); - - it('headerClicked should toggle detailsOpen', () => { - expect(component.detailsOpen).toBe(false); - component.headerClicked(); - expect(component.detailsOpen).toBe(true); - component.headerClicked(); - expect(component.detailsOpen).toBe(false); - }); - describe('entry', () => { it('should handle undefined values', () => { component.entry = undefined; @@ -114,19 +102,20 @@ describe('DraftHistoryEntryComponent', () => { expect(component.entry).toBe(entry); expect(component.sourceLanguage).toBe('fr'); expect(component.targetLanguage).toBe('en'); - expect(component.trainingData).toEqual([ + expect(component.trainingConfiguration).toEqual([ { bookNames: ['Exodus'], source: 'src', target: 'tar' } ]); - expect(component.trainingDataOpen).toBe(false); + expect(component.trainingConfigurationOpen).toBe(false); })); it('should handle builds where the draft cannot be downloaded yet', fakeAsync(() => { when(mockedI18nService.localizeBook('GEN')).thenReturn('Genesis'); when(mockedI18nService.localizeBook('EXO')).thenReturn('Exodus'); + when(mockedI18nService.translateStatic('draft_history_entry.draft_unknown')).thenReturn('Unknown'); const targetProjectDoc = { id: 'project01' } as SFProjectProfileDoc; @@ -159,14 +148,14 @@ describe('DraftHistoryEntryComponent', () => { expect(component.entry).toBe(entry); expect(component.sourceLanguage).toBe(''); expect(component.targetLanguage).toBe(''); - expect(component.trainingData).toEqual([ + expect(component.trainingConfiguration).toEqual([ { bookNames: ['Exodus'], - source: '', - target: '' + source: 'Unknown', + target: 'Unknown' } ]); - expect(component.trainingDataOpen).toBe(true); + expect(component.trainingConfigurationOpen).toBe(true); })); it('should handle builds with additional info referencing a deleted user', fakeAsync(() => { @@ -233,11 +222,11 @@ describe('DraftHistoryEntryComponent', () => { describe('getStatus', () => { it('should handle known build state strings', () => { - expect(component.getStatus(BuildStates.Active)).not.toBeUndefined(); + expect(component.getStatus(BuildStates.Active)).toBeDefined(); }); it('should handle unknown build state strings', () => { - expect(component.getStatus('unknown build state' as BuildStates)).not.toBeUndefined(); + expect(component.getStatus('unknown build state' as BuildStates)).toBeDefined(); }); }); }); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts index d72b16c8d9a..dc2d05ee29f 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts @@ -1,5 +1,6 @@ import { CommonModule } from '@angular/common'; import { Component, Input } from '@angular/core'; +import { MatExpansionModule } from '@angular/material/expansion'; import { MatIconModule } from '@angular/material/icon'; import { MatTableModule } from '@angular/material/table'; import { TranslocoModule } from '@ngneat/transloco'; @@ -22,7 +23,7 @@ const STATUS_INFO: Record { @@ -78,8 +80,8 @@ export class DraftHistoryEntryComponent { // Get the target language, if it is not already set this._targetLanguage ??= target?.data?.writingSystem.tag; - // Get the source project - const source = await this.projectService.getProfile(r.projectId); + // Get the source project, if it is configured + const source = r.projectId === '' ? undefined : await this.projectService.getProfile(r.projectId); // Get the source language, if it is not already set this._sourceLanguage ??= source?.data?.writingSystem.tag; @@ -87,17 +89,17 @@ export class DraftHistoryEntryComponent { // Return the data for this training range return { bookNames: r.scriptureRange.split(';').map(id => this.i18n.localizeBook(id)), - source: source?.data?.shortName ?? '', - target: target?.data?.shortName ?? '' - } as TrainingDataRow; + source: source?.data?.shortName ?? this.i18n.translateStatic('draft_history_entry.draft_unknown'), + target: target?.data?.shortName ?? this.i18n.translateStatic('draft_history_entry.draft_unknown') + } as TrainingConfigurationRow; }) - ).then(trainingData => { + ).then(trainingConfiguration => { // Set the training data for the table - this._trainingData = trainingData; + this._trainingConfiguration = trainingConfiguration; - // If we can only show training data, expand the training data - if (!this.canDownloadBuild && this.hasTrainingData) { - this.trainingDataOpen = true; + // If we can only show training data, expand the training configuration + if (!this.canDownloadBuild && this.hasTrainingConfiguration) { + this.trainingConfigurationOpen = true; } }); } @@ -105,15 +107,6 @@ export class DraftHistoryEntryComponent { return this._entry; } - private _forceDetailsOpen = false; - @Input() set forceDetailsOpen(value: boolean) { - this._forceDetailsOpen = value; - if (value) this.detailsOpen = true; - } - get forceDetailsOpen(): boolean { - return this._forceDetailsOpen; - } - private _buildRequestedByUserName: string | undefined; get buildRequestedByUserName(): string | undefined { return this._buildRequestedByUserName; @@ -124,16 +117,12 @@ export class DraftHistoryEntryComponent { return this.i18n.formatDate(new Date(this._entry?.additionalInfo?.dateRequested)); } - get columnsToDisplay(): string[] { - return ['bookNames', 'source', 'target']; - } - get hasDetails(): boolean { - return this.hasTrainingData || this.canDownloadBuild; + return this.hasTrainingConfiguration || this.canDownloadBuild; } - get hasTrainingData(): boolean { - return this._trainingData.length > 0; + get hasTrainingConfiguration(): boolean { + return this._trainingConfiguration.length > 0; } private _sourceLanguage?: string = undefined; @@ -146,15 +135,17 @@ export class DraftHistoryEntryComponent { return this._targetLanguage ?? ''; } - private _trainingData: TrainingDataRow[] = []; - get trainingData(): TrainingDataRow[] { - return this._trainingData; + private _trainingConfiguration: TrainingConfigurationRow[] = []; + get trainingConfiguration(): TrainingConfigurationRow[] { + return this._trainingConfiguration; } @Input() canDownloadBuild = false; + @Input() forceDetailsOpen = false; + + trainingConfigurationOpen = false; - detailsOpen = false; - trainingDataOpen = false; + readonly columnsToDisplay: string[] = ['bookNames', 'source', 'target']; constructor( readonly i18n: I18nService, @@ -180,8 +171,4 @@ export class DraftHistoryEntryComponent { getStatus(state: BuildStates): { icons: string; text: string; color: string } { return STATUS_INFO[state] ?? { icons: 'help_outline', text: 'draft_unknown', color: 'grey' }; } - - headerClicked(): void { - if (!this.forceDetailsOpen) this.detailsOpen = !this.detailsOpen; - } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html index 4ac4b1be5c9..ad8088d7e26 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.html @@ -1,17 +1,13 @@ @if (latestBuild != null) {

{{ lastCompletedBuildMessage }}

- + } @if (historicalBuilds.length > 0) {

{{ t("previously_generated_drafts") }}

@for (entry of historicalBuilds; track entry.id) { - + } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts index 5210fe0a287..4b54e5b219a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.spec.ts @@ -1,4 +1,5 @@ import { ComponentFixture, fakeAsync, TestBed } from '@angular/core/testing'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { BehaviorSubject, of } from 'rxjs'; import { mock, when } from 'ts-mockito'; import { ActivatedProjectService } from 'xforge-common/activated-project.service'; @@ -21,7 +22,7 @@ const mockedUserService = mock(UserService); describe('DraftHistoryListComponent', () => { configureTestingModule(() => ({ - imports: [TestTranslocoModule, TestRealtimeModule.forRoot(SF_TYPE_REGISTRY)], + imports: [NoopAnimationsModule, TestTranslocoModule, TestRealtimeModule.forRoot(SF_TYPE_REGISTRY)], providers: [ { provide: ActivatedProjectService, useMock: mockedActivatedProjectService }, { provide: DraftGenerationService, useMock: mockedDraftGenerationService }, @@ -116,7 +117,7 @@ describe('DraftHistoryListComponent', () => { constructor(buildHistory: BuildDto[] | undefined) { when(mockedActivatedProjectService.projectId$).thenReturn(of('project01')); - when(mockedActivatedProjectService.changes$).thenReturn(of(undefined)); + when(mockedActivatedProjectService.changes$).thenReturn(of(undefined)); // Required for DraftPreviewBooksComponent when(mockedDraftGenerationService.getBuildHistory('project01')).thenReturn(new BehaviorSubject(buildHistory)); this.fixture = TestBed.createComponent(DraftHistoryListComponent); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts index bd7fc9883d9..72138465cd4 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-list.component.ts @@ -1,9 +1,9 @@ -import { Component } from '@angular/core'; +import { Component, DestroyRef } from '@angular/core'; import { MatIconModule } from '@angular/material/icon'; import { TranslocoModule, TranslocoService } from '@ngneat/transloco'; import { take } from 'rxjs'; import { ActivatedProjectService } from 'xforge-common/activated-project.service'; -import { filterNullish } from 'xforge-common/util/rxjs-util'; +import { filterNullish, quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; import { BuildDto } from '../../../machine-api/build-dto'; import { BuildStates } from '../../../machine-api/build-states'; import { activeBuildStates } from '../draft-generation'; @@ -22,14 +22,20 @@ export class DraftHistoryListComponent { constructor( private readonly activatedProject: ActivatedProjectService, + destroyRef: DestroyRef, private readonly draftGenerationService: DraftGenerationService, private readonly transloco: TranslocoService ) { - this.activatedProject.projectId$.pipe(filterNullish(), take(1)).subscribe(projectId => { - this.draftGenerationService.getBuildHistory(projectId).subscribe(result => { - this.history = result?.reverse() ?? []; + this.activatedProject.projectId$ + .pipe(quietTakeUntilDestroyed(destroyRef), filterNullish(), take(1)) + .subscribe(projectId => { + this.draftGenerationService + .getBuildHistory(projectId) + .pipe(quietTakeUntilDestroyed(destroyRef)) + .subscribe(result => { + this.history = result?.reverse() ?? []; + }); }); - }); } get nonActiveBuilds(): BuildDto[] { @@ -49,7 +55,7 @@ export class DraftHistoryListComponent { case BuildStates.Faulted: return this.transloco.translate('draft_history_list.draft_faulted'); default: - // The latest build must abe a build that has finished + // The latest build must be a build that has finished return ''; } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts index 0cb3b59a371..1e35d233d23 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts @@ -68,6 +68,7 @@ export class DraftPreviewBooksComponent { .sort((a, b) => a.bookNumber - b.bookNumber) .filter(book => book.chaptersWithDrafts.length > 0) as BookWithDraft[]; } else { + // TODO: Support books from multiple translation projects draftBooks = this.build.additionalInfo?.translationScriptureRanges .flatMap(range => booksFromScriptureRange(range.scriptureRange)) .map(bookNum => { 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 c22aab4e25d..8055108c8e6 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 @@ -18,27 +18,29 @@ @if (isDraftReady) {
- - - - {{ selectedRevision ?? "" | revisionFormat }} - - @for (r of draftRevisions; track r) { - -
- {{ r | revisionFormat }} -
- - } - - + @if (featureFlags.newDraftHistory.enabled) { + + + + {{ selectedRevision ?? "" | revisionFormat }} + + @for (r of draftRevisions; track r) { + +
+ {{ r | revisionFormat }} +
+ + } + + + }
@if (userAppliedDraft) { 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 fb5e4843317..a6f34a51065 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 @@ -26,6 +26,7 @@ import { ActivatedProjectService } from 'xforge-common/activated-project.service import { isNetworkError } from 'xforge-common/command.service'; import { DialogService } from 'xforge-common/dialog.service'; import { ErrorReportingService } from 'xforge-common/error-reporting.service'; +import { FeatureFlagService } from 'xforge-common/feature-flags/feature-flag.service'; import { FontService } from 'xforge-common/font.service'; import { I18nService } from 'xforge-common/i18n.service'; import { NoticeService } from 'xforge-common/notice.service'; @@ -86,6 +87,7 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { private readonly dialogService: DialogService, private readonly draftGenerationService: DraftGenerationService, private readonly draftHandlingService: DraftHandlingService, + readonly featureFlags: FeatureFlagService, readonly fontService: FontService, private readonly i18n: I18nService, private readonly projectService: SFProjectService, @@ -256,7 +258,7 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { return this.draftGenerationService.draftExists(this.projectId!, this.bookNum!, this.chapter!); } - findClosestRevision(date: Date, revisions: Revision[]): Revision { + private findClosestRevision(date: Date, revisions: Revision[]): Revision { const targetTime: number = date.getTime(); const oneHour: number = 60 * 60 * 1000; 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 44b40827076..0e5652b08ad 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 @@ -270,13 +270,13 @@ "draft_faulted": "Failed", "draft_pending": "Pending", "draft_unknown": "Unknown", - "hide_model_training_data": "Hide model training data", - "show_model_training_data": "Show model training data", - "training_data_configuration": "The language model was trained on this configuration" + "hide_model_training_configuration": "Hide model training configuration", + "show_model_training_configuration": "Show model training configuration", + "training_model_description": "The language model was trained on this configuration" }, "draft_history_list": { - "draft_canceled": "Your draft was canceled", - "draft_completed": "Your draft is ready", + "draft_canceled": "The draft was canceled", + "draft_completed": "The draft is ready", "draft_faulted": "The draft failed", "previously_generated_drafts": "Previously generated drafts" }, diff --git a/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs b/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs index 8d6173655de..9204350382d 100644 --- a/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs +++ b/src/SIL.XForge.Scripture/Models/ServalBuildAdditionalInfo.cs @@ -8,8 +8,8 @@ public class ServalBuildAdditionalInfo public string BuildId { get; init; } = string.Empty; public IEnumerable? CorporaIds { get; init; } public DateTimeOffset? DateFinished { get; init; } - public DateTimeOffset? DateRequested { get; set; } public DateTimeOffset? DateGenerated { get; set; } + public DateTimeOffset? DateRequested { get; set; } public IEnumerable? ParallelCorporaIds { get; init; } public string? RequestedByUserId { get; set; } public int Step { get; init; } diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index 0affa6f9fcb..b3fb4c57bf5 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -118,7 +118,7 @@ await projectSecrets.UpdateAsync( } // Get the translation engine id - string translationEngineId = GetTranslationId(projectSecret, preTranslate: true); + string translationEngineId = GetTranslationEngineId(projectSecret, preTranslate: true); try { @@ -344,12 +344,15 @@ CancellationToken cancellationToken // Fallback for builds previous to the event metric being recorded: // - As there is no event metric, get the translation scripture range from the pre-translation corpus // - We cannot accurately determine the source projects, so do not record the training scripture ranges. - PretranslateCorpus corpus = translationBuild.Pretranslate?.FirstOrDefault(); - if (corpus is not null) + + // Get the translation scripture range + PretranslateCorpus translationCorpus = translationBuild.Pretranslate?.FirstOrDefault(); + if (translationCorpus is not null) { #pragma warning disable CS0612 // Type or member is obsolete string scriptureRange = - corpus.SourceFilters?.FirstOrDefault()?.ScriptureRange ?? corpus.ScriptureRange; + translationCorpus.SourceFilters?.FirstOrDefault()?.ScriptureRange + ?? translationCorpus.ScriptureRange; #pragma warning restore CS0612 // Type or member is obsolete if (!string.IsNullOrWhiteSpace(scriptureRange)) { @@ -358,6 +361,23 @@ CancellationToken cancellationToken ); } } + + // Get the training scripture range + TrainingCorpus trainingCorpus = translationBuild.TrainOn?.FirstOrDefault(); + if (trainingCorpus is not null) + { +#pragma warning disable CS0612 // Type or member is obsolete + string scriptureRange = + trainingCorpus.SourceFilters?.FirstOrDefault()?.ScriptureRange ?? trainingCorpus.ScriptureRange; +#pragma warning restore CS0612 // Type or member is obsolete + if (!string.IsNullOrWhiteSpace(scriptureRange)) + { + // We do not accurately know the training, project, so leave it blank + buildDto.AdditionalInfo!.TrainingScriptureRanges.Add( + new ProjectScriptureRange { ProjectId = string.Empty, ScriptureRange = scriptureRange } + ); + } + } } // Make sure the DTO conforms to the machine-api URLs @@ -1055,7 +1075,7 @@ CancellationToken cancellationToken if (projectSecret.ServalData?.PreTranslationsRetrieved ?? true) { // Get the last completed build - string translationEngineId = GetTranslationId(projectSecret, preTranslate: true); + string translationEngineId = GetTranslationEngineId(projectSecret, preTranslate: true); TranslationBuild? translationBuild = ( await translationEnginesClient.GetAllBuildsAsync(translationEngineId, cancellationToken) ) @@ -1517,8 +1537,9 @@ CancellationToken cancellationToken /// /// The translation build from Serval. /// The build DTO. - private static ServalBuildDto CreateDto(TranslationBuild translationBuild) => - new ServalBuildDto + private static ServalBuildDto CreateDto(TranslationBuild translationBuild) + { + var buildDto = new ServalBuildDto { Id = translationBuild.Id, Revision = translationBuild.Revision, @@ -1558,6 +1579,16 @@ .. translationBuild }, }; + // Create an initial value for the date requested, based on the object id from Mongo + // This will be overwritten with the value from the EventMetric, if that exists + if (ObjectId.TryParse(translationBuild.Id, out ObjectId objectId)) + { + buildDto.AdditionalInfo!.DateRequested = new DateTimeOffset(objectId.CreationTime, TimeSpan.Zero); + } + + return buildDto; + } + private static ServalEngineDto CreateDto(TranslationEngine translationEngine) => new ServalEngineDto { @@ -1592,7 +1623,7 @@ private static string GetHighestRankedUserId(SFProject project) .Key; } - private static string GetTranslationId( + private static string GetTranslationEngineId( SFProjectSecret projectSecret, bool preTranslate, bool returnEmptyStringIfMissing = false @@ -1786,6 +1817,6 @@ private async Task GetTranslationIdAsync( : throw new DataNotFoundException("The project secret is missing"); } - return GetTranslationId(projectSecret, preTranslate, returnEmptyStringIfMissing); + return GetTranslationEngineId(projectSecret, preTranslate, returnEmptyStringIfMissing); } } diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index c474f1d3178..1e469f2146c 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -424,6 +424,8 @@ public async Task GetBuildAsync_IncludesAdditionalInfo() { // Set up test environment var env = new TestEnvironment(); + ObjectId buildObjectId = new ObjectId(); + string buildId = buildObjectId.ToString(); const string message = "Finalizing"; const double percentCompleted = 0.95; const int revision = 553; @@ -443,7 +445,7 @@ public async Task GetBuildAsync_IncludesAdditionalInfo() TranslationBuild translationBuild = new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = buildId, Engine = { Id = engineId, Url = "https://example.com" }, Message = message, PercentCompleted = percentCompleted, @@ -500,7 +502,7 @@ public async Task GetBuildAsync_IncludesAdditionalInfo() ServalBuildDto? actual = await env.Service.GetBuildAsync( User01, Project01, - Build01, + buildId, minRevision: null, preTranslate: false, isServalAdmin: false, @@ -511,8 +513,12 @@ public async Task GetBuildAsync_IncludesAdditionalInfo() TestEnvironment.AssertCoreBuildProperties(translationBuild, actual); Assert.AreEqual(queueDepth, actual!.QueueDepth); Assert.IsNotNull(actual.AdditionalInfo); - Assert.AreEqual(Build01, actual.AdditionalInfo!.BuildId); + Assert.AreEqual(buildId, actual.AdditionalInfo!.BuildId); Assert.AreEqual(dateFinished, actual.AdditionalInfo.DateFinished); + Assert.AreEqual( + new DateTimeOffset(buildObjectId.CreationTime, TimeSpan.Zero), + actual.AdditionalInfo!.DateRequested + ); Assert.AreEqual(step, actual.AdditionalInfo.Step); Assert.AreEqual(engineId, actual.AdditionalInfo.TranslationEngineId); Assert.IsNotNull(actual.AdditionalInfo.CorporaIds); @@ -834,9 +840,11 @@ public async Task GetBuildsAsync_SuccessWithFallbackToLegacyBuild() TranslationBuild translationBuild = env.ConfigureTranslationBuild(); // Add additional build properties - const string scriptureRange = "GEN;EXO"; + const string translationScriptureRange = "GEN;EXO"; + const string trainingScriptureRange = "LEV;NUM"; #pragma warning disable CS0612 // Type or member is obsolete - translationBuild.Pretranslate = [new PretranslateCorpus { ScriptureRange = scriptureRange }]; + translationBuild.Pretranslate = [new PretranslateCorpus { ScriptureRange = translationScriptureRange }]; + translationBuild.TrainOn = [new TrainingCorpus { ScriptureRange = trainingScriptureRange }]; #pragma warning restore CS0612 // Type or member is obsolete env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) @@ -855,7 +863,14 @@ public async Task GetBuildsAsync_SuccessWithFallbackToLegacyBuild() TestEnvironment.AssertCoreBuildProperties(translationBuild, builds[0]); Assert.NotNull(builds[0].AdditionalInfo); Assert.AreEqual(Project01, builds[0].AdditionalInfo?.TranslationScriptureRanges.Single().ProjectId); - Assert.AreEqual(scriptureRange, builds[0].AdditionalInfo?.TranslationScriptureRanges.Single().ScriptureRange); + Assert.AreEqual( + translationScriptureRange, + builds[0].AdditionalInfo?.TranslationScriptureRanges.Single().ScriptureRange + ); + Assert.AreEqual( + trainingScriptureRange, + builds[0].AdditionalInfo?.TrainingScriptureRanges.Single().ScriptureRange + ); } [Test] @@ -866,10 +881,21 @@ public async Task GetBuildsAsync_SuccessWithFallbackToPreTranslateBuild() TranslationBuild translationBuild = env.ConfigureTranslationBuild(); // Add additional build properties - const string scriptureRange = "GEN;EXO"; + const string translationScriptureRange = "GEN;EXO"; + const string trainingScriptureRange = "LEV;NUM"; translationBuild.Pretranslate = [ - new PretranslateCorpus { SourceFilters = [new ParallelCorpusFilter { ScriptureRange = scriptureRange }] }, + new PretranslateCorpus + { + SourceFilters = [new ParallelCorpusFilter { ScriptureRange = translationScriptureRange }], + }, + ]; + translationBuild.TrainOn = + [ + new TrainingCorpus + { + SourceFilters = [new ParallelCorpusFilter { ScriptureRange = trainingScriptureRange }], + }, ]; env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) @@ -888,7 +914,14 @@ public async Task GetBuildsAsync_SuccessWithFallbackToPreTranslateBuild() TestEnvironment.AssertCoreBuildProperties(translationBuild, builds[0]); Assert.NotNull(builds[0].AdditionalInfo); Assert.AreEqual(Project01, builds[0].AdditionalInfo?.TranslationScriptureRanges.Single().ProjectId); - Assert.AreEqual(scriptureRange, builds[0].AdditionalInfo?.TranslationScriptureRanges.Single().ScriptureRange); + Assert.AreEqual( + translationScriptureRange, + builds[0].AdditionalInfo?.TranslationScriptureRanges.Single().ScriptureRange + ); + Assert.AreEqual( + trainingScriptureRange, + builds[0].AdditionalInfo?.TrainingScriptureRanges.Single().ScriptureRange + ); } [Test] @@ -1653,7 +1686,6 @@ public async Task GetPreTranslationRevisionsAsync_NoOps() ] ) ); - List revisions = []; // SUT IReadOnlyList revisions = await env.Service.GetPreTranslationRevisionsAsync( @@ -3623,7 +3655,7 @@ public TranslationBuild ConfigureTranslationBuild(TranslationBuild? translationB .CancelBuildAsync(TranslationEngine01, CancellationToken.None) .Returns(Task.FromResult(translationBuild)); TranslationEnginesClient - .GetBuildAsync(TranslationEngine01, Build01, minRevision: null, CancellationToken.None) + .GetBuildAsync(TranslationEngine01, translationBuild.Id, minRevision: null, CancellationToken.None) .Returns(Task.FromResult(translationBuild)); TranslationEnginesClient .GetCurrentBuildAsync(TranslationEngine01, minRevision: null, CancellationToken.None) @@ -3718,14 +3750,14 @@ public void SetupTextDocument(string textDocumentId, int bookNum, bool alreadyEx public static void AssertCoreBuildProperties(TranslationBuild translationBuild, ServalBuildDto? actual) { - const string buildDtoId = $"{Project01}.{Build01}"; + string buildDtoId = $"{Project01}.{translationBuild.Id}"; Assert.IsNotNull(actual); Assert.AreEqual(translationBuild.Message, actual!.Message); Assert.AreEqual(translationBuild.PercentCompleted, actual.PercentCompleted); Assert.AreEqual(translationBuild.Revision, actual.Revision); Assert.AreEqual(translationBuild.State.ToString().ToUpperInvariant(), actual.State); Assert.AreEqual(buildDtoId, actual.Id); - Assert.AreEqual(MachineApi.GetBuildHref(Project01, Build01), actual.Href); + Assert.AreEqual(MachineApi.GetBuildHref(Project01, translationBuild.Id), actual.Href); Assert.AreEqual(Project01, actual.Engine.Id); Assert.AreEqual(MachineApi.GetEngineHref(Project01), actual.Engine.Href); }