Move and handle writing systems effectively (#1890)

* create move WritingSystem api with test

* implement move writing system for FwData

* implement moving ws in LcmCrdt

* handle diff orderable writing systems

* Mark WritingSystem.Order as MiniLcmInternal

---------

Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org>

Author: hahn-kev

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

@@ -19,6 +19,7 @@
 using SIL.LCModel.Core.WritingSystems;
 using SIL.LCModel.DomainServices;
 using SIL.LCModel.Infrastructure;
+using CollectionExtensions = SIL.Extensions.CollectionExtensions;
 
 namespace FwDataMiniLcmBridge.Api;
 
@@ -94,7 +95,7 @@ public Task<WritingSystems> GetWritingSystems()
         {
             Vernacular = WritingSystemContainer.CurrentVernacularWritingSystems.Select((definition, index) =>
                 FromLcmWritingSystem(definition, index, WritingSystemType.Vernacular)).ToArray(),
-            Analysis = Cache.ServiceLocator.WritingSystems.CurrentAnalysisWritingSystems.Select((definition, index) =>
+            Analysis = WritingSystemContainer.CurrentAnalysisWritingSystems.Select((definition, index) =>
                 FromLcmWritingSystem(definition, index, WritingSystemType.Analysis)).ToArray()
         };
         CompleteExemplars(writingSystems);
@@ -117,15 +118,15 @@ private WritingSystem FromLcmWritingSystem(CoreWritingSystemDefinition ws, int i
         };
     }
 
-    public async Task<WritingSystem> GetWritingSystem(WritingSystemId id, WritingSystemType type)
+    public async Task<WritingSystem?> GetWritingSystem(WritingSystemId id, WritingSystemType type)
     {
         var writingSystems = await GetWritingSystems();
         return type switch
         {
             WritingSystemType.Vernacular => writingSystems.Vernacular.FirstOrDefault(ws => ws.WsId == id),
             WritingSystemType.Analysis => writingSystems.Analysis.FirstOrDefault(ws => ws.WsId == id),
             _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
-        } ?? throw new NullReferenceException($"unable to find writing system with id {id}");
+        };
     }
 
     internal void CompleteExemplars(WritingSystems writingSystems)
@@ -149,7 +150,7 @@ internal void CompleteExemplars(WritingSystems writingSystems)
         }
     }
 
-    public async Task<WritingSystem> CreateWritingSystem(WritingSystem writingSystem)
+    public async Task<WritingSystem> CreateWritingSystem(WritingSystem writingSystem, BetweenPosition<WritingSystemId?>? between = null)
     {
         var type = writingSystem.Type;
         var exitingWs = type == WritingSystemType.Analysis ? Cache.ServiceLocator.WritingSystems.AnalysisWritingSystems : Cache.ServiceLocator.WritingSystems.VernacularWritingSystems;
@@ -158,10 +159,9 @@ public async Task<WritingSystem> CreateWritingSystem(WritingSystem writingSystem
             throw new DuplicateObjectException($"Writing system {writingSystem.WsId.Code} already exists");
         }
         CoreWritingSystemDefinition? ws = null;
-        UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Writing System",
+        await Cache.DoUsingNewOrCurrentUOW("Create Writing System",
             "Remove writing system",
-            Cache.ServiceLocator.ActionHandler,
-            () =>
+            async () =>
             {
                 Cache.ServiceLocator.WritingSystemManager.GetOrSet(writingSystem.WsId.Code, out ws);
                 ws.Abbreviation = writingSystem.Abbreviation;
@@ -176,6 +176,9 @@ public async Task<WritingSystem> CreateWritingSystem(WritingSystem writingSystem
                     default:
                         throw new ArgumentOutOfRangeException(nameof(type), type, null);
                 }
+
+                if (between is not null && (between.Previous is not null || between.Next is not null))
+                    await MoveWritingSystem(writingSystem.WsId, type, between);
             });
         if (ws is null) throw new InvalidOperationException("Writing system not found");
         var index = type switch
@@ -205,7 +208,7 @@ await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",
                 update.Apply(updateProxy);
                 return ValueTask.CompletedTask;
             });
-        return await GetWritingSystem(id, type);
+        return await GetWritingSystem(id, type) ?? throw new NullReferenceException($"unable to find writing system with id {id}");
     }
 
     public async Task<WritingSystem> UpdateWritingSystem(WritingSystem before, WritingSystem after, IMiniLcmApi? api = null)
@@ -219,6 +222,61 @@ await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",
         return await GetWritingSystem(after.WsId, after.Type) ?? throw new NullReferenceException($"unable to find {after.Type} writing system with id {after.WsId}");
     }
 
+    public async Task MoveWritingSystem(WritingSystemId id, WritingSystemType type, BetweenPosition<WritingSystemId?> between)
+    {
+        var wsToUpdate = GetLexWritingSystem(id, type);
+        if (wsToUpdate is null) throw new NullReferenceException($"unable to find writing system with id {id}");
+        var previousWs = between.Previous is null ? null : GetLexWritingSystem(between.Previous.Value, type);
+        var nextWs = between.Next is null ? null : GetLexWritingSystem(between.Next.Value, type);
+        if (nextWs is null && previousWs is null) throw new NullReferenceException($"unable to find writing system with id {between.Previous} or {between.Next}");
+        await Cache.DoUsingNewOrCurrentUOW("Move WritingSystem",
+            "Revert Move WritingSystem",
+            () =>
+            {
+                var exitingWs = type == WritingSystemType.Analysis
+                    ? WritingSystemContainer.AnalysisWritingSystems
+                    : WritingSystemContainer.VernacularWritingSystems;
+                var currentExistingWs = type == WritingSystemType.Analysis
+                    ? WritingSystemContainer.CurrentAnalysisWritingSystems
+                    : WritingSystemContainer.CurrentVernacularWritingSystems;
+                MoveWs(wsToUpdate, previousWs, nextWs, exitingWs);
+                MoveWs(wsToUpdate, previousWs, nextWs, currentExistingWs);
+
+                void MoveWs(CoreWritingSystemDefinition ws,
+                    CoreWritingSystemDefinition? previous,
+                    CoreWritingSystemDefinition? next,
+                    ICollection<CoreWritingSystemDefinition> list)
+                {
+                    var index = -1;
+                    if (previous is not null)
+                    {
+                        index = CollectionExtensions.IndexOf(list, previous);
+                        if (index >= 0) index++;
+                    }
+
+                    if (index < 0 && next is not null)
+                    {
+                        index = CollectionExtensions.IndexOf(list, next);
+                    }
+
+                    if (index < 0)
+                        throw new InvalidOperationException("unable to find writing system with id " + between.Previous + " or " + between.Next);
+
+                    LcmHelpers.AddOrMoveInList(list, index, ws);
+                }
+
+                return ValueTask.CompletedTask;
+            });
+    }
+
+    private CoreWritingSystemDefinition? GetLexWritingSystem(WritingSystemId id, WritingSystemType type)
+    {
+        var exitingWs = type == WritingSystemType.Analysis
+            ? WritingSystemContainer.AnalysisWritingSystems
+            : WritingSystemContainer.VernacularWritingSystems;
+        return exitingWs.FirstOrDefault(ws => ws.Id == id);
+    }
+
     public IAsyncEnumerable<PartOfSpeech> GetPartsOfSpeech()
     {
         return PartOfSpeechRepository

backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs

@@ -1,9 +1,11 @@
 using System.Globalization;
 using MiniLcm.Culture;
 using MiniLcm.Models;
+using SIL.Extensions;
 using SIL.LCModel;
 using SIL.LCModel.Core.KernelInterfaces;
 using SIL.LCModel.Core.Text;
+using SIL.LCModel.Core.WritingSystems;
 
 namespace FwDataMiniLcmBridge.Api;
 
@@ -289,4 +291,41 @@ internal static void SetString(this ITsMultiString multiString, FwDataMiniLcmApi
             multiString.set_String(writingSystemHandle, tsString);
         }
     }
+
+    //mostly a copy of method in SIL.FieldWorks.FwCoreDlgs.FwWritingSystemSetupModel
+    internal static void AddOrMoveInList(
+        ICollection<CoreWritingSystemDefinition> allWritingSystems,
+        int desiredIndex,
+        CoreWritingSystemDefinition workingWs
+    )
+    {
+        if (desiredIndex < 0) throw new ArgumentOutOfRangeException(nameof(desiredIndex), desiredIndex, "desiredIndex must be >= 0");
+
+        // copy original contents into a list
+        var updatedList = new List<CoreWritingSystemDefinition>(allWritingSystems);
+        var ws = updatedList.Find(listItem =>
+        {
+            if (ReferenceEquals(listItem, workingWs)) return true;
+            var workingTag = string.IsNullOrEmpty(workingWs.Id) ? workingWs.LanguageTag : workingWs.Id;
+            var listItemTag = string.IsNullOrEmpty(listItem.Id) ? listItem.LanguageTag : listItem.Id;
+            return string.Equals(listItemTag, workingTag);
+        });
+
+        if (ws != null)
+        {
+            updatedList.Remove(ws);
+        }
+
+        if (desiredIndex > updatedList.Count)
+        {
+            updatedList.Add(workingWs);
+        }
+        else
+        {
+            updatedList.Insert(desiredIndex, workingWs);
+        }
+
+        allWritingSystems.Clear();
+        allWritingSystems.AddRange(updatedList);
+    }
 }

backend/FwLite/FwLiteProjectSync.Tests/Import/ResumableTests.cs

@@ -203,9 +203,9 @@ Task<Publication> IMiniLcmWriteApi.CreatePublication(Publication publication)
         ResumableTests.MaybeThrowRandom(random, 0.2);
         return _api.CreatePublication(publication);
     }
-    Task<WritingSystem> IMiniLcmWriteApi.CreateWritingSystem(WritingSystem writingSystems)
+    Task<WritingSystem> IMiniLcmWriteApi.CreateWritingSystem(WritingSystem writingSystems, BetweenPosition<WritingSystemId?>? between)
     {
         ResumableTests.MaybeThrowRandom(random, 0.2);
-        return _api.CreateWritingSystem(writingSystems);
+        return _api.CreateWritingSystem(writingSystems, between);
     }
 }

backend/FwLite/FwLiteProjectSync.Tests/WritingSystemSyncTests.cs

@@ -0,0 +1,79 @@
+using FwLiteProjectSync.Tests.Fixtures;
+using MiniLcm.Models;
+
+namespace FwLiteProjectSync.Tests;
+
+public class WritingSystemSyncTests : IClassFixture<SyncFixture>, IAsyncLifetime
+{
+
+    private readonly SyncFixture _fixture;
+    private readonly CrdtFwdataProjectSyncService _syncService;
+
+    public WritingSystemSyncTests(SyncFixture fixture)
+    {
+        _fixture = fixture;
+        _syncService = _fixture.SyncService;
+    }
+
+    public async Task InitializeAsync()
+    {
+        await _fixture.FwDataApi.CreateEntry(new Entry()
+        {
+            Id = Guid.NewGuid(),
+            LexemeForm = { { "en", "Pineapple" } },
+        });
+        await _fixture.FwDataApi.CreateWritingSystem(new WritingSystem()
+        {
+            Id = Guid.NewGuid(),
+            Type = WritingSystemType.Vernacular,
+            WsId = new WritingSystemId("fr"),
+            Name = "French",
+            Abbreviation = "fr",
+            Font = "Arial"
+        });
+        await _syncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi);
+    }
+
+    public async Task DisposeAsync()
+    {
+        await foreach (var entry in _fixture.FwDataApi.GetAllEntries())
+        {
+            await _fixture.FwDataApi.DeleteEntry(entry.Id);
+        }
+
+        foreach (var entry in await _fixture.CrdtApi.GetAllEntries().ToArrayAsync())
+        {
+            await _fixture.CrdtApi.DeleteEntry(entry.Id);
+        }
+    }
+
+    [Fact]
+    public async Task SyncWs_UpdatesOrder()
+    {
+        var en = await _fixture.FwDataApi.GetWritingSystem("en", WritingSystemType.Vernacular);
+        en.Should().NotBeNull();
+        en.Order.Should().Be(0);
+        var fr = await _fixture.FwDataApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
+        fr.Should().NotBeNull();
+        fr.Order.Should().Be(1);
+        await _fixture.FwDataApi.MoveWritingSystem("fr", WritingSystemType.Vernacular, new(null, "en"));
+        fr = await _fixture.FwDataApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
+        fr.Should().NotBeNull();
+        fr.Order.Should().Be(0);
+        var crdtFr = await _fixture.CrdtApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
+        crdtFr.Should().NotBeNull();
+        crdtFr.Order.Should().Be(1);
+
+        await _syncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi);
+
+        fr = await _fixture.FwDataApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
+        fr.Should().NotBeNull();
+        fr.Order.Should().Be(0);
+
+        var crdtEn = await _fixture.CrdtApi.GetWritingSystem("en", WritingSystemType.Vernacular);
+        crdtEn.Should().NotBeNull();
+        var actualFr = await _fixture.CrdtApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
+        actualFr.Should().NotBeNull();
+        actualFr.Order.Should().BeLessThan(crdtEn.Order);
+    }
+}

backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs

@@ -18,9 +18,10 @@ public void Dispose()
 
     public record DryRunRecord(string Method, string Description);
 
-    public Task<WritingSystem> CreateWritingSystem(WritingSystem writingSystem)
+    public Task<WritingSystem> CreateWritingSystem(WritingSystem writingSystem, BetweenPosition<WritingSystemId?>? position = null)
     {
-        DryRunRecords.Add(new DryRunRecord(nameof(CreateWritingSystem), $"Create writing system {writingSystem.Type}"));
+        DryRunRecords.Add(new DryRunRecord(nameof(CreateWritingSystem),
+        $"Create writing system {writingSystem.Type} between {position?.Previous} and {position?.Next}"));
         return Task.FromResult(writingSystem);
     }
 
@@ -45,6 +46,12 @@ public Task<WritingSystem> UpdateWritingSystem(WritingSystem before, WritingSyst
         return Task.FromResult(after);
     }
 
+    public async Task MoveWritingSystem(WritingSystemId id, WritingSystemType type, BetweenPosition<WritingSystemId?> between)
+    {
+        DryRunRecords.Add(new DryRunRecord(nameof(MoveWritingSystem), $"Move writing system {id} between {between.Previous} and {between.Next}"));
+        await Task.CompletedTask;
+    }
+
     public Task<PartOfSpeech> CreatePartOfSpeech(PartOfSpeech partOfSpeech)
     {
         DryRunRecords.Add(new DryRunRecord(nameof(CreatePartOfSpeech), $"Create part of speech {partOfSpeech.Name}"));

backend/FwLite/FwLiteProjectSync/Import/ResumableImportApi.cs

@@ -1,6 +1,7 @@
 using System.Runtime.CompilerServices;
 using MiniLcm;
 using MiniLcm.Models;
+using MiniLcm.SyncHelpers;
 
 namespace FwLiteProjectSync.Import;
 
@@ -65,9 +66,9 @@ async Task<Publication> IMiniLcmWriteApi.CreatePublication(Publication publicati
     {
         return await HasCreated(publication, _api.GetPublications(), () => _api.CreatePublication(publication));
     }
-    async Task<WritingSystem> IMiniLcmWriteApi.CreateWritingSystem(WritingSystem writingSystem)
+    async Task<WritingSystem> IMiniLcmWriteApi.CreateWritingSystem(WritingSystem writingSystem, BetweenPosition<WritingSystemId?>? between = null)
     {
-        return await HasCreated(writingSystem, AsyncWs(), () => _api.CreateWritingSystem(writingSystem), ws => ws.Type + ws.WsId.Code);
+        return await HasCreated(writingSystem, AsyncWs(), () => _api.CreateWritingSystem(writingSystem, between), ws => ws.Type + ws.WsId.Code);
     }
 
     private async IAsyncEnumerable<WritingSystem> AsyncWs()

backend/FwLite/LcmCrdt.Tests/Changes/RegressionDeserializationData.json

@@ -277,6 +277,11 @@
     },
     "EntityId": "e03826fb-e6d2-6e3c-1f43-9fc6a0f6c4c6"
   },
+  {
+    "$type": "SetOrderChange:WritingSystem",
+    "Order": 0.4870418422144669,
+    "EntityId": "9d3e9006-c4b4-4316-6628-faeede75af40"
+  },
   {
     "$type": "SetOrderChange:Sense",
     "Order": 0.4870418422144669,

backend/FwLite/LcmCrdt.Tests/Changes/UseChangesTests.cs

@@ -199,6 +199,9 @@ private static IEnumerable<ChangeWithDependencies> GetAllChanges()
         var setComplexFormComponentOrderChange = new LcmCrdt.Changes.SetOrderChange<ComplexFormComponent>(complexFormComponent.Id, 10);
         yield return new ChangeWithDependencies(setComplexFormComponentOrderChange, [createComplexFormComponentChange]);
 
+        var setWritingSystemOrderChange = new LcmCrdt.Changes.SetOrderChange<WritingSystem>(writingSystem.Id, 10);
+        yield return new ChangeWithDependencies(setWritingSystemOrderChange, [createWritingSystemChange]);
+
         var publication = new Publication { Id = Guid.NewGuid(), Name = { { "en", "Main" } } };
         var createPublicationChange = new CreatePublicationChange(publication.Id, publication.Name);
         yield return new ChangeWithDependencies(createPublicationChange);

backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txt

@@ -171,6 +171,10 @@
     {
       DerivedType: SetOrderChange<ComplexFormComponent>,
       TypeDiscriminator: SetOrderChange:ComplexFormComponent
+    },
+    {
+      DerivedType: SetOrderChange<WritingSystem>,
+      TypeDiscriminator: SetOrderChange:WritingSystem
     }
   ],
   IgnoreUnrecognizedTypeDiscriminators: false,

backend/FwLite/LcmCrdt/Changes/SetOrderChange.cs

@@ -5,7 +5,7 @@
 namespace LcmCrdt.Changes;
 
 public class SetOrderChange<T>(Guid entityId, double order) : EditChange<T>(entityId), IPolyType
-    where T : class, IOrderable
+    where T : class, IOrderableNoId
 {
     public static string TypeName => $"{nameof(SetOrderChange<T>)}:" + typeof(T).Name;