Skip to content

Commit 7390b24

Browse files
authored
SF-3610 Check for the latest draft for allowing formatting (#3533)
1 parent c5708ff commit 7390b24

File tree

13 files changed

+498
-13
lines changed

13 files changed

+498
-13
lines changed

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,67 @@ describe('DraftGenerationService', () => {
183183
}));
184184
});
185185

186+
describe('getLastPreTranslationBuild', () => {
187+
it('should get last pre-translation build and return an observable of BuildDto', fakeAsync(() => {
188+
// SUT
189+
service.getLastPreTranslationBuild(projectId).subscribe(result => {
190+
expect(result).toEqual(buildDto);
191+
});
192+
tick();
193+
194+
// Setup the HTTP request
195+
const req = httpTestingController.expectOne(
196+
`${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/getLastPreTranslationBuild`
197+
);
198+
expect(req.request.method).toEqual('GET');
199+
req.flush(buildDto);
200+
tick();
201+
}));
202+
203+
it('should return undefined when no build has ever run', fakeAsync(() => {
204+
// SUT
205+
service.getLastPreTranslationBuild(projectId).subscribe(result => {
206+
expect(result).toBeUndefined();
207+
});
208+
tick();
209+
210+
// Setup the HTTP request
211+
const req = httpTestingController.expectOne(
212+
`${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/getLastPreTranslationBuild`
213+
);
214+
expect(req.request.method).toEqual('GET');
215+
req.flush(null, { status: HttpStatusCode.NoContent, statusText: 'No Content' });
216+
tick();
217+
}));
218+
219+
it('should return undefined if offline', fakeAsync(() => {
220+
testOnlineStatusService.setIsOnline(false);
221+
222+
// SUT
223+
service.getLastPreTranslationBuild(projectId).subscribe(result => {
224+
expect(result).toBeUndefined();
225+
});
226+
tick();
227+
}));
228+
229+
it('should return undefined and show error when server returns 500', fakeAsync(() => {
230+
// SUT
231+
service.getLastPreTranslationBuild(projectId).subscribe(result => {
232+
expect(result).toBeUndefined();
233+
verify(mockNoticeService.showError(anything())).once();
234+
});
235+
tick();
236+
237+
// Setup the HTTP request
238+
const req = httpTestingController.expectOne(
239+
`${MACHINE_API_BASE_URL}translation/engines/project:${projectId}/actions/getLastPreTranslationBuild`
240+
);
241+
expect(req.request.method).toEqual('GET');
242+
req.flush(null, { status: HttpStatusCode.InternalServerError, statusText: 'Server Error' });
243+
tick();
244+
}));
245+
});
246+
186247
describe('getBuildHistory', () => {
187248
it('should get project builds and return an observable array of BuildDto', fakeAsync(() => {
188249
// SUT

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,33 @@ export class DraftGenerationService {
131131
);
132132
}
133133

134+
/**
135+
* Gets the last pre-translation build regardless of state (Completed, Running, Queued, Faulted, or Canceled).
136+
* This is a simpler accessor than getLastCompletedBuild() and can be used when the consumer
137+
* wants the most recent build even if it has not yet completed.
138+
* @param projectId The SF project id for the target translation.
139+
* @returns An observable BuildDto for the last pre-translation build, or undefined if no build has ever run.
140+
*/
141+
getLastPreTranslationBuild(projectId: string): Observable<BuildDto | undefined> {
142+
if (!this.onlineStatusService.isOnline) {
143+
return of(undefined);
144+
}
145+
return this.httpClient
146+
.get<BuildDto>(`translation/engines/project:${projectId}/actions/getLastPreTranslationBuild`)
147+
.pipe(
148+
map(res => res.data),
149+
catchError(err => {
150+
// If project doesn't exist on Serval, return undefined
151+
if (err.status === 403 || err.status === 404) {
152+
return of(undefined);
153+
}
154+
155+
this.noticeService.showError(this.i18n.translateStatic('draft_generation.temporarily_unavailable'));
156+
return of(undefined);
157+
})
158+
);
159+
}
160+
134161
/** Gets the build exactly as Serval returns it */
135162
getRawBuild(buildId: string): Observable<Object | undefined> {
136163
if (!this.onlineStatusService.isOnline) {

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,9 @@
4343
</mat-form-field>
4444
}
4545
<div class="apply-draft-button-container">
46-
@if (showDraftOptionsButton$ | async) {
47-
<span
48-
[matTooltip]="t(doesLatestHaveDraft ? 'format_draft_can' : 'format_draft_cannot')"
49-
[style.cursor]="doesLatestHaveDraft ? 'pointer' : 'not-allowed'"
50-
>
51-
<button mat-button (click)="navigateToFormatting()" [disabled]="!doesLatestHaveDraft">
46+
@if ((isFormattingSupportedForLatest$ | async) && canConfigureFormatting) {
47+
<span [matTooltip]="t('format_draft_tooltip')">
48+
<button mat-button (click)="navigateToFormatting()">
5249
<mat-icon>build</mat-icon>
5350
<transloco key="editor_draft_tab.format_draft"></transloco>
5451
</button>

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ describe('EditorDraftComponent', () => {
8686
buildProgress$.next({ state: BuildStates.Completed } as BuildDto);
8787
when(mockActivatedProjectService.projectId$).thenReturn(of('targetProjectId'));
8888
when(mockDraftGenerationService.getLastCompletedBuild(anything())).thenReturn(of(undefined));
89+
const defaultProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc;
90+
when(mockActivatedProjectService.projectDoc$).thenReturn(of(defaultProjectDoc));
91+
when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(
92+
of({ state: BuildStates.Completed } as BuildDto)
93+
);
8994

9095
fixture = TestBed.createComponent(EditorDraftComponent);
9196
component = fixture.componentInstance;
@@ -559,6 +564,129 @@ describe('EditorDraftComponent', () => {
559564
}));
560565
});
561566

567+
describe('canConfigureFormatting', () => {
568+
beforeEach(() => {
569+
when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true));
570+
when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn(
571+
of(draftHistory)
572+
);
573+
spyOn<any>(component, 'getTargetOps').and.returnValue(of(targetDelta.ops));
574+
when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftDelta.ops!));
575+
when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!);
576+
});
577+
578+
it('should be true when latest build has draft and selected revision is latest', fakeAsync(() => {
579+
const testProjectDoc: SFProjectProfileDoc = {
580+
data: createTestProjectProfile({
581+
texts: [
582+
{
583+
bookNum: 1,
584+
chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }]
585+
}
586+
]
587+
})
588+
} as SFProjectProfileDoc;
589+
when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true));
590+
when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc));
591+
when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc));
592+
593+
when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(
594+
of({ state: BuildStates.Completed } as BuildDto)
595+
);
596+
fixture.detectChanges();
597+
tick(EDITOR_READY_TIMEOUT);
598+
599+
expect(component.isSelectedDraftLatest).toBe(true);
600+
expect(component.canConfigureFormatting).toBe(true);
601+
flush();
602+
}));
603+
604+
it('should be false when selected revision is not the latest', fakeAsync(() => {
605+
const testProjectDoc: SFProjectProfileDoc = {
606+
data: createTestProjectProfile({
607+
texts: [
608+
{
609+
bookNum: 1,
610+
chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }]
611+
}
612+
]
613+
})
614+
} as SFProjectProfileDoc;
615+
when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true));
616+
when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc));
617+
when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc));
618+
619+
when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(
620+
of({ state: BuildStates.Completed } as BuildDto)
621+
);
622+
fixture.detectChanges();
623+
tick(EDITOR_READY_TIMEOUT);
624+
expect(component.canConfigureFormatting).toBe(true);
625+
626+
// Select earlier revision
627+
component.onSelectionChanged({ value: draftHistory[1] } as MatSelectChange);
628+
fixture.detectChanges();
629+
tick(EDITOR_READY_TIMEOUT);
630+
631+
expect(component.isSelectedDraftLatest).toBe(false);
632+
expect(component.canConfigureFormatting).toBe(false);
633+
flush();
634+
}));
635+
636+
it('should be false when latest build does not have a draft', fakeAsync(() => {
637+
const testProjectDoc: SFProjectProfileDoc = {
638+
data: createTestProjectProfile({
639+
texts: [
640+
{
641+
bookNum: 1,
642+
chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: false }]
643+
}
644+
]
645+
})
646+
} as SFProjectProfileDoc;
647+
when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false));
648+
when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc));
649+
when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc));
650+
651+
when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(
652+
of({ state: BuildStates.Completed } as BuildDto)
653+
);
654+
fixture.detectChanges();
655+
tick(EDITOR_READY_TIMEOUT);
656+
tick();
657+
expect(component.isSelectedDraftLatest).toBe(true);
658+
expect(component.canConfigureFormatting).toBe(false);
659+
flush();
660+
}));
661+
662+
it('should be false when latest build is canceled even if draft exists and selected revision is latest', fakeAsync(() => {
663+
const testProjectDoc: SFProjectProfileDoc = {
664+
data: createTestProjectProfile({
665+
texts: [
666+
{
667+
bookNum: 1,
668+
chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }]
669+
}
670+
]
671+
})
672+
} as SFProjectProfileDoc;
673+
when(mockDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(
674+
of({ state: BuildStates.Canceled } as BuildDto)
675+
);
676+
when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true));
677+
when(mockActivatedProjectService.projectDoc$).thenReturn(of(testProjectDoc));
678+
when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc));
679+
680+
fixture.detectChanges();
681+
tick(EDITOR_READY_TIMEOUT);
682+
683+
expect(component.isSelectedDraftLatest).toBe(true);
684+
expect(component.doesLatestCompletedHaveDraft).toBe(true);
685+
expect(component.canConfigureFormatting).toBe(false);
686+
flush();
687+
}));
688+
});
689+
562690
describe('getLocalizedBookChapter', () => {
563691
it('should return an empty string if bookNum or chapter is undefined', () => {
564692
component.bookNum = undefined;

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
startWith,
3131
Subject,
3232
switchMap,
33+
take,
3334
tap,
3435
throttleTime
3536
} from 'rxjs';
@@ -47,6 +48,7 @@ import { isString } from '../../../../type-utils';
4748
import { TextDocId } from '../../../core/models/text-doc';
4849
import { Revision } from '../../../core/paratext.service';
4950
import { SFProjectService } from '../../../core/sf-project.service';
51+
import { BuildDto } from '../../../machine-api/build-dto';
5052
import { BuildStates } from '../../../machine-api/build-states';
5153
import { NoticeComponent } from '../../../shared/notice/notice.component';
5254
import { TextComponent } from '../../../shared/text/text.component';
@@ -113,14 +115,15 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges {
113115
this.onlineStatusService.onlineStatus$
114116
]).pipe(map(([isLoading, isOnline]) => isLoading || !isOnline));
115117

116-
showDraftOptionsButton$: Observable<boolean> = this.activatedProjectService.projectId$.pipe(
118+
isFormattingSupportedForLatest$: Observable<boolean> = this.activatedProjectService.projectId$.pipe(
117119
filterNullish(),
118120
switchMap(projectId => this.draftGenerationService.getLastCompletedBuild(projectId)),
119121
map(build => this.draftOptionsService.areFormattingOptionsSupportedForBuild(build))
120122
);
121123

122124
private draftDelta?: Delta;
123125
private targetDelta?: Delta;
126+
private _latestPreTranslationBuild: BuildDto | undefined;
124127

125128
constructor(
126129
private readonly activatedProjectService: ActivatedProjectService,
@@ -150,13 +153,25 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges {
150153
return this.draftHandlingService.canApplyDraft(this.targetProject, this.bookNum, this.chapter, this.draftDelta.ops);
151154
}
152155

153-
get doesLatestHaveDraft(): boolean {
156+
get canConfigureFormatting(): boolean {
157+
return this.doesLatestCompletedHaveDraft && this.isSelectedDraftLatest && this.isLatestBuildCompleted;
158+
}
159+
160+
get doesLatestCompletedHaveDraft(): boolean {
154161
return (
155162
this.targetProject?.texts.find(t => t.bookNum === this.bookNum)?.chapters.find(c => c.number === this.chapter)
156163
?.hasDraft ?? false
157164
);
158165
}
159166

167+
get isLatestBuildCompleted(): boolean {
168+
return this._latestPreTranslationBuild?.state === BuildStates.Completed;
169+
}
170+
171+
get isSelectedDraftLatest(): boolean {
172+
return this.selectedRevision?.timestamp === this._draftRevisions[0].timestamp;
173+
}
174+
160175
set draftRevisions(value: Revision[]) {
161176
this._draftRevisions = value;
162177
}
@@ -239,9 +254,6 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges {
239254
// Respond to project changes
240255
return this.activatedProjectService.changes$.pipe(
241256
filterNullish(),
242-
tap(projectDoc => {
243-
this.targetProject = projectDoc.data;
244-
}),
245257
distinctUntilChanged(),
246258
map(() => initialTimestamp)
247259
);
@@ -299,6 +311,35 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges {
299311

300312
this.isDraftReady = this.draftCheckState === 'draft-present' || this.draftCheckState === 'draft-legacy';
301313
});
314+
315+
combineLatest([
316+
this.onlineStatusService.onlineStatus$,
317+
this.activatedProjectService.projectDoc$,
318+
this.draftGenerationService.pollBuildProgress(this.textDocId!.projectId),
319+
this.draftText.editorCreated as EventEmitter<any>,
320+
this.inputChanged$.pipe(startWith(undefined))
321+
])
322+
.pipe(
323+
quietTakeUntilDestroyed(this.destroyRef),
324+
filter(([_, projectDoc]) => projectDoc !== undefined),
325+
tap(([_, projectDoc]) => {
326+
this.targetProject = projectDoc!.data;
327+
}),
328+
filter(([isOnline, _]) => {
329+
return isOnline && this.doesLatestCompletedHaveDraft;
330+
}),
331+
switchMap(() => this.refreshLastPreTranslationBuild())
332+
)
333+
.subscribe((build: BuildDto | undefined) => {
334+
this._latestPreTranslationBuild = build;
335+
});
336+
}
337+
338+
private refreshLastPreTranslationBuild(): Observable<BuildDto | undefined> {
339+
if (this.projectId == null) {
340+
return of<BuildDto | undefined>(undefined);
341+
}
342+
return this.draftGenerationService.getLastPreTranslationBuild(this.projectId).pipe(take(1));
302343
}
303344

304345
navigateToFormatting(): void {

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4905,6 +4905,7 @@ class TestEnvironment {
49054905
when(mockedDraftGenerationService.pollBuildProgress(anything())).thenReturn(
49064906
of({ state: BuildStates.Completed } as BuildDto)
49074907
);
4908+
when(mockedDraftGenerationService.getLastPreTranslationBuild(anything())).thenReturn(of(undefined));
49084909
when(
49094910
mockedDraftGenerationService.getGeneratedDraftDeltaOperations(
49104911
anything(),

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,7 @@
410410
"draft_legacy_warning": "We have updated our drafting functionality. You can take advantage of this by [link:generateDraftUrl]generating a new draft[/link].",
411411
"error_applying_draft": "Failed to add the draft to the project. Try again later.",
412412
"format_draft": "Formatting options",
413-
"format_draft_can": "Customize formatting options for the draft",
414-
"format_draft_cannot": "You can only change formatting for books from the latest draft",
413+
"format_draft_tooltip": "Customize formatting options for the draft",
415414
"no_draft_notice": "{{ bookChapterName }} has no draft.",
416415
"offline_notice": "Generated drafts are not available offline.",
417416
"overwrite": "Adding the draft will overwrite the current chapter. Are you sure you want to continue?",

0 commit comments

Comments
 (0)