Feat/1246 allow reordering complex form components

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

@@ -206,7 +206,7 @@
         }
         await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",
             "Revert WritingSystem",
             async () =>
             {
                 var updateProxy = new UpdateWritingSystemProxy(lcmWritingSystem, this)
                 {
@@ -715,20 +715,44 @@
         return await GetEntry(entry.Id) ?? throw new InvalidOperationException("Entry was not created");
     }
 
-    public Task<ComplexFormComponent> CreateComplexFormComponent(ComplexFormComponent complexFormComponent)
+    public Task<ComplexFormComponent> CreateComplexFormComponent(ComplexFormComponent complexFormComponent, BetweenPosition? position = null)
     {
         UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Complex Form Component",
             "Remove Complex Form Component",
             Cache.ServiceLocator.ActionHandler,
             () =>
             {
                 var lexEntry = EntriesRepository.GetObject(complexFormComponent.ComplexFormEntryId);
-                AddComplexFormComponent(lexEntry, complexFormComponent);
+                AddComplexFormComponent(lexEntry, complexFormComponent, position);
             });
         return Task.FromResult(ToComplexFormComponents(EntriesRepository.GetObject(complexFormComponent.ComplexFormEntryId))
             .Single(c => c.ComponentEntryId == complexFormComponent.ComponentEntryId && c.ComponentSenseId == complexFormComponent.ComponentSenseId));
     }
 
+    public Task MoveComplexFormComponent(Guid complexFormEntryId, Guid complexFormComponentEntityId, BetweenPosition between)
+    {
+        if (!EntriesRepository.TryGetObject(complexFormEntryId, out var lexComplexFormEntry))
+            throw new InvalidOperationException("Entry not found");
+
+        var lexComponent = FindSenseOrEntryComponent(complexFormComponentEntityId);
+
+        UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Move Complex Form Component",
+            "Move Complex Form Component back",
+            Cache.ServiceLocator.ActionHandler,
+            () =>
+            {
+                InsertComplexFormComponent(lexComplexFormEntry, lexComponent, between);
+            });
+        return Task.CompletedTask;
+    }
+
+    private ICmObject FindSenseOrEntryComponent(Guid id)
+    {
+        if (SenseRepository.TryGetObject(id, out var sense)) return sense;
+        if (EntriesRepository.TryGetObject(id, out var entry)) return entry;
+        throw new InvalidOperationException("Component not found");
+    }
+
     public Task DeleteComplexFormComponent(ComplexFormComponent complexFormComponent)
     {
         UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Delete Complex Form Component",
@@ -769,12 +793,58 @@
     /// <summary>
     /// must be called as part of an lcm action
     /// </summary>
-    internal void AddComplexFormComponent(ILexEntry lexEntry, ComplexFormComponent component)
+    internal void AddComplexFormComponent(ILexEntry lexComplexForm, ComplexFormComponent component, BetweenPosition? between = null)
     {
         ICmObject lexComponent = component.ComponentSenseId is not null
             ? SenseRepository.GetObject(component.ComponentSenseId.Value)
             : EntriesRepository.GetObject(component.ComponentEntryId);
-        lexEntry.AddComponent(lexComponent);
+        InsertComplexFormComponent(lexComplexForm, lexComponent, between);
+    }
+
+    internal void InsertComplexFormComponent(ILexEntry lexComplexForm, ICmObject lexComponent, BetweenPosition? between = null)
+    {
+        var previousComponentId = between?.Previous;
+        var nextComponentId = between?.Next;
+
+        var entryRef = lexComplexForm.ComplexFormEntryRefs.SingleOrDefault();
+        if (entryRef is null || entryRef.ComponentLexemesRS.Count == 0)
+        {
+            lexComplexForm.AddComponent(lexComponent);
+            return;
+        }
+
+        // Prevents adding duplicates (which ComponentLexemesRS.Insert is susceptible to)
+        if (entryRef.ComponentLexemesRS.Contains(lexComponent))
+        {
+            if (between is null) return;
+            entryRef.ComponentLexemesRS.Remove(lexComponent);
+        }
+
+        var previousComponent = previousComponentId.HasValue ? entryRef.ComponentLexemesRS.FirstOrDefault(s => s.Guid == previousComponentId) : null;
+        if (previousComponent is not null)
+        {
+            var insertI = entryRef.ComponentLexemesRS.IndexOf(previousComponent) + 1;
+            if (insertI >= entryRef.ComponentLexemesRS.Count)
+            {
+                // Prefer AddComponent as it does some extra magical stuff 🤷
+                lexComplexForm.AddComponent(lexComponent);
+            }
+            else
+            {
+                entryRef.ComponentLexemesRS.Insert(insertI, lexComponent);
+            }
+            return;
+        }
+
+        var nextComponent = nextComponentId.HasValue ? entryRef.ComponentLexemesRS.FirstOrDefault(s => s.Guid == nextComponentId) : null;
+        if (nextComponent is not null)
+        {
+            var insertI = entryRef.ComponentLexemesRS.IndexOf(nextComponent);
+            entryRef.ComponentLexemesRS.Insert(insertI, lexComponent);
+            return;
+        }
+
+        lexComplexForm.AddComponent(lexComponent);
     }
 
     internal void RemoveComplexFormComponent(ILexEntry lexEntry, ComplexFormComponent component)
@@ -909,6 +979,7 @@
             if (previousSense.SensesOS.Count > 0)
             {
                 // if the sense has sub-senses, our sense will only come directly after it if it is the first sub-sense
+                // ILcmOwningSequence treats an insert as a move if the item is already in it
                 previousSense.SensesOS.Insert(0, lexSense);
             }
             else
@@ -918,6 +989,7 @@
                     : previousSense.Owner is ILexSense parentSense ? parentSense.SensesOS
                     : throw new InvalidOperationException("Sense parent is not a sense or the expected entry");
                 var insertI = allSiblings.IndexOf(previousSense) + 1;
+                // ILcmOwningSequence treats an insert as a move if the item is already in it
                 lexEntry.SensesOS.Insert(insertI, lexSense);
             }
             return;
@@ -931,6 +1003,7 @@
                     : nextSense.Owner is ILexSense parentSense ? parentSense.SensesOS
                     : throw new InvalidOperationException("Sense parent is not a sense or the expected entry");
             var insertI = allSiblings.IndexOf(nextSense);
+            // ILcmOwningSequence treats an insert as a move if the item is already in it
             lexEntry.SensesOS.Insert(insertI, lexSense);
             return;
         }
@@ -947,6 +1020,7 @@
         if (previousExample is not null)
         {
             var insertI = lexSense.ExamplesOS.IndexOf(previousExample) + 1;
+            // ILcmOwningSequence treats an insert as a move if the item is already in it
             lexSense.ExamplesOS.Insert(insertI, lexExample);
             return;
         }
@@ -955,6 +1029,7 @@
         if (nextExample is not null)
         {
             var insertI = lexSense.ExamplesOS.IndexOf(nextExample);
+            // ILcmOwningSequence treats an insert as a move if the item is already in it
             lexSense.ExamplesOS.Insert(insertI, lexExample);
             return;
         }
@@ -1045,7 +1120,6 @@
             Cache.ServiceLocator.ActionHandler,
             () =>
             {
-                // LibLCM treats an insert as a move if the sense is already in the entry
                 InsertSense(lexEntry, lexSense, between);
             });
         return Task.CompletedTask;
@@ -1168,7 +1242,6 @@
             Cache.ServiceLocator.ActionHandler,
             () =>
             {
-                // LibLCM treats an insert as a move if the example sentence is already on the sense
                 InsertExampleSentence(lexSense, lexExample, between);
             });
         return Task.CompletedTask;
@@ -1200,4 +1273,9 @@
                                                 lexExampleSentence.Owner.ClassName);
         }
     }
+
+    public ProjectDataFormat GetDataFormat()
+    {
+        return ProjectDataFormat.FwData;
+    }
 }

backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs

@@ -51,17 +51,9 @@ public override List<Sense> Senses
         set => throw new NotImplementedException();
     }
 
-    public override IList<ComplexFormComponent> Components
+    public override List<ComplexFormComponent> Components
     {
-        get =>
-            new UpdateListProxy<ComplexFormComponent>(
-                component => _lexboxLcmApi.AddComplexFormComponent(_lcmEntry, component),
-                component => _lexboxLcmApi.RemoveComplexFormComponent(_lcmEntry, component),
-                i => new UpdateComplexFormComponentProxy(_lcmEntry,
-                    _lcmEntry.ComplexFormEntryRefs.Single().ComponentLexemesRS[i],
-                    _lexboxLcmApi),
-                _lcmEntry.ComplexFormEntryRefs.SingleOrDefault()?.ComponentLexemesRS.Count ?? 0
-            );
+        get => throw new NotImplementedException();
         set => throw new NotImplementedException();
     }
 

backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs

@@ -1,4 +1,5 @@
 using FwLiteProjectSync.Tests.Fixtures;
+using MiniLcm;
 using MiniLcm.Models;
 using MiniLcm.SyncHelpers;
 using MiniLcm.Tests.AutoFakerHelpers;
@@ -44,7 +45,9 @@ public async Task CanSyncRandomEntries()
         actual.Should().NotBeNull();
         actual.Should().BeEquivalentTo(after, options => options
             .For(e => e.Senses).Exclude(s => s.Order)
-            .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
+            .For(e => e.Components).Exclude(c => c.Order)
+            .For(e => e.ComplexForms).Exclude(c => c.Order)
+            .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order)
             );
     }
 
@@ -125,4 +128,56 @@ public async Task CanChangeComplexFormTypeViaSync()
         actual.Should().NotBeNull();
         actual.Should().BeEquivalentTo(after, options => options);
     }
+
+    [Theory]
+    [InlineData(true, ProjectDataFormat.Harmony)]
+    [InlineData(true, ProjectDataFormat.FwData)]
+    [InlineData(false, ProjectDataFormat.Harmony)]
+    [InlineData(false, ProjectDataFormat.FwData)]
+    public async Task CanInsertComplexFormComponentViaSync(bool componentThenComplexForm, ProjectDataFormat apiType)
+    {
+        // arrange
+        IMiniLcmApi api = apiType == ProjectDataFormat.Harmony ? _fixture.CrdtApi : _fixture.FwDataApi;
+
+        var existingComponent = await api.CreateEntry(new() { LexemeForm = { { "en", "existing-component" } } });
+        var complexFormBefore = await api.CreateEntry(new() { LexemeForm = { { "en", "complex-form" } } });
+        await api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(complexFormBefore, existingComponent));
+        complexFormBefore = (await api.GetEntry(complexFormBefore.Id))!;
+
+        var newComponentBefore = await api.CreateEntry(new() { LexemeForm = { { "en", "component" } } });
+        var newComplexFormComponent = ComplexFormComponent.FromEntries(complexFormBefore, newComponentBefore);
+
+        var newComponentAfter = newComponentBefore.Copy();
+        newComponentAfter.ComplexForms.Add(newComplexFormComponent);
+        var complexFormAfter = complexFormBefore.Copy();
+        complexFormAfter.Components.Insert(0, newComplexFormComponent);
+
+        // act - The big question is: both entries want to add the same component to the complex form. One cares about its order, the other doesn't. Will it land in the right place?
+        if (componentThenComplexForm)
+        {
+            // this results in 2 crdt changes:
+            // (1) add complex-form (i.e. implicitly add component)
+            // (2) move component to the right place
+            await EntrySync.Sync([newComponentBefore, complexFormBefore], [newComponentAfter, complexFormAfter], api);
+        }
+        else
+        {
+            // this results in 1 crdt change:
+            // the component is added in the right place
+            // (adding the complex-form becomes a no-op, because it already exists and a BetweenPosition is not specified)
+            await EntrySync.Sync([complexFormBefore, newComponentBefore], [complexFormAfter, newComponentAfter], api);
+        }
+
+        // assert
+        var actual = await api.GetEntry(complexFormAfter.Id);
+        actual.Should().NotBeNull();
+        actual.Should().BeEquivalentTo(complexFormAfter, options => options
+            .WithStrictOrdering()
+            .For(e => e.Components).Exclude(c => c.Order)
+            .For(e => e.Components).Exclude(c => c.Id));
+        actual.Components.Count.Should().Be(2);
+        actual.Components.First().Should().BeEquivalentTo(newComplexFormComponent, options => options
+            .Excluding(c => c.Order)
+            .Excluding(c => c.Id));
+    }
 }

backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs

@@ -52,9 +52,11 @@ private void ShouldAllBeEquivalentTo(Dictionary<Guid, Entry> crdtEntries, Dictio
                 crdtEntry.Should().BeEquivalentTo(fwdataEntry,
                     options => options
                         .For(e => e.Components).Exclude(c => c.Id)
+                        .For(e => e.Components).Exclude(c => c.Order)
                         .For(e => e.ComplexForms).Exclude(c => c.Id)
+                        .For(e => e.ComplexForms).Exclude(c => c.Order)
                         .For(e => e.Senses).Exclude(s => s.Order)
-                        .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order),
+                        .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order),
                     $"CRDT entry {crdtEntry.Id} was synced with FwData");
             }
         }

backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs

@@ -88,7 +88,10 @@ public async Task FirstSyncJustDoesAnImport()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                .For(e => e.ComplexForms).Exclude(c => c.Id));
+                .For(e => e.Components).Exclude(c => c.Order)
+                .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                );
     }
 
     [Fact]
@@ -156,7 +159,10 @@ await crdtApi.CreateEntry(new Entry()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                .For(e => e.ComplexForms).Exclude(c => c.Id));
+                .For(e => e.Components).Exclude(c => c.Order)
+                .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                );
     }
 
     [Fact]
@@ -239,7 +245,10 @@ public async Task CreatingAComplexEntryInFwDataSyncsWithoutIssue()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                .For(e => e.ComplexForms).Exclude(c => c.Id));
+                .For(e => e.Components).Exclude(c => c.Order)
+                .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                );
 
         // Sync again, ensure no problems or changes
         var secondSync = await _syncService.Sync(crdtApi, fwdataApi);
@@ -334,7 +343,10 @@ await crdtApi.CreateEntry(new Entry()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                .For(e => e.ComplexForms).Exclude(c => c.Id));
+                .For(e => e.Components).Exclude(c => c.Order)
+                .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                );
     }
 
     [Fact]
@@ -418,7 +430,10 @@ await crdtApi.CreateEntry(new Entry()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                .For(e => e.ComplexForms).Exclude(c => c.Id));
+                .For(e => e.Components).Exclude(c => c.Order)
+                .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                );
     }
 
     [Fact]
@@ -445,9 +460,11 @@ public async Task UpdatingAnEntryInEachProjectSyncsAcrossBoth()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                //todo the headword should be changed
-                .For(e => e.Components).Exclude(c => c.ComponentHeadword)
+                .For(e => e.Components).Exclude(c => c.Order)
                 .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                //todo the headwords should be changed
+                .For(e => e.Components).Exclude(c => c.ComponentHeadword)
                 .For(e => e.ComplexForms).Exclude(c => c.ComponentHeadword));
     }
 
@@ -517,7 +534,10 @@ public async Task AddingASenseToAnEntryInEachProjectSyncsAcrossBoth()
                 .For(e => e.Senses).Exclude(s => s.Order)
                 .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order)
                 .For(e => e.Components).Exclude(c => c.Id)
-                .For(e => e.ComplexForms).Exclude(c => c.Id));
+                .For(e => e.Components).Exclude(c => c.Order)
+                .For(e => e.ComplexForms).Exclude(c => c.Id)
+                .For(e => e.ComplexForms).Exclude(c => c.Order)
+                );
     }
 
     [Fact]