From 213abfee9d2bd91adffa94a6f4fd27f27510d3a1 Mon Sep 17 00:00:00 2001 From: Raymond Luong Date: Thu, 12 Feb 2026 12:32:21 -0700 Subject: [PATCH 1/3] SF-3709 Hide resolved note threads when importing questions from PT --- .../Services/ParatextDataHelper.cs | 7 +++--- .../Services/ParatextDataHelperTests.cs | 22 ++++++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs index 06c803bf8b..2162e6fa07 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs @@ -42,14 +42,15 @@ public IReadOnlyList GetNotes( CommentManager? commentManager, CommentTags? commentTags, Func? predicate = null, - bool includeInactiveThreads = true + bool activeThreadsOnly = true ) { if (commentManager == null) return Array.Empty(); - Func filter = predicate ?? (_ => true); - IEnumerable threads = commentManager.FindThreads(filter, includeInactiveThreads); + // When no predicate is provided, return note threads that are unresolved + Func filter = predicate ?? (t => t.Status != NoteStatus.Resolved); + IEnumerable threads = commentManager.FindThreads(filter, activeThreadsOnly); var notes = new List(); foreach (CommentThread thread in threads) diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs index 6c0913b4c3..82330927db 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs @@ -13,6 +13,7 @@ using Paratext.Data.ProjectFileAccess; using Paratext.Data.ProjectSettingsAccess; using Paratext.Data.Repository; +using PtxUtils; using SIL.WritingSystems; using SIL.XForge.Scripture.Models; using SIL.XForge.Services; @@ -125,6 +126,23 @@ public void GetNotes_IncludesThreadsWithDeletedComments() Assert.AreEqual(1, notes.Count); } + [Test] + public void GetNotes_ExcludeResolvedThreads() + { + var env = new TestEnvironment(); + using MockScrText scrText = env.GetScrText(HexId.CreateNew().ToString()); + var commentManager = CommentManager.Get(scrText); + var commentTags = new MockCommentTags(scrText); + commentTags.InitializeTagList([5]); + TestEnvironment.AddComment(scrText, "thread-04", "RUT 1:4", "Note to resolve", "5"); + TestEnvironment.AddComment(scrText, "thread-04", "RUT 1:4", "This is resolved", "5", resolved: true); + + // SUT + IReadOnlyList notes = env.Service.GetNotes(commentManager, commentTags); + + Assert.AreEqual(0, notes.Count); + } + [Test] public void GetNotes_FiltersThreadsWithPredicate() { @@ -345,7 +363,8 @@ public static void AddComment( string verseRef, string content, string? tagValue, - bool deleted = false + bool deleted = false, + bool resolved = false ) { XmlDocument doc = new XmlDocument(); @@ -362,6 +381,7 @@ public static void AddComment( Contents = root, DateTime = DateTimeOffset.UtcNow, Deleted = deleted, + Status = resolved ? NoteStatus.Resolved : NoteStatus.Todo, SelectedText = string.Empty, StartPosition = 0, }; From 3dbdaf4ca1a6218bf5c8850db93b5feff69f1d88 Mon Sep 17 00:00:00 2001 From: Raymond Luong Date: Fri, 13 Feb 2026 09:41:10 -0700 Subject: [PATCH 2/3] clean up --- .../Services/IParatextDataHelper.cs | 8 +------ .../Services/ParatextDataHelper.cs | 15 +++++-------- .../Services/ParatextDataHelperTests.cs | 22 ------------------- .../Services/ParatextServiceTests.cs | 15 ++----------- 4 files changed, 9 insertions(+), 51 deletions(-) diff --git a/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs index ec740aad89..e54fc7d2a8 100644 --- a/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -12,11 +11,6 @@ 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 - ); + IReadOnlyList GetNotes(CommentManager? commentManager, CommentTags? commentTags); Task MigrateResourceIfRequiredAsync(ScrText scrText, LanguageId? overrideLanguage, CancellationToken token); } diff --git a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs index 2162e6fa07..aeabec6022 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs @@ -38,19 +38,16 @@ public void CommitVersionedText(ScrText scrText, string comment) vText.Commit(comment, null, false); } - public IReadOnlyList GetNotes( - CommentManager? commentManager, - CommentTags? commentTags, - Func? predicate = null, - bool activeThreadsOnly = true - ) + public IReadOnlyList GetNotes(CommentManager? commentManager, CommentTags? commentTags) { if (commentManager == null) return Array.Empty(); - // When no predicate is provided, return note threads that are unresolved - Func filter = predicate ?? (t => t.Status != NoteStatus.Resolved); - IEnumerable threads = commentManager.FindThreads(filter, activeThreadsOnly); + // Only return note threads that are not resolved and active + IEnumerable threads = commentManager.FindThreads( + t => t.Status != NoteStatus.Resolved, + activeOnly: true + ); var notes = new List(); foreach (CommentThread thread in threads) diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs index 82330927db..553519c845 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextDataHelperTests.cs @@ -143,28 +143,6 @@ public void GetNotes_ExcludeResolvedThreads() Assert.AreEqual(0, notes.Count); } - [Test] - public void GetNotes_FiltersThreadsWithPredicate() - { - var env = new TestEnvironment(); - using MockScrText scrText = env.GetScrText(HexId.CreateNew().ToString()); - var commentManager = CommentManager.Get(scrText); - var commentTags = new MockCommentTags(scrText); - commentTags.InitializeTagList([2]); - TestEnvironment.AddComment(scrText, "thread-05", "RUT 1:5", "First", "2"); - TestEnvironment.AddComment(scrText, "thread-06", "RUT 1:6", "Second", "2"); - - // SUT - IReadOnlyList notes = env.Service.GetNotes( - commentManager, - 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() { diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs index 54efbac3a6..5a5e3c77d9 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs @@ -867,24 +867,13 @@ public void GetNoteThreads_ReturnsNotesFromDataHelper() Comments = Array.Empty(), }, }; - env.MockParatextDataHelper.GetNotes( - Arg.Any(), - Arg.Any(), - Arg.Any>(), - Arg.Any() - ) - .Returns(expectedNotes); + env.MockParatextDataHelper.GetNotes(Arg.Any(), Arg.Any()).Returns(expectedNotes); IReadOnlyList actual = env.Service.GetNoteThreads(userSecret, paratextId); Assert.AreSame(expectedNotes, actual); env.MockParatextDataHelper.Received(1) - .GetNotes( - env.ProjectCommentManager, - Arg.Is(tags => tags != null), - Arg.Any>(), - true - ); + .GetNotes(env.ProjectCommentManager, Arg.Is(tags => tags != null)); } [Test] From a9495b04371d4c5e6e412d2eb949096323990728 Mon Sep 17 00:00:00 2001 From: Raymond Luong Date: Thu, 19 Feb 2026 12:56:30 -0700 Subject: [PATCH 3/3] Explicitly assign comment manager and comment tags --- .../Services/IParatextDataHelper.cs | 2 +- .../Services/ParatextDataHelper.cs | 27 +++++-------------- .../Services/ParatextService.cs | 2 +- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs index e54fc7d2a8..1833b49a42 100644 --- a/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/IParatextDataHelper.cs @@ -11,6 +11,6 @@ namespace SIL.XForge.Scripture.Services; public interface IParatextDataHelper { void CommitVersionedText(ScrText scrText, string comment); - IReadOnlyList GetNotes(CommentManager? commentManager, CommentTags? commentTags); + IReadOnlyList GetNotes(CommentManager commentManager, CommentTags commentTags); Task MigrateResourceIfRequiredAsync(ScrText scrText, LanguageId? overrideLanguage, CancellationToken token); } diff --git a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs index aeabec6022..f73f20f0cd 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextDataHelper.cs @@ -38,11 +38,8 @@ public void CommitVersionedText(ScrText scrText, string comment) vText.Commit(comment, null, false); } - public IReadOnlyList GetNotes(CommentManager? commentManager, CommentTags? commentTags) + public IReadOnlyList GetNotes(CommentManager commentManager, CommentTags commentTags) { - if (commentManager == null) - return Array.Empty(); - // Only return note threads that are not resolved and active IEnumerable threads = commentManager.FindThreads( t => t.Status != NoteStatus.Resolved, @@ -154,7 +151,7 @@ CancellationToken token } } - private static ParatextNoteComment CreateNoteComment(ParatextComment comment, CommentTags? commentTags) + private static ParatextNoteComment CreateNoteComment(ParatextComment comment, CommentTags commentTags) { ParatextNoteTag? tag = null; if (comment.TagsAdded is { Length: > 0 }) @@ -179,24 +176,14 @@ private static ParatextNoteComment CreateNoteComment(ParatextComment comment, Co }; } - private static ParatextNoteTag CreateNoteTag(int tagId, CommentTags? commentTags) + 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, - }; - } - + CommentTag commentTag = commentTags.Get(tagId); return new ParatextNoteTag { - Id = tagId, - Name = string.Empty, - Icon = string.Empty, + Id = commentTag.Id, + Name = commentTag.Name ?? string.Empty, + Icon = commentTag.Icon ?? string.Empty, }; } diff --git a/src/SIL.XForge.Scripture/Services/ParatextService.cs b/src/SIL.XForge.Scripture/Services/ParatextService.cs index 2679635e60..0aea1ae527 100644 --- a/src/SIL.XForge.Scripture/Services/ParatextService.cs +++ b/src/SIL.XForge.Scripture/Services/ParatextService.cs @@ -1276,7 +1276,7 @@ public async Task PutBookText( return null; CommentManager manager = CommentManager.Get(scrText); - CommentTags? commentTags = CommentTags.Get(scrText); + CommentTags commentTags = CommentTags.Get(scrText); return _paratextDataHelper.GetNotes(manager, commentTags); }