Skip to content

Commit 0be0ab7

Browse files
josephmyersjosephmyers
committed
Mark training file as deleted, instead of deleting
This fixes a bug where deleted training files couldn't be found in the project, since their file name would be missing and the app wouldn't register that there was a training file associated with a build entry.
1 parent 47ee299 commit 0be0ab7

File tree

16 files changed

+223
-53
lines changed

16 files changed

+223
-53
lines changed

‎src/RealtimeServer/scriptureforge/models/training-data.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ export interface TrainingData extends ProjectData {
1313
mimeType: string;
1414
skipRows: number;
1515
title: string;
16+
deleted: boolean;
1617
}

‎src/RealtimeServer/scriptureforge/services/training-data-service.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ export class TrainingDataService extends SFProjectDataService<TrainingData> {
3636
},
3737
title: {
3838
bsonType: 'string'
39+
},
40+
deleted: {
41+
bsonType: 'bool'
3942
}
4043
},
4144
additionalProperties: false

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ export class ConfirmSourcesComponent {
5858
)
5959
.pipe(quietTakeUntilDestroyed(this.destroyRef, { logWarnings: false }))
6060
.subscribe(() => {
61-
this.trainingDataFiles = this.trainingDataQuery?.docs.map(doc => doc.data).filter(d => d != null) ?? [];
61+
this.trainingDataFiles =
62+
(this.trainingDataQuery?.docs
63+
.map(doc => doc.data)
64+
.filter(d => d != null && !d.deleted) as TrainingData[]) ?? [];
6265
});
6366
}
6467
});

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { MatCheckboxHarness } from '@angular/material/checkbox/testing';
77
import { By } from '@angular/platform-browser';
88
import { provideNoopAnimations } from '@angular/platform-browser/animations';
99
import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data';
10+
import { getTrainingDataId, TrainingData } from 'realtime-server/lib/esm/scriptureforge/models/training-data';
1011
import { BehaviorSubject, of } from 'rxjs';
1112
import { anything, mock, verify, when } from 'ts-mockito';
1213
import { ActivatedProjectService } from 'xforge-common/activated-project.service';
@@ -16,11 +17,13 @@ import { UserDoc } from 'xforge-common/models/user-doc';
1617
import { NoticeService } from 'xforge-common/notice.service';
1718
import { OnlineStatusService } from 'xforge-common/online-status.service';
1819
import { provideTestRealtime } from 'xforge-common/test-realtime-providers';
20+
import { TestRealtimeService } from 'xforge-common/test-realtime.service';
1921
import { configureTestingModule, getTestTranslocoModule } from 'xforge-common/test-utils';
2022
import { UserService } from 'xforge-common/user.service';
2123
import { ParatextProject } from '../../../core/models/paratext-project';
2224
import { SFProjectProfileDoc } from '../../../core/models/sf-project-profile-doc';
2325
import { SF_TYPE_REGISTRY } from '../../../core/models/sf-type-registry';
26+
import { TrainingDataDoc } from '../../../core/models/training-data-doc';
2427
import { ParatextService } from '../../../core/paratext.service';
2528
import { SFProjectService } from '../../../core/sf-project.service';
2629
import { ProgressService, TextProgress } from '../../../shared/progress-service/progress.service';
@@ -1262,6 +1265,60 @@ describe('DraftGenerationStepsComponent', () => {
12621265
sendEmailOnBuildFinished: false
12631266
});
12641267
});
1268+
1269+
it('sets trainingDataFiles from realtime query results', fakeAsync(() => {
1270+
const realtimeService = TestBed.inject(TestRealtimeService);
1271+
realtimeService.addSnapshots<TrainingData>(TrainingDataDoc.COLLECTION, [
1272+
{
1273+
id: getTrainingDataId('project01', 'keep1'),
1274+
data: {
1275+
projectRef: 'project01',
1276+
dataId: 'keep1',
1277+
fileUrl: 'project01/keep1.csv',
1278+
mimeType: 'text/csv',
1279+
skipRows: 0,
1280+
title: 'keep1.csv',
1281+
ownerRef: 'user01',
1282+
deleted: false
1283+
}
1284+
},
1285+
{
1286+
id: getTrainingDataId('project01', 'skip-deleted'),
1287+
data: {
1288+
projectRef: 'project01',
1289+
dataId: 'skip-deleted',
1290+
fileUrl: 'project01/deleted.csv',
1291+
mimeType: 'text/csv',
1292+
skipRows: 0,
1293+
title: 'deleted.csv',
1294+
ownerRef: 'user01',
1295+
deleted: true
1296+
}
1297+
},
1298+
{
1299+
id: getTrainingDataId('otherProject', 'other'),
1300+
data: {
1301+
projectRef: 'otherProject',
1302+
dataId: 'other',
1303+
fileUrl: 'other/other.csv',
1304+
mimeType: 'text/csv',
1305+
skipRows: 0,
1306+
title: 'other.csv',
1307+
ownerRef: 'user02',
1308+
deleted: false
1309+
}
1310+
}
1311+
]);
1312+
1313+
fixture = TestBed.createComponent(DraftGenerationStepsComponent);
1314+
component = fixture.componentInstance;
1315+
fixture.detectChanges();
1316+
tick();
1317+
fixture.detectChanges();
1318+
1319+
const trainingDataFiles: Readonly<TrainingData>[] = (component as any).trainingDataFiles;
1320+
expect(trainingDataFiles.map(td => td.dataId)).toEqual(['keep1']);
1321+
}));
12651322
});
12661323

12671324
describe('confirm step', () => {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,10 @@ export class DraftGenerationStepsComponent implements OnInit {
254254
)
255255
.pipe(quietTakeUntilDestroyed(this.destroyRef, { logWarnings: false }))
256256
.subscribe(() => {
257-
this.trainingDataFiles = this.trainingDataQuery?.docs.map(doc => doc.data).filter(d => d != null) ?? [];
257+
this.trainingDataFiles =
258+
(this.trainingDataQuery?.docs
259+
.map(doc => doc.data)
260+
.filter(d => d != null && !d.deleted) as Readonly<TrainingData>[]) ?? [];
258261
});
259262

260263
// Reset the field that toggles a notice that books were automatically selected

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,11 @@ describe('DraftSourcesComponent', () => {
329329
env.clickLanguageCodesConfirmationCheckbox();
330330

331331
const savedFile = {} as TrainingData;
332-
when(mockTrainingDataQuery.docs).thenReturn([{ data: savedFile } as TrainingDataDoc]);
332+
const deletedFile: TrainingData = { dataId: 'deleted', deleted: true } as TrainingData;
333+
when(mockTrainingDataQuery.docs).thenReturn([
334+
{ data: savedFile } as TrainingDataDoc,
335+
{ data: deletedFile } as TrainingDataDoc
336+
]);
333337
trainingDataQueryLocalChanges$.next();
334338

335339
expect(env.component.availableTrainingFiles.length).toEqual(1);
@@ -349,9 +353,11 @@ describe('DraftSourcesComponent', () => {
349353

350354
const savedFile1 = { dataId: 'file1' } as TrainingData;
351355
const savedFile2 = { dataId: 'file2' } as TrainingData;
356+
const deletedFile: TrainingData = { dataId: 'deleted', deleted: true } as TrainingData;
352357
when(mockTrainingDataQuery.docs).thenReturn([
353358
{ data: savedFile1 } as TrainingDataDoc,
354-
{ data: savedFile2 } as TrainingDataDoc
359+
{ data: savedFile2 } as TrainingDataDoc,
360+
{ data: deletedFile } as TrainingDataDoc
355361
]);
356362
trainingDataQueryLocalChanges$.next();
357363
tick();
@@ -375,7 +381,11 @@ describe('DraftSourcesComponent', () => {
375381
when(mockedDialogService.confirm(anything(), anything(), anything())).thenResolve(true);
376382

377383
const savedFile = { dataId: 'saved_file', ownerRef: 'user01' } as TrainingData;
378-
when(mockTrainingDataQuery.docs).thenReturn([{ data: savedFile } as TrainingDataDoc]);
384+
const deletedFile: TrainingData = { dataId: 'deleted', ownerRef: 'user01', deleted: true } as TrainingData;
385+
when(mockTrainingDataQuery.docs).thenReturn([
386+
{ data: savedFile } as TrainingDataDoc,
387+
{ data: deletedFile } as TrainingDataDoc
388+
]);
379389
trainingDataQueryLocalChanges$.next();
380390
tick();
381391

@@ -419,9 +429,11 @@ describe('DraftSourcesComponent', () => {
419429

420430
const initialFile1 = { dataId: 'file1' } as TrainingData;
421431
const initialFile2 = { dataId: 'file2' } as TrainingData;
432+
const deletedFile: TrainingData = { dataId: 'deleted', deleted: true } as TrainingData;
422433
when(mockTrainingDataQuery.docs).thenReturn([
423434
{ data: initialFile1 } as TrainingDataDoc,
424-
{ data: initialFile2 } as TrainingDataDoc
435+
{ data: initialFile2 } as TrainingDataDoc,
436+
{ data: deletedFile } as TrainingDataDoc
425437
]);
426438
trainingDataQueryLocalChanges$.next();
427439
tick();
@@ -438,10 +450,12 @@ describe('DraftSourcesComponent', () => {
438450

439451
// Another client updates the query
440452
const remoteFile = { dataId: 'remote_file' } as TrainingData;
453+
const remoteDeletedFile: TrainingData = { dataId: 'remote_deleted', deleted: true } as TrainingData;
441454
when(mockTrainingDataQuery.docs).thenReturn([
442455
{ data: initialFile1 } as TrainingDataDoc,
443456
{ data: initialFile2 } as TrainingDataDoc,
444-
{ data: remoteFile } as TrainingDataDoc
457+
{ data: remoteFile } as TrainingDataDoc,
458+
{ data: remoteDeletedFile } as TrainingDataDoc
445459
]);
446460
trainingDataQueryLocalChanges$.next();
447461
tick();

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ export class DraftSourcesComponent extends DataLoadingComponent implements OnIni
184184
this.savedTrainingFiles?.filter(saved => !this.availableTrainingFiles.includes(saved)) ?? [];
185185
const addedFiles = this.availableTrainingFiles.filter(selected => !this.savedTrainingFiles?.includes(selected));
186186

187-
this.savedTrainingFiles = this.trainingDataQuery?.docs.filter(d => d.data != null).map(d => d.data!) ?? [];
187+
this.savedTrainingFiles =
188+
this.trainingDataQuery?.docs.filter(d => d.data != null && !d.data.deleted).map(d => d.data!) ?? [];
188189
this.availableTrainingFiles = this.savedTrainingFiles
189190
.filter(d => removedFiles.findIndex(f => f.dataId === d.dataId) === -1)
190191
.concat(addedFiles);

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-multi-select.component.spec.ts‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,36 @@ describe('TrainingDataMultiSelectComponent', () => {
5959

6060
beforeEach(fakeAsync(() => {
6161
mockTrainingData = [
62-
{ dataId: 'data01', fileUrl: '', mimeType: '', skipRows: 0, title: '', projectRef: '', ownerRef: 'user01' },
63-
{ dataId: 'data02', fileUrl: '', mimeType: '', skipRows: 0, title: '', projectRef: '', ownerRef: 'user01' },
64-
{ dataId: 'data03', fileUrl: '', mimeType: '', skipRows: 0, title: '', projectRef: '', ownerRef: 'user01' }
62+
{
63+
dataId: 'data01',
64+
fileUrl: '',
65+
mimeType: '',
66+
skipRows: 0,
67+
title: '',
68+
projectRef: '',
69+
ownerRef: 'user01',
70+
deleted: false
71+
},
72+
{
73+
dataId: 'data02',
74+
fileUrl: '',
75+
mimeType: '',
76+
skipRows: 0,
77+
title: '',
78+
projectRef: '',
79+
ownerRef: 'user01',
80+
deleted: false
81+
},
82+
{
83+
dataId: 'data03',
84+
fileUrl: '',
85+
mimeType: '',
86+
skipRows: 0,
87+
title: '',
88+
projectRef: '',
89+
ownerRef: 'user01',
90+
deleted: false
91+
}
6592
];
6693
when(mockActivatedProjectService.projectId).thenReturn(mockProjectId);
6794
when(mockActivatedProjectService.projectId$).thenReturn(mockProjectId$);

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/training-data/training-data-upload-dialog.component.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ export class TrainingDataUploadDialogComponent implements AfterViewInit {
142142
fileUrl,
143143
mimeType: 'text/csv',
144144
skipRows: (this.skipFirstRow?.checked ?? false) ? 1 : 0,
145-
title: this.trainingDataFile!.fileName!
145+
title: this.trainingDataFile!.fileName!,
146+
deleted: false
146147
};
147148

148149
this.dialogRef.close(trainingData);

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

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
22
import { getTrainingDataId, TrainingData } from 'realtime-server/lib/esm/scriptureforge/models/training-data';
3-
import { anything, mock, verify } from 'ts-mockito';
4-
import { FileService } from 'xforge-common/file.service';
5-
import { FileType } from 'xforge-common/models/file-offline-data';
3+
import { anything, deepEqual, mock, verify, when } from 'ts-mockito';
4+
import { CommandService } from 'xforge-common/command.service';
65
import { RealtimeQuery } from 'xforge-common/models/realtime-query';
76
import { Snapshot } from 'xforge-common/models/snapshot';
87
import { noopDestroyRef } from 'xforge-common/realtime.service';
98
import { provideTestRealtime } from 'xforge-common/test-realtime-providers';
109
import { TestRealtimeService } from 'xforge-common/test-realtime.service';
1110
import { configureTestingModule } from 'xforge-common/test-utils';
1211
import { TypeRegistry } from 'xforge-common/type-registry';
12+
import { PROJECTS_URL } from 'xforge-common/url-constants';
1313
import { TrainingDataDoc } from '../../../core/models/training-data-doc';
1414
import { TrainingDataService } from './training-data.service';
1515

@@ -24,7 +24,8 @@ describe('TrainingDataService', () => {
2424
mimeType: 'text/csv',
2525
skipRows: 0,
2626
title: 'test.csv',
27-
ownerRef: 'user01'
27+
ownerRef: 'user01',
28+
deleted: false
2829
}
2930
},
3031
{
@@ -36,25 +37,28 @@ describe('TrainingDataService', () => {
3637
mimeType: 'text/csv',
3738
skipRows: 0,
3839
title: 'test2.csv',
39-
ownerRef: 'user01'
40+
ownerRef: 'user01',
41+
deleted: false
4042
}
4143
}
4244
];
4345
let trainingDataService: TrainingDataService;
4446
let realtimeService: TestRealtimeService;
45-
const mockedFileService = mock(FileService);
47+
const mockedCommandService = mock(CommandService);
4648

4749
configureTestingModule(() => ({
4850
providers: [
49-
provideTestRealtime(new TypeRegistry([TrainingDataDoc], [FileType.TrainingData], [])),
50-
{ provide: FileService, useMock: mockedFileService }
51+
provideTestRealtime(new TypeRegistry([TrainingDataDoc], [], [])),
52+
{ provide: CommandService, useMock: mockedCommandService }
5153
]
5254
}));
5355

5456
beforeEach(() => {
5557
realtimeService = TestBed.inject(TestRealtimeService);
5658
realtimeService.addSnapshots<TrainingData>(TrainingDataDoc.COLLECTION, trainingData);
5759
trainingDataService = TestBed.inject(TrainingDataService);
60+
61+
when(mockedCommandService.onlineInvoke<void>(anything(), anything(), anything())).thenResolve();
5862
});
5963

6064
it('should create a training data doc', fakeAsync(async () => {
@@ -65,7 +69,8 @@ describe('TrainingDataService', () => {
6569
mimeType: 'text/csv',
6670
skipRows: 0,
6771
title: 'test3.csv',
68-
ownerRef: 'user01'
72+
ownerRef: 'user01',
73+
deleted: false
6974
};
7075
await trainingDataService.createTrainingDataAsync(newTrainingData);
7176
tick();
@@ -77,40 +82,26 @@ describe('TrainingDataService', () => {
7782
expect(trainingDataDoc.data).toEqual(newTrainingData);
7883
}));
7984

80-
it('should delete a training data doc', fakeAsync(async () => {
81-
// Verify the document exists
82-
const existingTrainingDataDoc = realtimeService.get<TrainingDataDoc>(
83-
TrainingDataDoc.COLLECTION,
84-
getTrainingDataId('project01', 'data01')
85-
);
86-
expect(existingTrainingDataDoc.data?.dataId).toBe('data01');
87-
expect(existingTrainingDataDoc.data?.projectRef).toBe('project01');
88-
89-
// SUT
85+
it('should request deletion via RPC', fakeAsync(async () => {
9086
const trainingDataToDelete: TrainingData = {
9187
projectRef: 'project01',
9288
dataId: 'data01',
9389
fileUrl: '',
9490
mimeType: '',
9591
skipRows: 0,
9692
title: '',
97-
ownerRef: 'user01'
93+
ownerRef: 'user01',
94+
deleted: false
9895
};
96+
9997
await trainingDataService.deleteTrainingDataAsync(trainingDataToDelete);
10098
tick();
10199

102-
const trainingDataDoc = realtimeService.get<TrainingDataDoc>(
103-
TrainingDataDoc.COLLECTION,
104-
getTrainingDataId('project01', 'data01')
105-
);
106-
expect(trainingDataDoc.data).toBeUndefined();
107100
verify(
108-
mockedFileService.deleteFile(
109-
FileType.TrainingData,
110-
'project01',
111-
TrainingDataDoc.COLLECTION,
112-
anything(),
113-
anything()
101+
mockedCommandService.onlineInvoke<void>(
102+
PROJECTS_URL,
103+
'markTrainingDataDeleted',
104+
deepEqual({ projectId: 'project01', dataId: 'data01' })
114105
)
115106
).once();
116107
}));

0 commit comments

Comments
 (0)