Skip to content

SF-3191 Improve latest draft retrieval when webhook fails #3132

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SF-3191 Improve latest draft retrieval when webhook fails #3132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/SIL.XForge.Scripture/Models/ParatextSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using SIL.Scripture;

namespace SIL.XForge.Scripture.Models;

Expand Down Expand Up @@ -37,4 +38,5 @@ public class ParatextSettings
public string? BaseProjectParatextId { get; init; }
public string? CopyrightBanner { get; init; }
public string? CopyrightNotice { get; init; }
public ScrVers? Versification { get; init; }
}
1 change: 1 addition & 0 deletions src/SIL.XForge.Scripture/SIL.XForge.Scripture.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<PackageReference Include="ParatextData" Version="9.5.0.8" />
<PackageReference Include="Serval.Client" Version="1.9.1" />
<PackageReference Include="SharpZipLib" Version="1.4.2" />
<PackageReference Include="SIL.Machine" Version="3.6.4" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="8.0.0" />
<PackageReference Include="Swashbuckle.AspNetCore.Newtonsoft" Version="8.0.0" />
<PackageReference Include="System.Text.Encoding.CodePages" Version="8.0.0" />
Expand Down
88 changes: 87 additions & 1 deletion src/SIL.XForge.Scripture/Services/MachineApiService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Serval.Client;
using SIL.Converters.Usj;
using SIL.ObjectModel;
using SIL.Scripture;
using SIL.XForge.Configuration;
using SIL.XForge.DataAccess;
using SIL.XForge.EventMetrics;
Expand Down Expand Up @@ -380,7 +381,7 @@
ServalBuildDto? buildDto = null;

// Ensure that the user has permission
await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin);
SFProject project = await EnsureProjectPermissionAsync(curUserId, sfProjectId, isServalAdmin);

// Get the translation engine
string translationEngineId = await GetTranslationIdAsync(sfProjectId, preTranslate: true);
Expand All @@ -395,6 +396,91 @@
.MaxBy(b => b.DateFinished);
if (translationBuild is not null)
{
// Verify that each book/chapter from the translationBuild is marked HasDraft = true
// If the projects texts chapters are not all marked as having a draft, then the webhook likely failed
// and we want to retrieve the pre-translation status to update the chapters as having a draft
Dictionary<string, List<int>> scriptureRanges = [];

IList<PretranslateCorpus> pretranslateCorpus = translationBuild.Pretranslate ?? [];

// Retrieve the user secret
Attempt<UserSecret> attempt = await userSecrets.TryGetAsync(curUserId);
if (!attempt.TryResult(out UserSecret userSecret))
{
throw new DataNotFoundException("The user does not exist.");
}

ScrVers versification = paratextService
.GetParatextSettings(userSecret, project.ParatextId)
.Versification;

ScriptureRangeParser scriptureRangeParser = new ScriptureRangeParser(versification);

// Create the dictionary of scripture range bookIds and bookNums to check against the project texts
Dictionary<string, int> scriptureRangeBooks = [];

foreach (PretranslateCorpus ptc in pretranslateCorpus)
{
// We are using the TranslationBuild.Pretranslate.SourceFilters.ScriptureRange to find the
// books selected for drafting. Some projects may have used the now obsolete field
// TranslationBuild.Pretranslate.ScriptureRange and will not get checked for webhook failures.
foreach (ParallelCorpusFilter source in ptc.SourceFilters ?? [])
{
foreach (
(string book, List<int> bookChapters) in scriptureRangeParser.GetChapters(
source.ScriptureRange
)
)
{
int bookNum = Canon.BookIdToNumber(book);
scriptureRangeBooks.Add(book, bookNum);
// Ensure that if chapters is blank, it contains every chapter in the book
List<int> chapters = bookChapters;
if (chapters.Count == 0)
{
chapters = [.. Enumerable.Range(1, versification.GetLastChapter(bookNum))];
}

// Set or merge the list of chapters
if (!scriptureRanges.TryGetValue(book, out List<int> existingChapters))
{
scriptureRanges[book] = chapters;
}
else
{
// Merge new chapters into existing list, avoiding duplicates
foreach (int chapter in chapters.Where(chapter => !existingChapters.Contains(chapter)))
{
existingChapters.Add(chapter);
}
// add existing chapters to the books chapter list
scriptureRanges[book].AddRange(existingChapters);
}
}
}
}

string[] scriptureRangeIds = [.. scriptureRanges.Keys];

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
scriptureRangeIds
Loading
is useless, since its value is never read.

Copilot Autofix

AI 19 days ago

To fix the problem, we need to remove the assignment to the scriptureRangeIds variable since it is not used anywhere in the provided code snippet. This will clean up the code and eliminate the unnecessary assignment.

  • Remove the line that assigns a value to scriptureRangeIds.
  • Ensure that the removal does not affect the existing functionality of the code.
Suggested changeset 1
src/SIL.XForge.Scripture/Services/MachineApiService.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs
--- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs
+++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs
@@ -462,3 +462,3 @@
 
-                string[] scriptureRangeIds = [.. scriptureRanges.Keys];
+                // Removed unused variable assignment
 
EOF
@@ -462,3 +462,3 @@

string[] scriptureRangeIds = [.. scriptureRanges.Keys];
// Removed unused variable assignment

Copilot is powered by AI and may make mistakes. Always verify output.

// check if any chapters from the scripture range are marked as HasDraft = false or null
bool hasDraftIsFalseOrNullInScriptureRange =
scriptureRangeBooks.Count > 0
? scriptureRangeBooks.All(kvp =>
{
return project.Texts.Any(text =>
text.BookNum == kvp.Value
&& text.Chapters.Where(chapter => scriptureRanges[kvp.Key].Contains(chapter.Number))
.Any(c => !c.HasDraft ?? false)
);
})
: false;
Comment on lines +467 to +476

Check notice

Code scanning / CodeQL

Unnecessarily complex Boolean expression Note

The expression 'A ? B : false' can be simplified to 'A && B'.

Copilot Autofix

AI 19 days ago

To fix the problem, we need to simplify the Boolean expression by removing the unnecessary ternary conditional. Specifically, we will replace the expression scriptureRangeBooks.Count > 0 ? scriptureRangeBooks.All(...) : false with scriptureRangeBooks.Count > 0 && scriptureRangeBooks.All(...). This change will maintain the existing functionality while making the code more readable.

Suggested changeset 1
src/SIL.XForge.Scripture/Services/MachineApiService.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs
--- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs
+++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs
@@ -466,12 +466,11 @@
                 bool hasDraftIsFalseOrNullInScriptureRange =
-                    scriptureRangeBooks.Count > 0
-                        ? scriptureRangeBooks.All(kvp =>
-                        {
-                            return project.Texts.Any(text =>
-                                text.BookNum == kvp.Value
-                                && text.Chapters.Where(chapter => scriptureRanges[kvp.Key].Contains(chapter.Number))
-                                    .Any(c => !c.HasDraft ?? false)
-                            );
-                        })
-                        : false;
+                    scriptureRangeBooks.Count > 0 &&
+                    scriptureRangeBooks.All(kvp =>
+                    {
+                        return project.Texts.Any(text =>
+                            text.BookNum == kvp.Value
+                            && text.Chapters.Where(chapter => scriptureRanges[kvp.Key].Contains(chapter.Number))
+                                .Any(c => !c.HasDraft ?? false)
+                        );
+                    });
 
EOF
@@ -466,12 +466,11 @@
bool hasDraftIsFalseOrNullInScriptureRange =
scriptureRangeBooks.Count > 0
? scriptureRangeBooks.All(kvp =>
{
return project.Texts.Any(text =>
text.BookNum == kvp.Value
&& text.Chapters.Where(chapter => scriptureRanges[kvp.Key].Contains(chapter.Number))
.Any(c => !c.HasDraft ?? false)
);
})
: false;
scriptureRangeBooks.Count > 0 &&
scriptureRangeBooks.All(kvp =>
{
return project.Texts.Any(text =>
text.BookNum == kvp.Value
&& text.Chapters.Where(chapter => scriptureRanges[kvp.Key].Contains(chapter.Number))
.Any(c => !c.HasDraft ?? false)
);
});

Copilot is powered by AI and may make mistakes. Always verify output.

if (hasDraftIsFalseOrNullInScriptureRange)
{
// Chapters HasDraft is missing or false but should be true, retrieve the pre-translation status to update them.
await RetrievePreTranslationStatusAsync(sfProjectId, cancellationToken);
}

buildDto = CreateDto(translationBuild);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/SIL.XForge.Scripture/Services/ParatextService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,7 @@ string contextInformation
BaseProjectShortName = scrText.Settings.TranslationInfo.BaseProjectName,
CopyrightBanner = copyrightBanner,
CopyrightNotice = copyrightNotice,
Versification = scrText.Settings.Versification,
};
}

Expand Down
121 changes: 121 additions & 0 deletions src/SIL.XForge.Scripture/packages.lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,21 @@
"resolved": "1.4.2",
"contentHash": "yjj+3zgz8zgXpiiC3ZdF/iyTBbz2fFvMxZFEBPUcwZjIvXOf37Ylm+K58hqMfIBt5JgU/Z2uoUS67JmTLe973A=="
},
"SIL.Machine": {
"type": "Direct",
"requested": "[3.6.4, )",
"resolved": "3.6.4",
"contentHash": "TLdST7VKN1yu3GuvpSfnSK70hq8VQmJ0ftdOmjEtf5oHu5sSDIjc7N+KJ95ayd58I0MEGvFFlFHDz3awNNvo3Q==",
"dependencies": {
"CaseExtensions": "1.1.0",
"Newtonsoft.Json": "13.0.2",
"Nito.AsyncEx": "5.1.2",
"SIL.Scripture": "12.0.1",
"Sandwych.QuickGraph.Core": "1.0.0",
"System.Text.Encoding.CodePages": "6.0.0",
"System.Threading.Tasks.Dataflow": "6.0.0"
}
},
"Swashbuckle.AspNetCore": {
"type": "Direct",
"requested": "[8.0.0, )",
Expand Down Expand Up @@ -279,6 +294,11 @@
"Microsoft.Extensions.DiagnosticAdapter": "3.1.32"
}
},
"CaseExtensions": {
"type": "Transitive",
"resolved": "1.1.0",
"contentHash": "zEvNeodNmNai8aQ4YfhHKCTWmlMwXM8NHR+8x8WsgY5eT4eDW0yFGrO+voQ1lCzyAXJ1Djv6dUAiyraVTueapA=="
},
"Castle.Core": {
"type": "Transitive",
"resolved": "5.1.1",
Expand Down Expand Up @@ -882,6 +902,81 @@
"Newtonsoft.Json": "12.0.1"
}
},
"Nito.AsyncEx": {
"type": "Transitive",
"resolved": "5.1.2",
"contentHash": "hq+N63M/2znx2z1VzvPDHNg+HIWKdIloEZre+P7E0O+2iRf1Q4HBOgeiJU6SzFD/fWoyKyKSSSrekk4RgiXaeQ==",
"dependencies": {
"Nito.AsyncEx.Context": "5.1.2",
"Nito.AsyncEx.Coordination": "5.1.2",
"Nito.AsyncEx.Interop.WaitHandles": "5.1.2",
"Nito.AsyncEx.Oop": "5.1.2",
"Nito.AsyncEx.Tasks": "5.1.2",
"Nito.Cancellation": "1.1.2"
}
},
"Nito.AsyncEx.Context": {
"type": "Transitive",
"resolved": "5.1.2",
"contentHash": "rMwL7Nj3oNyvFu/jxUzQ/YBobEkM2RQHe+5mpCDRyq6mfD7vCj7Z3rjB6XgpM6Mqcx1CA2xGv0ascU/2Xk8IIg==",
"dependencies": {
"Nito.AsyncEx.Tasks": "5.1.2"
}
},
"Nito.AsyncEx.Coordination": {
"type": "Transitive",
"resolved": "5.1.2",
"contentHash": "QMyUfsaxov//0ZMbOHWr9hJaBFteZd66DV1ay4J5wRODDb8+K/uHC7+3VsOflo6SVw/29mu8OWZp8vMDSuzc0w==",
"dependencies": {
"Nito.AsyncEx.Tasks": "5.1.2",
"Nito.Collections.Deque": "1.1.1"
}
},
"Nito.AsyncEx.Interop.WaitHandles": {
"type": "Transitive",
"resolved": "5.1.2",
"contentHash": "qym29lFBCSIacKvFcJDW+beXzuO+6y9lWdd1KecxzzAqtNuvlYgNPwIsxwdhEINLhTT4aDuCM3JalpUZYWI51Q==",
"dependencies": {
"Nito.AsyncEx.Tasks": "5.1.2"
}
},
"Nito.AsyncEx.Oop": {
"type": "Transitive",
"resolved": "5.1.2",
"contentHash": "MxQl/NFoPgMApyjbB2fSZBrjdf9r6ODd/BTrWLyJKYX6UeNfw0Ocr0cPiTg2LRN0Ayud8Gj4dh67AdasNn709Q==",
"dependencies": {
"Nito.AsyncEx.Coordination": "5.1.2"
}
},
"Nito.AsyncEx.Tasks": {
"type": "Transitive",
"resolved": "5.1.2",
"contentHash": "jEkCfR2/M26OK/U4G7SEN063EU/F4LiVA06TtpZILMdX/quIHCg+wn31Zerl2LC+u1cyFancjTY3cNAr2/89PA==",
"dependencies": {
"Nito.Disposables": "2.2.1"
}
},
"Nito.Cancellation": {
"type": "Transitive",
"resolved": "1.1.2",
"contentHash": "Z+SZKp0KxMC6tEVbXe8ah4pBJadyqP0pObQMaZcBavhIDEIsGuxt7PL+B9AiNJD3Ni5VgnZsnii5HPJgVDE81w==",
"dependencies": {
"Nito.Disposables": "2.2.1"
}
},
"Nito.Collections.Deque": {
"type": "Transitive",
"resolved": "1.1.1",
"contentHash": "CU0/Iuv5VDynK8I8pDLwkgF0rZhbQoZahtodfL0M3x2gFkpBRApKs8RyMyNlAi1mwExE4gsmqQXk4aFVvW9a4Q=="
},
"Nito.Disposables": {
"type": "Transitive",
"resolved": "2.2.1",
"contentHash": "6sZ5uynQeAE9dPWBQGKebNmxbY4xsvcc5VplB5WkYEESUS7oy4AwnFp0FhqxTSKm/PaFrFqLrYr696CYN8cugg==",
"dependencies": {
"System.Collections.Immutable": "1.7.1"
}
},
"ParatextCorePluginInterfaces": {
"type": "Transitive",
"resolved": "2.0.100",
Expand Down Expand Up @@ -1069,6 +1164,14 @@
"resolved": "4.4.0",
"contentHash": "YhEdSQUsTx+C8m8Bw7ar5/VesXvCFMItyZF7G1AUY+OM0VPZUOeAVpJ4Wl6fydBGUYZxojTDR3I6Bj/+BPkJNA=="
},
"Sandwych.QuickGraph.Core": {
"type": "Transitive",
"resolved": "1.0.0",
"contentHash": "OJEAm5kik+51yBaDfDuSD9+bx3dI23IvcfFcSL+H6v/YfAYtHbiMkAn5aOljNlkHSig3uakMRGhCTYr3lDKrEA==",
"dependencies": {
"System.Diagnostics.Contracts": "4.3.0"
}
},
"SharpCompress": {
"type": "Transitive",
"resolved": "0.30.1",
Expand Down Expand Up @@ -1186,6 +1289,11 @@
"System.Threading.Tasks": "4.3.0"
}
},
"System.Collections.Immutable": {
"type": "Transitive",
"resolved": "1.7.1",
"contentHash": "B43Zsz5EfMwyEbnObwRxW5u85fzJma3lrDeGcSAV1qkhSRTNY5uXAByTn9h9ddNdhM+4/YoLc/CI43umjwIl9Q=="
},
"System.ComponentModel.Annotations": {
"type": "Transitive",
"resolved": "5.0.0",
Expand Down Expand Up @@ -1228,6 +1336,14 @@
"System.Diagnostics.PerformanceCounter": "6.0.2"
}
},
"System.Diagnostics.Contracts": {
"type": "Transitive",
"resolved": "4.3.0",
"contentHash": "eelRRbnm+OloiQvp9CXS0ixjNQldjjkHO4iIkR5XH2VIP8sUB/SIpa1TdUW6/+HDcQ+MlhP3pNa1u5SbzYuWGA==",
"dependencies": {
"System.Runtime": "4.3.0"
}
},
"System.Diagnostics.Debug": {
"type": "Transitive",
"resolved": "4.3.0",
Expand Down Expand Up @@ -1939,6 +2055,11 @@
"System.Runtime": "4.3.0"
}
},
"System.Threading.Tasks.Dataflow": {
"type": "Transitive",
"resolved": "6.0.0",
"contentHash": "+tyDCU3/B1lDdOOAJywHQoFwyXIUghIaP2BxG79uvhfTnO+D9qIgjVlL/JV2NTliYbMHpd6eKDmHp2VHpij7MA=="
},
"System.ValueTuple": {
"type": "Transitive",
"resolved": "4.5.0",
Expand Down
Loading
Loading