Skip to content

Commit 0c26af4

Browse files
josephmyersjosephmyers
committed
Force refresh when draft config changes remotely
I also added an optional parameter to the dialog service to disallow closing by clicking outside the dialog, since for this case, we want them to explicitly click the button.
1 parent 9426d38 commit 0c26af4

File tree

4 files changed

+177
-26
lines changed

4 files changed

+177
-26
lines changed

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

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge
1010
import { BehaviorSubject, of } from 'rxjs';
1111
import { anything, mock, verify, when } from 'ts-mockito';
1212
import { ActivatedProjectService } from 'xforge-common/activated-project.service';
13+
import { DialogService } from 'xforge-common/dialog.service';
1314
import { createTestFeatureFlag, FeatureFlagService } from 'xforge-common/feature-flags/feature-flag.service';
15+
import { LocationService } from 'xforge-common/location.service';
1416
import { NoticeService } from 'xforge-common/notice.service';
1517
import { OnlineStatusService } from 'xforge-common/online-status.service';
1618
import { TestRealtimeModule } from 'xforge-common/test-realtime.module';
@@ -34,6 +36,8 @@ describe('DraftGenerationStepsComponent', () => {
3436
const mockOnlineStatusService = mock(OnlineStatusService);
3537
const mockNoticeService = mock(NoticeService);
3638
const mockDraftSourceService = mock(DraftSourcesService);
39+
const mockDialogService = mock(DialogService);
40+
const mockLocationService = mock(LocationService);
3741

3842
when(mockActivatedProjectService.projectId).thenReturn('project01');
3943

@@ -47,6 +51,8 @@ describe('DraftGenerationStepsComponent', () => {
4751
{ provide: ProgressService, useMock: mockProgressService },
4852
{ provide: OnlineStatusService, useMock: mockOnlineStatusService },
4953
{ provide: NoticeService, useMock: mockNoticeService },
54+
{ provide: DialogService, useMock: mockDialogService },
55+
{ provide: LocationService, useMock: mockLocationService },
5056
provideHttpClient(withInterceptorsFromDi()),
5157
provideHttpClientTesting()
5258
]
@@ -66,6 +72,115 @@ describe('DraftGenerationStepsComponent', () => {
6672
when(mockOnlineStatusService.isOnline).thenReturn(true);
6773
}));
6874

75+
describe('ngOnInit', () => {
76+
let draftSources$: BehaviorSubject<DraftSourcesAsArrays>;
77+
const initialConfig: DraftSourcesAsArrays = {
78+
trainingSources: [
79+
{
80+
projectRef: 'sourceProject',
81+
paratextId: 'PT_SP',
82+
name: 'Source Project',
83+
shortName: 'sP',
84+
writingSystem: { tag: 'eng' },
85+
texts: [{ bookNum: 1 }]
86+
}
87+
],
88+
trainingTargets: [
89+
{
90+
projectRef: 'project01',
91+
paratextId: 'PT_TT',
92+
name: 'Target Project',
93+
shortName: 'tT',
94+
writingSystem: { tag: 'xyz' },
95+
texts: [{ bookNum: 1 }]
96+
}
97+
],
98+
draftingSources: [
99+
{
100+
projectRef: 'sourceProject',
101+
paratextId: 'PT_SP',
102+
name: 'Source Project',
103+
shortName: 'sP',
104+
writingSystem: { tag: 'eng' },
105+
texts: [{ bookNum: 1 }]
106+
}
107+
]
108+
};
109+
110+
beforeEach(() => {
111+
draftSources$ = new BehaviorSubject<DraftSourcesAsArrays>(initialConfig);
112+
when(mockDraftSourceService.getDraftProjectSources()).thenReturn(draftSources$);
113+
when(mockDialogService.message(anything(), anything(), anything())).thenResolve();
114+
when(mockNllbLanguageService.isNllbLanguageAsync(anything())).thenResolve(false);
115+
when(mockFeatureFlagService.showDeveloperTools).thenReturn(createTestFeatureFlag(false));
116+
const mockTargetProjectDoc = {
117+
id: 'project01',
118+
data: createTestProjectProfile({
119+
texts: [{ bookNum: 1 }],
120+
translateConfig: {
121+
source: { projectRef: 'sourceProject', shortName: 'sP', writingSystem: { tag: 'xyz' } }
122+
},
123+
writingSystem: { tag: 'eng' }
124+
})
125+
} as SFProjectProfileDoc;
126+
when(mockActivatedProjectService.projectDoc).thenReturn(mockTargetProjectDoc);
127+
when(mockActivatedProjectService.projectDoc$).thenReturn(of(mockTargetProjectDoc));
128+
when(mockActivatedProjectService.changes$).thenReturn(new BehaviorSubject(undefined));
129+
});
130+
131+
it('should show dialog and reload if sources change', fakeAsync(() => {
132+
when(mockDialogService.openDialogCount).thenReturn(0);
133+
fixture = TestBed.createComponent(DraftGenerationStepsComponent);
134+
component = fixture.componentInstance;
135+
fixture.detectChanges();
136+
tick();
137+
fixture.detectChanges();
138+
139+
const reload = new Error('RELOAD');
140+
expect(component['draftingSources'].length).toBe(1);
141+
verify(mockDialogService.message(anything(), anything(), anything())).never();
142+
when(mockLocationService.reload()).thenCall(() => {
143+
verify(mockDialogService.message(anything(), anything(), anything())).once();
144+
verify(mockLocationService.reload()).once();
145+
throw reload;
146+
});
147+
148+
// Simulate a remote change
149+
const newConfig: DraftSourcesAsArrays = {
150+
...initialConfig,
151+
draftingSources: [{ ...initialConfig.draftingSources[0], texts: [{ bookNum: 2 }] }]
152+
};
153+
draftSources$.next(newConfig);
154+
155+
try {
156+
tick();
157+
fail('tick() did not throw');
158+
} catch (e: any) {
159+
expect(e.rejection).toBe(reload);
160+
}
161+
}));
162+
163+
it('should not show dialog or reload if a dialog is already open', fakeAsync(() => {
164+
when(mockDialogService.openDialogCount).thenReturn(1);
165+
fixture = TestBed.createComponent(DraftGenerationStepsComponent);
166+
component = fixture.componentInstance;
167+
fixture.detectChanges();
168+
tick();
169+
fixture.detectChanges();
170+
171+
expect(component['draftingSources'].length).toBe(1);
172+
173+
// Simulate a remote change
174+
const newConfig = { ...initialConfig, draftingSources: [] };
175+
draftSources$.next(newConfig);
176+
tick();
177+
fixture.detectChanges();
178+
179+
verify(mockDialogService.message(anything(), anything(), anything())).never();
180+
verify(mockLocationService.reload()).never();
181+
}));
182+
});
183+
69184
describe('one training source', async () => {
70185
const availableBooks = [{ bookNum: 1 }, { bookNum: 2 }, { bookNum: 3 }];
71186
const allBooks = [...availableBooks, { bookNum: 6 }, { bookNum: 7 }];

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import { ProjectScriptureRange, TranslateSource } from 'realtime-server/lib/esm/
88
import { combineLatest, merge, Subscription } from 'rxjs';
99
import { filter } from 'rxjs/operators';
1010
import { ActivatedProjectService } from 'xforge-common/activated-project.service';
11+
import { DialogService } from 'xforge-common/dialog.service';
1112
import { FeatureFlagService } from 'xforge-common/feature-flags/feature-flag.service';
1213
import { I18nService } from 'xforge-common/i18n.service';
14+
import { LocationService } from 'xforge-common/location.service';
1315
import { RealtimeQuery } from 'xforge-common/models/realtime-query';
1416
import { NoticeService } from 'xforge-common/notice.service';
1517
import { OnlineStatusService } from 'xforge-common/online-status.service';
@@ -111,7 +113,9 @@ export class DraftGenerationStepsComponent implements OnInit {
111113
private readonly onlineStatusService: OnlineStatusService,
112114
private readonly noticeService: NoticeService,
113115
private readonly progressService: ProgressService,
114-
private readonly trainingFileService: TrainingDataService
116+
private readonly trainingFileService: TrainingDataService,
117+
private readonly dialogService: DialogService,
118+
private readonly locationService: LocationService
115119
) {}
116120

117121
ngOnInit(): void {
@@ -126,6 +130,17 @@ export class DraftGenerationStepsComponent implements OnInit {
126130
.subscribe(
127131
// Build book lists
128132
async ([{ trainingTargets, trainingSources, draftingSources }, projectId]) => {
133+
// Force refresh on remote changes
134+
if (this.draftingSources.length > 0 || this.trainingSources.length > 0 || this.trainingTargets.length > 0) {
135+
if (this.dialogService.openDialogCount > 0) return;
136+
await this.dialogService.message(
137+
'draft_generation_steps.remote_changes',
138+
'draft_generation_steps.remote_changes_start_over',
139+
true
140+
);
141+
this.locationService.reload();
142+
}
143+
129144
// The null values will have been filtered above
130145
const target = trainingTargets[0]!;
131146
const draftingSource = draftingSources[0]!;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,8 @@
246246
"no_training_books": "No training books selected",
247247
"overview": "Overview",
248248
"reference_books": "Reference books",
249+
"remote_changes": "The draft generation settings for this project have been modified, either by another user or in a separate window. Please review the latest configuration and try starting the draft again.",
250+
"remote_changes_start_over": "Start over",
249251
"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",
250252
"summary_header": "Summary",
251253
"summary_title": "Summary",

‎src/SIL.XForge.Scripture/ClientApp/src/xforge-common/dialog.service.ts‎

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,28 @@ export class DialogService {
2222

2323
diagnosticOverlay: OverlayRef | undefined;
2424

25-
openMatDialog<T, D = any, R = any>(component: ComponentType<T>, config?: MatDialogConfig<D>): MatDialogRef<T, R> {
25+
openMatDialog<T, D = any, R = any>(
26+
component: ComponentType<T>,
27+
config?: MatDialogConfig<D>,
28+
disableClose: boolean = false
29+
): MatDialogRef<T, R> {
2630
const defaults: MatDialogConfig = { direction: this.i18n.direction, autoFocus: false };
2731
const dialogDefaults: MatDialogConfig = hasObjectProp(component, 'defaultMatDialogConfig')
2832
? component.defaultMatDialogConfig
2933
: {};
30-
return this.matDialog.open(component, { ...defaults, ...dialogDefaults, ...(config ?? {}) });
34+
return this.matDialog.open(component, { ...defaults, ...dialogDefaults, ...(config ?? {}), disableClose });
3135
}
3236

33-
openGenericDialog<T>(options: GenericDialogOptions<T>): GenericDialogRef<T> {
37+
openGenericDialog<T>(options: GenericDialogOptions<T>, disableClose: boolean = false): GenericDialogRef<T> {
3438
const dialogRef: MatDialogRef<GenericDialogComponent<T>, T> = this.matDialog.open<
3539
GenericDialogComponent<T>,
3640
GenericDialogOptions<T>,
3741
T
3842
>(GenericDialogComponent, {
3943
direction: this.i18n.direction,
4044
autoFocus: false,
41-
data: options
45+
data: options,
46+
disableClose
4247
});
4348

4449
return {
@@ -50,25 +55,32 @@ export class DialogService {
5055
async confirm(
5156
question: I18nKey | Observable<string>,
5257
affirmative: I18nKey | Observable<string>,
53-
negative?: I18nKey | Observable<string>
58+
negative?: I18nKey | Observable<string>,
59+
disableClose: boolean = false
5460
): Promise<boolean> {
55-
return await this.confirmWithOptions({ title: question, affirmative, negative });
61+
return await this.confirmWithOptions({ title: question, affirmative, negative }, disableClose);
5662
}
5763

58-
async confirmWithOptions(options: {
59-
title: I18nKey | Observable<string>;
60-
message?: I18nKey | Observable<string>;
61-
affirmative: I18nKey | Observable<string>;
62-
negative?: I18nKey | Observable<string>;
63-
}): Promise<boolean> {
64-
const result: boolean | undefined = await this.openGenericDialog({
65-
title: this.ensureLocalized(options.title),
66-
message: options.message == null ? undefined : this.ensureLocalized(options.message),
67-
options: [
68-
{ value: false, label: this.ensureLocalized(options.negative ?? 'dialog.cancel') },
69-
{ value: true, label: this.ensureLocalized(options.affirmative), highlight: true }
70-
]
71-
}).result;
64+
async confirmWithOptions(
65+
options: {
66+
title: I18nKey | Observable<string>;
67+
message?: I18nKey | Observable<string>;
68+
affirmative: I18nKey | Observable<string>;
69+
negative?: I18nKey | Observable<string>;
70+
},
71+
disableClose: boolean = false
72+
): Promise<boolean> {
73+
const result: boolean | undefined = await this.openGenericDialog(
74+
{
75+
title: this.ensureLocalized(options.title),
76+
message: options.message == null ? undefined : this.ensureLocalized(options.message),
77+
options: [
78+
{ value: false, label: this.ensureLocalized(options.negative ?? 'dialog.cancel') },
79+
{ value: true, label: this.ensureLocalized(options.affirmative), highlight: true }
80+
]
81+
},
82+
disableClose
83+
).result;
7284
return result === true;
7385
}
7486

@@ -80,11 +92,18 @@ export class DialogService {
8092
* @param close (optional) May be an Observable<string>, or an I18nKey which will be used as a translation key. If not
8193
* provided the button will use a default label for the close button.
8294
*/
83-
async message(message: I18nKey | Observable<string>, close?: I18nKey | Observable<string>): Promise<void> {
84-
return await this.openGenericDialog({
85-
title: this.ensureLocalized(message),
86-
options: [{ value: undefined, label: this.ensureLocalized(close ?? 'dialog.close'), highlight: true }]
87-
}).result;
95+
async message(
96+
message: I18nKey | Observable<string>,
97+
close?: I18nKey | Observable<string>,
98+
disableClose: boolean = false
99+
): Promise<void> {
100+
return await this.openGenericDialog(
101+
{
102+
title: this.ensureLocalized(message),
103+
options: [{ value: undefined, label: this.ensureLocalized(close ?? 'dialog.close'), highlight: true }]
104+
},
105+
disableClose
106+
).result;
88107
}
89108

90109
get openDialogCount(): number {

0 commit comments

Comments
 (0)