From 998e10ece5854c1de79319a64b8f658d8d7f54b7 Mon Sep 17 00:00:00 2001
From: Peter Chapman <peter@conglomo.co.nz>
Date: Thu, 9 Jan 2025 12:51:17 +1300
Subject: [PATCH] SF-3149 Only update resource permissions when necessary

---
 .../Services/ParatextSyncRunner.cs            | 15 +++++----
 .../Services/ParatextSyncRunnerTests.cs       | 32 ++++++++++++++++---
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs b/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs
index 6f6a4073a4..4a20aeef2a 100644
--- a/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs
+++ b/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs
@@ -128,7 +128,6 @@ IGuidService guidService
         _guidService = guidService;
     }
 
-    private bool TranslationSuggestionsEnabled => _projectDoc.Data.TranslateConfig.TranslationSuggestionsEnabled;
     private bool CheckingEnabled => _projectDoc.Data.CheckingConfig.CheckingEnabled;
 
     /// <summary>
@@ -327,8 +326,7 @@ await GetAndUpdateParatextBooksAndNotes(
             // Update user resource access, if this project has a source resource
             // The updating of a source project's permissions is done when that project is synced.
             if (
-                TranslationSuggestionsEnabled
-                && !string.IsNullOrWhiteSpace(sourceParatextId)
+                !string.IsNullOrWhiteSpace(sourceParatextId)
                 && !string.IsNullOrWhiteSpace(sourceProjectRef)
                 && _paratextService.IsResource(sourceParatextId)
             )
@@ -440,9 +438,14 @@ await UpdateBiblicalTermsAsync(
                 await UpdateResourceConfig(paratextProject);
             }
 
-            // We will always update permissions, even if this is a resource project
-            LogMetric("Updating permissions");
-            await _projectService.UpdatePermissionsAsync(userId, _projectDoc, _paratextUsers, token);
+            // Update permissions if not a resource, or if it is a resource and needs updating.
+            // A resource will need updating if its text or permissions have changed on the DBL.
+            // Source resources have their permissions updated above in the section "Updating user resource access".
+            if (!_paratextService.IsResource(targetParatextId) || resourceNeedsUpdating)
+            {
+                LogMetric("Updating permissions");
+                await _projectService.UpdatePermissionsAsync(userId, _projectDoc, _paratextUsers, token);
+            }
 
             await NotifySyncProgress(SyncPhase.Phase9, 40.0);
 
diff --git a/test/SIL.XForge.Scripture.Tests/Services/ParatextSyncRunnerTests.cs b/test/SIL.XForge.Scripture.Tests/Services/ParatextSyncRunnerTests.cs
index 65a733af97..092f0e4922 100644
--- a/test/SIL.XForge.Scripture.Tests/Services/ParatextSyncRunnerTests.cs
+++ b/test/SIL.XForge.Scripture.Tests/Services/ParatextSyncRunnerTests.cs
@@ -638,7 +638,7 @@ public async Task SyncAsync_UserRoleChangedAndUserRemoved()
     }
 
     [Test]
-    public async Task SyncAsync_SetsUserPermissions()
+    public async Task SyncAsync_UpdatesPermissionsForProjects()
     {
         var env = new TestEnvironment();
         Book[] books = [new Book("MAT", 2), new Book("MRK", 2)];
@@ -650,6 +650,7 @@ public async Task SyncAsync_SetsUserPermissions()
                 Arg.Any<CancellationToken>()
             )
             .Returns([TestEnvironment.ParatextProjectUser01 with { Role = SFProjectRole.Translator }]);
+        env.ParatextService.IsResource(Arg.Any<string>()).Returns(false);
 
         // SUT
         await env.Runner.RunAsync("project01", "user01", "project01", false, CancellationToken.None);
@@ -658,9 +659,8 @@ await env
             .SFProjectService.Received()
             .UpdatePermissionsAsync(
                 "user01",
-                Arg.Is<IDocument<SFProject>>(
-                    (IDocument<SFProject> sfProjDoc) =>
-                        sfProjDoc.Data.Id == "project01" && sfProjDoc.Data.ParatextId == "target"
+                Arg.Is<IDocument<SFProject>>(sfProjDoc =>
+                    sfProjDoc.Data.Id == "project01" && sfProjDoc.Data.ParatextId == "target"
                 ),
                 Arg.Any<IReadOnlyList<ParatextProjectUser>>(),
                 Arg.Any<CancellationToken>()
@@ -2382,6 +2382,18 @@ public async Task SyncAsync_ResourceChanged()
         // Check that the resource users metrics have been updated
         SyncMetrics syncMetrics = env.GetSyncMetrics("project01");
         Assert.That(syncMetrics.ResourceUsers, Is.EqualTo(new SyncMetricInfo(2, 0, 0)));
+
+        // Check that the permissions were updated
+        await env
+            .SFProjectService.Received()
+            .UpdatePermissionsAsync(
+                "user01",
+                Arg.Is<IDocument<SFProject>>(sfProjDoc =>
+                    sfProjDoc.Data.Id == "project01" && sfProjDoc.Data.ParatextId == "target"
+                ),
+                Arg.Any<IReadOnlyList<ParatextProjectUser>>(),
+                Arg.Any<CancellationToken>()
+            );
     }
 
     [Test]
@@ -2419,6 +2431,18 @@ public async Task SyncAsync_ResourceNotChanged()
         Assert.IsNull(env.GetProject().ResourceConfig);
         Assert.That(env.ContainsText("project01", "MAT", 2), Is.True);
         Assert.That(env.ContainsText("project01", "MRK", 2), Is.True);
+
+        // Ensure that the permissions were not updated
+        await env
+            .SFProjectService.DidNotReceive()
+            .UpdatePermissionsAsync(
+                "user01",
+                Arg.Is<IDocument<SFProject>>(sfProjDoc =>
+                    sfProjDoc.Data.Id == "project01" && sfProjDoc.Data.ParatextId == "target"
+                ),
+                Arg.Any<IReadOnlyList<ParatextProjectUser>>(),
+                Arg.Any<CancellationToken>()
+            );
     }
 
     [Test]