diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts index c71fa5f4b51..d1b3c56174d 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts @@ -24,6 +24,7 @@ import { QueryParameters, QueryResults } from 'xforge-common/query-parameters'; import { RealtimeService } from 'xforge-common/realtime.service'; import { RetryingRequest, RetryingRequestService } from 'xforge-common/retrying-request.service'; import { EventMetric } from '../event-metrics/event-metric'; +import { BookProgress } from '../shared/progress-service/progress.service'; import { booksFromScriptureRange } from '../shared/utils'; import { BiblicalTermDoc } from './models/biblical-term-doc'; import { InviteeStatus } from './models/invitee-status'; @@ -404,4 +405,9 @@ export class SFProjectService extends ProjectService { return await this.onlineInvoke>('eventMetrics', params); } + + /** Gets project progress by calling the backend aggregation endpoint. */ + async getProjectProgress(projectId: string): Promise { + return await this.onlineInvoke('getProjectProgress', { projectId }); + } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.html index 7253dd5d1fb..21a92917f17 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.html @@ -49,14 +49,14 @@ [disabled]="readonly" (change)="onChipListChange(book)" > - @if (!basicMode) { + @if (book.progress != null && basicMode === false) { {{ "canon.book_names." + book.bookId | transloco }} -
+
} @else { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.spec.ts index f39b7a892f2..852050447a7 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.spec.ts @@ -1,9 +1,8 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { of } from 'rxjs'; -import { mock, when } from 'ts-mockito'; +import { anything, mock, when } from 'ts-mockito'; import { I18nService } from 'xforge-common/i18n.service'; import { configureTestingModule, getTestTranslocoModule } from 'xforge-common/test-utils'; -import { ProgressService, TextProgress } from '../progress-service/progress.service'; +import { ProgressService, ProjectProgress } from '../progress-service/progress.service'; import { Book } from './book-multi-select'; import { BookMultiSelectComponent } from './book-multi-select.component'; @@ -35,22 +34,24 @@ describe('BookMultiSelectComponent', () => { { number: 1, selected: true }, { number: 3, selected: true } ]; - when(mockedProgressService.isLoaded$).thenReturn(of(true)); - when(mockedProgressService.texts).thenReturn([ - { text: { bookNum: 1 }, percentage: 0 } as TextProgress, - { text: { bookNum: 2 }, percentage: 15 } as TextProgress, - { text: { bookNum: 3 }, percentage: 30 } as TextProgress, - { text: { bookNum: 40 }, percentage: 45 } as TextProgress, - { text: { bookNum: 42 }, percentage: 60 } as TextProgress, - { text: { bookNum: 67 }, percentage: 80 } as TextProgress, - { text: { bookNum: 70 }, percentage: 100 } as TextProgress - ]); + when(mockedProgressService.getProgress(anything(), anything())).thenResolve( + new ProjectProgress([ + { bookId: 'GEN', verseSegments: 0, blankVerseSegments: 0 }, + { bookId: 'EXO', verseSegments: 10_000, blankVerseSegments: 8_500 }, + { bookId: 'LEV', verseSegments: 10_000, blankVerseSegments: 7_000 }, + { bookId: 'MAT', verseSegments: 10_000, blankVerseSegments: 5_500 }, + { bookId: 'LUK', verseSegments: 10_000, blankVerseSegments: 4_000 }, + { bookId: 'TOB', verseSegments: 10_000, blankVerseSegments: 2_000 }, + { bookId: 'WIS', verseSegments: 10_000, blankVerseSegments: 0 } + ]) + ); when(mockedI18nService.localeCode).thenReturn('en'); fixture = TestBed.createComponent(BookMultiSelectComponent); component = fixture.componentInstance; component.availableBooks = mockBooks; component.selectedBooks = mockSelectedBooks; + component.projectId = 'test-project-id'; fixture.detectChanges(); }); @@ -65,28 +66,28 @@ describe('BookMultiSelectComponent', () => { it('should initialize book options on ngOnChanges', async () => { await component.ngOnChanges(); expect(component.bookOptions).toEqual([ - { bookNum: 1, bookId: 'GEN', selected: true, progressPercentage: 0 }, - { bookNum: 2, bookId: 'EXO', selected: false, progressPercentage: 15 }, - { bookNum: 3, bookId: 'LEV', selected: true, progressPercentage: 30 }, - { bookNum: 40, bookId: 'MAT', selected: false, progressPercentage: 45 }, - { bookNum: 42, bookId: 'LUK', selected: false, progressPercentage: 60 }, - { bookNum: 67, bookId: 'TOB', selected: false, progressPercentage: 80 }, - { bookNum: 70, bookId: 'WIS', selected: false, progressPercentage: 100 } + { bookNum: 1, bookId: 'GEN', selected: true, progress: 0.0 }, + { bookNum: 2, bookId: 'EXO', selected: false, progress: 0.15 }, + { bookNum: 3, bookId: 'LEV', selected: true, progress: 0.3 }, + { bookNum: 40, bookId: 'MAT', selected: false, progress: 0.45 }, + { bookNum: 42, bookId: 'LUK', selected: false, progress: 0.6 }, + { bookNum: 67, bookId: 'TOB', selected: false, progress: 0.8 }, + { bookNum: 70, bookId: 'WIS', selected: false, progress: 1 } ]); }); it('should not crash when texts have not yet loaded', async () => { - when(mockedProgressService.texts).thenReturn([]); + when(mockedProgressService.getProgress(anything(), anything())).thenResolve(new ProjectProgress([])); await component.ngOnChanges(); expect(component.bookOptions).toEqual([ - { bookNum: 1, bookId: 'GEN', selected: true, progressPercentage: 0 }, - { bookNum: 2, bookId: 'EXO', selected: false, progressPercentage: 0 }, - { bookNum: 3, bookId: 'LEV', selected: true, progressPercentage: 0 }, - { bookNum: 40, bookId: 'MAT', selected: false, progressPercentage: 0 }, - { bookNum: 42, bookId: 'LUK', selected: false, progressPercentage: 0 }, - { bookNum: 67, bookId: 'TOB', selected: false, progressPercentage: 0 }, - { bookNum: 70, bookId: 'WIS', selected: false, progressPercentage: 0 } + { bookNum: 1, bookId: 'GEN', selected: true, progress: null }, + { bookNum: 2, bookId: 'EXO', selected: false, progress: null }, + { bookNum: 3, bookId: 'LEV', selected: true, progress: null }, + { bookNum: 40, bookId: 'MAT', selected: false, progress: null }, + { bookNum: 42, bookId: 'LUK', selected: false, progress: null }, + { bookNum: 67, bookId: 'TOB', selected: false, progress: null }, + { bookNum: 70, bookId: 'WIS', selected: false, progress: null } ]); }); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts index 7835d63e47c..895360be740 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts @@ -5,16 +5,16 @@ import { MatProgressSpinner } from '@angular/material/progress-spinner'; import { MatTooltip } from '@angular/material/tooltip'; import { TranslocoModule } from '@ngneat/transloco'; import { Canon } from '@sillsdev/scripture'; -import { filter, firstValueFrom } from 'rxjs'; import { L10nPercentPipe } from 'xforge-common/l10n-percent.pipe'; -import { ProgressService } from '../progress-service/progress.service'; +import { estimatedActualBookProgress, ProgressService, ProjectProgress } from '../progress-service/progress.service'; import { Book } from './book-multi-select'; export interface BookOption { bookNum: number; bookId: string; selected: boolean; - progressPercentage: number; + /** The progress of the book as a ratio between 0 and 1, or null if not available. */ + progress: number | null; } type Scope = 'OT' | 'NT' | 'DC'; @@ -37,6 +37,8 @@ export class BookMultiSelectComponent implements OnChanges { @Input() availableBooks: Book[] = []; @Input() selectedBooks: Book[] = []; @Input() readonly: boolean = false; + /** The ID of the project to get the progress. */ + @Input() projectId?: string; @Input() projectName?: string; @Input() basicMode: boolean = false; @Output() bookSelect = new EventEmitter(); @@ -66,15 +68,26 @@ export class BookMultiSelectComponent implements OnChanges { } async initBookOptions(): Promise { - await firstValueFrom(this.progressService.isLoaded$.pipe(filter(loaded => loaded))); + // Only load progress if not in basic mode + let progress: ProjectProgress | undefined; + if (this.basicMode === false) { + if (this.projectId == null) { + throw new Error('app-book-multi-select requires a projectId input to initialize when not in basic mode'); + } + progress = await this.progressService.getProgress(this.projectId, { maxStalenessMs: 30_000 }); + } this.loaded = true; - const progress = this.progressService.texts; + + const progressByBookNum = (progress?.books ?? []).map(b => ({ + bookNum: Canon.bookIdToNumber(b.bookId), + progress: estimatedActualBookProgress(b) + })); this.bookOptions = this.availableBooks.map((book: Book) => ({ bookNum: book.number, bookId: Canon.bookNumberToId(book.number), selected: this.selectedBooks.find(b => book.number === b.number) !== undefined, - progressPercentage: progress.find(p => p.text.bookNum === book.number)?.percentage ?? 0 + progress: progressByBookNum.find(p => p.bookNum === book.number)?.progress ?? null })); this.booksOT = this.selectedBooks.filter(n => Canon.isBookOT(n.number)); @@ -136,8 +149,11 @@ export class BookMultiSelectComponent implements OnChanges { return this.availableBooks.findIndex(n => Canon.isBookDC(n.number)) > -1; } - getPercentage(book: BookOption): number { - // avoid showing 100% when it's not quite there - return (book.progressPercentage > 99 && book.progressPercentage < 100 ? 99 : book.progressPercentage) / 100; + /** + * Takes a number between 0 and 1, and if it's between 0.99 and 1, returns 0.99 to prevent showing a book as 100% + * complete when it's not. + */ + normalizeRatioForDisplay(ratio: number): number { + return ratio > 0.99 && ratio < 1 ? 0.99 : ratio; } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts index d727be07e2a..62767a6f86d 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts @@ -1,293 +1,304 @@ -import { NgZone } from '@angular/core'; -import { discardPeriodicTasks, fakeAsync, TestBed, tick } from '@angular/core/testing'; -import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; -import { TextData } from 'realtime-server/lib/esm/scriptureforge/models/text-data'; -import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; -import { BehaviorSubject, of } from 'rxjs'; -import { anything, instance, mock, when } from 'ts-mockito'; -import { ActivatedProjectService } from 'xforge-common/activated-project.service'; +import { fakeAsync, flushMicrotasks } from '@angular/core/testing'; +import { instance, mock, reset, verify, when } from 'ts-mockito'; import { NoticeService } from 'xforge-common/notice.service'; -import { provideTestOnlineStatus } from 'xforge-common/test-online-status-providers'; -import { configureTestingModule } from 'xforge-common/test-utils'; -import { SFProjectProfileDoc } from '../../core/models/sf-project-profile-doc'; -import { TextDocId } from '../../core/models/text-doc'; -import { PermissionsService } from '../../core/permissions.service'; import { SFProjectService } from '../../core/sf-project.service'; -import { ProgressService, TextProgress } from './progress.service'; - -const mockSFProjectService = mock(SFProjectService); -const mockNoticeService = mock(NoticeService); -const mockPermissionService = mock(PermissionsService); -const mockProjectService = mock(ActivatedProjectService); - -describe('progress service', () => { - configureTestingModule(() => ({ - providers: [ - provideTestOnlineStatus(), - { provide: NoticeService, useMock: mockNoticeService }, - { provide: PermissionsService, useMock: mockPermissionService }, - { provide: SFProjectService, useMock: mockSFProjectService }, - { provide: ActivatedProjectService, useMock: mockProjectService } - ] - })); +import { BookProgress, estimatedActualBookProgress, ProgressService, ProjectProgress } from './progress.service'; + +const mockedNoticeService = mock(NoticeService); +const mockedProjectService = mock(SFProjectService); + +describe('ProgressService', () => { + beforeEach(() => { + reset(mockedNoticeService); + reset(mockedProjectService); + }); - it('populates progress and texts on construction', fakeAsync(() => { - // Create segments for 20 chapters multiplied by 20 books - const env = new TestEnvironment(3600, 2000); - // Override the verse counts to be less than half of the number created for each book - spyOn(TextProgress.prototype as any, 'getVerseCount').and.callFake(() => 200); - const calculate = spyOn(env.service, 'calculateProgress').and.callThrough(); - - tick(); - - expect(env.service.overallProgress.translated).toEqual(3600); - expect(env.service.overallProgress.notTranslated).toEqual(2000); - expect(env.service.overallProgress.blank).toEqual(2000); - expect(env.service.overallProgress.total).toEqual(5600); - expect(env.service.overallProgress.percentage).toEqual(64); - expect(env.service.texts.length).toBeGreaterThan(0); - let i = 0; - for (const book of env.service.texts) { - expect(book.text.bookNum).toEqual(++i); - expect(book.expectedNumberOfVerses).toEqual(200); - expect(book.useExpectedNumberOfVerses).toEqual(false); - expect(book.blank).toEqual(100); - expect(book.translated).toEqual(180); - expect(book.total).toEqual(280); - expect(book.text.chapters.length).toBeGreaterThan(0); - let j = 0; - for (const chapter of book.text.chapters) { - expect(chapter.number).toEqual(j++); - } - } - expect(calculate).toHaveBeenCalledTimes(1); - - discardPeriodicTasks(); + it('should get fresh progress data', fakeAsync(() => { + const env = new TestEnvironment(); + const projectId = 'project1'; + const expectedBooks: BookProgress[] = [ + { bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }, + { bookId: 'MAT', verseSegments: 50, blankVerseSegments: 10 } + ]; + when(mockedProjectService.getProjectProgress(projectId)).thenResolve(expectedBooks); + + let result: ProjectProgress | undefined; + env.service.getProgress(projectId, { maxStalenessMs: 1000 }).then(r => (result = r)); + flushMicrotasks(); + + expect(result).toBeInstanceOf(ProjectProgress); + expect(result?.books).toEqual(expectedBooks); + expect(result?.verseSegments).toBe(150); + expect(result?.blankVerseSegments).toBe(30); + expect(result?.translatedVerseSegments).toBe(120); + expect(result?.ratio).toBeCloseTo(0.8); + verify(mockedProjectService.getProjectProgress(projectId)).once(); })); - it('re-initializes when project changes', fakeAsync(() => { - const env = new TestEnvironment(100, 50); - tick(); + it('should return cached data when fresh', fakeAsync(() => { + const env = new TestEnvironment(); + const projectId = 'project1'; + const expectedBooks: BookProgress[] = [{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }]; + when(mockedProjectService.getProjectProgress(projectId)).thenResolve(expectedBooks); - const initialize = spyOn(env.service, 'initialize').and.callThrough(); + let result1: ProjectProgress | undefined; + let result2: ProjectProgress | undefined; - when(env.mockProject.id).thenReturn('project02'); - env.project$.next(instance(env.mockProject)); - tick(); + env.service.getProgress(projectId, { maxStalenessMs: 10000 }).then(r => (result1 = r)); + flushMicrotasks(); + env.service.getProgress(projectId, { maxStalenessMs: 10000 }).then(r => (result2 = r)); + flushMicrotasks(); - expect(initialize).toHaveBeenCalledTimes(1); - discardPeriodicTasks(); + expect(result1?.books).toEqual(expectedBooks); + expect(result2?.books).toEqual(expectedBooks); + verify(mockedProjectService.getProjectProgress(projectId)).once(); })); - it('updates total progress when chapter content changes', fakeAsync(async () => { + it('should fetch fresh data when cache is stale', fakeAsync(() => { const env = new TestEnvironment(); - const changeEvent = new BehaviorSubject({}); - env.setTextData('project01', 1, 2, 'target', 12, 2, changeEvent); - - tick(); + const projectId = 'project1'; + const firstBooks: BookProgress[] = [{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }]; + const secondBooks: BookProgress[] = [{ bookId: 'GEN', verseSegments: 120, blankVerseSegments: 15 }]; - // mock a change - env.setTextData('project01', 1, 2, 'target', 13, 1, changeEvent); + when(mockedProjectService.getProjectProgress(projectId)).thenResolve(firstBooks).thenResolve(secondBooks); - const originalProgress = env.service.overallProgress.translated; - tick(1000); // wait for the throttle time + const nowSpy = spyOn(Date, 'now'); + nowSpy.and.returnValues(1000, 2000, 2000); - changeEvent.next({}); + let result1: ProjectProgress | undefined; + let result2: ProjectProgress | undefined; + env.service.getProgress(projectId, { maxStalenessMs: 1000 }).then(r => (result1 = r)); + flushMicrotasks(); + env.service.getProgress(projectId, { maxStalenessMs: 5 }).then(r => (result2 = r)); + flushMicrotasks(); - expect(env.service.overallProgress.translated).toEqual(originalProgress + 1); - discardPeriodicTasks(); + expect(result1?.books).toEqual(firstBooks); + expect(result2?.books).toEqual(secondBooks); + verify(mockedProjectService.getProjectProgress(projectId)).twice(); })); - it('uses the verse counts when there are too few segments', fakeAsync(() => { - const notTranslatedVerses = 17380; - const translatedVerses = 5; - const env = new TestEnvironment(translatedVerses, 1); - tick(); - - expect(env.service.overallProgress.translated).toEqual(translatedVerses); - expect(env.service.overallProgress.notTranslated).toEqual(notTranslatedVerses); - expect(env.service.overallProgress.blank).toEqual(notTranslatedVerses); - expect(env.service.overallProgress.total).toEqual(notTranslatedVerses + translatedVerses); - expect(env.service.overallProgress.percentage).toEqual(0); - expect(env.service.texts.length).toBeGreaterThan(0); - let i = 0; - for (const book of env.service.texts) { - expect(book.text.bookNum).toEqual(++i); - expect(book.useExpectedNumberOfVerses).toEqual(true); - expect(book.total).toEqual(book.expectedNumberOfVerses); - expect(book.text.chapters.length).toBeGreaterThan(0); - } - - discardPeriodicTasks(); - })); - - it('can train suggestions', fakeAsync(async () => { + it('should deduplicate concurrent requests for the same project', fakeAsync(() => { const env = new TestEnvironment(); - tick(); + const projectId = 'project1'; + const expectedBooks: BookProgress[] = [{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }]; - expect(env.service.canTrainSuggestions).toBeTruthy(); - discardPeriodicTasks(); - })); + let resolvePromise: ((value: BookProgress[]) => void) | undefined; + const delayedPromise: Promise = new Promise(resolve => { + resolvePromise = resolve; + }); + when(mockedProjectService.getProjectProgress(projectId)).thenReturn(delayedPromise); - it('cannot train suggestions if too few segments', fakeAsync(async () => { - const env = new TestEnvironment(9); - tick(); + let result1: ProjectProgress | undefined; + let result2: ProjectProgress | undefined; - expect(env.service.canTrainSuggestions).toBeFalsy(); - discardPeriodicTasks(); - })); + env.service.getProgress(projectId, { maxStalenessMs: 1000 }).then(r => (result1 = r)); + env.service.getProgress(projectId, { maxStalenessMs: 1000 }).then(r => (result2 = r)); - it('cannot train suggestions if no source permission', fakeAsync(async () => { - const env = new TestEnvironment(); - when(mockPermissionService.canAccessText(anything())).thenCall((textDocId: TextDocId) => { - return Promise.resolve(textDocId.projectId !== 'sourceId'); - }); - tick(); + resolvePromise!(expectedBooks); + flushMicrotasks(); - expect(env.service.canTrainSuggestions).toBeFalsy(); - discardPeriodicTasks(); + expect(result1?.books).toEqual(expectedBooks); + expect(result2?.books).toEqual(expectedBooks); + verify(mockedProjectService.getProjectProgress(projectId)).once(); })); - it('resets train suggestions flag when switching projects', fakeAsync(async () => { + it('should clean up request cache on error and allow retry', fakeAsync(() => { const env = new TestEnvironment(); - tick(); + const projectId = 'project1'; + const error = new Error('Network error'); + when(mockedProjectService.getProjectProgress(projectId)) + .thenReject(error) + .thenResolve([{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }]); + + let firstError: unknown; + env.service.getProgress(projectId, { maxStalenessMs: 1000 }).catch(e => (firstError = e)); + flushMicrotasks(); - expect(env.service.canTrainSuggestions).toBeTruthy(); + expect(firstError).toBe(error); - when(env.mockProject.id).thenReturn('project02'); - env.project$.next(instance(env.mockProject)); - tick(); + let result2: ProjectProgress | undefined; + env.service.getProgress(projectId, { maxStalenessMs: 1000 }).then(r => (result2 = r)); + flushMicrotasks(); + + expect(result2?.books).toEqual([{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }]); + verify(mockedProjectService.getProjectProgress(projectId)).twice(); + })); - expect(env.service.canTrainSuggestions).toBeFalsy(); - discardPeriodicTasks(); + it('should cache independently per project', fakeAsync(() => { + const env = new TestEnvironment(); + const project1Books: BookProgress[] = [{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }]; + const project2Books: BookProgress[] = [{ bookId: 'MAT', verseSegments: 50, blankVerseSegments: 10 }]; + + when(mockedProjectService.getProjectProgress('project1')).thenReturn(Promise.resolve(project1Books)); + when(mockedProjectService.getProjectProgress('project2')).thenReturn(Promise.resolve(project2Books)); + + let result1: ProjectProgress | undefined; + let result2: ProjectProgress | undefined; + env.service.getProgress('project1', { maxStalenessMs: 1000 }).then(r => (result1 = r)); + env.service.getProgress('project2', { maxStalenessMs: 1000 }).then(r => (result2 = r)); + flushMicrotasks(); + + expect(result1?.books).toEqual(project1Books); + expect(result2?.books).toEqual(project2Books); + verify(mockedProjectService.getProjectProgress('project1')).once(); + verify(mockedProjectService.getProjectProgress('project2')).once(); })); }); class TestEnvironment { - readonly ngZone: NgZone = TestBed.inject(NgZone); readonly service: ProgressService; - private readonly numBooks = 20; - private readonly numChapters = 20; - - readonly mockProject = mock(SFProjectProfileDoc); - readonly project$ = new BehaviorSubject(instance(this.mockProject)); - - // Store all text data in a single map to avoid repeated deepEqual calls - private readonly allTextData = new Map(); - - constructor( - private readonly translatedSegments: number = 1000, - private readonly blankSegments: number = 500 - ) { - const data = createTestProjectProfile({ - texts: this.createTexts(), - translateConfig: { - translationSuggestionsEnabled: true, - source: { - projectRef: 'sourceId' - } - } - }); - - when(this.mockProject.id).thenReturn('project01'); - when(mockProjectService.changes$).thenReturn(this.project$); - - when(mockPermissionService.canAccessText(anything())).thenResolve(true); - when(mockSFProjectService.getProfile('project01')).thenResolve({ - data, - id: 'project01', - remoteChanges$: new BehaviorSubject([]) - } as unknown as SFProjectProfileDoc); - - // set up blank project - when(mockSFProjectService.getProfile('project02')).thenResolve({ - data, - id: 'project02', - remoteChanges$: new BehaviorSubject([]) - } as unknown as SFProjectProfileDoc); - this.populateTextData('project02', 0, 1000); - - this.populateTextData('sourceId', this.translatedSegments, this.blankSegments); - this.populateTextData('project01', this.translatedSegments, this.blankSegments); - - // Set up a single mock for getText that handles all TextDocId instances - when(mockSFProjectService.getText(anything())).thenCall((textDocId: TextDocId) => { - const key = `${textDocId.projectId}:${textDocId.bookNum}:${textDocId.chapterNum}:${textDocId.textType}`; - const data = this.allTextData.get(key); - const changeKey = `${key}:changes`; - const customChanges = (this.allTextData as any).get(changeKey); - - if (data != null) { - return { - getSegmentCount: () => { - return { translated: data.translated, blank: data.blank }; - }, - getNonEmptyVerses: () => this.createVerses(data.translated), - changes$: customChanges ?? of({} as TextData) - }; - } - - // Return a default value if not found - return { - getSegmentCount: () => { - return { translated: 0, blank: 0 }; - }, - getNonEmptyVerses: () => [], - changes$: of({} as TextData) - }; - }); - this.service = TestBed.inject(ProgressService); + constructor() { + this.service = new ProgressService(instance(mockedNoticeService), instance(mockedProjectService)); } +} - private populateTextData(projectId: string, translatedSegments: number, blankSegments: number): void { - for (let book = 1; book <= this.numBooks; book++) { - for (let chapter = 0; chapter < this.numChapters; chapter++) { - const translated = translatedSegments >= 9 ? 9 : translatedSegments; - translatedSegments -= translated; - const blank = blankSegments >= 5 ? 5 : blankSegments; - blankSegments -= blank; - - const key = `${projectId}:${book}:${chapter}:target`; - this.allTextData.set(key, { translated, blank }); - } - } - } +describe('ProjectProgress', () => { + it('should calculate totals correctly with multiple books', () => { + const books: BookProgress[] = [ + { bookId: 'GEN', verseSegments: 100, blankVerseSegments: 20 }, + { bookId: 'EXO', verseSegments: 80, blankVerseSegments: 15 }, + { bookId: 'MAT', verseSegments: 50, blankVerseSegments: 5 } + ]; - setTextData( - projectId: string, - book: number, - chapter: number, - textType: string, - translated: number, - blank: number, - changes$?: BehaviorSubject - ): void { - const key = `${projectId}:${book}:${chapter}:${textType}`; - this.allTextData.set(key, { translated, blank }); - - // If a custom changes$ observable is provided, we need to store it - // so the mock can return it - if (changes$ != null) { - const changeKey = `${key}:changes`; - (this.allTextData as any).set(changeKey, changes$); - } - } + const progress = new ProjectProgress(books); - createVerses(num: number): string[] { - let count = 0; - return Array.from({ length: num }, () => 'verse' + ++count); - } + expect(progress.verseSegments).toBe(230); + expect(progress.blankVerseSegments).toBe(40); + expect(progress.translatedVerseSegments).toBe(190); + expect(progress.ratio).toBeCloseTo(0.8261); + }); - createTexts(): TextInfo[] { - const texts: TextInfo[] = []; - for (let book = 1; book <= this.numBooks; book++) { - const chapters: Chapter[] = []; - for (let chapter = 0; chapter < this.numChapters; chapter++) { - chapters.push({ isValid: true, lastVerse: 1, number: chapter, permissions: {}, hasAudio: false }); - } - texts.push({ bookNum: book, chapters: chapters, hasSource: true, permissions: {} }); - } - return texts; - } -} + it('should handle empty books array', () => { + const progress = new ProjectProgress([]); + + expect(progress.verseSegments).toBe(0); + expect(progress.blankVerseSegments).toBe(0); + expect(progress.translatedVerseSegments).toBe(0); + expect(progress.ratio).toBe(0); + }); + + it('should handle all blank verses', () => { + const books: BookProgress[] = [{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 100 }]; + + const progress = new ProjectProgress(books); + + expect(progress.verseSegments).toBe(100); + expect(progress.blankVerseSegments).toBe(100); + expect(progress.translatedVerseSegments).toBe(0); + expect(progress.ratio).toBe(0); + }); + + it('should handle no blank verses', () => { + const books: BookProgress[] = [{ bookId: 'GEN', verseSegments: 100, blankVerseSegments: 0 }]; + + const progress = new ProjectProgress(books); + + expect(progress.verseSegments).toBe(100); + expect(progress.blankVerseSegments).toBe(0); + expect(progress.translatedVerseSegments).toBe(100); + expect(progress.ratio).toBe(1); + }); +}); + +describe('estimatedActualBookProgress', () => { + it('should return normal ratio for books with sufficient segments', () => { + const bookProgress: BookProgress = { + bookId: 'GEN', + verseSegments: 1500, // Close to expected 1533 + blankVerseSegments: 300 + }; + + const result = estimatedActualBookProgress(bookProgress); + + expect(result).toBeCloseTo(0.8); // 1200 / 1500 + }); + + it('should return estimated ratio for books with very few segments compared to expected verses', () => { + const bookProgress: BookProgress = { + bookId: 'GEN', // Expected verses: 1533 + verseSegments: 50, // Much less than 10% of 1533 (153.3) + blankVerseSegments: 10 + }; + + const result = estimatedActualBookProgress(bookProgress); + + // Should use 40 / 1533 instead of 40 / 50 + expect(result).toBeCloseTo(40 / 1533); + }); + + it('should handle books at the threshold (exactly 10% of expected verses)', () => { + const bookProgress: BookProgress = { + bookId: 'GEN', // Expected verses: 1533 + // 10% of 1533 is 153.3, and the implementation uses a strict "<" comparison. + // Use a value just above the threshold to ensure we are in the "normal" ratio branch. + verseSegments: 154, + blankVerseSegments: 50 + }; + + const result = estimatedActualBookProgress(bookProgress); + + // Should use normal ratio since it's at 10% + expect(result).toBeCloseTo(104 / 154); + }); + + it('should handle books with no expected verse count', () => { + const bookProgress: BookProgress = { + bookId: 'UNKNOWN', + verseSegments: 10, + blankVerseSegments: 2 + }; + + const result = estimatedActualBookProgress(bookProgress); + + expect(result).toBeCloseTo(0.8); // 8 / 10 + }); + + it('should handle books with zero segments', () => { + const bookProgress: BookProgress = { + bookId: 'GEN', + verseSegments: 0, + blankVerseSegments: 0 + }; + + const result = estimatedActualBookProgress(bookProgress); + + expect(result).toBe(0); + }); + + it('should handle books with all blank segments', () => { + const bookProgress: BookProgress = { + bookId: 'GEN', + verseSegments: 100, + blankVerseSegments: 100 + }; + + const result = estimatedActualBookProgress(bookProgress); + + expect(result).toBe(0); + }); + + it('should handle various book IDs correctly', () => { + // Ensure the "estimated" branch is used for each book by keeping verseSegments < 10% of expected verses. + const testCases = [ + { bookId: 'MAT', expectedVerses: 1071, verseSegments: 5, blankVerseSegments: 1 }, + { bookId: 'RUT', expectedVerses: 85, verseSegments: 5, blankVerseSegments: 1 }, + { bookId: 'PSA', expectedVerses: 2527, verseSegments: 5, blankVerseSegments: 1 }, + // For JUD (25 verses), 10% is 2.5, so 5 segments would *not* trigger the estimated branch. + { bookId: 'JUD', expectedVerses: 25, verseSegments: 2, blankVerseSegments: 1 } + ]; + + testCases.forEach(testCase => { + const bookProgress: BookProgress = { + bookId: testCase.bookId, + verseSegments: testCase.verseSegments, + blankVerseSegments: testCase.blankVerseSegments + }; + + const result = estimatedActualBookProgress(bookProgress); + + const translatedSegments: number = testCase.verseSegments - testCase.blankVerseSegments; + expect(result).toBeCloseTo(translatedSegments / testCase.expectedVerses); + }); + }); +}); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts index ca4cf94a3e4..a9f1d5e0db9 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts @@ -1,20 +1,8 @@ -import { DestroyRef, Injectable, OnDestroy } from '@angular/core'; -import { Canon } from '@sillsdev/scripture'; -import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; -import { asyncScheduler, merge, startWith, Subscription, tap, throttleTime } from 'rxjs'; -import { ActivatedProjectService } from 'xforge-common/activated-project.service'; -import { DataLoadingComponent } from 'xforge-common/data-loading-component'; +import { Injectable } from '@angular/core'; import { NoticeService } from 'xforge-common/notice.service'; -import { OnlineStatusService } from 'xforge-common/online-status.service'; -import { filterNullish, quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; -import { SFProjectProfileDoc } from '../../core/models/sf-project-profile-doc'; -import { TextDoc, TextDocId } from '../../core/models/text-doc'; -import { PermissionsService } from '../../core/permissions.service'; import { SFProjectService } from '../../core/sf-project.service'; -/** - * The expected number of verses per book, calculated from the libpalaso versification files. - */ +/** The expected number of verses per book, calculated from the libpalaso versification files. */ const verseCounts: Record = { GEN: 1533, EXO: 1213, @@ -126,202 +114,83 @@ const verseCounts: Record = { LAO: 20 }; -export class Progress { - translated: number = 0; - blank: number = 0; - - get total(): number { - return this.translated + this.blank; - } +export interface BookProgress { + /** The book identifier (e.g. "GEN", "MAT"). */ + bookId: string; - get ratio(): number { - return this.total === 0 ? 0 : this.translated / this.total; - } - - get percentage(): number { - return Math.round(this.ratio * 100); - } + /** The total number of verse segments in this book. */ + verseSegments: number; - get notTranslated(): number { - return this.blank; - } - - set notTranslated(value: number) { - this.blank = value; - } - - reset(): void { - this.translated = 0; - this.blank = 0; - } + /** The number of blank verse segments in this book. */ + blankVerseSegments: number; } -export class TextProgress extends Progress { - expectedNumberOfVerses: number = 0; +export class ProjectProgress { + verseSegments = this.books.reduce((acc, book) => acc + book.verseSegments, 0); + blankVerseSegments = this.books.reduce((acc, book) => acc + book.blankVerseSegments, 0); + translatedVerseSegments = this.verseSegments - this.blankVerseSegments; + ratio = this.verseSegments === 0 ? 0 : this.translatedVerseSegments / this.verseSegments; - constructor(public readonly text: TextInfo) { - super(); - this.expectedNumberOfVerses = this.getVerseCount(text.bookNum); - } - - get notTranslated(): number { - // This value will be the number of blanks unless useExpectedNumberOfVerses is true. - return this.total - this.translated; - } - - get total(): number { - return this.useExpectedNumberOfVerses ? this.expectedNumberOfVerses : super.total; - } - - get useExpectedNumberOfVerses(): boolean { - // If the total calculated from segments is at least 2/3 of the expected number of verses possible, use the total. - return this.expectedNumberOfVerses > 0 && super.total < this.expectedNumberOfVerses * 0.66; - } + constructor(readonly books: BookProgress[]) {} +} - getVerseCount(bookNum: number): number { - return verseCounts[Canon.bookNumberToId(bookNum)] ?? 0; +/** + * Given a BookProgress object that indicates the total number of verse segments and number of blank verse segments, + * determines what the actual likely progress ratio is, based on the number of verses in the book. + * + * For most books, this will be the same as the ratio of translated segments to total segments, but if a book has very + * few segments but many expected verses (total segments < 10% of expected verses), it's unlikely the book actually + * combines verses so much that it's produced this ratio, and more likely the book is just missing most verses. In this + * case the function will return the ratio of translated segments to expected verses, which will provide a very rough + * approximation (since it assumes a 1:1 ratio of segments to verses). + */ +export function estimatedActualBookProgress(bookProgress: BookProgress): number { + const MAX_PLAUSIBLE_AVERAGE_VERSES_PER_SEGMENT = 10; + const translatedSegments = bookProgress.verseSegments - bookProgress.blankVerseSegments; + const expectedNumberOfVerses = verseCounts[bookProgress.bookId] ?? 0; + if ( + expectedNumberOfVerses !== 0 && + bookProgress.verseSegments * MAX_PLAUSIBLE_AVERAGE_VERSES_PER_SEGMENT < expectedNumberOfVerses + ) { + return translatedSegments / expectedNumberOfVerses; + } else { + return bookProgress.verseSegments === 0 ? 0 : translatedSegments / bookProgress.verseSegments; } } @Injectable({ providedIn: 'root' }) -export class ProgressService extends DataLoadingComponent implements OnDestroy { - readonly overallProgress = new Progress(); - - private _texts?: TextProgress[]; - private _projectDoc?: SFProjectProfileDoc; - private _allChaptersChangeSub?: Subscription; - private _canTrainSuggestions: boolean = false; - +export class ProgressService { constructor( readonly noticeService: NoticeService, - private readonly activatedProject: ActivatedProjectService, - private readonly onlineStatusService: OnlineStatusService, - private readonly projectService: SFProjectService, - private readonly permissionsService: PermissionsService, - private destroyRef: DestroyRef - ) { - super(noticeService); - - this.activatedProject.changes$ - .pipe( - startWith(this.activatedProject.projectDoc), - filterNullish(), - tap(async project => { - void this.initialize(project.id); - }), - throttleTime(1000, asyncScheduler, { leading: false, trailing: true }), - quietTakeUntilDestroyed(this.destroyRef) - ) - .subscribe(project => { - void this.initialize(project.id); - }); - } + private readonly projectService: SFProjectService + ) {} - get texts(): TextProgress[] { - return this._texts ?? []; - } - - // Whether or not we have the minimum number of segment pairs - get canTrainSuggestions(): boolean { - return this._canTrainSuggestions; - } + private projectProgressCache = new Map(); + private requestCache = new Map>(); - ngOnDestroy(): void { - this._allChaptersChangeSub?.unsubscribe(); - } - - private async initialize(projectId: string): Promise { - this._canTrainSuggestions = false; - this._projectDoc = await this.projectService.getProfile(projectId); - - // If we are offline, just update the progress with what we have - if (!this.onlineStatusService.isOnline) { - await this.calculateProgress(); + async getProgress(projectId: string, options: { maxStalenessMs: number }): Promise { + const cachedProgress = this.projectProgressCache.get(projectId); + if (cachedProgress != null && Date.now() - cachedProgress.timestampMs < options.maxStalenessMs) { + return cachedProgress.progress; } - - const chapterDocPromises: Promise[] = []; - for (const book of this._projectDoc.data!.texts) { - for (const chapter of book.chapters) { - const textDocId = new TextDocId(this._projectDoc.id, book.bookNum, chapter.number, 'target'); - chapterDocPromises.push(this.projectService.getText(textDocId)); - } + const existingRequest = this.requestCache.get(projectId); + if (existingRequest) { + return existingRequest; } - - const chapterDocs = await Promise.all(chapterDocPromises); - - const chapterObservables = chapterDocs.map(p => p.changes$); - - this._allChaptersChangeSub?.unsubscribe(); - this._allChaptersChangeSub = merge(...chapterObservables, this.onlineStatusService.online) - .pipe(throttleTime(1000, asyncScheduler, { leading: true, trailing: true })) - .subscribe(async () => { - await this.calculateProgress(); + const requestTimestamp = Date.now(); + const requestPromise = this.projectService + .getProjectProgress(projectId) + .then(bookProgressList => { + const progress = new ProjectProgress(bookProgressList); + this.projectProgressCache.set(projectId, { timestampMs: requestTimestamp, progress }); + this.requestCache.delete(projectId); + return progress; + }) + .catch(error => { + this.requestCache.delete(projectId); + throw error; }); - } - - private async calculateProgress(): Promise { - this.loadingStarted(); - try { - if (this._projectDoc == null || this._projectDoc.data == null) { - return; - } - this._texts = this._projectDoc.data.texts - .map(t => new TextProgress(t)) - .sort((a, b) => a.text.bookNum - b.text.bookNum); - this.overallProgress.reset(); - const bookProgressPromises: Promise[] = []; - for (const book of this.texts) { - bookProgressPromises.push(this.incorporateBookProgress(this._projectDoc, book)); - } - await Promise.all(bookProgressPromises); - } finally { - this.loadingFinished(); - } - } - - private async incorporateBookProgress(project: SFProjectProfileDoc, book: TextProgress): Promise { - // NOTE: This will stop being incremented when the minimum required number of pairs for training is reached - let numTranslatedSegments: number = 0; - for (const chapter of book.text.chapters) { - const textDocId = new TextDocId(project.id, book.text.bookNum, chapter.number, 'target'); - const chapterText: TextDoc = await this.projectService.getText(textDocId); - - // Calculate Segment Count - const { translated, blank } = chapterText.getSegmentCount(); - book.translated += translated; - book.blank += blank; - - // If translation suggestions are enabled, collect the number of segment pairs up to the minimum required - // We don't go any further so we don't load all of the source texts while this is running - if ( - project.data?.translateConfig.translationSuggestionsEnabled && - project.data.translateConfig.source != null && - book.text.hasSource && - !this.canTrainSuggestions - ) { - const sourceId: string = project.data.translateConfig.source.projectRef; - const sourceTextDocId = new TextDocId(sourceId, book.text.bookNum, chapter.number, 'target'); - - // Only retrieve the source text if the user has permission - let sourceNonEmptyVerses: string[] = []; - if (await this.permissionsService.canAccessText(sourceTextDocId)) { - const sourceChapterText: TextDoc = await this.projectService.getText(sourceTextDocId); - sourceNonEmptyVerses = sourceChapterText.getNonEmptyVerses(); - } - - // Get the intersect of the source and target arrays of non-empty verses - // i.e. The verses with data that both texts have in common - const targetNonEmptyVerses: string[] = chapterText.getNonEmptyVerses(); - numTranslatedSegments += targetNonEmptyVerses.filter(item => sourceNonEmptyVerses.includes(item)).length; - // 9 is the minimum number found in testing, but we will use 10 to be safe - if (numTranslatedSegments >= 10) { - this._canTrainSuggestions = true; - } - } - } - - // Add the book to the overall progress - this.overallProgress.translated += book.translated; - this.overallProgress.notTranslated += book.notTranslated; + this.requestCache.set(projectId, requestPromise); + return requestPromise; } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.html index 3e730109357..5a973224340 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.html @@ -130,6 +130,7 @@

{{ t("translated_books") }}

[availableBooks]="selectableTrainingBooksByProj(activatedProject.projectId)" [selectedBooks]="selectedTrainingBooksByProj(activatedProject.projectId)" [projectName]="targetProjectName" + [projectId]="activatedProject.projectId" (bookSelect)="onTranslatedBookSelect($event)" data-test-id="draft-stepper-training-books" > diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.spec.ts index 384c55ee7db..5348cccfee1 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.spec.ts @@ -23,7 +23,7 @@ import { SFProjectProfileDoc } from '../../../core/models/sf-project-profile-doc import { SF_TYPE_REGISTRY } from '../../../core/models/sf-type-registry'; import { ParatextService } from '../../../core/paratext.service'; import { SFProjectService } from '../../../core/sf-project.service'; -import { ProgressService, TextProgress } from '../../../shared/progress-service/progress.service'; +import { ProgressService, ProjectProgress } from '../../../shared/progress-service/progress.service'; import { NllbLanguageService } from '../../nllb-language.service'; import { DraftSource, DraftSourcesAsArrays } from '../draft-source'; import { DraftSourcesService } from '../draft-sources.service'; @@ -72,14 +72,15 @@ describe('DraftGenerationStepsComponent', () => { beforeEach(fakeAsync(() => { when(mockActivatedProjectService.projectId).thenReturn('project01'); when(mockActivatedProjectService.projectId$).thenReturn(of('project01')); - when(mockProgressService.isLoaded$).thenReturn(of(true)); - when(mockProgressService.texts).thenReturn([ - { text: { bookNum: 1 }, translated: 100, percentage: 100 } as TextProgress, - { text: { bookNum: 2 }, translated: 100, percentage: 100 } as TextProgress, - { text: { bookNum: 3 }, translated: 0 } as TextProgress, - { text: { bookNum: 6 }, translated: 20, blank: 2, percentage: 90 } as TextProgress, - { text: { bookNum: 7 }, translated: 0 } as TextProgress - ]); + when(mockProgressService.getProgress(anything(), anything())).thenResolve( + new ProjectProgress([ + { bookId: 'GEN', verseSegments: 100, blankVerseSegments: 0 }, + { bookId: 'EXO', verseSegments: 100, blankVerseSegments: 0 }, + { bookId: 'LEV', verseSegments: 100, blankVerseSegments: 100 }, + { bookId: 'NUM', verseSegments: 22, blankVerseSegments: 2 }, + { bookId: 'DEU', verseSegments: 0, blankVerseSegments: 0 } + ]) + ); when(mockOnlineStatusService.isOnline).thenReturn(true); })); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts index 172e7212e67..9001ced7e6a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts @@ -50,7 +50,7 @@ import { SFProjectService } from '../../../core/sf-project.service'; import { Book } from '../../../shared/book-multi-select/book-multi-select'; import { BookMultiSelectComponent } from '../../../shared/book-multi-select/book-multi-select.component'; import { NoticeComponent } from '../../../shared/notice/notice.component'; -import { ProgressService, TextProgress } from '../../../shared/progress-service/progress.service'; +import { BookProgress, ProgressService } from '../../../shared/progress-service/progress.service'; import { booksFromScriptureRange, projectLabel } from '../../../shared/utils'; import { NllbLanguageService } from '../../nllb-language.service'; import { ConfirmSourcesComponent } from '../confirm-sources/confirm-sources.component'; @@ -58,8 +58,7 @@ import { DraftSource } from '../draft-source'; import { DraftSourcesService } from '../draft-sources.service'; import { TrainingDataService } from '../training-data/training-data.service'; -// We consider books with more than 10 translated segments as translated -const minimumTranslatedSegments: number = 10; +const MIN_TRANSLATED_SEGMENTS_TO_AUTO_SELECT_BOOK = 10 as const; export interface DraftGenerationStepsResult { trainingDataFiles: string[]; @@ -264,6 +263,9 @@ export class DraftGenerationStepsComponent implements OnInit { // Otherwise, add to unusable books. // Ensure books are displayed in ascending canonical order. const targetBooks = new Set(); + const projectProgress = await this.progressService.getProgress(projectId, { + maxStalenessMs: 30_000 + }); for (const text of target.texts.slice().sort((a, b) => a.bookNum - b.bookNum)) { const bookNum = text.bookNum; targetBooks.add(bookNum); @@ -296,16 +298,17 @@ export class DraftGenerationStepsComponent implements OnInit { // Determine if this book should be auto selected. The requirements are: // 1. The project does not have any previous training selections made. - // 2. At least 10 verses have been translated. + // 2. At least 10 non-blank segments in the book // 3. At least 99 percent of the book has been translated or 3 or fewer blank segments. - const textProgress: TextProgress | undefined = this.progressService.texts.find( - t => t.text.bookNum === bookNum - ); + const bookId = Canon.bookNumberToId(bookNum); + const bookProgress: BookProgress | undefined = projectProgress.books.find(b => b.bookId === bookId); const selected: boolean = !hasPreviousTrainingRange && - textProgress != null && - textProgress.translated > minimumTranslatedSegments && - (textProgress.percentage >= 99 || textProgress.notTranslated <= 3); + bookProgress != null && + bookProgress.verseSegments - bookProgress.blankVerseSegments > + MIN_TRANSLATED_SEGMENTS_TO_AUTO_SELECT_BOOK && + (bookProgress.blankVerseSegments / bookProgress.verseSegments <= 0.01 || + bookProgress.blankVerseSegments <= 3); // If books were automatically selected, reflect this in the UI via a notice this.trainingBooksWereAutoSelected ||= selected; diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html index 51cedf1509f..ee7e255c85a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html @@ -10,10 +10,10 @@

{{ t("translate_overview") }}

{{ t("progress") }} -
+
{{ t("translate_overview") }} - @if (progressService.texts != null) { + @if (projectProgress != null) { - @for (textProgress of progressService.texts; track trackTextByBookNum($index, textProgress)) { - + @for (bookProgress of projectProgress.books; track bookProgress.bookId) { + book - {{ getBookName(textProgress.text) }} + {{ getBookNameFromId(bookProgress.bookId) }} {{ t("translated_segments", { - translatedSegments: (textProgress.translated | l10nNumber), - total: (textProgress.total | l10nNumber) + translatedSegments: (bookTranslatedSegments(bookProgress) | l10nNumber), + total: (bookProgress.verseSegments | l10nNumber) }) }} -
+
{{ t("translate_overview") }} {{ trainedSegmentCount | l10nNumber }}{{ t("trained_segments") }}
- @if (showCannotTrainEngineMessage || !progressService.canTrainSuggestions) { + @if (showCannotTrainEngineMessage || !hasMinimumSegmentsToTrainStatisticalEngine) {
{{ showCannotTrainEngineMessage ? t("cannot_train_suggestion_engine") : t("not_enough_verses_translated") diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.spec.ts index 3171f202b05..e0b61c76e9c 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.spec.ts @@ -12,7 +12,6 @@ import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/ import { SFProjectRole } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-role'; import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; import { getTextDocId } from 'realtime-server/lib/esm/scriptureforge/models/text-data'; -import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; import { TextInfoPermission } from 'realtime-server/lib/esm/scriptureforge/models/text-info-permission'; import * as RichText from 'rich-text'; import { defer, of, Subject } from 'rxjs'; @@ -34,7 +33,7 @@ import { TextDoc, TextDocId } from '../../core/models/text-doc'; import { PermissionsService } from '../../core/permissions.service'; import { TranslationEngineService } from '../../core/translation-engine.service'; import { RemoteTranslationEngine } from '../../machine-api/remote-translation-engine'; -import { Progress, ProgressService, TextProgress } from '../../shared/progress-service/progress.service'; +import { ProgressService, ProjectProgress } from '../../shared/progress-service/progress.service'; import { FontUnsupportedMessageComponent } from '../font-unsupported-message/font-unsupported-message.component'; import { TrainingProgressComponent } from '../training-progress/training-progress.component'; import { TranslateOverviewComponent } from './translate-overview.component'; @@ -76,7 +75,7 @@ describe('TranslateOverviewComponent', () => { env.wait(); expect(env.progressTitle.textContent).toContain('Progress'); - expect(env.component.progressService.texts.length).toEqual(4); + expect(env.component.projectProgress?.books.length).toEqual(4); env.expectContainsTextProgress(0, 'Matthew', '10 of 20 segments'); env.expectContainsTextProgress(1, 'Mark', '10 of 20 segments'); env.expectContainsTextProgress(2, 'Luke', '10 of 20 segments'); @@ -268,14 +267,14 @@ class TestEnvironment { when(this.mockedRemoteTranslationEngine.getStats()).thenResolve({ confidence: 0.25, trainedSegmentCount: 100 }); when(this.mockedRemoteTranslationEngine.listenForTrainingStatus()).thenReturn(defer(() => this.trainingProgress$)); when(this.mockedRemoteTranslationEngine.startTraining()).thenResolve(); - when(mockedProgressService.isLoaded$).thenReturn(of(true)); - when(mockedProgressService.overallProgress).thenReturn(new Progress()); - when(mockedProgressService.texts).thenReturn([ - { translated: 10, blank: 10, total: 20, percentage: 50, text: { bookNum: 40 } as TextInfo } as TextProgress, - { translated: 10, blank: 10, total: 20, percentage: 50, text: { bookNum: 41 } as TextInfo } as TextProgress, - { translated: 10, blank: 10, total: 20, percentage: 50, text: { bookNum: 42 } as TextInfo } as TextProgress, - { translated: 10, blank: 10, total: 20, percentage: 50, text: { bookNum: 43 } as TextInfo } as TextProgress - ]); + when(mockedProgressService.getProgress(anything(), anything())).thenResolve( + new ProjectProgress([ + { bookId: 'MAT', verseSegments: 20, blankVerseSegments: 10 }, + { bookId: 'MRK', verseSegments: 20, blankVerseSegments: 10 }, + { bookId: 'LUK', verseSegments: 20, blankVerseSegments: 10 }, + { bookId: 'JHN', verseSegments: 20, blankVerseSegments: 10 } + ]) + ); this.setCurrentUser(); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts index 9e58c32d5a9..2078694aed9 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts @@ -23,7 +23,7 @@ import { SFProject } from 'realtime-server/lib/esm/scriptureforge/models/sf-proj import { SF_PROJECT_RIGHTS, SFProjectDomain } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-rights'; import { isParatextRole } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-role'; import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; -import { asyncScheduler, firstValueFrom, Subscription, timer } from 'rxjs'; +import { asyncScheduler, Subscription, timer } from 'rxjs'; import { filter, map, repeat, retry, tap, throttleTime } from 'rxjs/operators'; import { AuthService } from 'xforge-common/auth.service'; import { DataLoadingComponent } from 'xforge-common/data-loading-component'; @@ -40,7 +40,7 @@ import { SFProjectProfileDoc } from '../../core/models/sf-project-profile-doc'; import { SFProjectService } from '../../core/sf-project.service'; import { TranslationEngineService } from '../../core/translation-engine.service'; import { RemoteTranslationEngine } from '../../machine-api/remote-translation-engine'; -import { ProgressService, TextProgress } from '../../shared/progress-service/progress.service'; +import { BookProgress, ProgressService, ProjectProgress } from '../../shared/progress-service/progress.service'; import { FontUnsupportedMessageComponent } from '../font-unsupported-message/font-unsupported-message.component'; import { TrainingProgressComponent } from '../training-progress/training-progress.component'; const ENGINE_QUALITY_STAR_COUNT = 3; @@ -83,6 +83,7 @@ export class TranslateOverviewComponent extends DataLoadingComponent implements engineQuality: number = 0; engineConfidence: number = 0; trainedSegmentCount: number = 0; + projectProgress?: ProjectProgress; private trainingSub?: Subscription; private translationEngine?: RemoteTranslationEngine; @@ -153,7 +154,7 @@ export class TranslateOverviewComponent extends DataLoadingComponent implements this.setupTranslationEngine(); } await this.updateEngineStats(); - await firstValueFrom(this.progressService.isLoaded$.pipe(filter(loaded => loaded))); + this.projectProgress = await this.progressService.getProgress(this.projectId!, { maxStalenessMs: 30_000 }); } finally { this.loadingFinished(); } @@ -177,7 +178,9 @@ export class TranslateOverviewComponent extends DataLoadingComponent implements this.loadingStarted(); try { await this.updateEngineStats(); - await firstValueFrom(this.progressService.isLoaded$.pipe(filter(loaded => loaded))); + this.projectProgress = await this.progressService.getProgress(this.projectId!, { + maxStalenessMs: 30_000 + }); } finally { this.loadingFinished(); } @@ -210,6 +213,26 @@ export class TranslateOverviewComponent extends DataLoadingComponent implements .then(() => void this.listenForStatus()); } + getBookNameFromId(bookId: string): string { + const bookNum = Canon.bookIdToNumber(bookId); + return this.i18n.localizeBook(bookNum); + } + + get hasMinimumSegmentsToTrainStatisticalEngine(): boolean { + return (this.projectProgress?.translatedVerseSegments ?? 0) >= 10; + } + + bookTranslatedSegments(bookProgress: BookProgress): number { + return bookProgress.verseSegments - bookProgress.blankVerseSegments; + } + + bookTranslationRatio(bookProgress: BookProgress): number { + if (bookProgress.verseSegments === 0) { + return 0; + } + return this.bookTranslatedSegments(bookProgress) / bookProgress.verseSegments; + } + getBookName(text: TextInfo): string { return this.i18n.localizeBook(text.bookNum); } @@ -218,10 +241,6 @@ export class TranslateOverviewComponent extends DataLoadingComponent implements return Canon.bookNumberToId(text.bookNum); } - trackTextByBookNum(_index: number, item: TextProgress): number { - return item.text.bookNum; - } - get isPTUser(): boolean { return isParatextRole(this.projectDoc?.data?.userRoles[this.userService.currentUserId]); } diff --git a/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs b/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs index 1c39d8de2aa..64748a9261d 100644 --- a/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs +++ b/src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs @@ -1180,4 +1180,28 @@ public async Task SetIsValid(string projectId, int book, int c throw; } } + + public async Task GetProjectProgress(string projectId) + { + try + { + BookProgress[] progress = await projectService.GetProjectProgressAsync(UserId, projectId); + return Ok(progress); + } + catch (ForbiddenException) + { + return ForbiddenError(); + } + catch (DataNotFoundException dnfe) + { + return NotFoundError(dnfe.Message); + } + catch (Exception) + { + _exceptionHandler.RecordEndpointInfoForException( + new Dictionary { { "method", "GetProjectProgress" }, { "projectId", projectId } } + ); + throw; + } + } } diff --git a/src/SIL.XForge.Scripture/Models/BookProgress.cs b/src/SIL.XForge.Scripture/Models/BookProgress.cs new file mode 100644 index 00000000000..34e6bda2adc --- /dev/null +++ b/src/SIL.XForge.Scripture/Models/BookProgress.cs @@ -0,0 +1,22 @@ +namespace SIL.XForge.Scripture.Models; + +/// +/// Represents the progress data for a single book calculated from the database aggregation. +/// +public class BookProgress +{ + /// + /// The book identifier (e.g. "GEN", "MAT"). + /// + public string BookId { get; set; } = string.Empty; + + /// + /// The total number of verse segments in this book. + /// + public int VerseSegments { get; set; } + + /// + /// The number of blank verse segments in this book. + /// + public int BlankVerseSegments { get; set; } +} diff --git a/src/SIL.XForge.Scripture/Services/ISFProjectService.cs b/src/SIL.XForge.Scripture/Services/ISFProjectService.cs index c3c412a6b7e..2590c59f2df 100644 --- a/src/SIL.XForge.Scripture/Services/ISFProjectService.cs +++ b/src/SIL.XForge.Scripture/Services/ISFProjectService.cs @@ -97,4 +97,5 @@ string audioUrl Task SetIsValidAsync(string userId, string projectId, int book, int chapter, bool isValid); Task SetRoleProjectPermissionsAsync(string curUserId, string projectId, string role, string[] permissions); Task SetUserProjectPermissionsAsync(string curUserId, string projectId, string userId, string[] permissions); + Task GetProjectProgressAsync(string curUserId, string projectId); } diff --git a/src/SIL.XForge.Scripture/Services/SFProjectService.cs b/src/SIL.XForge.Scripture/Services/SFProjectService.cs index abb4acc399f..e2f1795ce4b 100644 --- a/src/SIL.XForge.Scripture/Services/SFProjectService.cs +++ b/src/SIL.XForge.Scripture/Services/SFProjectService.cs @@ -11,6 +11,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using MongoDB.Bson; +using MongoDB.Driver; using Newtonsoft.Json.Linq; using SIL.Extensions; using SIL.XForge.Configuration; @@ -50,10 +51,12 @@ public class SFProjectService : ProjectService, ISFP private readonly IEventMetricService _eventMetricService; private readonly ISFProjectRights _projectRights; private readonly IGuidService _guidService; + private readonly IMongoDatabase _database; public SFProjectService( IRealtimeService realtimeService, IOptions siteOptions, + IOptions dataAccessOptions, IAudioService audioService, IEmailService emailService, IRepository projectSecrets, @@ -70,7 +73,8 @@ public SFProjectService( IBackgroundJobClient backgroundJobClient, IEventMetricService eventMetricService, ISFProjectRights projectRights, - IGuidService guidService + IGuidService guidService, + IMongoClient mongoClient ) : base(realtimeService, siteOptions, audioService, projectSecrets, fileSystemService) { @@ -89,6 +93,7 @@ IGuidService guidService _backgroundJobClient = backgroundJobClient; _projectRights = projectRights; _guidService = guidService; + _database = mongoClient.GetDatabase(dataAccessOptions.Value.MongoDatabaseName); } protected override string ProjectAdminRole => SFProjectRole.Administrator; @@ -1824,6 +1829,110 @@ protected override async Task> TryGetProjectRoleAsync(SFProject return Attempt.Failure(ProjectRole.None); } + /// + /// Calculates project progress by aggregating verse segment data from the MongoDB texts collection. + /// This method uses a MongoDB aggregation pipeline to efficiently compute progress at the database level. + /// + /// The current user identifier. + /// The project identifier. + /// The calculated project progress data. + /// The project does not exist. + /// The user does not have permission to view the project. + public async Task GetProjectProgressAsync(string curUserId, string projectId) + { + // Check that the project exists and user has permission to view it + SFProject project = await GetProjectAsync(projectId); + if (!project.UserRoles.ContainsKey(curUserId)) + throw new ForbiddenException(); + + // Checks whether a segment is a verse segment by checking if the attributes.segment field starts with "verse_" + BsonDocument isVerseSegmentIdExpression = new BsonDocument( + "$regexMatch", + new BsonDocument + { + { "input", new BsonDocument("$ifNull", new BsonArray { "$$segment.attributes.segment", "" }) }, + { "regex", "^verse_" }, + } + ); + + // Filters for ops that are verse segments (i.e., attributes.segment starts with "verse_") + BsonDocument verseSegmentOpsFilterExpression = new BsonDocument + { + { "input", "$ops" }, + { "as", "segment" }, + { "cond", isVerseSegmentIdExpression }, + }; + // Same as above filter, except that insert.blank must also be true in order to match a segment + BsonDocument blankVerseSegmentOpsFilterExpression = new BsonDocument + { + { "input", "$ops" }, + { "as", "segment" }, + { + "cond", + new BsonDocument( + "$and", + new BsonArray + { + isVerseSegmentIdExpression, + new BsonDocument("$eq", new BsonArray { "$$segment.insert.blank", true }), + } + ) + }, + }; + + List results = await _database + .GetCollection("texts") + .Aggregate() + // Filter for text documents that belong to the specified project + .Match(Builders.Filter.Regex("_id", new BsonRegularExpression($"^{projectId}:"))) + // Project: + // - Extract the book ID from the document ID + // - Count the number of verse segments + // - Count the number of blank verse segments + .Project( + new BsonDocument + { + { "_id", 1 }, + { + "book", + new BsonDocument( + "$arrayElemAt", + new BsonArray { new BsonDocument("$split", new BsonArray { "$_id", ":" }), 1 } + ) + }, + { + "verseSegments", + new BsonDocument("$size", new BsonDocument("$filter", verseSegmentOpsFilterExpression)) + }, + { + "blankVerseSegments", + new BsonDocument("$size", new BsonDocument("$filter", blankVerseSegmentOpsFilterExpression)) + }, + } + ) + // Group progress by book and count the total verse segments and blank verse segments for each book + .Group( + new BsonDocument + { + { "_id", "$book" }, + { "verseSegments", new BsonDocument("$sum", "$verseSegments") }, + { "blankVerseSegments", new BsonDocument("$sum", "$blankVerseSegments") }, + } + ) + .ToListAsync(); + + var books = results + .Select(doc => new BookProgress + { + BookId = doc["_id"].AsString, + VerseSegments = doc["verseSegments"].AsInt32, + BlankVerseSegments = doc["blankVerseSegments"].AsInt32, + }) + .ToArray(); + + return books; + } + private async Task AddUserToSourceProjectAsync( IConnection conn, TranslateSource? source, diff --git a/test/SIL.XForge.Scripture.Tests/Services/SFProjectServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/SFProjectServiceTests.cs index 75d96ce84ae..d080365da75 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/SFProjectServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/SFProjectServiceTests.cs @@ -11,6 +11,7 @@ using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using MongoDB.Driver; using Newtonsoft.Json; using NSubstitute; using NSubstitute.ExceptionExtensions; @@ -4571,6 +4572,13 @@ public async Task SyncUserRoleAsync_UpgradesRole() await env.SyncService.Received().SyncAsync(Arg.Any()); } + [Test] + public void GetProjectProgressAsync_UserNotOnProject_Forbidden() + { + var env = new TestEnvironment(); + Assert.ThrowsAsync(() => env.Service.GetProjectProgressAsync(User04, Project01)); + } + private class TestEnvironment { public static readonly Uri WebsiteUrl = new Uri("http://localhost/", UriKind.Absolute); @@ -5137,6 +5145,12 @@ public TestEnvironment() SiteDir = "xforge", } ); + IOptions dataAccessOptions = Microsoft.Extensions.Options.Options.Create( + new DataAccessOptions { MongoDatabaseName = "mongoDatabaseName" } + ); + + MongoClient = Substitute.For(); + var audioService = Substitute.For(); EmailService = Substitute.For(); EmailService.ValidateEmail(Arg.Any()).Returns(true); @@ -5510,6 +5524,7 @@ public TestEnvironment() Service = new SFProjectService( RealtimeService, siteOptions, + dataAccessOptions, audioService, EmailService, ProjectSecrets, @@ -5526,7 +5541,8 @@ public TestEnvironment() BackgroundJobClient, EventMetricService, ProjectRights, - GuidService + GuidService, + MongoClient ); } @@ -5547,6 +5563,7 @@ public TestEnvironment() public IBackgroundJobClient BackgroundJobClient { get; } public ISFProjectRights ProjectRights { get; } public IGuidService GuidService { get; } + public IMongoClient MongoClient { get; } public SFProject GetProject(string id) => RealtimeService.GetRepository().Get(id);