From c13b842680694b7771b7920edf78d8d32308362f Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Thu, 13 Nov 2025 13:44:44 -0600 Subject: [PATCH 01/18] SF-3578 Add Import Questions from Paratext The notes are fetched from the Paratext local copy, and the relevant pieces are parsed. The back end replaces the tag numbers with their user-displayed text (for readability). The front end makes this call when the user clicks the Paratext Import option in the import dialog. It first gathers the available tags and displays them as a dropdown. Once a value is chosen, it filters the notes down to the selection, and feeds them to the same display used for the Transcelerator questions. The brunt of this is done, but there are a number of edge cases to work through. And tests. --- .../import-questions-dialog.component.html | 51 +++++ .../import-questions-dialog.component.scss | 24 +++ .../import-questions-dialog.component.ts | 192 +++++++++++++++++- .../src/app/core/paratext.service.ts | 28 +++ .../src/assets/i18n/non_checking_en.json | 8 + .../Controllers/ParatextController.cs | 42 ++++ .../Models/ParatextNote.cs | 40 ++++ .../Services/INotesService.cs | 19 ++ .../Services/IParatextService.cs | 3 + .../Services/NotesFormatter.cs | 95 +++++++++ .../Services/NotesService.cs | 96 +++++++++ .../Services/ParatextService.cs | 36 +++- .../Services/ParatextSyncRunner.cs | 21 +- .../Services/SFServiceCollectionExtensions.cs | 1 + 14 files changed, 623 insertions(+), 33 deletions(-) create mode 100644 src/SIL.XForge.Scripture/Models/ParatextNote.cs create mode 100644 src/SIL.XForge.Scripture/Services/INotesService.cs create mode 100644 src/SIL.XForge.Scripture/Services/NotesService.cs diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html index 527482691b5..f1de4e65b38 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html @@ -97,6 +97,17 @@

{{ t("learn_more") }} + + {{ t("import_from_paratext") }} + +

{{ t("import_from_paratext_description") }}

+
+ + + +
@for (i of helpInstructions | async; track i) { @@ -112,6 +123,33 @@

} + @case ("paratext_tag_selection") { +
+ @if (loadingParatextTags) { + + } @else if (paratextTagLoadError) { +
{{ t("import_from_paratext_tag_load_error") }}
+ } @else if (paratextTagOptions.length === 0) { +
{{ t("import_from_paratext_no_tags_available") }}
+ } @else { +

{{ t("import_from_paratext_select_tag_instructions") }}

+ @if (paratextTagConversionError) { +
+ {{ t("import_from_paratext_tag_conversion_error") }} +
+ } + + {{ t("import_from_paratext_select_tag") }} + + @for (tag of paratextTagOptions; track tag.id) { + {{ tag.name }} + } + + + } +
+ } + @case ("loading") {
} @@ -247,6 +285,19 @@

@if (questionSource != null && importCanceled === false) { + @if (status === "paratext_tag_selection") { + + + } @if (status === "filter" || status === "file_import_errors") { + {{ t("learn_more") }} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.scss index e8509909abc..43286d04fe5 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.scss @@ -38,7 +38,7 @@ display: flex; column-gap: 1em; - @include breakpoints.media-breakpoint-only(xs) { + @include breakpoints.media-breakpoint-down(md) { flex-direction: column; row-gap: 1em; } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts index 817294e6e12..99dba32a128 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts @@ -296,6 +296,13 @@ export class ImportQuestionsDialogComponent implements OnDestroy { .split('\n'); } + get paratextInstructions(): string[] { + const importFromParatext = this.transloco.translate('import_questions_dialog.import_from_paratext'); + return this.i18n + .translateAndInsertTags('import_questions_dialog.paratext_instructions', { importFromParatext }) + .split('\n'); + } + get showDuplicateImportNote(): boolean { return this.filteredList.some(item => item.checked && item.sfVersionOfQuestion != null); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json index 6d07ad4f9b7..a43250e248f 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json +++ b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json @@ -543,6 +543,7 @@ "transcelerator_instructions": "Translate the questions in Transcelerator.\nWithin Transcelerator, export the questions to Scripture Forge.\nSend and receive the project in Paratext.\nSync the project in Scripture Forge.\nClick {{ boldStart }}{{ importFromTranscelerator }}{{ boldEnd }} below.", "transcelerator_paratext": "{1}Transcelerator{2} is a plugin for {3}Paratext{4} to aid with creating and translating scripture checking questions.", "transcelerator_some_questions_already_imported": "Note: Some selected questions have already been imported. Questions that have been edited in Transcelerator will be updated in Scripture Forge, and exact duplicates will be skipped.", + "paratext_instructions": "In Paratext, open Project Properties, go to the Notes tab, and create a tag for Scripture Forge.\nUse Project Note from the Project menu to add notes with the tag you created.\nSend and receive the project in Paratext.\nSync the project in Scripture Forge.\nClick {{ boldStart }}{{ importFromParatext }}{{ boldEnd }} below to import the tagged notes.", "update_transcelerator": "The version of Transcelerator used in this project is not supported. Please update to at least Transcelerator version 1.5.3." }, "language_codes_confirmation": { From 3e3f61ec205d4fe08ac5bec353baceecb8c049ac Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Thu, 20 Nov 2025 12:06:51 -0600 Subject: [PATCH 05/18] Changed Cancel to Back for the tag selection state --- .../import-questions-dialog.component.html | 2 +- .../ClientApp/src/assets/i18n/non_checking_en.json | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html index f2a0341249e..0866602a5e8 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html @@ -298,7 +298,7 @@

@if (status === "paratext_tag_selection") { - @if (fromControl.invalid) { - {{ t("must_be_valid_reference") }} - } - - - {{ t("reference_to") }} - - - @if (toControl.invalid) { - {{ t("must_be_valid_reference") }} - } - - -
-
- - - - -
-
- - - - - - - - - - - -
- {{ - t("select_all") - }} - - {{ referenceForDisplay(element.question) }} - {{ t("question") }}{{ element.question.text }}
- + @case ("filter_questions") { + + } + + @case ("filter_notes") { + } @case ("no_questions") { @@ -262,7 +212,7 @@

} - @if (status === "filter") { + @if (status === "filter_questions" || status === "filter_notes") { } + +
+
+ + {{ t("reference_from") }} + + + @if (fromControl.invalid) { + {{ t("must_be_valid_reference") }} + } + + + {{ t("reference_to") }} + + + @if (toControl.invalid) { + {{ t("must_be_valid_reference") }} + } + +
+
+
+ + + + +
+
+ + + + + + + + + + + +
+ {{ + t("select_all") + }} + + {{ referenceForDisplay(element.question) }} + {{ t("question") }}{{ element.question.text }}
+
+
@if (questionSource != null && importCanceled === false) { @if (status === "paratext_tag_selection") { - } - @if (status === "filter" || status === "file_import_errors") { - + } + @if (status === "filter_notes") { + } @if (status === "no_questions" || status === "update_transcelerator" || status === "missing_header_row") { @@ -324,7 +336,7 @@

{{ t("continue_import") }} } - @if (status === "filter") { + @if (status === "filter_questions" || status === "filter_notes") { @@ -311,7 +306,7 @@

mat-flat-button color="primary" (click)="confirmParatextTagSelection()" - [disabled]="loadingParatextTags" + [disabled]="errorState === 'paratext_tag_load_error'" > {{ t("import_from_paratext_next") }} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts index ed9bab3323f..ff0974b5c9a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts @@ -87,7 +87,12 @@ interface DialogListItem { sfVersionOfQuestion?: QuestionDoc; } -type DialogErrorState = 'update_transcelerator' | 'file_import_errors' | 'missing_header_row' | 'offline_conversion'; +type DialogErrorState = + | 'update_transcelerator' + | 'file_import_errors' + | 'missing_header_row' + | 'offline_conversion' + | 'paratext_tag_load_error'; type DialogStatus = | 'initial' | 'no_questions' @@ -162,9 +167,6 @@ export class ImportQuestionsDialogComponent implements OnDestroy { fileExtensions: string = '.csv,.tsv'; showParatextTagSelector = false; - loadingParatextTags = false; - paratextTagLoadError = false; - paratextTagConversionError = false; paratextTagOptions: ParatextNoteTag[] = []; selectedParatextTagId: number | null = null; private paratextNotes: ParatextNote[] = []; @@ -495,20 +497,14 @@ export class ImportQuestionsDialogComponent implements OnDestroy { } async importFromParatext(): Promise { - if (this.loadingParatextTags) { - return; - } - + this.loading = true; this.importClicked = false; this.errorState = undefined; this.questionSource = 'paratext'; this.questionList = []; this.filteredList = []; this.invalidRows = []; - this.loadingParatextTags = true; this.showParatextTagSelector = true; - this.paratextTagLoadError = false; - this.paratextTagConversionError = false; this.selectedParatextTagId = null; this.paratextNotes = []; this.paratextTagOptions = []; @@ -516,7 +512,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { try { const paratextId = await this.getParatextProjectId(); if (paratextId == null) { - this.paratextTagLoadError = true; + this.errorState = 'paratext_tag_load_error'; return; } @@ -526,22 +522,22 @@ export class ImportQuestionsDialogComponent implements OnDestroy { if (this.paratextTagOptions.length > 0) { this.selectedParatextTagId = this.paratextTagOptions[0].id; } + throw new Error(); } catch (error) { + console.error('Failed to load notes from Paratext: ', error); this.paratextNotes = []; this.paratextTagOptions = []; this.selectedParatextTagId = null; - this.paratextTagLoadError = true; + this.errorState = 'paratext_tag_load_error'; } finally { - this.loadingParatextTags = false; + this.loading = false; } } reset(): void { this.showParatextTagSelector = false; this.questionSource = null; - this.loadingParatextTags = false; - this.paratextTagLoadError = false; - this.paratextTagConversionError = false; + this.errorState = undefined; this.paratextNotes = []; this.paratextTagOptions = []; this.selectedParatextTagId = null; @@ -559,18 +555,12 @@ export class ImportQuestionsDialogComponent implements OnDestroy { this.questionList = []; this.filteredList = []; this.invalidRows = []; - this.paratextTagConversionError = false; + this.errorState = undefined; - try { - const questions = this.convertParatextCommentsToQuestions(this.paratextNotes, tagId); - await this.setUpQuestionList(questions, true); - } catch (error) { - this.paratextTagConversionError = true; - this.showParatextTagSelector = true; - return; - } finally { - this.loading = false; - } + const questions = this.convertParatextCommentsToQuestions(this.paratextNotes, tagId); + await this.setUpQuestionList(questions, true); + + this.loading = false; } private async getParatextProjectId(): Promise { @@ -580,7 +570,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { const project = projects?.find(p => p.projectId === this.data.projectId); return project?.paratextId; } catch (error) { - console.error('Failed to load Paratext project list', error); + console.error('Failed to load Paratext project list: ', error); this.paratextProjectsPromise = undefined; throw error; } From 87969888c472f6f68c51811fb91823934dce68b7 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Tue, 2 Dec 2025 14:26:11 -0600 Subject: [PATCH 09/18] Fixed C# failing tests and added new ones. Front end tests still remaining --- .../Controllers/ParatextControllerTests.cs | 23 +++ .../Services/NotesFormatterTests.cs | 57 +++++++ .../Services/NotesServiceTests.cs | 142 ++++++++++++++++++ .../Services/ParatextServiceTests.cs | 54 ++++++- 4 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs diff --git a/test/SIL.XForge.Scripture.Tests/Controllers/ParatextControllerTests.cs b/test/SIL.XForge.Scripture.Tests/Controllers/ParatextControllerTests.cs index c98e9d2d980..97b099c59c7 100644 --- a/test/SIL.XForge.Scripture.Tests/Controllers/ParatextControllerTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Controllers/ParatextControllerTests.cs @@ -237,6 +237,29 @@ public async Task GetAsync_CannotConnectException() Assert.AreEqual(StatusCodes.Status503ServiceUnavailable, ((ObjectResult)actual.Result).StatusCode); } + [Test] + public async Task GetNotesAsync_Success() + { + // Set up test environment + var env = new TestEnvironment(); + var expectedNotes = new[] + { + new ParatextNote + { + Id = "thread-01", + VerseRef = "GEN 1:1", + Comments = [new ParatextNoteComment { VerseRef = "GEN 1:1", Content = "

Content

" }], + }, + }; + env.ParatextService.GetNoteThreads(Arg.Any(), Project01).Returns(expectedNotes); + + // SUT + ActionResult> actual = await env.Controller.GetNotesAsync(Project01); + + Assert.IsInstanceOf(actual.Result); + Assert.AreSame(expectedNotes, ((OkObjectResult)actual.Result!).Value); + } + [Test] public async Task GetRevisionHistoryAsync_Forbidden() { diff --git a/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs b/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs index 7df78fe1cf1..a4c6fa09cee 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs @@ -1,6 +1,9 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; +using System.Linq; +using System.Reflection; using System.Text; using System.Xml; using System.Xml.Linq; @@ -398,6 +401,28 @@ public void FormatNotes_NoComments() Assert.AreEqual(expected, actual); } + [Test] + public void ReplaceTagIdentifiersWithCommentTags_ReplacesNumericIdentifiers() + { + // Setup comment tag data + string ptProjectId = Guid.NewGuid().ToString("N"); + MockCommentTags commentTags = MockCommentTags.GetCommentTags("user", ptProjectId); + commentTags.InitializeTagList([5]); + XElement notesElement = XElement.Parse( + "5" + ); + + // SUT + NotesFormatter.ReplaceTagIdentifiersWithCommentTags(notesElement, commentTags); + + // Verify tag gets replaced with the comment tag metadata + XElement actualTagElement = notesElement.Descendants("tagAdded").Single(); + CommentTag? expectedTag = commentTags.Get(5); + Assert.IsNotNull(expectedTag); + XElement expectedTagElement = BuildExpectedTagElement("tagAdded", expectedTag); + Assert.IsTrue(XNode.DeepEquals(expectedTagElement, actualTagElement)); + } + [Test] public void ParseNotes_CommentText() { @@ -629,6 +654,38 @@ public void ParseNotes_NoComments() Assert.AreEqual(0, actual.Count); } + private static XElement BuildExpectedTagElement(string elementName, CommentTag? commentTag) + { + XElement element = new XElement(elementName); + if (commentTag == null) + return element; + + PropertyInfo[] properties = typeof(CommentTag).GetProperties(BindingFlags.Instance | BindingFlags.Public); + foreach (PropertyInfo property in properties) + { + if (!property.CanRead) + continue; + + object? value = property.GetValue(commentTag); + if (value == null) + continue; + + string? propertyValue = value switch + { + string stringValue when !string.IsNullOrEmpty(stringValue) => stringValue, + int intValue => intValue.ToString(CultureInfo.InvariantCulture), + bool boolValue => boolValue ? bool.TrueString : bool.FalseString, + Enum enumValue => enumValue.ToString(), + _ => null, + }; + + if (propertyValue != null) + element.Add(new XElement(property.Name, propertyValue)); + } + + return element; + } + private class TestEnvironment { public readonly string Project01 = "project01"; diff --git a/test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs new file mode 100644 index 00000000000..c500f902160 --- /dev/null +++ b/test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs @@ -0,0 +1,142 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Xml; +using NSubstitute; +using NUnit.Framework; +using Paratext.Data; +using Paratext.Data.ProjectComments; +using Paratext.Data.ProjectFileAccess; +using SIL.XForge.Scripture.Models; + +namespace SIL.XForge.Scripture.Services; + +/// +/// Unit tests for covering note and tag mapping behaviour. +/// +[TestFixture] +public class NotesServiceTests +{ + [Test] + public void GetNotes_ReturnsMappedTagData() + { + var env = new TestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-01", "RUT 1:1", "Mapped tag", "5"); + var service = new NotesService(); + + IReadOnlyList notes = service.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(1, notes.Count); + ParatextNote note = notes[0]; + Assert.AreEqual("thread-01", note.Id); + Assert.AreEqual("RUT 1:1", note.VerseRef); + Assert.AreEqual(1, note.Comments.Count); + ParatextNoteComment comment = note.Comments[0]; + Assert.AreEqual("

Mapped tag

", comment.Content); + Assert.IsNotNull(comment.Tag); + Assert.AreEqual(5, comment.Tag.Id); + Assert.AreEqual("tag5", comment.Tag!.Name); + Assert.AreEqual("icon5", comment.Tag!.Icon); + } + + [Test] + public void GetNotes_SkipsThreadsWithOnlyDeletedComments() + { + var env = new TestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-04", "RUT 1:4", "Deleted", "5", deleted: true); + var service = new NotesService(); + + IReadOnlyList notes = service.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(0, notes.Count); + } + + [Test] + public void GetNotes_IncludesThreadsWithDeletedComments() + { + var env = new TestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-04", "RUT 1:4", "Deleted", "5", deleted: true); + env.AddComment("thread-04", "RUT 1:4", "Active", "5"); + var service = new NotesService(); + + IReadOnlyList notes = service.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(1, notes.Count); + } + + [Test] + public void GetNotes_FiltersThreadsWithPredicate() + { + var env = new TestEnvironment(); + env.CommentTags.InitializeTagList([2]); + env.AddComment("thread-05", "RUT 1:5", "First", "2"); + env.AddComment("thread-06", "RUT 1:6", "Second", "2"); + var service = new NotesService(); + + IReadOnlyList notes = service.GetNotes( + env.CommentManager, + env.CommentTags, + thread => string.Equals(thread.Id, "thread-06", StringComparison.Ordinal) + ); + + Assert.AreEqual(1, notes.Count); + Assert.AreEqual("thread-06", notes[0].Id); + } + + /// + /// Provides minimal Paratext setup to exercise against real comment threads. + /// + private sealed class TestEnvironment + { + private readonly string _ptProjectId; + + public TestEnvironment() + { + Username = "User 01"; + _ptProjectId = Guid.NewGuid().ToString("N"); + string projectPath = Path.Join(Path.GetTempPath(), _ptProjectId, "target"); + ParatextUser = new SFParatextUser(Username); + ProjectName projectName = new ProjectName { ProjectPath = projectPath, ShortName = "Proj" }; + ScrText = new MockScrText(ParatextUser, projectName) { CachedGuid = HexId.FromStr(_ptProjectId) }; + ScrText.Permissions.CreateFirstAdminUser(); + ProjectFileManager fileManager = Substitute.For(ScrText, null); + fileManager.IsWritable.Returns(true); + ScrText.SetFileManager(fileManager); + CommentManager = CommentManager.Get(ScrText); + CommentTags = MockCommentTags.GetCommentTags(Username, _ptProjectId); + } + + public string Username { get; } + public SFParatextUser ParatextUser { get; } + public MockScrText ScrText { get; } + public CommentManager CommentManager { get; } + public MockCommentTags CommentTags { get; } + + public void AddComment(string threadId, string verseRef, string content, string? tagValue, bool deleted = false) + { + XmlDocument doc = new XmlDocument(); + XmlElement root = doc.CreateElement("content"); + XmlElement paragraph = doc.CreateElement("p"); + paragraph.InnerText = content; + root.AppendChild(paragraph); + doc.AppendChild(root); + + var comment = new Paratext.Data.ProjectComments.Comment(ParatextUser) + { + Thread = threadId, + VerseRefStr = verseRef, + Contents = root, + DateTime = DateTimeOffset.UtcNow, + Deleted = deleted, + SelectedText = string.Empty, + StartPosition = 0, + }; + if (tagValue != null) + comment.TagsAdded = [tagValue]; + CommentManager.AddComment(comment); + } + } +} diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs index b68f6d86653..689e59eefc5 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs @@ -851,6 +851,37 @@ public void GetNotes_RetrievesNotes() Assert.True(notes.StartsWith(expected)); } + [Test] + public void GetNoteThreads_ReturnsNotesFromNotesService() + { + var env = new TestEnvironment(); + UserSecret userSecret = TestEnvironment.MakeUserSecret(env.User01, env.Username01, env.ParatextUserId01); + var associatedPtUser = new SFParatextUser(env.Username01); + string paratextId = env.SetupProject(env.Project01, associatedPtUser); + IReadOnlyList expectedNotes = new List + { + new ParatextNote { Id = "thread-01", VerseRef = "RUT 1:1" }, + }; + env.MockNotesService.GetNotes( + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any() + ) + .Returns(expectedNotes); + + IReadOnlyList actual = env.Service.GetNoteThreads(userSecret, paratextId); + + Assert.AreSame(expectedNotes, actual); + env.MockNotesService.Received(1) + .GetNotes( + env.ProjectCommentManager, + Arg.Is(tags => tags != null), + Arg.Any>(), + true + ); + } + [Test] public void PutNotes_AddEditDeleteComment_ThreadCorrectlyUpdated() { @@ -3931,6 +3962,24 @@ public void GetParatextSettings_RetrievesTagIcons() Assert.That(resultTag.CreatorResolve, Is.True); } + [Test] + public void GetCommentTags_ReturnsProjectTags() + { + var env = new TestEnvironment(); + var associatedPtUser = new SFParatextUser(env.Username01); + string paratextId = env.SetupProject(env.Project01, associatedPtUser); + UserSecret userSecret = TestEnvironment.MakeUserSecret(env.User01, env.Username01, env.ParatextUserId01); + env.SetupCommentTags(env.ProjectScrText, null); + CommentTags.ClearCacheForProject(env.ProjectScrText); + + CommentTags? tags = env.Service.GetCommentTags(userSecret, paratextId); + + Assert.That(tags, Is.Not.Null); + List projectTags = tags == null ? [] : [.. tags.GetAllTags()]; + Assert.AreEqual(projectTags.Count, env.TagCount - 1); //indexing starts at 1 + Assert.That(projectTags.Select(t => t.Name), Does.Contain("tag1")); + } + [Test] public void GetParatextSettings_NoteTagSetToNull() { @@ -6846,6 +6895,7 @@ private class TestEnvironment : IDisposable public readonly IInternetSharedRepositorySourceProvider MockInternetSharedRepositorySourceProvider; public readonly ISFRestClientFactory MockRestClientFactory; public readonly IGuidService MockGuidService; + public readonly INotesService MockNotesService; public readonly ParatextService Service; public readonly HttpClient MockRegistryHttpClient; public readonly IDeltaUsxMapper MockDeltaUsxMapper; @@ -6872,6 +6922,7 @@ public TestEnvironment() MockRegistryHttpClient = Substitute.For(); MockDeltaUsxMapper = Substitute.For(); MockAuthService = Substitute.For(); + MockNotesService = Substitute.For(); DateTime aSecondAgo = DateTime.Now - TimeSpan.FromSeconds(1); string accessToken1 = TokenHelper.CreateAccessToken( @@ -6942,7 +6993,8 @@ public TestEnvironment() MockRestClientFactory, MockHgWrapper, MockDeltaUsxMapper, - MockAuthService + MockAuthService, + MockNotesService ) { ScrTextCollection = MockScrTextCollection, From 9eea8302140cf7a85d729526ad28a5230133f5cf Mon Sep 17 00:00:00 2001 From: josephmyers <43733878+josephmyers@users.noreply.github.com> Date: Tue, 2 Dec 2025 15:26:27 -0600 Subject: [PATCH 10/18] Potential fix for code scanning alert no. 1251: Missed opportunity to use Where Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- src/SIL.XForge.Scripture/Services/NotesFormatter.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/SIL.XForge.Scripture/Services/NotesFormatter.cs b/src/SIL.XForge.Scripture/Services/NotesFormatter.cs index fc218d7b8ce..b7186624783 100644 --- a/src/SIL.XForge.Scripture/Services/NotesFormatter.cs +++ b/src/SIL.XForge.Scripture/Services/NotesFormatter.cs @@ -206,11 +206,8 @@ private static XElement CreateTagElement(XName elementName, CommentTag commentTa return element; PropertyInfo[] properties = typeof(CommentTag).GetProperties(BindingFlags.Instance | BindingFlags.Public); - foreach (PropertyInfo property in properties) + foreach (PropertyInfo property in properties.Where(property => property.CanRead)) { - if (!property.CanRead) - continue; - object value = property.GetValue(commentTag); if (value == null) continue; From 3f2d5a3888019082b001ee400398429b60f8da9c Mon Sep 17 00:00:00 2001 From: josephmyers <43733878+josephmyers@users.noreply.github.com> Date: Tue, 2 Dec 2025 15:27:41 -0600 Subject: [PATCH 11/18] Potential fix for code scanning alert no. 1252: Missed opportunity to use Where Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Services/NotesFormatterTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs b/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs index a4c6fa09cee..1ce3e8f1adc 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs @@ -661,10 +661,8 @@ private static XElement BuildExpectedTagElement(string elementName, CommentTag? return element; PropertyInfo[] properties = typeof(CommentTag).GetProperties(BindingFlags.Instance | BindingFlags.Public); - foreach (PropertyInfo property in properties) + foreach (PropertyInfo property in properties.Where(p => p.CanRead)) { - if (!property.CanRead) - continue; object? value = property.GetValue(commentTag); if (value == null) From af932cc723e8dc4f91eca92bb8549ca2d5db3694 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Thu, 4 Dec 2025 07:39:20 -0600 Subject: [PATCH 12/18] A couple renames --- src/RealtimeServer/scriptureforge/models/question.ts | 2 +- .../scriptureforge/services/question-service.ts | 2 +- .../import-questions-dialog.component.html | 2 +- .../import-questions-dialog.component.spec.ts | 4 ++-- .../import-questions-dialog.component.ts | 5 ++--- .../ClientApp/src/assets/i18n/non_checking_en.json | 2 +- src/SIL.XForge.Scripture/Models/Question.cs | 2 +- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/RealtimeServer/scriptureforge/models/question.ts b/src/RealtimeServer/scriptureforge/models/question.ts index 53e815a8c0b..50155f9688a 100644 --- a/src/RealtimeServer/scriptureforge/models/question.ts +++ b/src/RealtimeServer/scriptureforge/models/question.ts @@ -29,5 +29,5 @@ export interface Question extends ProjectData { dateArchived?: string; dateModified: string; dateCreated: string; - transceleratorQuestionId?: string; + sourceQuestionId?: string; } diff --git a/src/RealtimeServer/scriptureforge/services/question-service.ts b/src/RealtimeServer/scriptureforge/services/question-service.ts index 2e4ab0b3962..e904c88e582 100644 --- a/src/RealtimeServer/scriptureforge/services/question-service.ts +++ b/src/RealtimeServer/scriptureforge/services/question-service.ts @@ -187,7 +187,7 @@ export class QuestionService extends SFProjectDataService { dateCreated: { bsonType: 'string' }, - transceleratorQuestionId: { + sourceQuestionId: { bsonType: 'string' } }, diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html index 66910cfc537..8775c9bf5cd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html @@ -308,7 +308,7 @@

(click)="confirmParatextTagSelection()" [disabled]="errorState === 'paratext_tag_load_error'" > - {{ t("import_from_paratext_next") }} + {{ t("next") }} } @if (status === "filter_questions" || status === "file_import_errors") { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts index 6f45b1c58af..504a17256d9 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts @@ -245,7 +245,7 @@ describe('ImportQuestionsDialogComponent', () => { verseNum: 1, verse: '1-3' }); - expect(question.transceleratorQuestionId).toBe('2'); + expect(question.sourceQuestionId).toBe('2'); })); it('should properly import questions that are on a single verse', fakeAsync(() => { @@ -754,7 +754,7 @@ class TestEnvironment { this.existingQuestions.push({ data: { text: doc.text + ' [before edit]', - transceleratorQuestionId: doc.id, + sourceQuestionId: doc.id, answers: [] as Answer[], verseRef: fromVerseRef(new VerseRef(doc.book, doc.startChapter, verse)) } as Question, diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts index ff0974b5c9a..113e7f2f8b2 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts @@ -344,7 +344,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { const sfVersionOfQuestion: QuestionDoc | undefined = questionQuery.docs.find( doc => doc.data != null && - (useQuestionIds ? doc.data.transceleratorQuestionId === question.id : doc.data.text === question.text) && + (useQuestionIds ? doc.data.sourceQuestionId === question.id : doc.data.text === question.text) && toVerseRef(doc.data.verseRef).equals(question.verseRef) ); @@ -443,8 +443,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { isArchived: false, dateCreated: currentDate, dateModified: currentDate, - //todo rename - transceleratorQuestionId: listItem.question.id + sourceQuestionId: listItem.question.id }; await this.zone.runOutsideAngular(() => this.checkingQuestionsService.createQuestion(this.data.projectId, newQuestion, undefined, undefined) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json index 6d998c189b9..c2dd1762d4e 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json +++ b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json @@ -519,7 +519,6 @@ "import_from_paratext_no_tags_available": "There are no tagged notes available to import.", "import_from_paratext_tag_load_error": "We could not load Paratext notes. Please try again later.", "import_from_paratext_tag_conversion_error": "We could not parse the notes for this Paratext type. Please try again or try a different type.", - "import_from_paratext_next": "Next", "import_from_transcelerator": "Import from Transcelerator", "import_progress": "Imported {{ completed }} of {{ total }} questions...", "import_questions": "Import Questions", @@ -530,6 +529,7 @@ "missing_header_row": "The file you imported needs to have a header row with columns titled {{ boldStart }}Reference{{ boldEnd }} and {{ boldStart }}Question{{ boldEnd }}.", "must_be_valid_reference": "Must be a valid reference", "network_error_transcelerator": "Network error fetching Transcelerator questions. Retrying ({{ count }}).", + "next": "Next", "no_questions_available": "There are no questions for the books in this project.", "no_transcelerator_offline": "Importing from Transcelerator is not available offline.", "question_for_verse": "Question for verse {{ number }}", diff --git a/src/SIL.XForge.Scripture/Models/Question.cs b/src/SIL.XForge.Scripture/Models/Question.cs index 86526864ac2..c15703097c4 100644 --- a/src/SIL.XForge.Scripture/Models/Question.cs +++ b/src/SIL.XForge.Scripture/Models/Question.cs @@ -15,5 +15,5 @@ public class Question : ProjectData public DateTime? DateArchived { get; set; } public DateTime DateModified { get; set; } public DateTime DateCreated { get; set; } - public string TransceleratorQuestionId { get; set; } + public string SourceQuestionId { get; set; } } From 6c4856ddd7a6d40b303a84628542358a6132cb4d Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Thu, 4 Dec 2025 07:54:19 -0600 Subject: [PATCH 13/18] Front end tests --- .../import-questions-dialog.component.spec.ts | 241 ++++++++++++++++-- .../src/app/core/paratext.service.ts | 1 - 2 files changed, 218 insertions(+), 24 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts index 504a17256d9..e33e6f36667 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts @@ -3,7 +3,6 @@ import { ComponentFixture, fakeAsync, flush, TestBed, tick } from '@angular/core import { FormControl, ReactiveFormsModule } from '@angular/forms'; import { MatCheckbox } from '@angular/material/checkbox'; import { MatDialog, MatDialogRef } from '@angular/material/dialog'; -import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { Canon, VerseRef } from '@sillsdev/scripture'; import { ngfModule } from 'angular-file'; import { Answer } from 'realtime-server/lib/esm/scriptureforge/models/answer'; @@ -11,7 +10,7 @@ import { Question } from 'realtime-server/lib/esm/scriptureforge/models/question import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; import { fromVerseRef } from 'realtime-server/lib/esm/scriptureforge/models/verse-ref-data'; import { BehaviorSubject, of, throwError } from 'rxjs'; -import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; +import { anything, capture, instance, mock, reset, verify, when } from 'ts-mockito'; import { CsvService } from 'xforge-common/csv-service.service'; import { DialogService } from 'xforge-common/dialog.service'; import { RealtimeQuery } from 'xforge-common/models/realtime-query'; @@ -20,9 +19,11 @@ import { provideTestOnlineStatus } from 'xforge-common/test-online-status-provid import { TestOnlineStatusService } from 'xforge-common/test-online-status.service'; import { ChildViewContainerComponent, configureTestingModule, getTestTranslocoModule } from 'xforge-common/test-utils'; import { TestingRetryingRequestService } from 'xforge-common/testing-retrying-request.service'; +import { ParatextProject } from '../../core/models/paratext-project'; import { QuestionDoc } from '../../core/models/question-doc'; import { TextsByBookId } from '../../core/models/texts-by-book-id'; import { TransceleratorQuestion } from '../../core/models/transcelerator-question'; +import { ParatextNote, ParatextNoteTag, ParatextService } from '../../core/paratext.service'; import { SFProjectService } from '../../core/sf-project.service'; import { ScriptureChooserDialogComponent } from '../../scripture-chooser-dialog/scripture-chooser-dialog.component'; import { CheckingQuestionsService } from '../checking/checking-questions.service'; @@ -34,6 +35,7 @@ const mockedQuestionsService = mock(CheckingQuestionsService); const mockedDialogService = mock(DialogService); const mockedCsvService = mock(CsvService); const mockedRealtimeQuery: RealtimeQuery = mock(RealtimeQuery); +const mockedParatextService = mock(ParatextService); describe('ImportQuestionsDialogComponent', () => { configureTestingModule(() => ({ @@ -50,8 +52,8 @@ describe('ImportQuestionsDialogComponent', () => { { provide: CheckingQuestionsService, useMock: mockedQuestionsService }, { provide: DialogService, useMock: mockedDialogService }, { provide: CsvService, useMock: mockedCsvService }, - { provide: OnlineStatusService, useClass: TestOnlineStatusService }, - provideNoopAnimations() + { provide: ParatextService, useMock: mockedParatextService }, + { provide: OnlineStatusService, useClass: TestOnlineStatusService } ] })); @@ -73,7 +75,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.getRowQuestion(questions[0])).toBe('Transcelerator question 1:1'); expect(env.getRowReference(questions[1])).toBe('MAT 1:2'); expect(env.getRowQuestion(questions[1])).toBe('Transcelerator question 1:2'); - env.click(env.cancelButton); + env.click(env.backButton); })); it('can select questions in the list', fakeAsync(() => { @@ -101,7 +103,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.component.filteredList[0].checked).toBe(false); expect(env.selectAllCheckbox.checked).toBe(false); expect(env.selectAllCheckbox.indeterminate).toBe(true); - env.click(env.cancelButton); + env.click(env.backButton); })); it('select all selects and deselects all visible questions', fakeAsync(() => { @@ -131,7 +133,7 @@ describe('ImportQuestionsDialogComponent', () => { env.clickSelectAll(); expect(env.component.filteredList[0].checked).toBe(true); expect(env.component.filteredList[1].checked).toBe(true); - env.click(env.cancelButton); + env.click(env.backButton); })); it('can filter questions for text', fakeAsync(() => { @@ -140,7 +142,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(2); env.setControlValue(env.component.filterControl, '1:2'); expect(env.tableRows.length).toBe(1); - env.click(env.cancelButton); + env.click(env.backButton); })); it('clears text from filter when show all is clicked', fakeAsync(() => { @@ -156,7 +158,7 @@ describe('ImportQuestionsDialogComponent', () => { env.click(env.showAllButton); expect(env.component.fromControl.value).toBe(''); expect(env.component.toControl.value).toBe(''); - env.click(env.cancelButton); + env.click(env.backButton); })); it('can filter questions with verse reference', fakeAsync(() => { @@ -177,7 +179,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(1); env.setControlValue(env.component.toControl, 'MAL 1:1'); expect(env.tableRows.length).toBe(0); - env.click(env.cancelButton); + env.click(env.backButton); })); it('show scripture chooser dialog', fakeAsync(() => { @@ -187,7 +189,7 @@ describe('ImportQuestionsDialogComponent', () => { env.openFromScriptureChooser(); verify(mockedDialogService.openMatDialog(anything(), anything())).once(); expect(env.component.fromControl.value).toBe('MAT 1:1'); - env.click(env.cancelButton); + env.click(env.backButton); })); it('prompts for edited questions that have already been imported', fakeAsync(() => { @@ -207,7 +209,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.component.filteredList[1].checked).toBe(false); expect(env.component.filteredList[2].checked).toBe(true); expect(env.component.filteredList[3].checked).toBe(true); - env.click(env.cancelButton); + env.click(env.backButton); })); it('allows updating questions that have been edited in Transcelerator', fakeAsync(() => { @@ -330,7 +332,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(1); expect(env.getColumnTwoText(env.tableRows[0])).toEqual('Genesis 1:1'); env.click(env.continueImportButton); - env.click(env.cancelButton); + env.click(env.backButton); })); it('can import from an Excel 97-2003 file', fakeAsync(() => { @@ -344,7 +346,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(1); expect(env.getColumnTwoText(env.tableRows[0])).toEqual('Genesis 1:1'); env.click(env.continueImportButton); - env.click(env.cancelButton); + env.click(env.backButton); })); it('can import from an Excel 2007 file', fakeAsync(() => { @@ -358,7 +360,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(1); expect(env.getColumnTwoText(env.tableRows[0])).toEqual('Genesis 1:1'); env.click(env.continueImportButton); - env.click(env.cancelButton); + env.click(env.backButton); })); it('does not import from an Excel file when offline', fakeAsync(() => { @@ -388,7 +390,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.footerText).toBe( 'Note: Some of the selected questions are exact duplicates of questions that are already part of your project. They will not be re-imported.' ); - env.click(env.cancelButton); + env.click(env.backButton); })); it('it informs the user about invalid rows in the CSV file and skips them', fakeAsync(() => { @@ -413,7 +415,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(questionRows.length).toBe(1); expect(env.getRowReference(env.tableRows[0])).toEqual('MAT 1:2'); - env.click(env.cancelButton); + env.click(env.backButton); })); it('allows reference and questions columns to be anywhere and ignores irrelevant columns', fakeAsync(() => { @@ -430,7 +432,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(2); expect(env.getRowReference(env.tableRows[1])).toEqual('MAT 1:2'); - env.click(env.cancelButton); + env.click(env.backButton); })); it('allows ignores white space and capitalization in question and reference heading', fakeAsync(() => { @@ -447,7 +449,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(2); expect(env.getRowReference(env.tableRows[1])).toEqual('GEN 1:2'); - env.click(env.cancelButton); + env.click(env.backButton); })); it('informs when there are no questions', fakeAsync(() => { @@ -461,7 +463,7 @@ describe('ImportQuestionsDialogComponent', () => { const env = new TestEnvironment(); env.click(env.importFromTransceleratorButton); expect(env.bodyText).not.toContain('There are no questions for the books in this project.'); - env.click(env.cancelButton); + env.click(env.backButton); })); it('infinite scrolls questions', fakeAsync(() => { @@ -479,7 +481,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.tableRows.length).toBe(125); env.scrollDialogContentBodyToBottom(); expect(env.tableRows.length).toBe(150); - env.click(env.cancelButton); + env.click(env.backButton); })); it('does not try to load transcelerator questions when the user is online', fakeAsync(() => { @@ -490,7 +492,7 @@ describe('ImportQuestionsDialogComponent', () => { expect(env.importFromTransceleratorButton.disabled).toBe(false); expect(env.errorMessages).toEqual([]); env.click(env.importFromTransceleratorButton); - env.click(env.cancelButton); + env.click(env.backButton); })); it('has a close button on the initial view', fakeAsync(() => { @@ -499,6 +501,116 @@ describe('ImportQuestionsDialogComponent', () => { env.click(env.closeButton); expect(env.overlayContainerElement.hasChildNodes()).withContext('close button closes dialog').toBeFalse(); })); + + it('collects unique Paratext tags in alphabetical order', fakeAsync(() => { + const env = new TestEnvironment(); + const tagAlpha: ParatextNoteTag = { id: 1, name: 'Alpha' }; + const tagBeta: ParatextNoteTag = { id: 3, name: 'Beta' }; + const tagGamma: ParatextNoteTag = { id: 2, name: 'Gamma' }; + const notes: ParatextNote[] = [ + { + id: 'note-1', + verseRef: 'MAT 1:1', + comments: [ + { verseRef: 'MAT 1:1', content: '

Alpha

', tag: tagGamma }, + { verseRef: 'MAT 1:1', content: '

Alpha again

', tag: tagAlpha } + ] + }, + { + id: 'note-2', + verseRef: 'MAT 1:2', + comments: [ + { verseRef: 'MAT 1:2', content: '

Beta

', tag: tagBeta }, + { verseRef: 'MAT 1:2', content: '

Beta duplicate

', tag: tagBeta } + ] + } + ]; + + const tags = env.collectParatextTagOptions(env.component, notes); + + expect(tags.length).toBe(3); + expect(tags.map(tag => tag.name)).toEqual(['Alpha', 'Beta', 'Gamma']); + expect(tags.map(tag => tag.id)).toEqual([1, 3, 2]); + })); + + it('shows a message when no notes have tagged comments', fakeAsync(() => { + const env = new TestEnvironment(); + const notes: ParatextNote[] = [ + { + id: 'note-1', + verseRef: 'MAT 1:1', + comments: [{ verseRef: 'MAT 1:1', content: '

Note without tag

' }] + } + ]; + env.setParatextNotes(env.component, notes); + env.setParatextTagOptions(env.component, env.collectParatextTagOptions(env.component, notes)); + env.component.questionSource = 'paratext'; + env.component.showParatextTagSelector = true; + env.component.selectedParatextTagId = null; + env.component.errorState = undefined; + + env.fixture.detectChanges(); + tick(); + + expect(env.component.status).toBe('paratext_tag_selection'); + expect(env.getParatextTagMessage()).toBe('There are no tagged notes available to import.'); + })); + + it('converts Paratext notes for the selected tag into questions', fakeAsync(() => { + const tagQuestions: ParatextNoteTag = { id: 7, name: 'Questions' }; + const notes: ParatextNote[] = [ + { + id: 'note-1', + verseRef: 'MAT 1:1', + comments: [ + { verseRef: 'MAT 1:1', content: '

Ignore

', tag: { id: 6, name: 'Other' } }, + { verseRef: 'MAT 1:1', content: '

Question text

', tag: tagQuestions } + ] + }, + { + id: 'note-2', + verseRef: 'MAT 1:2', + comments: [{ verseRef: 'MAT 1:2', content: '

Question 2

', tag: tagQuestions }] + }, + { + id: 'note-3', + verseRef: 'GEN 1:1', + comments: [{ verseRef: 'GEN 1:1', content: '

Different book

', tag: tagQuestions }] + } + ]; + const preexistingQuestion = TestEnvironment.createQuestionDocWithSource('note-1', new VerseRef('MAT 1:1'), 'text'); + const env = new TestEnvironment({ existingQuestions: [preexistingQuestion], paratextNotes: notes }); + env.setParatextNotes(env.component, notes); + env.setParatextTagOptions(env.component, env.collectParatextTagOptions(env.component, notes)); + env.component.selectedParatextTagId = tagQuestions.id; + env.component.questionSource = 'paratext'; + env.component.showParatextTagSelector = true; + + void env.component.confirmParatextTagSelection(); + tick(); + env.fixture.detectChanges(); + + expect(env.component.status).toBe('filter_notes'); + expect(env.component.filteredList.length).toBe(2); + const questionAlreadyImported = env.component.filteredList[0]; + expect(questionAlreadyImported.question.id).toBe('note-1'); + expect(questionAlreadyImported.question.text).toBe('Question text'); + expect(questionAlreadyImported.sfVersionOfQuestion).not.toBeUndefined(); + + expect(env.component.showDuplicateImportNote).toBeFalse(); + questionAlreadyImported.checked = true; + expect(env.component.showDuplicateImportNote).toBeTrue(); + })); + + it('shows an error when no Paratext project can be found', fakeAsync(() => { + const env = new TestEnvironment({ paratextProjects: [], paratextNotes: [] }); + + env.click(env.importFromParatextButton); + + expect(env.component.errorState).toBe('paratext_tag_load_error'); + expect(env.component.status).toBe('paratext_tag_load_error'); + expect(env.getParatextTagOptions(env.component).length).toBe(0); + })); }); class TestEnvironment { @@ -559,6 +671,9 @@ class TestEnvironment { transceleratorQuestions?: TransceleratorQuestion[]; offline?: boolean; existingQuestions?: QuestionDoc[]; + paratextProjects?: ParatextProject[]; + paratextNotes?: ParatextNote[]; + paratextNotesError?: Error; } = {} ) { this.questions = options.transceleratorQuestions || this.questions; @@ -574,6 +689,7 @@ class TestEnvironment { if (options.existingQuestions) { this.existingQuestions = options.existingQuestions; } + this.setupParatext(options.paratextProjects, options.paratextNotes, options.paratextNotesError); this.setupTransceleratorQuestions(); const gen: TextInfo = { @@ -624,7 +740,11 @@ class TestEnvironment { } get importFromCsvFile(): HTMLButtonElement { - return this.overlayContainerElement.querySelector('mat-card:last-child button') as HTMLButtonElement; + return this.overlayContainerElement.querySelector('mat-card:nth-child(2) button') as HTMLButtonElement; + } + + get importFromParatextButton(): HTMLButtonElement { + return this.overlayContainerElement.querySelector('mat-card:nth-child(3) button') as HTMLButtonElement; } get tableRows(): HTMLElement[] { @@ -647,6 +767,15 @@ class TestEnvironment { return this.dialogContentBody.textContent || ''; } + getParatextTagMessage(): string { + const element = this.overlayContainerElement.querySelector('.paratext-tag-selection-message'); + if (element == null) { + return ''; + } + const text = element.textContent; + return text != null ? text.trim() : ''; + } + get errorMessages(): string[] { return Array.from(this.overlayContainerElement.querySelectorAll('mat-error')).map(node => (node.textContent || '').trim() @@ -665,6 +794,10 @@ class TestEnvironment { return this.getButtonByText('Show All'); } + get backButton(): HTMLButtonElement { + return this.getButtonByText('Back'); + } + get closeButton(): HTMLButtonElement { return this.getButtonByText('Close'); } @@ -673,6 +806,10 @@ class TestEnvironment { return this.getButtonByText('Continue Import'); } + get nextButton(): HTMLButtonElement { + return this.getButtonByText('Next'); + } + get headerText(): string { return this.overlayContainerElement.querySelector('.dialog-content-header')?.textContent?.trim() || ''; } @@ -766,6 +903,64 @@ class TestEnvironment { }); } + collectParatextTagOptions(component: ImportQuestionsDialogComponent, notes: ParatextNote[]): ParatextNoteTag[] { + return ( + component as unknown as { collectParatextTagOptions(notes: ParatextNote[]): ParatextNoteTag[] } + ).collectParatextTagOptions(notes); + } + + setParatextNotes(component: ImportQuestionsDialogComponent, notes: ParatextNote[]): void { + (component as unknown as { paratextNotes: ParatextNote[] }).paratextNotes = notes; + } + + setParatextTagOptions(component: ImportQuestionsDialogComponent, tags: ParatextNoteTag[]): void { + (component as unknown as { paratextTagOptions: ParatextNoteTag[] }).paratextTagOptions = tags; + } + + getParatextTagOptions(component: ImportQuestionsDialogComponent): ParatextNoteTag[] { + return (component as unknown as { paratextTagOptions: ParatextNoteTag[] }).paratextTagOptions; + } + + static createQuestionDocWithSource(sourceId: string, verse: VerseRef, text: string): QuestionDoc { + return { + data: { + sourceQuestionId: sourceId, + verseRef: fromVerseRef(verse), + text, + answers: [] as Answer[] + } as Question, + submitJson0Op: () => Promise.resolve(), + getAnswers: () => [] as Answer[] + } as unknown as QuestionDoc; + } + + private setupParatext( + projects: ParatextProject[] | undefined, + notes: ParatextNote[] | undefined, + notesError: Error | undefined + ): void { + reset(mockedParatextService); + const defaultProjects: ParatextProject[] = [ + { + paratextId: `pt-project01`, + name: 'Project 01', + shortName: 'P01', + languageTag: 'en', + projectId: 'project01', + isConnectable: false, + isConnected: true, + hasUserRoleChanged: false, + hasUpdate: false + } + ]; + when(mockedParatextService.getProjects()).thenResolve(projects ?? defaultProjects); + if (notesError !== undefined) { + when(mockedParatextService.getNotes(anything())).thenReject(notesError); + } else { + when(mockedParatextService.getNotes(anything())).thenResolve(notes ?? []); + } + } + private setupTransceleratorQuestions(): void { if (this.errorOnFetchQuestions) { when(mockedProjectService.transceleratorQuestions('project01', anything())).thenCall(() => diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/core/paratext.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/core/paratext.service.ts index 8755f25ba35..1f755a2472e 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/core/paratext.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/core/paratext.service.ts @@ -47,7 +47,6 @@ export interface ParatextNoteComment { export interface ParatextNoteTag { id: number; name: string; - icon: string; } @Injectable({ From cebdc61eff06aa1cf9426252e54bc557c6f5f6e6 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Mon, 8 Dec 2025 15:52:04 -0600 Subject: [PATCH 14/18] Revert "A couple renames" (only the big one) This reverts commit 1b6840fe0d977e8593afaba66662ce73d8db76e4. --- src/RealtimeServer/scriptureforge/models/question.ts | 2 +- .../scriptureforge/services/question-service.ts | 2 +- .../import-questions-dialog.component.spec.ts | 6 +++--- .../import-questions-dialog.component.ts | 5 ++--- src/SIL.XForge.Scripture/Models/Question.cs | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/RealtimeServer/scriptureforge/models/question.ts b/src/RealtimeServer/scriptureforge/models/question.ts index 50155f9688a..53e815a8c0b 100644 --- a/src/RealtimeServer/scriptureforge/models/question.ts +++ b/src/RealtimeServer/scriptureforge/models/question.ts @@ -29,5 +29,5 @@ export interface Question extends ProjectData { dateArchived?: string; dateModified: string; dateCreated: string; - sourceQuestionId?: string; + transceleratorQuestionId?: string; } diff --git a/src/RealtimeServer/scriptureforge/services/question-service.ts b/src/RealtimeServer/scriptureforge/services/question-service.ts index e904c88e582..2e4ab0b3962 100644 --- a/src/RealtimeServer/scriptureforge/services/question-service.ts +++ b/src/RealtimeServer/scriptureforge/services/question-service.ts @@ -187,7 +187,7 @@ export class QuestionService extends SFProjectDataService { dateCreated: { bsonType: 'string' }, - sourceQuestionId: { + transceleratorQuestionId: { bsonType: 'string' } }, diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts index e33e6f36667..d4ad67541e8 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.spec.ts @@ -247,7 +247,7 @@ describe('ImportQuestionsDialogComponent', () => { verseNum: 1, verse: '1-3' }); - expect(question.sourceQuestionId).toBe('2'); + expect(question.transceleratorQuestionId).toBe('2'); })); it('should properly import questions that are on a single verse', fakeAsync(() => { @@ -891,7 +891,7 @@ class TestEnvironment { this.existingQuestions.push({ data: { text: doc.text + ' [before edit]', - sourceQuestionId: doc.id, + transceleratorQuestionId: doc.id, answers: [] as Answer[], verseRef: fromVerseRef(new VerseRef(doc.book, doc.startChapter, verse)) } as Question, @@ -924,7 +924,7 @@ class TestEnvironment { static createQuestionDocWithSource(sourceId: string, verse: VerseRef, text: string): QuestionDoc { return { data: { - sourceQuestionId: sourceId, + transceleratorQuestionId: sourceId, verseRef: fromVerseRef(verse), text, answers: [] as Answer[] diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts index 113e7f2f8b2..af99beabc25 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts @@ -344,7 +344,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { const sfVersionOfQuestion: QuestionDoc | undefined = questionQuery.docs.find( doc => doc.data != null && - (useQuestionIds ? doc.data.sourceQuestionId === question.id : doc.data.text === question.text) && + (useQuestionIds ? doc.data.transceleratorQuestionId === question.id : doc.data.text === question.text) && toVerseRef(doc.data.verseRef).equals(question.verseRef) ); @@ -443,7 +443,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { isArchived: false, dateCreated: currentDate, dateModified: currentDate, - sourceQuestionId: listItem.question.id + transceleratorQuestionId: listItem.question.id }; await this.zone.runOutsideAngular(() => this.checkingQuestionsService.createQuestion(this.data.projectId, newQuestion, undefined, undefined) @@ -521,7 +521,6 @@ export class ImportQuestionsDialogComponent implements OnDestroy { if (this.paratextTagOptions.length > 0) { this.selectedParatextTagId = this.paratextTagOptions[0].id; } - throw new Error(); } catch (error) { console.error('Failed to load notes from Paratext: ', error); this.paratextNotes = []; diff --git a/src/SIL.XForge.Scripture/Models/Question.cs b/src/SIL.XForge.Scripture/Models/Question.cs index c15703097c4..86526864ac2 100644 --- a/src/SIL.XForge.Scripture/Models/Question.cs +++ b/src/SIL.XForge.Scripture/Models/Question.cs @@ -15,5 +15,5 @@ public class Question : ProjectData public DateTime? DateArchived { get; set; } public DateTime DateModified { get; set; } public DateTime DateCreated { get; set; } - public string SourceQuestionId { get; set; } + public string TransceleratorQuestionId { get; set; } } From 7affa4a01a86b1bb2dde8f91c2dfbec223ae8332 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Tue, 9 Dec 2025 10:02:03 -0600 Subject: [PATCH 15/18] Splitting out note id to its own property --- .../scriptureforge/models/question.ts | 1 + .../services/question-service.ts | 3 +++ .../import-questions-dialog.component.ts | 18 +++++++++++++----- src/SIL.XForge.Scripture/Models/Question.cs | 1 + 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/RealtimeServer/scriptureforge/models/question.ts b/src/RealtimeServer/scriptureforge/models/question.ts index 53e815a8c0b..52f0e696382 100644 --- a/src/RealtimeServer/scriptureforge/models/question.ts +++ b/src/RealtimeServer/scriptureforge/models/question.ts @@ -30,4 +30,5 @@ export interface Question extends ProjectData { dateModified: string; dateCreated: string; transceleratorQuestionId?: string; + paratextNoteId?: string; } diff --git a/src/RealtimeServer/scriptureforge/services/question-service.ts b/src/RealtimeServer/scriptureforge/services/question-service.ts index 2e4ab0b3962..0c30d6aba0a 100644 --- a/src/RealtimeServer/scriptureforge/services/question-service.ts +++ b/src/RealtimeServer/scriptureforge/services/question-service.ts @@ -189,6 +189,9 @@ export class QuestionService extends SFProjectDataService { }, transceleratorQuestionId: { bsonType: 'string' + }, + paratextNoteId: { + bsonType: 'string' } }, additionalProperties: false diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts index af99beabc25..a35fca4c22e 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts @@ -343,9 +343,10 @@ export class ImportQuestionsDialogComponent implements OnDestroy { // Questions imported from a file should be skipped only if they are exactly the same as what is currently in SF. const sfVersionOfQuestion: QuestionDoc | undefined = questionQuery.docs.find( doc => - doc.data != null && - (useQuestionIds ? doc.data.transceleratorQuestionId === question.id : doc.data.text === question.text) && - toVerseRef(doc.data.verseRef).equals(question.verseRef) + (doc.data != null && + (useQuestionIds ? doc.data.transceleratorQuestionId === question.id : doc.data.text === question.text) && + toVerseRef(doc.data.verseRef).equals(question.verseRef)) || + (useQuestionIds ? doc.data?.paratextNoteId === question.id : doc.data?.text === question.text) ); this.questionList.push({ @@ -414,6 +415,7 @@ export class ImportQuestionsDialogComponent implements OnDestroy { async importQuestions(): Promise { this.importClicked = true; + const status = this.status; //capture the state before we update it if (this.selectedCount < 1) { return; @@ -442,9 +444,15 @@ export class ImportQuestionsDialogComponent implements OnDestroy { answers: [], isArchived: false, dateCreated: currentDate, - dateModified: currentDate, - transceleratorQuestionId: listItem.question.id + dateModified: currentDate }; + + if (status === 'filter_questions') { + newQuestion.transceleratorQuestionId = listItem.question.id; + } else if (status === 'filter_notes') { + newQuestion.paratextNoteId = listItem.question.id; + } + await this.zone.runOutsideAngular(() => this.checkingQuestionsService.createQuestion(this.data.projectId, newQuestion, undefined, undefined) ); diff --git a/src/SIL.XForge.Scripture/Models/Question.cs b/src/SIL.XForge.Scripture/Models/Question.cs index 86526864ac2..e25d831f3ae 100644 --- a/src/SIL.XForge.Scripture/Models/Question.cs +++ b/src/SIL.XForge.Scripture/Models/Question.cs @@ -16,4 +16,5 @@ public class Question : ProjectData public DateTime DateModified { get; set; } public DateTime DateCreated { get; set; } public string TransceleratorQuestionId { get; set; } + public string ParatextNoteId { get; set; } } From 14cf49344c8ebc2aa2203434c2574888b1e04ad7 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Tue, 9 Dec 2025 15:11:33 -0600 Subject: [PATCH 16/18] PR comments --- .../Models/ParatextNote.cs | 19 ++- src/SIL.XForge.Scripture/Models/Question.cs | 4 +- .../Services/INotesService.cs | 19 --- .../Services/IParatextDataHelper.cs | 11 ++ .../Services/IParatextService.cs | 4 +- .../Services/NotesFormatter.cs | 78 +--------- .../Services/NotesService.cs | 96 ------------ .../Services/ParatextDataHelper.cs | 86 +++++++++++ .../Services/ParatextService.cs | 11 +- .../Services/ParatextSyncRunner.cs | 5 +- .../Services/SFServiceCollectionExtensions.cs | 1 - .../Services/NotesFormatterTests.cs | 55 ------- .../Services/NotesServiceTests.cs | 142 ------------------ .../Services/ParatextDataHelperTests.cs | 139 ++++++++++++++++- .../Services/ParatextServiceTests.cs | 18 ++- 15 files changed, 266 insertions(+), 422 deletions(-) delete mode 100644 src/SIL.XForge.Scripture/Services/INotesService.cs delete mode 100644 src/SIL.XForge.Scripture/Services/NotesService.cs delete mode 100644 test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs diff --git a/src/SIL.XForge.Scripture/Models/ParatextNote.cs b/src/SIL.XForge.Scripture/Models/ParatextNote.cs index 337351fea63..9eb21b47c2c 100644 --- a/src/SIL.XForge.Scripture/Models/ParatextNote.cs +++ b/src/SIL.XForge.Scripture/Models/ParatextNote.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; namespace SIL.XForge.Scripture.Models; @@ -8,11 +7,11 @@ namespace SIL.XForge.Scripture.Models; /// public class ParatextNote { - public string Id { get; set; } = string.Empty; + public required string Id { get; init; } - public string VerseRef { get; set; } = string.Empty; + public required string VerseRef { get; init; } - public IReadOnlyList Comments { get; set; } = Array.Empty(); + public required IReadOnlyList Comments { get; init; } } /// @@ -20,11 +19,11 @@ public class ParatextNote /// public class ParatextNoteComment { - public string VerseRef { get; set; } = string.Empty; + public required string VerseRef { get; init; } - public string Content { get; set; } = string.Empty; + public required string Content { get; init; } - public ParatextNoteTag? Tag { get; set; } + public ParatextNoteTag? Tag { get; init; } } /// @@ -32,9 +31,9 @@ public class ParatextNoteComment /// public class ParatextNoteTag { - public int Id { get; set; } + public int Id { get; init; } - public string Name { get; set; } = string.Empty; + public required string Name { get; init; } - public string Icon { get; set; } = string.Empty; + public required string Icon { get; init; } } diff --git a/src/SIL.XForge.Scripture/Models/Question.cs b/src/SIL.XForge.Scripture/Models/Question.cs index e25d831f3ae..7dabd51b33f 100644 --- a/src/SIL.XForge.Scripture/Models/Question.cs +++ b/src/SIL.XForge.Scripture/Models/Question.cs @@ -15,6 +15,6 @@ public class Question : ProjectData public DateTime? DateArchived { get; set; } public DateTime DateModified { get; set; } public DateTime DateCreated { get; set; } - public string TransceleratorQuestionId { get; set; } - public string ParatextNoteId { get; set; } + public string? TransceleratorQuestionId { get; set; } + public string? ParatextNoteId { get; set; } } diff --git a/src/SIL.XForge.Scripture/Services/INotesService.cs b/src/SIL.XForge.Scripture/Services/INotesService.cs deleted file mode 100644 index ae31830b49c..00000000000 --- a/src/SIL.XForge.Scripture/Services/INotesService.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System; -using System.Collections.Generic; -using Paratext.Data.ProjectComments; -using SIL.XForge.Scripture.Models; - -namespace SIL.XForge.Scripture.Services; - -/// -/// Provides helpers for retrieving and mapping Paratext note threads into Scripture Forge models. -/// -public interface INotesService -{ - IReadOnlyList GetNotes( - CommentManager commentManager, - CommentTags? commentTags, - Func? predicate = null, - bool includeInactiveThreads = true - ); -} diff --git a/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs index 0246eb21916..ae9b55476e3 100644 --- a/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs @@ -1,8 +1,19 @@ +using System; +using System.Collections.Generic; using Paratext.Data; +using Paratext.Data.ProjectComments; +using SIL.XForge.Scripture.Models; namespace SIL.XForge.Scripture.Services; public interface IParatextDataHelper { void CommitVersionedText(ScrText scrText, string comment); + + IReadOnlyList GetNotes( + CommentManager? commentManager, + CommentTags? commentTags, + Func? predicate = null, + bool includeInactiveThreads = true + ); } diff --git a/src/SIL.XForge.Scripture/Services/IParatextService.cs b/src/SIL.XForge.Scripture/Services/IParatextService.cs index e795a5966dd..a1644cd1c2e 100644 --- a/src/SIL.XForge.Scripture/Services/IParatextService.cs +++ b/src/SIL.XForge.Scripture/Services/IParatextService.cs @@ -3,7 +3,6 @@ using System.Threading; using System.Threading.Tasks; using System.Xml.Linq; -using Paratext.Data.ProjectComments; using SIL.Converters.Usj; using SIL.XForge.Models; using SIL.XForge.Realtime; @@ -54,9 +53,8 @@ Task PutBookText( Dictionary chapterAuthors = null ); string GetNotes(UserSecret userSecret, string paratextId, int bookNum); - IReadOnlyList GetNoteThreads(UserSecret userSecret, string paratextId); + IReadOnlyList? GetNoteThreads(UserSecret userSecret, string paratextId); SyncMetricInfo PutNotes(UserSecret userSecret, string paratextId, XElement notesElement); - CommentTags? GetCommentTags(UserSecret userSecret, string paratextId); Task UpdateParatextCommentsAsync( UserSecret userSecret, string paratextId, diff --git a/src/SIL.XForge.Scripture/Services/NotesFormatter.cs b/src/SIL.XForge.Scripture/Services/NotesFormatter.cs index b7186624783..588ea3cb7bd 100644 --- a/src/SIL.XForge.Scripture/Services/NotesFormatter.cs +++ b/src/SIL.XForge.Scripture/Services/NotesFormatter.cs @@ -1,8 +1,5 @@ -using System; using System.Collections.Generic; -using System.Globalization; using System.Linq; -using System.Reflection; using System.Text; using System.Text.RegularExpressions; using System.Xml; @@ -23,7 +20,7 @@ namespace SIL.XForge.Scripture.Services; /// * Additional null checking /// * Code reformatting /// -public static class NotesFormatter +public static partial class NotesFormatter { private const string NotesSchemaVersion = "1.1"; @@ -31,10 +28,8 @@ public static class NotesFormatter /// The regular expression for finding whitespace before XML tags. /// /// This is used by . - private static readonly Regex WhitespaceBeforeTagsRegex = new Regex( - @"\n\s*<", - RegexOptions.CultureInvariant | RegexOptions.Compiled - ); + [GeneratedRegex(@"\n\s*<", RegexOptions.CultureInvariant)] + private static partial Regex WhitespaceBeforeTagsRegex(); #region Public methods /// @@ -71,33 +66,10 @@ public static List> ParseNotes(XElement noteXml, ParatextUser ptUs public static XElement ParseNotesToXElement(string text) { text = text.Trim().Replace("\r\n", "\n"); - text = WhitespaceBeforeTagsRegex.Replace(text, "<"); + text = WhitespaceBeforeTagsRegex().Replace(text, "<"); return XElement.Parse(text, LoadOptions.PreserveWhitespace); } - /// - /// Replaces numeric tag identifiers in note XML with the corresponding Paratext comment tag data. - /// - /// The notes XML element. - /// The Paratext comment tag collection. - public static void ReplaceTagIdentifiersWithCommentTags(XElement notesElement, CommentTags commentTags) - { - if (notesElement == null || commentTags == null) - return; - - List tagElements = [.. notesElement.Descendants("tagAdded")]; - foreach (XElement tagElement in tagElements) - { - string elementValue = tagElement.Value; - if (!int.TryParse(elementValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out int tagId)) - continue; - - CommentTag mappedTag = commentTags.Get(tagId); - XElement replacement = CreateTagElement(tagElement.Name, mappedTag); - tagElement.ReplaceWith(replacement); - } - } - #endregion #region Private helper methods for formatting @@ -199,48 +171,6 @@ private static XElement FormatSelection(ScriptureSelection selection) return selElem; } - private static XElement CreateTagElement(XName elementName, CommentTag commentTag) - { - XElement element = new XElement(elementName); - if (commentTag == null) - return element; - - PropertyInfo[] properties = typeof(CommentTag).GetProperties(BindingFlags.Instance | BindingFlags.Public); - foreach (PropertyInfo property in properties.Where(property => property.CanRead)) - { - object value = property.GetValue(commentTag); - if (value == null) - continue; - - string propertyValue; - if (value is string stringValue) - { - if (string.IsNullOrEmpty(stringValue)) - continue; - propertyValue = stringValue; - } - else if (value is int intValue) - { - propertyValue = intValue.ToString(CultureInfo.InvariantCulture); - } - else if (value is bool boolValue) - { - propertyValue = boolValue ? bool.TrueString : bool.FalseString; - } - else if (value is Enum enumValue) - { - propertyValue = enumValue.ToString(); - } - else - { - continue; - } - - element.Add(new XElement(property.Name, propertyValue)); - } - - return element; - } #endregion #region Private helper methods for parsing diff --git a/src/SIL.XForge.Scripture/Services/NotesService.cs b/src/SIL.XForge.Scripture/Services/NotesService.cs deleted file mode 100644 index 1f699115b44..00000000000 --- a/src/SIL.XForge.Scripture/Services/NotesService.cs +++ /dev/null @@ -1,96 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Globalization; -using System.Linq; -using Paratext.Data.ProjectComments; -using SIL.XForge.Scripture.Models; -using ParatextComment = Paratext.Data.ProjectComments.Comment; - -namespace SIL.XForge.Scripture.Services; - -/// -/// Maps Paratext comment threads into lightweight Scripture Forge note models. -/// -public class NotesService : INotesService -{ - public IReadOnlyList GetNotes( - CommentManager commentManager, - CommentTags? commentTags, - Func? predicate = null, - bool includeInactiveThreads = true - ) - { - if (commentManager == null) - return Array.Empty(); - - Func filter = predicate ?? (_ => true); - IEnumerable threads = commentManager.FindThreads(filter, includeInactiveThreads); - - var notes = new List(); - foreach (CommentThread thread in threads) - { - IReadOnlyList activeComments = thread.ActiveComments.ToList(); - if (activeComments.Count == 0) - continue; - - var comments = new List(); - foreach (ParatextComment comment in activeComments) - { - comments.Add(CreateNoteComment(comment, commentTags)); - } - - if (comments.Count == 0) - continue; - - string verseRef = thread.ScriptureSelection?.VerseRef.ToString(); - if (string.IsNullOrEmpty(verseRef)) - verseRef = comments[0].VerseRef; - - notes.Add( - new ParatextNote - { - Id = thread.Id ?? string.Empty, - VerseRef = verseRef ?? string.Empty, - Comments = comments, - } - ); - } - - return notes; - } - - private static ParatextNoteComment CreateNoteComment(ParatextComment comment, CommentTags? commentTags) - { - ParatextNoteTag? tag = null; - if (comment.TagsAdded is { Length: > 0 }) - { - string tagValue = comment.TagsAdded[0]; - if (int.TryParse(tagValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out int tagId)) - tag = CreateNoteTag(tagId, commentTags); - } - - string content = comment.Contents?.InnerXml ?? string.Empty; - return new ParatextNoteComment - { - VerseRef = comment.VerseRefStr ?? string.Empty, - Content = content, - Tag = tag, - }; - } - - private static ParatextNoteTag CreateNoteTag(int tagId, CommentTags? commentTags) - { - if (commentTags != null) - { - CommentTag commentTag = commentTags.Get(tagId); - return new ParatextNoteTag - { - Id = commentTag.Id, - Name = commentTag.Name ?? string.Empty, - Icon = commentTag.Icon ?? string.Empty, - }; - } - - return new ParatextNoteTag { Id = tagId }; - } -} diff --git a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs index ac7cf86e783..7cc5f59bab3 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs @@ -1,6 +1,12 @@ using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; using Paratext.Data; +using Paratext.Data.ProjectComments; using Paratext.Data.Repository; +using SIL.XForge.Scripture.Models; +using ParatextComment = Paratext.Data.ProjectComments.Comment; namespace SIL.XForge.Scripture.Services; @@ -20,4 +26,84 @@ public void CommitVersionedText(ScrText scrText, string comment) VersionedText vText = VersioningManager.Get(scrText); vText.Commit(comment, null, false); } + + public IReadOnlyList GetNotes( + CommentManager? commentManager, + CommentTags? commentTags, + Func? predicate = null, + bool includeInactiveThreads = true + ) + { + if (commentManager == null) + return Array.Empty(); + + Func filter = predicate ?? (_ => true); + IEnumerable threads = commentManager.FindThreads(filter, includeInactiveThreads); + + var notes = new List(); + foreach (CommentThread thread in threads) + { + List comments = + [ + .. thread.ActiveComments.Select(comment => CreateNoteComment(comment, commentTags)), + ]; + if (comments.Count == 0) + continue; + + string verseRef = thread.ScriptureSelection?.VerseRef.ToString(); + if (string.IsNullOrEmpty(verseRef)) + verseRef = comments[0].VerseRef; + + notes.Add( + new ParatextNote + { + Id = thread.Id ?? string.Empty, + VerseRef = verseRef, + Comments = comments, + } + ); + } + + return notes; + } + + private static ParatextNoteComment CreateNoteComment(ParatextComment comment, CommentTags? commentTags) + { + ParatextNoteTag? tag = null; + if (comment.TagsAdded is { Length: > 0 }) + { + string tagValue = comment.TagsAdded[0]; + if (int.TryParse(tagValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out int tagId)) + tag = CreateNoteTag(tagId, commentTags); + } + + string content = comment.Contents?.InnerXml ?? string.Empty; + return new ParatextNoteComment + { + VerseRef = comment.VerseRefStr ?? string.Empty, + Content = content, + Tag = tag, + }; + } + + private static ParatextNoteTag CreateNoteTag(int tagId, CommentTags? commentTags) + { + if (commentTags != null) + { + CommentTag commentTag = commentTags.Get(tagId); + return new ParatextNoteTag + { + Id = commentTag.Id, + Name = commentTag.Name ?? string.Empty, + Icon = commentTag.Icon ?? string.Empty, + }; + } + + return new ParatextNoteTag + { + Id = tagId, + Name = string.Empty, + Icon = string.Empty, + }; + } } diff --git a/src/SIL.XForge.Scripture/Services/ParatextService.cs b/src/SIL.XForge.Scripture/Services/ParatextService.cs index ed685448223..6ebd4f5fa96 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextService.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextService.cs @@ -83,7 +83,6 @@ public class ParatextService : DisposableBase, IParatextService private readonly DotNetCoreAlert _alertSystem; private readonly IDeltaUsxMapper _deltaUsxMapper; private readonly IAuthService _authService; - private readonly INotesService _notesService; private readonly Dictionary _forcedUsernames = []; public ParatextService( @@ -102,8 +101,7 @@ public ParatextService( ISFRestClientFactory restClientFactory, IHgWrapper hgWrapper, IDeltaUsxMapper deltaUsxMapper, - IAuthService authService, - INotesService notesService + IAuthService authService ) { _paratextOptions = paratextOptions; @@ -123,7 +121,6 @@ INotesService notesService _alertSystem = new DotNetCoreAlert(_logger); _deltaUsxMapper = deltaUsxMapper; _authService = authService; - _notesService = notesService; _httpClientHandler = new HttpClientHandler(); _registryClient = new HttpClient(_httpClientHandler); @@ -1272,7 +1269,7 @@ public async Task PutBookText( } /// Gets note threads from the Paratext project and maps them to Scripture Forge models. - public IReadOnlyList GetNoteThreads(UserSecret userSecret, string paratextId) + public IReadOnlyList? GetNoteThreads(UserSecret userSecret, string paratextId) { using ScrText scrText = ScrTextCollection.FindById(GetParatextUsername(userSecret), paratextId); if (scrText == null) @@ -1281,7 +1278,7 @@ public IReadOnlyList GetNoteThreads(UserSecret userSecret, string CommentManager manager = CommentManager.Get(scrText); CommentTags? commentTags = CommentTags.Get(scrText); - return _notesService.GetNotes(manager, commentTags); + return _paratextDataHelper.GetNotes(manager, commentTags); } /// Get notes from the Paratext project folder. @@ -1492,7 +1489,7 @@ int sfNoteTagId return PutCommentThreads(userSecret, paratextId, noteThreadChangeList); } - public CommentTags? GetCommentTags(UserSecret userSecret, string paratextId) + internal CommentTags? GetCommentTags(UserSecret userSecret, string paratextId) { string? ptUsername = GetParatextUsername(userSecret); if (ptUsername == null) diff --git a/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs b/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs index eeff379ef5f..ab5619e6807 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs @@ -559,7 +559,10 @@ .. _projectDoc.Data.Texts.SelectMany(t => if (!_paratextService.IsResource(paratextId)) { LogMetric("Updating Paratext notes for questions"); - await UpdateParatextNotesAsync(text, questionDocsByBook[text.BookNum]); + if (questionDocsByBook[text.BookNum].Count > 0) + { + await UpdateParatextNotesAsync(text, questionDocsByBook[text.BookNum]); + } List> noteThreadDocs = [ diff --git a/src/SIL.XForge.Scripture/Services/SFServiceCollectionExtensions.cs b/src/SIL.XForge.Scripture/Services/SFServiceCollectionExtensions.cs index 527095ed459..c65280ffec3 100644 --- a/src/SIL.XForge.Scripture/Services/SFServiceCollectionExtensions.cs +++ b/src/SIL.XForge.Scripture/Services/SFServiceCollectionExtensions.cs @@ -24,7 +24,6 @@ public static IServiceCollection AddSFServices(this IServiceCollection services) services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs b/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs index 1ce3e8f1adc..7df78fe1cf1 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/NotesFormatterTests.cs @@ -1,9 +1,6 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; -using System.Linq; -using System.Reflection; using System.Text; using System.Xml; using System.Xml.Linq; @@ -401,28 +398,6 @@ public void FormatNotes_NoComments() Assert.AreEqual(expected, actual); } - [Test] - public void ReplaceTagIdentifiersWithCommentTags_ReplacesNumericIdentifiers() - { - // Setup comment tag data - string ptProjectId = Guid.NewGuid().ToString("N"); - MockCommentTags commentTags = MockCommentTags.GetCommentTags("user", ptProjectId); - commentTags.InitializeTagList([5]); - XElement notesElement = XElement.Parse( - "5" - ); - - // SUT - NotesFormatter.ReplaceTagIdentifiersWithCommentTags(notesElement, commentTags); - - // Verify tag gets replaced with the comment tag metadata - XElement actualTagElement = notesElement.Descendants("tagAdded").Single(); - CommentTag? expectedTag = commentTags.Get(5); - Assert.IsNotNull(expectedTag); - XElement expectedTagElement = BuildExpectedTagElement("tagAdded", expectedTag); - Assert.IsTrue(XNode.DeepEquals(expectedTagElement, actualTagElement)); - } - [Test] public void ParseNotes_CommentText() { @@ -654,36 +629,6 @@ public void ParseNotes_NoComments() Assert.AreEqual(0, actual.Count); } - private static XElement BuildExpectedTagElement(string elementName, CommentTag? commentTag) - { - XElement element = new XElement(elementName); - if (commentTag == null) - return element; - - PropertyInfo[] properties = typeof(CommentTag).GetProperties(BindingFlags.Instance | BindingFlags.Public); - foreach (PropertyInfo property in properties.Where(p => p.CanRead)) - { - - object? value = property.GetValue(commentTag); - if (value == null) - continue; - - string? propertyValue = value switch - { - string stringValue when !string.IsNullOrEmpty(stringValue) => stringValue, - int intValue => intValue.ToString(CultureInfo.InvariantCulture), - bool boolValue => boolValue ? bool.TrueString : bool.FalseString, - Enum enumValue => enumValue.ToString(), - _ => null, - }; - - if (propertyValue != null) - element.Add(new XElement(property.Name, propertyValue)); - } - - return element; - } - private class TestEnvironment { public readonly string Project01 = "project01"; diff --git a/test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs deleted file mode 100644 index c500f902160..00000000000 --- a/test/SIL.XForge.Scripture.Tests/Services/NotesServiceTests.cs +++ /dev/null @@ -1,142 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Xml; -using NSubstitute; -using NUnit.Framework; -using Paratext.Data; -using Paratext.Data.ProjectComments; -using Paratext.Data.ProjectFileAccess; -using SIL.XForge.Scripture.Models; - -namespace SIL.XForge.Scripture.Services; - -/// -/// Unit tests for covering note and tag mapping behaviour. -/// -[TestFixture] -public class NotesServiceTests -{ - [Test] - public void GetNotes_ReturnsMappedTagData() - { - var env = new TestEnvironment(); - env.CommentTags.InitializeTagList([5]); - env.AddComment("thread-01", "RUT 1:1", "Mapped tag", "5"); - var service = new NotesService(); - - IReadOnlyList notes = service.GetNotes(env.CommentManager, env.CommentTags); - - Assert.AreEqual(1, notes.Count); - ParatextNote note = notes[0]; - Assert.AreEqual("thread-01", note.Id); - Assert.AreEqual("RUT 1:1", note.VerseRef); - Assert.AreEqual(1, note.Comments.Count); - ParatextNoteComment comment = note.Comments[0]; - Assert.AreEqual("

Mapped tag

", comment.Content); - Assert.IsNotNull(comment.Tag); - Assert.AreEqual(5, comment.Tag.Id); - Assert.AreEqual("tag5", comment.Tag!.Name); - Assert.AreEqual("icon5", comment.Tag!.Icon); - } - - [Test] - public void GetNotes_SkipsThreadsWithOnlyDeletedComments() - { - var env = new TestEnvironment(); - env.CommentTags.InitializeTagList([5]); - env.AddComment("thread-04", "RUT 1:4", "Deleted", "5", deleted: true); - var service = new NotesService(); - - IReadOnlyList notes = service.GetNotes(env.CommentManager, env.CommentTags); - - Assert.AreEqual(0, notes.Count); - } - - [Test] - public void GetNotes_IncludesThreadsWithDeletedComments() - { - var env = new TestEnvironment(); - env.CommentTags.InitializeTagList([5]); - env.AddComment("thread-04", "RUT 1:4", "Deleted", "5", deleted: true); - env.AddComment("thread-04", "RUT 1:4", "Active", "5"); - var service = new NotesService(); - - IReadOnlyList notes = service.GetNotes(env.CommentManager, env.CommentTags); - - Assert.AreEqual(1, notes.Count); - } - - [Test] - public void GetNotes_FiltersThreadsWithPredicate() - { - var env = new TestEnvironment(); - env.CommentTags.InitializeTagList([2]); - env.AddComment("thread-05", "RUT 1:5", "First", "2"); - env.AddComment("thread-06", "RUT 1:6", "Second", "2"); - var service = new NotesService(); - - IReadOnlyList notes = service.GetNotes( - env.CommentManager, - env.CommentTags, - thread => string.Equals(thread.Id, "thread-06", StringComparison.Ordinal) - ); - - Assert.AreEqual(1, notes.Count); - Assert.AreEqual("thread-06", notes[0].Id); - } - - /// - /// Provides minimal Paratext setup to exercise against real comment threads. - /// - private sealed class TestEnvironment - { - private readonly string _ptProjectId; - - public TestEnvironment() - { - Username = "User 01"; - _ptProjectId = Guid.NewGuid().ToString("N"); - string projectPath = Path.Join(Path.GetTempPath(), _ptProjectId, "target"); - ParatextUser = new SFParatextUser(Username); - ProjectName projectName = new ProjectName { ProjectPath = projectPath, ShortName = "Proj" }; - ScrText = new MockScrText(ParatextUser, projectName) { CachedGuid = HexId.FromStr(_ptProjectId) }; - ScrText.Permissions.CreateFirstAdminUser(); - ProjectFileManager fileManager = Substitute.For(ScrText, null); - fileManager.IsWritable.Returns(true); - ScrText.SetFileManager(fileManager); - CommentManager = CommentManager.Get(ScrText); - CommentTags = MockCommentTags.GetCommentTags(Username, _ptProjectId); - } - - public string Username { get; } - public SFParatextUser ParatextUser { get; } - public MockScrText ScrText { get; } - public CommentManager CommentManager { get; } - public MockCommentTags CommentTags { get; } - - public void AddComment(string threadId, string verseRef, string content, string? tagValue, bool deleted = false) - { - XmlDocument doc = new XmlDocument(); - XmlElement root = doc.CreateElement("content"); - XmlElement paragraph = doc.CreateElement("p"); - paragraph.InnerText = content; - root.AppendChild(paragraph); - doc.AppendChild(root); - - var comment = new Paratext.Data.ProjectComments.Comment(ParatextUser) - { - Thread = threadId, - VerseRefStr = verseRef, - Contents = root, - DateTime = DateTimeOffset.UtcNow, - Deleted = deleted, - SelectedText = string.Empty, - StartPosition = 0, - }; - if (tagValue != null) - comment.TagsAdded = [tagValue]; - CommentManager.AddComment(comment); - } - } -} diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs index 83efcedb0c1..c3d7e186dd1 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs @@ -1,9 +1,16 @@ using System; +using System.Collections.Generic; +using System.IO; +using System.Xml; +using NSubstitute; using NUnit.Framework; using Paratext.Data; +using Paratext.Data.ProjectComments; +using Paratext.Data.ProjectFileAccess; using Paratext.Data.Repository; using SIL.WritingSystems; using SIL.XForge.Scripture.Models; +using ParatextComment = Paratext.Data.ProjectComments.Comment; namespace SIL.XForge.Scripture.Services; @@ -12,11 +19,80 @@ public class ParatextDataHelperTests { private const string ParatextUser01 = "ParatextUser01"; + [Test] + public void GetNotes_ReturnsMappedTagData() + { + var env = new NotesTestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-01", "RUT 1:1", "Mapped tag", "5"); + var helper = new ParatextDataHelper(); + + IReadOnlyList notes = helper.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(1, notes.Count); + ParatextNote note = notes[0]; + Assert.AreEqual("thread-01", note.Id); + Assert.AreEqual("RUT 1:1", note.VerseRef); + Assert.AreEqual(1, note.Comments.Count); + ParatextNoteComment comment = note.Comments[0]; + Assert.AreEqual("

Mapped tag

", comment.Content); + Assert.IsNotNull(comment.Tag); + Assert.AreEqual(5, comment.Tag!.Id); + Assert.AreEqual("tag5", comment.Tag!.Name); + Assert.AreEqual("icon5", comment.Tag!.Icon); + } + + [Test] + public void GetNotes_SkipsThreadsWithOnlyDeletedComments() + { + var env = new NotesTestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-04", "RUT 1:4", "Deleted", "5", deleted: true); + var helper = new ParatextDataHelper(); + + IReadOnlyList notes = helper.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(0, notes.Count); + } + + [Test] + public void GetNotes_IncludesThreadsWithDeletedComments() + { + var env = new NotesTestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-04", "RUT 1:4", "Deleted", "5", deleted: true); + env.AddComment("thread-04", "RUT 1:4", "Active", "5"); + var helper = new ParatextDataHelper(); + + IReadOnlyList notes = helper.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(1, notes.Count); + } + + [Test] + public void GetNotes_FiltersThreadsWithPredicate() + { + var env = new NotesTestEnvironment(); + env.CommentTags.InitializeTagList([2]); + env.AddComment("thread-05", "RUT 1:5", "First", "2"); + env.AddComment("thread-06", "RUT 1:6", "Second", "2"); + var helper = new ParatextDataHelper(); + + IReadOnlyList notes = helper.GetNotes( + env.CommentManager, + env.CommentTags, + thread => string.Equals(thread.Id, "thread-06", StringComparison.Ordinal) + ); + + Assert.AreEqual(1, notes.Count); + Assert.AreEqual("thread-06", notes[0].Id); + } + [Test] public void CommitVersionedText_Success() { // Setup - var env = new TestEnvironment(); + var env = new CommitTestEnvironment(); using MockScrText scrText = new MockScrText(new SFParatextUser(ParatextUser01), new ProjectName()); scrText.CachedGuid = HexId.CreateNew(); scrText.Permissions.CreateFirstAdminUser(); @@ -29,7 +105,7 @@ public void CommitVersionedText_Success() public void CommitVersionedText_ThrowsExceptionIfObserver() { // Setup - var env = new TestEnvironment(); + var env = new CommitTestEnvironment(); using MockScrText scrText = new MockScrText(new SFParatextUser(ParatextUser01), new ProjectName()); scrText.Permissions.CreateUser(ParatextUser01); // A user is an observer by default @@ -37,9 +113,9 @@ public void CommitVersionedText_ThrowsExceptionIfObserver() Assert.Throws(() => env.Service.CommitVersionedText(scrText, "comment text")); } - private class TestEnvironment + private sealed class CommitTestEnvironment { - public TestEnvironment() + public CommitTestEnvironment() { // Ensure that the SLDR is initialized for LanguageID.Code to be retrieved correctly if (!Sldr.IsInitialized) @@ -53,4 +129,59 @@ public TestEnvironment() public ParatextDataHelper Service { get; } = new ParatextDataHelper(); } + + private sealed class NotesTestEnvironment + { + private readonly string _ptProjectId; + + public NotesTestEnvironment() + { + // Ensure that the SLDR is initialized for LanguageID.Code to be retrieved correctly + if (!Sldr.IsInitialized) + Sldr.Initialize(true); + + Username = "User 01"; + _ptProjectId = Guid.NewGuid().ToString("N"); + string projectPath = Path.Join(Path.GetTempPath(), _ptProjectId, "target"); + ParatextUser = new SFParatextUser(Username); + ProjectName projectName = new ProjectName { ProjectPath = projectPath, ShortName = "Proj" }; + ScrText = new MockScrText(ParatextUser, projectName) { CachedGuid = HexId.FromStr(_ptProjectId) }; + ScrText.Permissions.CreateFirstAdminUser(); + ProjectFileManager fileManager = Substitute.For(ScrText, null); + fileManager.IsWritable.Returns(true); + ScrText.SetFileManager(fileManager); + CommentManager = CommentManager.Get(ScrText); + CommentTags = MockCommentTags.GetCommentTags(Username, _ptProjectId); + } + + public string Username { get; } + public SFParatextUser ParatextUser { get; } + public MockScrText ScrText { get; } + public CommentManager CommentManager { get; } + public MockCommentTags CommentTags { get; } + + public void AddComment(string threadId, string verseRef, string content, string? tagValue, bool deleted = false) + { + XmlDocument doc = new XmlDocument(); + XmlElement root = doc.CreateElement("content"); + XmlElement paragraph = doc.CreateElement("p"); + paragraph.InnerText = content; + root.AppendChild(paragraph); + doc.AppendChild(root); + + var comment = new ParatextComment(ParatextUser) + { + Thread = threadId, + VerseRefStr = verseRef, + Contents = root, + DateTime = DateTimeOffset.UtcNow, + Deleted = deleted, + SelectedText = string.Empty, + StartPosition = 0, + }; + if (tagValue != null) + comment.TagsAdded = [tagValue]; + CommentManager.AddComment(comment); + } + } } diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs index 689e59eefc5..805cff43a39 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs @@ -852,7 +852,7 @@ public void GetNotes_RetrievesNotes() } [Test] - public void GetNoteThreads_ReturnsNotesFromNotesService() + public void GetNoteThreads_ReturnsNotesFromDataHelper() { var env = new TestEnvironment(); UserSecret userSecret = TestEnvironment.MakeUserSecret(env.User01, env.Username01, env.ParatextUserId01); @@ -860,9 +860,14 @@ public void GetNoteThreads_ReturnsNotesFromNotesService() string paratextId = env.SetupProject(env.Project01, associatedPtUser); IReadOnlyList expectedNotes = new List { - new ParatextNote { Id = "thread-01", VerseRef = "RUT 1:1" }, + new ParatextNote + { + Id = "thread-01", + VerseRef = "RUT 1:1", + Comments = Array.Empty(), + }, }; - env.MockNotesService.GetNotes( + env.MockParatextDataHelper.GetNotes( Arg.Any(), Arg.Any(), Arg.Any>(), @@ -873,7 +878,7 @@ public void GetNoteThreads_ReturnsNotesFromNotesService() IReadOnlyList actual = env.Service.GetNoteThreads(userSecret, paratextId); Assert.AreSame(expectedNotes, actual); - env.MockNotesService.Received(1) + env.MockParatextDataHelper.Received(1) .GetNotes( env.ProjectCommentManager, Arg.Is(tags => tags != null), @@ -6895,7 +6900,6 @@ private class TestEnvironment : IDisposable public readonly IInternetSharedRepositorySourceProvider MockInternetSharedRepositorySourceProvider; public readonly ISFRestClientFactory MockRestClientFactory; public readonly IGuidService MockGuidService; - public readonly INotesService MockNotesService; public readonly ParatextService Service; public readonly HttpClient MockRegistryHttpClient; public readonly IDeltaUsxMapper MockDeltaUsxMapper; @@ -6922,7 +6926,6 @@ public TestEnvironment() MockRegistryHttpClient = Substitute.For(); MockDeltaUsxMapper = Substitute.For(); MockAuthService = Substitute.For(); - MockNotesService = Substitute.For(); DateTime aSecondAgo = DateTime.Now - TimeSpan.FromSeconds(1); string accessToken1 = TokenHelper.CreateAccessToken( @@ -6993,8 +6996,7 @@ public TestEnvironment() MockRestClientFactory, MockHgWrapper, MockDeltaUsxMapper, - MockAuthService, - MockNotesService + MockAuthService ) { ScrTextCollection = MockScrTextCollection, From 3aa3db9beb2f05b6dd629a40e98299bf709d7220 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Thu, 18 Dec 2025 11:09:25 -0600 Subject: [PATCH 17/18] Disable Import from Paratext when offline --- .../import-questions-dialog.component.html | 2 +- .../import-questions-dialog.component.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html index 8775c9bf5cd..f64a92eb5be 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.html @@ -108,7 +108,7 @@

- {{ t("learn_more") }} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts index a35fca4c22e..8ae60e0e564 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-dialog.component.ts @@ -316,6 +316,10 @@ export class ImportQuestionsDialogComponent implements OnDestroy { return this.status === 'initial' || this.status === 'loading'; } + get isOnline(): boolean { + return this.onlineStatusService.isOnline; + } + ngOnDestroy(): void { void this.promiseForQuestionDocQuery.then(query => query.dispose()); } From 347a9097d3597205c820708042b0badc1d12eca2 Mon Sep 17 00:00:00 2001 From: Joseph Myers Date: Mon, 5 Jan 2026 12:05:54 -0600 Subject: [PATCH 18/18] Filtering out Paratext's reserved ID's (like Rendering Discussion) and unknown/deleted ID's. --- .../Services/ParatextDataHelper.cs | 8 ++++- .../Services/ParatextDataHelperTests.cs | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs index 7cc5f59bab3..d3772affdae 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs @@ -73,7 +73,13 @@ private static ParatextNoteComment CreateNoteComment(ParatextComment comment, Co if (comment.TagsAdded is { Length: > 0 }) { string tagValue = comment.TagsAdded[0]; - if (int.TryParse(tagValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out int tagId)) + //Based on Paratext's CommentTags.cs, their reserved tag ID's are -3 through -1. + //We don't want to show those to the users when they're dealing with their notes. + if ( + int.TryParse(tagValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out int tagId) + && tagId >= 0 + && commentTags.IsKnown(tagId) + ) tag = CreateNoteTag(tagId, commentTags); } diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs index c3d7e186dd1..bf26ff1944e 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs @@ -42,6 +42,39 @@ public void GetNotes_ReturnsMappedTagData() Assert.AreEqual("icon5", comment.Tag!.Icon); } + [Test] + public void GetNotes_SkipsUnknownTags() + { + var env = new NotesTestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-02", "RUT 1:2", "Unknown tag", "6"); + var helper = new ParatextDataHelper(); + + IReadOnlyList notes = helper.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(1, notes.Count); + ParatextNoteComment comment = notes[0].Comments[0]; + Assert.IsNull(comment.Tag); + Assert.AreEqual("

Unknown tag

", comment.Content); + } + + [Test] + public void GetNotes_SkipsNegativeTagIds() + { + var env = new NotesTestEnvironment(); + env.CommentTags.InitializeTagList([5]); + env.AddComment("thread-03", "RUT 1:3", "Negative tag id", "-3"); + var helper = new ParatextDataHelper(); + + IReadOnlyList notes = helper.GetNotes(env.CommentManager, env.CommentTags); + + Assert.AreEqual(1, notes.Count); + ParatextNoteComment comment = notes[0].Comments[0]; + Assert.IsNull(comment.Tag); + Assert.AreEqual("

Negative tag id

", comment.Content); + Assert.AreEqual("RUT 1:3", notes[0].VerseRef); + } + [Test] public void GetNotes_SkipsThreadsWithOnlyDeletedComments() {