Skip to content

Commit 207f3c0

Browse files
pmachapmanpmachapman
committed
Further updates from code review feedback
1 parent 42801d6 commit 207f3c0

File tree

5 files changed

+35
-36
lines changed

5 files changed

+35
-36
lines changed

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.spec.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
22
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
33
import { fakeAsync, TestBed } from '@angular/core/testing';
4-
import { anything, mock, verify } from 'ts-mockito';
4+
import { anything, mock, verify, when } from 'ts-mockito';
55
import { CommandService } from 'xforge-common/command.service';
66
import { RealtimeService } from 'xforge-common/realtime.service';
77
import { configureTestingModule } from 'xforge-common/test-utils';
@@ -76,14 +76,23 @@ describe('SFProjectService', () => {
7676

7777
describe('onlineApplyPreTranslationToProject', () => {
7878
it('should invoke the command service', fakeAsync(async () => {
79+
const jobId: string = 'job01';
7980
const env = new TestEnvironment();
81+
when(mockedCommandService.onlineInvoke(anything(), 'applyPreTranslationToProject', anything())).thenReturn(
82+
Promise.resolve(jobId)
83+
);
8084
const projectId = 'project01';
8185
const scriptureRange = 'GEN-REV';
8286
const targetProjectId = 'project01';
8387
const timestamp = new Date();
84-
await env.service.onlineApplyPreTranslationToProject(projectId, scriptureRange, targetProjectId, timestamp);
88+
const actual = await env.service.onlineApplyPreTranslationToProject(
89+
projectId,
90+
scriptureRange,
91+
targetProjectId,
92+
timestamp
93+
);
8594
verify(mockedCommandService.onlineInvoke(anything(), 'applyPreTranslationToProject', anything())).once();
86-
expect().nothing();
95+
expect(actual).toBe(jobId);
8796
}));
8897
});
8998

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ export class SFProjectService extends ProjectService<SFProject, SFProjectDoc> {
185185
scriptureRange: string,
186186
targetProjectId: string,
187187
timestamp: Date
188-
): Promise<void> {
189-
return this.onlineInvoke<void>('applyPreTranslationToProject', {
188+
): Promise<string | undefined> {
189+
return this.onlineInvoke<string>('applyPreTranslationToProject', {
190190
projectId,
191191
scriptureRange,
192192
targetProjectId,

‎src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ DateTime timestamp
4545
try
4646
{
4747
// Run the background job
48-
backgroundJobClient.Enqueue<IMachineApiService>(r =>
48+
string jobId = backgroundJobClient.Enqueue<IMachineApiService>(r =>
4949
r.ApplyPreTranslationToProjectAsync(
5050
UserId,
5151
projectId,
@@ -55,7 +55,7 @@ DateTime timestamp
5555
CancellationToken.None
5656
)
5757
);
58-
return Ok();
58+
return Ok(jobId);
5959
}
6060
catch (Exception)
6161
{

‎src/SIL.XForge.Scripture/Models/DraftApplyResult.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ public class DraftApplyResult
1313
public bool ChangesSaved { get; set; }
1414

1515
/// <summary>
16-
/// A list of any chapters that failed to apply in the format "GEN 1".
16+
/// A list of any books and chapters that failed to apply in the format "GEN" and "GEN 1".
1717
/// </summary>
18-
public List<string> Failures = [];
18+
public HashSet<string> Failures = [];
1919

2020
/// <summary>
2121
/// A log containing any warnings or errors that occurred while applying the draft.

‎src/SIL.XForge.Scripture/Services/MachineApiService.cs‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ CancellationToken cancellationToken
151151
{
152152
await hubContext.NotifyDraftApplyProgress(
153153
sfProjectId,
154-
new DraftApplyState { State = $"Retrieving draft for {Canon.BookIdToEnglishName(book)}." }
154+
new DraftApplyState { State = $"Retrieving draft for {book}." }
155155
);
156156
int bookNum = Canon.BookIdToNumber(book);
157157

@@ -212,11 +212,12 @@ await hubContext.NotifyDraftApplyProgress(
212212
// If the usfm is invalid, skip this book
213213
if (string.IsNullOrWhiteSpace(usfm))
214214
{
215+
result.Failures.Add(book);
215216
await hubContext.NotifyDraftApplyProgress(
216217
sfProjectId,
217218
new DraftApplyState
218219
{
219-
State = $"No book number for {Canon.BookNumberToEnglishName(bookNum)}.",
220+
State = $"No draft available for {Canon.BookNumberToId(bookNum)}.",
220221
}
221222
);
222223
break;
@@ -231,7 +232,7 @@ await hubContext.NotifyDraftApplyProgress(
231232
new DraftApplyState
232233
{
233234
State =
234-
$"Could not retrieve draft for {Canon.BookNumberToEnglishName(bookNum)}.",
235+
$"Could not retrieve a valid draft for {Canon.BookNumberToId(bookNum)}.",
235236
}
236237
);
237238

@@ -265,7 +266,7 @@ await hubContext.NotifyDraftApplyProgress(
265266
new DraftApplyState
266267
{
267268
State =
268-
$"Could not retrieve draft for {Canon.BookNumberToEnglishName(bookNum)} {chapterNum}.",
269+
$"Could not retrieve draft for {Canon.BookNumberToId(bookNum)} {chapterNum}.",
269270
}
270271
);
271272
continue;
@@ -284,8 +285,7 @@ await hubContext.NotifyDraftApplyProgress(
284285
sfProjectId,
285286
new DraftApplyState
286287
{
287-
State =
288-
$"Could not retrieve draft for {Canon.BookNumberToEnglishName(bookNum)} {chapterNum}.",
288+
State = $"Could not retrieve draft for {Canon.BookNumberToId(bookNum)} {chapterNum}.",
289289
}
290290
);
291291
continue;
@@ -327,7 +327,7 @@ await hubContext.NotifyDraftApplyProgress(
327327
bool successful = false;
328328
try
329329
{
330-
// Being the transaction
330+
// Begin the transaction
331331
connection.BeginTransaction();
332332

333333
// Begin a transaction, and update the project
@@ -413,16 +413,12 @@ await projectService.UpdatePermissionsAsync(
413413
if (textIndex == -1)
414414
{
415415
string bookId = Canon.BookNumberToId(bookNum);
416-
if (result.Failures.LastOrDefault() != bookId)
416+
if (result.Failures.Add(bookId))
417417
{
418-
// Only list the book failure once
419-
result.Failures.Add(bookId);
418+
// Only notify the book failure once per book
420419
await hubContext.NotifyDraftApplyProgress(
421420
sfProjectId,
422-
new DraftApplyState
423-
{
424-
State = $"Could not save draft for {Canon.BookNumberToEnglishName(bookNum)}.",
425-
}
421+
new DraftApplyState { State = $"Could not save draft for {Canon.BookNumberToId(bookNum)}." }
426422
);
427423
}
428424

@@ -441,16 +437,12 @@ await hubContext.NotifyDraftApplyProgress(
441437
}
442438

443439
string bookId = Canon.BookNumberToId(bookNum);
444-
if (result.Failures.LastOrDefault() != bookId)
440+
if (result.Failures.Add(bookId))
445441
{
446-
// Only list the book failure once
447-
result.Failures.Add(bookId);
442+
// Only notify the book failure once per book
448443
await hubContext.NotifyDraftApplyProgress(
449444
sfProjectId,
450-
new DraftApplyState
451-
{
452-
State = $"Could not save draft for {Canon.BookNumberToEnglishName(bookNum)}.",
453-
}
445+
new DraftApplyState { State = $"Could not save draft for {Canon.BookNumberToId(bookNum)}." }
454446
);
455447
}
456448

@@ -468,8 +460,7 @@ await hubContext.NotifyDraftApplyProgress(
468460
sfProjectId,
469461
new DraftApplyState
470462
{
471-
State =
472-
$"Could not save draft for {Canon.BookNumberToEnglishName(bookNum)} {chapterDelta.Number}.",
463+
State = $"Could not save draft for {Canon.BookNumberToId(bookNum)} {chapterDelta.Number}.",
473464
}
474465
);
475466
continue;
@@ -499,8 +490,7 @@ await hubContext.NotifyDraftApplyProgress(
499490
sfProjectId,
500491
new DraftApplyState
501492
{
502-
State =
503-
$"Could not save draft for {Canon.BookNumberToEnglishName(bookNum)} {chapterDelta.Number}.",
493+
State = $"Could not save draft for {Canon.BookNumberToId(bookNum)} {chapterDelta.Number}.",
504494
}
505495
);
506496
continue;
@@ -523,7 +513,7 @@ await hubContext.NotifyDraftApplyProgress(
523513
sfProjectId,
524514
new DraftApplyState
525515
{
526-
State = $"Updating {Canon.BookNumberToEnglishName(bookNum)} {chapterDelta.Number}.",
516+
State = $"Updating {Canon.BookNumberToId(bookNum)} {chapterDelta.Number}.",
527517
}
528518
);
529519
}
@@ -535,7 +525,7 @@ await hubContext.NotifyDraftApplyProgress(
535525
sfProjectId,
536526
new DraftApplyState
537527
{
538-
State = $"Creating {Canon.BookNumberToEnglishName(bookNum)} {chapterDelta.Number}.",
528+
State = $"Creating {Canon.BookNumberToId(bookNum)} {chapterDelta.Number}.",
539529
}
540530
);
541531
}

0 commit comments

Comments
 (0)