-
-
Notifications
You must be signed in to change notification settings - Fork 5
SF-3655 Do not update the draft in Mongo when another draft has started #3595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1063,24 +1063,37 @@ | |
|
|
||
| try | ||
| { | ||
| // Get the last completed build | ||
| TranslationBuild? translationBuild = ( | ||
| await translationEnginesClient.GetAllBuildsAsync(translationEngineId, cancellationToken) | ||
| ) | ||
| // Get the last build and the last completed build | ||
| IList<TranslationBuild> translationBuilds = await translationEnginesClient.GetAllBuildsAsync( | ||
| translationEngineId, | ||
| cancellationToken | ||
| ); | ||
| TranslationBuild? lastTranslationBuild = translationBuilds.LastOrDefault(); | ||
| TranslationBuild? lastCompletedTranslationBuild = translationBuilds | ||
| .Where(b => b.State == JobState.Completed) | ||
| .MaxBy(b => b.DateFinished); | ||
| if (translationBuild is not null) | ||
|
|
||
| // See if the very last build has completed | ||
| if (lastTranslationBuild?.State == JobState.Completed) | ||
| { | ||
| // See if the current scripture range has changed | ||
| string currentScriptureRange = GetDraftedScriptureRange(translationBuild); | ||
| if (project.TranslateConfig.DraftConfig.CurrentScriptureRange != currentScriptureRange) | ||
| string currentScriptureRange = GetDraftedScriptureRange(lastTranslationBuild); | ||
| if ( | ||
| !string.IsNullOrWhiteSpace(currentScriptureRange) | ||
| && project.TranslateConfig.DraftConfig.CurrentScriptureRange != currentScriptureRange | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📌 Actually, how would this be possible? If
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes. |
||
| ) | ||
| { | ||
| backgroundJobClient.Enqueue<IMachineApiService>(r => | ||
| r.RetrievePreTranslationStatusAsync(sfProjectId, CancellationToken.None) | ||
| ); | ||
| } | ||
|
|
||
| buildDto = CreateDto(translationBuild); | ||
| buildDto = CreateDto(lastTranslationBuild); | ||
| } | ||
| else if (lastCompletedTranslationBuild is not null) | ||
| { | ||
| // The very last build is active, so return the last completed build | ||
| buildDto = CreateDto(lastCompletedTranslationBuild); | ||
| } | ||
| } | ||
| catch (ServalApiException e) | ||
|
|
@@ -2289,7 +2302,7 @@ | |
| parallelCorpusId is not null | ||
| && translationBuild.Analysis?.FirstOrDefault(a => | ||
| a.ParallelCorpusRef == parallelCorpusId | ||
| && !string.IsNullOrEmpty(a.SourceQuoteConvention) | ||
|
Check warning on line 2305 in src/SIL.XForge.Scripture/Services/MachineApiService.cs
|
||
| && !string.IsNullOrEmpty(a.TargetQuoteConvention) | ||
| ) | ||
| is not null | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -987,7 +987,7 @@ | |
| new ParallelCorpusAnalysis | ||
| { | ||
| ParallelCorpusRef = parallelCorpusId1, | ||
| SourceQuoteConvention = "standard_english", | ||
|
Check warning on line 990 in test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs
|
||
| TargetQuoteConvention = "standard_english", | ||
| }, | ||
| ], | ||
|
|
@@ -1272,7 +1272,7 @@ | |
| new ParallelCorpusAnalysis | ||
| { | ||
| ParallelCorpusRef = ParallelCorpusId01, | ||
| SourceQuoteConvention = "standard_english", | ||
|
Check warning on line 1275 in test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs
|
||
| TargetQuoteConvention = "standard_english", | ||
| }, | ||
| ]; | ||
|
|
@@ -1938,7 +1938,7 @@ | |
| } | ||
|
|
||
| [Test] | ||
| public async Task GetLastCompletedPreTranslationBuildAsync_NoRetrievePreTranslationStatusAsyncCall_Success() | ||
| public async Task GetLastCompletedPreTranslationBuildAsync_NoRetrievePreTranslationStatusAsyncCallWhenActiveBuild_Success() | ||
| { | ||
| // Set up test environment | ||
| var env = new TestEnvironment(); | ||
|
|
@@ -1974,6 +1974,73 @@ | |
| }, | ||
| ], | ||
| }, | ||
| new TranslationBuild | ||
| { | ||
| Url = "https://example.com", | ||
| Id = Build02, | ||
| Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, | ||
| State = JobState.Active, | ||
| }, | ||
| ] | ||
| ) | ||
| ); | ||
|
|
||
| // SUT | ||
| ServalBuildDto? actual = await env.Service.GetLastCompletedPreTranslationBuildAsync( | ||
| User01, | ||
| Project01, | ||
| false, | ||
| CancellationToken.None | ||
| ); | ||
|
|
||
| // RetrievePreTranslationStatusAsync is run via a background job, so we verify that no new job was scheduled | ||
| env.BackgroundJobClient.DidNotReceive().Create(Arg.Any<Job>(), Arg.Any<IState>()); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📌 Oh, yes. Good fixing this check.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
|
||
| TestEnvironment.AssertCoreBuildProperties(CompletedTranslationBuild, actual); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetLastCompletedPreTranslationBuildAsync_NoRetrievePreTranslationStatusAsyncCallWhenAlreadyRun_Success() | ||
| { | ||
| // Set up test environment | ||
| var env = new TestEnvironment(); | ||
| const string scriptureRange = "GEN"; | ||
| await env.Projects.UpdateAsync( | ||
| Project01, | ||
| u => u.Set(p => p.TranslateConfig.DraftConfig.CurrentScriptureRange, scriptureRange) | ||
| ); | ||
| const double percentCompleted = 0; | ||
| const int revision = 43; | ||
| const JobState state = JobState.Completed; | ||
| env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None) | ||
| .Returns( | ||
| Task.FromResult<IList<TranslationBuild>>( | ||
| [ | ||
| new TranslationBuild | ||
| { | ||
| Url = "https://example.com", | ||
| Id = Build01, | ||
| Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, | ||
| Message = MachineApiService.BuildStateCompleted, | ||
| Progress = percentCompleted, | ||
| Revision = revision, | ||
| State = state, | ||
| DateFinished = DateTimeOffset.UtcNow, | ||
| Pretranslate = | ||
| [ | ||
| new PretranslateCorpus | ||
| { | ||
| SourceFilters = | ||
| [ | ||
| new ParallelCorpusFilter | ||
| { | ||
| Corpus = new ResourceLink { Id = "corpusId", Url = "https://example.com" }, | ||
| ScriptureRange = scriptureRange, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| ] | ||
| ) | ||
| ); | ||
|
|
@@ -1986,7 +2053,8 @@ | |
| CancellationToken.None | ||
| ); | ||
|
|
||
| await env.Service.DidNotReceive().RetrievePreTranslationStatusAsync(Project01, CancellationToken.None); | ||
| // RetrievePreTranslationStatusAsync is run via a background job, so we verify that no new job was scheduled | ||
| env.BackgroundJobClient.DidNotReceive().Create(Arg.Any<Job>(), Arg.Any<IState>()); | ||
|
|
||
| TestEnvironment.AssertCoreBuildProperties(CompletedTranslationBuild, actual); | ||
| } | ||
|
|
@@ -2040,7 +2108,8 @@ | |
| CancellationToken.None | ||
| ); | ||
|
|
||
| await env.Service.DidNotReceive().RetrievePreTranslationStatusAsync(Project01, CancellationToken.None); | ||
| // RetrievePreTranslationStatusAsync is run via a background job, so we verify that no new job was scheduled | ||
| env.BackgroundJobClient.DidNotReceive().Create(Arg.Any<Job>(), Arg.Any<IState>()); | ||
|
|
||
| TestEnvironment.AssertCoreBuildProperties(CompletedTranslationBuild, actual); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📌 That's interesting. What are you thinking with regard to
currentScriptureRangepossibly being null or whitespace?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These will be older builds that did not use the scripture range format.