Skip to content

Commit 20a6f85

Browse files
NateowamiNateowami
authored
Revert "SF-3617 Warn user when training source books are blank (#3562)" (#3581)
1 parent 0e9d096 commit 20a6f85

File tree

6 files changed

+53
-217
lines changed

6 files changed

+53
-217
lines changed

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts‎

Lines changed: 6 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@ const mockNoticeService = mock(NoticeService);
2020
const mockPermissionService = mock(PermissionsService);
2121
const mockProjectService = mock(ActivatedProjectService);
2222

23-
const defaultChaptersNum = 20;
24-
const defaultTranslatedNum = 9;
25-
const defaultBlankNum = 5;
26-
2723
describe('progress service', () => {
2824
configureTestingModule(() => ({
2925
providers: [
@@ -165,63 +161,17 @@ describe('progress service', () => {
165161
expect(env.service.canTrainSuggestions).toBeFalsy();
166162
discardPeriodicTasks();
167163
}));
168-
169-
it('returns text progress for texts on a project', fakeAsync(async () => {
170-
const booksWithTexts = 5;
171-
const totalTranslated = booksWithTexts * defaultChaptersNum * defaultTranslatedNum;
172-
const totalBlank = booksWithTexts * defaultChaptersNum * defaultBlankNum;
173-
const env = new TestEnvironment(totalTranslated, totalBlank);
174-
tick();
175-
176-
const texts: TextInfo[] = env.createTexts();
177-
const projectDoc: SFProjectProfileDoc = {
178-
id: 'sourceId',
179-
data: createTestProjectProfile({ texts })
180-
} as SFProjectProfileDoc;
181-
when(mockSFProjectService.getProfile(projectDoc.id)).thenResolve(projectDoc);
182-
when(mockPermissionService.isUserOnProject(anything())).thenResolve(true);
183-
184-
// SUT
185-
const progressList = await env.service.getTextProgressForProject(projectDoc.id);
186-
tick();
187-
expect(progressList.length).toEqual(texts.length);
188-
for (let i = 0; i < progressList.length; i++) {
189-
const progress = progressList[i];
190-
if (i < booksWithTexts) {
191-
expect(progress.translated).toEqual(defaultTranslatedNum * defaultChaptersNum);
192-
expect(progress.blank).toEqual(defaultBlankNum * defaultChaptersNum);
193-
} else {
194-
expect(progress.translated).toEqual(0);
195-
expect(progress.blank).toEqual(0);
196-
}
197-
}
198-
}));
199-
200-
it('returns empty text progress if user does not have permission', fakeAsync(async () => {
201-
const env = new TestEnvironment(1000, 500);
202-
tick();
203-
const texts: TextInfo[] = env.createTexts();
204-
const projectDoc: SFProjectProfileDoc = {
205-
id: 'sourceId',
206-
data: createTestProjectProfile({ texts })
207-
} as SFProjectProfileDoc;
208-
when(mockPermissionService.isUserOnProject(anything())).thenResolve(false);
209-
210-
// SUT
211-
const progressList = await env.service.getTextProgressForProject(projectDoc.id);
212-
tick();
213-
expect(progressList.length).toEqual(0);
214-
}));
215164
});
216165

217166
class TestEnvironment {
218167
readonly ngZone: NgZone = TestBed.inject(NgZone);
219168
readonly service: ProgressService;
169+
private readonly numBooks = 20;
170+
private readonly numChapters = 20;
220171

221172
readonly mockProject = mock(SFProjectProfileDoc);
222173
readonly project$ = new BehaviorSubject(instance(this.mockProject));
223174

224-
private readonly numBooks = 20;
225175
// Store all text data in a single map to avoid repeated deepEqual calls
226176
private readonly allTextData = new Map<string, { translated: number; blank: number }>();
227177

@@ -292,10 +242,10 @@ class TestEnvironment {
292242

293243
private populateTextData(projectId: string, translatedSegments: number, blankSegments: number): void {
294244
for (let book = 1; book <= this.numBooks; book++) {
295-
for (let chapter = 0; chapter < defaultChaptersNum; chapter++) {
296-
const translated = translatedSegments >= defaultTranslatedNum ? defaultTranslatedNum : translatedSegments;
245+
for (let chapter = 0; chapter < this.numChapters; chapter++) {
246+
const translated = translatedSegments >= 9 ? 9 : translatedSegments;
297247
translatedSegments -= translated;
298-
const blank = blankSegments >= defaultBlankNum ? defaultBlankNum : blankSegments;
248+
const blank = blankSegments >= 5 ? 5 : blankSegments;
299249
blankSegments -= blank;
300250

301251
const key = `${projectId}:${book}:${chapter}:target`;
@@ -333,7 +283,7 @@ class TestEnvironment {
333283
const texts: TextInfo[] = [];
334284
for (let book = 1; book <= this.numBooks; book++) {
335285
const chapters: Chapter[] = [];
336-
for (let chapter = 0; chapter < defaultChaptersNum; chapter++) {
286+
for (let chapter = 0; chapter < this.numChapters; chapter++) {
337287
chapters.push({ isValid: true, lastVerse: 1, number: chapter, permissions: {}, hasAudio: false });
338288
}
339289
texts.push({ bookNum: book, chapters: chapters, hasSource: true, permissions: {} });

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -230,45 +230,6 @@ export class ProgressService extends DataLoadingComponent implements OnDestroy {
230230
this._allChaptersChangeSub?.unsubscribe();
231231
}
232232

233-
/** Calculate the text progress for a project by reading every text doc for each book. */
234-
async getTextProgressForProject(projectId: string): Promise<TextProgress[]> {
235-
if (!(await this.permissionsService.isUserOnProject(projectId))) return [];
236-
237-
const projectDoc = await this.projectService.getProfile(projectId);
238-
if (projectDoc.data == null) {
239-
return [];
240-
}
241-
const chapterPromises: Promise<TextDoc[]>[] = [];
242-
const chaptersByBook: Map<number, TextDoc[]> = new Map();
243-
244-
// for every book that exists in the project calculate the translated verses
245-
for (const book of projectDoc.data.texts) {
246-
const bookChapters: TextDocId[] = book.chapters.map(
247-
c => new TextDocId(projectDoc.id, book.bookNum, c.number, 'target')
248-
);
249-
const chapterTextDocPromises = Promise.all(bookChapters.map(c => this.projectService.getText(c)));
250-
// set the map of books to text docs
251-
void chapterTextDocPromises.then(textDocs => {
252-
chaptersByBook.set(book.bookNum, textDocs);
253-
});
254-
chapterPromises.push(chapterTextDocPromises);
255-
}
256-
257-
await Promise.all(chapterPromises);
258-
const textProgressList = projectDoc.data.texts.map(t => new TextProgress(t));
259-
for (const textProgress of textProgressList) {
260-
const chapterTextDocs: TextDoc[] = chaptersByBook.get(textProgress.text.bookNum) ?? [];
261-
for (const chapterTextDoc of chapterTextDocs) {
262-
// get the translated and blank segments from the chapter docs
263-
const { translated, blank } = chapterTextDoc.getSegmentCount();
264-
textProgress.translated += translated;
265-
textProgress.blank += blank;
266-
}
267-
}
268-
269-
return textProgressList;
270-
}
271-
272233
private async initialize(projectId: string): Promise<void> {
273234
this._canTrainSuggestions = false;
274235
this._projectDoc = await this.projectService.getProfile(projectId);

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.html‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ <h2>{{ t("translated_books") }}</h2>
133133
(bookSelect)="onTranslatedBookSelect($event)"
134134
data-test-id="draft-stepper-training-books"
135135
></app-book-multi-select>
136-
@if (unusableTrainingSourceBooks.length > 0 || emptyTrainingSourceBooks.length > 0) {
136+
@if (unusableTrainingSourceBooks.length) {
137137
<app-notice icon="info" mode="basic" type="light" class="unusable-training-books">
138138
<div class="notice-container">
139139
<span
@@ -142,33 +142,22 @@ <h2>{{ t("translated_books") }}</h2>
142142
[innerHtml]="
143143
!expandUnusableTrainingBooks
144144
? i18n.translateAndInsertTags('draft_generation_steps.books_are_hidden_show_why', {
145-
numBooks: unusableTrainingSourceBooks.length + emptyTrainingSourceBooks.length
145+
numBooks: unusableTrainingSourceBooks.length
146146
})
147147
: i18n.translateAndInsertTags('draft_generation_steps.books_are_hidden_hide_explanation', {
148-
numBooks: unusableTrainingSourceBooks.length + emptyTrainingSourceBooks.length
148+
numBooks: unusableTrainingSourceBooks.length
149149
})
150150
"
151151
>
152152
</span>
153153
@if (expandUnusableTrainingBooks) {
154-
@if (emptyTrainingSourceBooks.length > 0) {
155-
<h4 class="explanation">
156-
<transloco
157-
key="draft_generation_steps.these_training_source_books_are_blank"
158-
[params]="{ firstTrainingSource }"
159-
></transloco>
160-
</h4>
161-
<span class="book-names">{{ bookNames(emptyTrainingSourceBooks) }}</span>
162-
}
163-
@if (unusableTrainingSourceBooks.length > 0) {
164-
<h4 class="explanation">
165-
<transloco
166-
key="draft_generation_steps.these_source_books_cannot_be_used_for_training"
167-
[params]="{ firstTrainingSource }"
168-
></transloco>
169-
</h4>
170-
<span class="book-names">{{ bookNames(unusableTrainingSourceBooks) }}</span>
171-
}
154+
<h4 class="explanation">
155+
<transloco
156+
key="draft_generation_steps.these_source_books_cannot_be_used_for_training"
157+
[params]="{ firstTrainingSource }"
158+
></transloco>
159+
</h4>
160+
<span class="book-names">{{ bookNames(unusableTrainingSourceBooks) }}</span>
172161
}
173162
</div>
174163
</app-notice>

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.spec.ts‎

Lines changed: 17 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -79,19 +79,6 @@ describe('DraftGenerationStepsComponent', () => {
7979
{ text: { bookNum: 6 }, translated: 20, blank: 2, percentage: 90 } as TextProgress,
8080
{ text: { bookNum: 7 }, translated: 0 } as TextProgress
8181
]);
82-
const defaultTextProgress = [
83-
{ text: { bookNum: 1 }, translated: 100, percentage: 100 } as TextProgress,
84-
{ text: { bookNum: 2 }, translated: 100, percentage: 100 } as TextProgress,
85-
{ text: { bookNum: 3 }, translated: 100, percentage: 100 } as TextProgress,
86-
{ text: { bookNum: 4 }, translated: 100, percentage: 100 } as TextProgress,
87-
{ text: { bookNum: 5 }, translated: 100, percentage: 100 } as TextProgress,
88-
{ text: { bookNum: 6 }, translated: 100, percentage: 100 } as TextProgress,
89-
{ text: { bookNum: 7 }, translated: 100, percentage: 100 } as TextProgress,
90-
{ text: { bookNum: 8 }, translated: 100, percentage: 100 } as TextProgress,
91-
{ text: { bookNum: 9 }, translated: 100, percentage: 100 } as TextProgress,
92-
{ text: { bookNum: 10 }, translated: 100, percentage: 100 } as TextProgress
93-
];
94-
when(mockProgressService.getTextProgressForProject(anything())).thenResolve(defaultTextProgress);
9582
when(mockOnlineStatusService.isOnline).thenReturn(true);
9683
}));
9784

@@ -304,7 +291,6 @@ describe('DraftGenerationStepsComponent', () => {
304291
when(mockActivatedProjectService.projectDoc).thenReturn(mockTargetProjectDoc);
305292
when(mockActivatedProjectService.projectDoc$).thenReturn(targetProjectDoc$);
306293
when(mockActivatedProjectService.changes$).thenReturn(targetProjectDoc$);
307-
// setup mock source with empty book
308294
setupProjectProfileMock(
309295
sourceProjectId,
310296
sourceBooks.map(b => b.bookNum),
@@ -345,12 +331,14 @@ describe('DraftGenerationStepsComponent', () => {
345331
project01: [
346332
{ number: 1, selected: true },
347333
{ number: 2, selected: true },
348-
{ number: 3, selected: false }
334+
{ number: 3, selected: false },
335+
{ number: 5, selected: false }
349336
],
350337
sourceProject: [
351338
{ number: 1, selected: true },
352339
{ number: 2, selected: true },
353-
{ number: 3, selected: false }
340+
{ number: 3, selected: false },
341+
{ number: 5, selected: false }
354342
]
355343
});
356344
}));
@@ -359,7 +347,8 @@ describe('DraftGenerationStepsComponent', () => {
359347
expect(component.selectableTrainingBooksByProj('project01')).toEqual([
360348
{ number: 1, selected: true },
361349
{ number: 2, selected: true },
362-
{ number: 3, selected: false }
350+
{ number: 3, selected: false },
351+
{ number: 5, selected: false }
363352
]);
364353
expect(component.selectableTrainingBooksByProj(sourceProjectId)).toEqual([
365354
{ number: 1, selected: true },
@@ -385,10 +374,6 @@ describe('DraftGenerationStepsComponent', () => {
385374
expect(component.emptyTranslateSourceBooks).toEqual([5]);
386375
}));
387376

388-
it('should set "emptyTrainingSourceBooks"', fakeAsync(() => {
389-
expect(component.emptyTrainingSourceBooks).toEqual([5]);
390-
}));
391-
392377
it('should set "unusableTranslateTargetBooks" and "unusableTrainingTargetBooks" correctly', fakeAsync(() => {
393378
expect(component.unusableTranslateTargetBooks).toEqual([7]);
394379
expect(component.unusableTrainingTargetBooks).toEqual([7]);
@@ -511,7 +496,10 @@ describe('DraftGenerationStepsComponent', () => {
511496
component.tryAdvanceStep();
512497
fixture.detectChanges();
513498
component.onTranslatedBookSelect([1]);
514-
expect(component.selectableTrainingBooksByProj('project01')).toEqual([{ number: 1, selected: true }]);
499+
expect(component.selectableTrainingBooksByProj('project01')).toEqual([
500+
{ number: 1, selected: true },
501+
{ number: 5, selected: false }
502+
]);
515503
expect(component.selectedTrainingBooksByProj('project01')).toEqual([{ number: 1, selected: true }]);
516504
expect(component.selectedTrainingBooksByProj('sourceProject')).toEqual([{ number: 1, selected: true }]);
517505
component.stepper.selectedIndex = 1;
@@ -523,7 +511,8 @@ describe('DraftGenerationStepsComponent', () => {
523511
// Exodus becomes a selectable training book
524512
expect(component.selectableTrainingBooksByProj('project01')).toEqual([
525513
{ number: 1, selected: true },
526-
{ number: 2, selected: false }
514+
{ number: 2, selected: false },
515+
{ number: 5, selected: false }
527516
]);
528517
expect(component.selectedTrainingBooksByProj('sourceProject')).toEqual([{ number: 1, selected: true }]);
529518
expect(component.selectedTrainingBooksByProj('project01')).toEqual([{ number: 1, selected: true }]);
@@ -629,11 +618,9 @@ describe('DraftGenerationStepsComponent', () => {
629618
});
630619

631620
describe('two training sources', () => {
632-
const availableBooks = [{ bookNum: 2 }, { bookNum: 3 }, { bookNum: 5 }];
621+
const availableBooks = [{ bookNum: 2 }, { bookNum: 3 }];
633622
const allBooks = [{ bookNum: 1 }, ...availableBooks, { bookNum: 6 }, { bookNum: 7 }, { bookNum: 8 }];
634623
const draftingSourceBooks = availableBooks.concat({ bookNum: 7 });
635-
const trainingSource1Books = availableBooks.concat({ bookNum: 1 });
636-
const trainingSource2Books = availableBooks.concat({ bookNum: 6 });
637624
const draftingSourceId = 'draftingSource';
638625
const config = {
639626
trainingSources: [
@@ -642,14 +629,14 @@ describe('DraftGenerationStepsComponent', () => {
642629
paratextId: 'PT_SP1',
643630
shortName: 'sP1',
644631
writingSystem: { tag: 'eng' },
645-
texts: trainingSource1Books
632+
texts: availableBooks.concat({ bookNum: 1 })
646633
},
647634
{
648635
projectRef: 'source2',
649636
paratextId: 'PT_SP2',
650637
shortName: 'sP2',
651638
writingSystem: { tag: 'eng' },
652-
texts: trainingSource2Books
639+
texts: availableBooks.concat({ bookNum: 6 })
653640
}
654641
] as [DraftSource, DraftSource],
655642
trainingTargets: [
@@ -670,27 +657,14 @@ describe('DraftGenerationStepsComponent', () => {
670657
] as [DraftSource]
671658
};
672659

673-
const emptyBooks = [5];
674660
beforeEach(fakeAsync(() => {
675661
when(mockDraftSourceService.getDraftProjectSources()).thenReturn(of(config));
676662
when(mockActivatedProjectService.projectDoc$).thenReturn(of({} as any));
677663
when(mockActivatedProjectService.changes$).thenReturn(of({} as any));
678664
when(mockActivatedProjectService.projectDoc).thenReturn({} as any);
679-
// setup mock sources with empty book
680665
setupProjectProfileMock(
681666
draftingSourceId,
682-
draftingSourceBooks.map(b => b.bookNum),
683-
emptyBooks
684-
);
685-
setupProjectProfileMock(
686-
'source1',
687-
trainingSource1Books.map(b => b.bookNum),
688-
emptyBooks
689-
);
690-
setupProjectProfileMock(
691-
'source2',
692-
trainingSource2Books.map(b => b.bookNum),
693-
emptyBooks
667+
draftingSourceBooks.map(b => b.bookNum)
694668
);
695669
when(mockFeatureFlagService.showDeveloperTools).thenReturn(createTestFeatureFlag(false));
696670
when(mockNllbLanguageService.isNllbLanguageAsync(anything())).thenResolve(true);
@@ -706,7 +680,6 @@ describe('DraftGenerationStepsComponent', () => {
706680
expect(component.allAvailableTranslateBooks).toEqual([
707681
{ number: 2, selected: false },
708682
{ number: 3, selected: false },
709-
{ number: 5, selected: false },
710683
{ number: 7, selected: false }
711684
]);
712685
}));
@@ -747,8 +720,6 @@ describe('DraftGenerationStepsComponent', () => {
747720
fixture.detectChanges();
748721
expect(component.unusableTranslateSourceBooks).toEqual([1, 6, 8]);
749722
expect(component.unusableTrainingSourceBooks).toEqual([6, 7, 8]);
750-
expect(component.emptyTranslateSourceBooks).toEqual([5]);
751-
expect(component.emptyTrainingSourceBooks).toEqual([5]);
752723

753724
// interact with unusable books notice
754725
const unusableTranslateBooks = fixture.nativeElement.querySelector('.unusable-translate-books');
@@ -1485,18 +1456,10 @@ describe('DraftGenerationStepsComponent', () => {
14851456
const profileDoc = {
14861457
id: projectId,
14871458
data: createTestProjectProfile({
1488-
texts: texts.map(b => ({ bookNum: b, chapters: [{ number: 1, lastVerse: 10 }] }))
1459+
texts: texts.map(b => ({ bookNum: b, chapters: [{ number: 1, lastVerse: emptyBooks.includes(b) ? 0 : 10 }] }))
14891460
})
14901461
} as SFProjectProfileDoc;
14911462

14921463
when(mockProjectService.getProfile(projectId)).thenResolve(profileDoc);
1493-
const textProgress = texts.map(bookNum => {
1494-
return {
1495-
text: { bookNum },
1496-
translated: emptyBooks.includes(bookNum) ? 0 : 100,
1497-
percentage: emptyBooks.includes(bookNum) ? 0 : 100
1498-
} as TextProgress;
1499-
});
1500-
when(mockProgressService.getTextProgressForProject(projectId)).thenResolve(textProgress);
15011464
}
15021465
});

0 commit comments

Comments
 (0)