Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SF-3156 Warn user when mismatch in translated and training books #2956

Merged
merged 3 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,16 @@ <h2>{{ t("reference_books") }}</h2>
{{ t("choose_books_for_training_error") }}
</app-notice>
}
@if (!translatedBooksSelectedInTrainingSources) {
<app-notice class="warn-translated-books-unselected" type="warning" icon="warning">
{{ t("translated_book_selected_no_training_pair") }}
</app-notice>
}
@if (translatedBooksWithNoSource.length > 0) {
<app-notice class="warn-source-books-missing" type="warning" icon="warning">
{{ t("source_books_missing") }}
</app-notice>
}
<div class="button-strip">
<button mat-stroked-button matStepperPrevious>{{ t("back") }}</button>
<button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ describe('DraftGenerationStepsComponent', () => {
when(mockActivatedRoute.params).thenReturn(of({ projectId: 'project01' }));
when(mockProgressService.isLoaded$).thenReturn(of(true));
when(mockProgressService.texts).thenReturn([
{ text: { bookNum: 1 } } as TextProgress,
{ text: { bookNum: 2 } } as TextProgress,
{ text: { bookNum: 3 } } as TextProgress,
{ text: { bookNum: 6 } } as TextProgress,
{ text: { bookNum: 7 } } as TextProgress
{ text: { bookNum: 1 }, translated: 100 } as TextProgress,
{ text: { bookNum: 2 }, translated: 100 } as TextProgress,
{ text: { bookNum: 3 }, translated: 0 } as TextProgress,
{ text: { bookNum: 6 }, translated: 20 } as TextProgress,
{ text: { bookNum: 7 }, translated: 0 } as TextProgress
]);
when(mockOnlineStatusService.isOnline).thenReturn(true);
}));
Expand Down Expand Up @@ -178,6 +178,8 @@ describe('DraftGenerationStepsComponent', () => {
it('should set "unusableTranslateSourceBooks" and "unusableTrainingSourceBooks" correctly', fakeAsync(() => {
expect(component.unusableTranslateSourceBooks).toEqual([6]);
expect(component.unusableTrainingSourceBooks).toEqual([6]);
// warn when translated book has no source
expect(fixture.nativeElement.querySelector('.warn-source-books-missing')).not.toBeNull();
}));

it('should set "unusableTranslateTargetBooks" and "unusableTrainingTargetBooks" correctly', fakeAsync(() => {
Expand Down Expand Up @@ -255,7 +257,7 @@ describe('DraftGenerationStepsComponent', () => {
{
projectRef: mockActivatedProjectService.projectId,
shortName: 'tT',
writingSystem: { tag: 'nllb' },
writingSystem: { tag: 'xyz' },
texts: allBooks
}
] as [DraftSource],
Expand Down Expand Up @@ -424,6 +426,32 @@ describe('DraftGenerationStepsComponent', () => {
} as DraftGenerationStepsResult);
expect(component.isStepsCompleted).toBe(true);
});

it('show warning when both source books missing translated books', () => {
component.onTranslateBookSelect([7]);
component.onTranslatedBookSelect([2, 6]);
component.onSourceTrainingBookSelect([2, 6], config.trainingSources[0]);
component.onSourceTrainingBookSelect([], config.trainingSources[1]);
fixture.detectChanges();

spyOn(component.done, 'emit');
fixture.detectChanges();
clickConfirmLanguages(fixture);
// Advance to the next step when at last step should emit books result
fixture.detectChanges();
component.tryAdvanceStep();
fixture.detectChanges();
component.tryAdvanceStep();
fixture.detectChanges();

expect(component.stepper.selectedIndex).toBe(2);
component.onSourceTrainingBookSelect([2], config.trainingSources[0]);
fixture.detectChanges();
expect(fixture.nativeElement.querySelector('.warn-translated-books-unselected')).not.toBeNull();
component.tryAdvanceStep();
fixture.detectChanges();
expect(component.stepper.selectedIndex).toBe(3);
});
});

describe('allow fast training feature flag is enabled', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { UICommonModule } from 'xforge-common/ui-common.module';
import { filterNullish } from 'xforge-common/util/rxjs-util';
import { TrainingDataDoc } from '../../../core/models/training-data-doc';
import { BookMultiSelectComponent } from '../../../shared/book-multi-select/book-multi-select.component';
import { ProgressService, TextProgress } from '../../../shared/progress-service/progress.service';
import { SharedModule } from '../../../shared/shared.module';
import { booksFromScriptureRange, projectLabel } from '../../../shared/utils';
import { NllbLanguageService } from '../../nllb-language.service';
Expand Down Expand Up @@ -101,6 +102,7 @@ export class DraftGenerationStepsComponent implements OnInit {

protected trainingSources: DraftSource[] = [];
protected trainingTargets: DraftSource[] = [];
protected translatedBooksWithNoSource: number[] = [];

private trainingDataQuery?: RealtimeQuery<TrainingDataDoc>;
private trainingDataSub?: Subscription;
Expand All @@ -114,7 +116,8 @@ export class DraftGenerationStepsComponent implements OnInit {
private readonly trainingDataService: TrainingDataService,
protected readonly i18n: I18nService,
private readonly onlineStatusService: OnlineStatusService,
private readonly noticeService: NoticeService
private readonly noticeService: NoticeService,
private readonly progressService: ProgressService
) {}

ngOnInit(): void {
Expand Down Expand Up @@ -184,6 +187,13 @@ export class DraftGenerationStepsComponent implements OnInit {
isPresentInASource = true;
} else {
this.unusableTrainingSourceBooks.push(bookNum);
const textProgress: TextProgress | undefined = this.progressService.texts.find(
t => t.text.bookNum === bookNum
);
if (textProgress != null && textProgress.translated > 10) {
// we consider books with more than 10 translated segments as translated
this.translatedBooksWithNoSource.push(bookNum);
}
}
if (trainingSources[1] != null && additionalTrainingSourceBooks.has(bookNum)) {
this.availableTrainingBooks[trainingSources[1].projectRef].push({ number: bookNum, selected: false });
Expand Down Expand Up @@ -254,6 +264,18 @@ export class DraftGenerationStepsComponent implements OnInit {
return false;
}

get translatedBooksSelectedInTrainingSources(): boolean {
const translatedBooksSelected: Book[] = this.selectedTrainingBooksByProj(this.activatedProject.projectId);
const selectedInSource1: Book[] =
this.trainingSources[0] != null ? this.selectedTrainingBooksByProj(this.trainingSources[0].projectRef) : [];
const selectedInSource2: Book[] =
this.trainingSources[1] != null ? this.selectedTrainingBooksByProj(this.trainingSources[1].projectRef) : [];
const selectedInSources: number[] = Array.from(
new Set<number>([...selectedInSource1, ...selectedInSource2].map(b => b.number))
);
return translatedBooksSelected.every(b => selectedInSources.includes(b.number));
}

get firstTrainingSource(): string {
return this.trainingSources[0]?.shortName ?? '';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { TrainingDataDoc } from '../../core/models/training-data-doc';
import { SFProjectService } from '../../core/sf-project.service';
import { BuildDto } from '../../machine-api/build-dto';
import { BuildStates } from '../../machine-api/build-states';
import { ProgressService } from '../../shared/progress-service/progress.service';
import { NllbLanguageService } from '../nllb-language.service';
import { DraftGenerationComponent } from './draft-generation.component';
import { DraftGenerationService } from './draft-generation.service';
Expand All @@ -53,6 +54,7 @@ describe('DraftGenerationComponent', () => {
let mockPreTranslationSignupUrlService: jasmine.SpyObj<PreTranslationSignupUrlService>;
let mockNllbLanguageService: jasmine.SpyObj<NllbLanguageService>;
let mockTrainingDataService: jasmine.SpyObj<TrainingDataService>;
let mockProgressService: jasmine.SpyObj<ProgressService>;

const buildDto: BuildDto = {
id: 'testId',
Expand Down Expand Up @@ -115,7 +117,8 @@ describe('DraftGenerationComponent', () => {
{ provide: PreTranslationSignupUrlService, useValue: mockPreTranslationSignupUrlService },
{ provide: NllbLanguageService, useValue: mockNllbLanguageService },
{ provide: OnlineStatusService, useClass: TestOnlineStatusService },
{ provide: TrainingDataService, useValue: mockTrainingDataService }
{ provide: TrainingDataService, useValue: mockTrainingDataService },
{ provide: ProgressService, useValue: mockProgressService }
]
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,12 @@
"no_training_books": "No training books selected",
"overview": "Overview",
"reference_books": "Reference books",
"source_books_missing": "The reference text is missing one or more of the translated books in your project. Consider choosing a reference text that contains a match for all the translated books",
"these_source_books_cannot_be_used_for_training": "The following books cannot be used for training as they are not in the training source text ({{ firstTrainingSource }}).",
"these_source_books_cannot_be_used_for_translating": "The following books cannot be translated as they are not in the drafting source text ({{ draftingSourceProjectName }}).",
"training_books_will_appear": "Training books will appear as you select books under translated books",
"translated_books": "Translated books",
"translated_book_selected_no_training_pair": "Some of the training books you selected have no matching reference books selected. Please select matching reference books.",
"unusable_target_books": "Can't find the book you're looking for? Be sure the book is created in Paratext, then sync your project."
},
"draft_preview_books": {
Expand Down
Loading