Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/1246 allow reordering complex form components #1385

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
+1,414 −515
Open

Feat/1246 allow reordering complex form components #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0e123ee
Enable reordering complex form components
myieye Jan 16, 2025
de7e10e
Remove outdated tests
myieye Jan 16, 2025
0a031e5
write deserialization tests to ensure we can always deserialize chang…
hahn-kev Jan 20, 2025
37f1a75
pull sync exclusions out into a helper method
hahn-kev Jan 22, 2025
a46fa12
remove OrderableObjectWithIdCollectionDiffApi and provide a default i…
hahn-kev Jan 22, 2025
acf57f6
rename ExistingJson.json to RegressionDeserializationData.json and ad…
hahn-kev Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
387 changes: 0 additions & 387 deletions backend/FwLite/FwDataMiniLcmBridge.Tests/UpdateComplexFormsTests.cs
View file
Open in desktop

This file was deleted.

90 changes: 84 additions & 6 deletions backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,20 +715,44 @@ public async Task<Entry> CreateEntry(Entry entry)
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",
Expand Down Expand Up @@ -769,12 +793,58 @@ public Task RemoveComplexFormType(Guid entryId, Guid complexFormTypeId)
/// <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)
Expand Down Expand Up @@ -909,6 +979,7 @@ internal void InsertSense(ILexEntry lexEntry, ILexSense lexSense, BetweenPositio
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
Expand All @@ -918,6 +989,7 @@ internal void InsertSense(ILexEntry lexEntry, ILexSense lexSense, BetweenPositio
: 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;
Expand All @@ -931,6 +1003,7 @@ internal void InsertSense(ILexEntry lexEntry, ILexSense lexSense, BetweenPositio
: 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;
}
Expand All @@ -947,6 +1020,7 @@ internal void InsertExampleSentence(ILexSense lexSense, ILexExampleSentence lexE
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;
}
Expand All @@ -955,6 +1029,7 @@ internal void InsertExampleSentence(ILexSense lexSense, ILexExampleSentence lexE
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;
}
Expand Down Expand Up @@ -1045,7 +1120,6 @@ public Task MoveSense(Guid entryId, Guid senseId, BetweenPosition between)
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;
Expand Down Expand Up @@ -1168,7 +1242,6 @@ public Task MoveExampleSentence(Guid entryId, Guid senseId, Guid exampleSentence
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;
Expand Down Expand Up @@ -1200,4 +1273,9 @@ private static void ValidateOwnership(ILexExampleSentence lexExampleSentence, Gu
lexExampleSentence.Owner.ClassName);
}
}

public ProjectDataFormat GetDataFormat()
{
return ProjectDataFormat.FwData;
}
}
12 changes: 2 additions & 10 deletions backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
57 changes: 56 additions & 1 deletion backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using FwLiteProjectSync.Tests.Fixtures;
using MiniLcm;
using MiniLcm.Models;
using MiniLcm.SyncHelpers;
using MiniLcm.Tests.AutoFakerHelpers;
Expand Down Expand Up @@ -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)
);
}

Expand Down Expand Up @@ -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));
}
}
6 changes: 1 addition & 5 deletions backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ private void ShouldAllBeEquivalentTo(Dictionary<Guid, Entry> crdtEntries, Dictio
{
var fwdataEntry = fwdataEntries[crdtEntry.Id];
crdtEntry.Should().BeEquivalentTo(fwdataEntry,
options => options
.For(e => e.Components).Exclude(c => c.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id)
.For(e => e.Senses).Exclude(s => s.Order)
.For(e => e.Senses).For(s => s.ExampleSentences).Exclude(s => s.Order),
SyncTests.SyncExclusions,
$"CRDT entry {crdtEntry.Id} was synced with FwData");
}
}
Expand Down
61 changes: 19 additions & 42 deletions backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,17 @@ public SyncTests(SyncFixture fixture)
_syncService = _fixture.SyncService;
}

internal static EquivalencyOptions<Entry> SyncExclusions(EquivalencyOptions<Entry> options)
{
return 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.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]
[Trait("Category", "Integration")]
public async Task FirstSyncJustDoesAnImport()
Expand All @@ -83,12 +94,7 @@ public async Task FirstSyncJustDoesAnImport()

var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id));
crdtEntries.Should().BeEquivalentTo(fwdataEntries, SyncExclusions);
}

[Fact]
Expand Down Expand Up @@ -151,12 +157,7 @@ await crdtApi.CreateEntry(new Entry()

var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id));
crdtEntries.Should().BeEquivalentTo(fwdataEntries, SyncExclusions);
}

[Fact]
Expand Down Expand Up @@ -234,12 +235,7 @@ public async Task CreatingAComplexEntryInFwDataSyncsWithoutIssue()

var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id));
crdtEntries.Should().BeEquivalentTo(fwdataEntries, SyncExclusions);

// Sync again, ensure no problems or changes
var secondSync = await _syncService.Sync(crdtApi, fwdataApi);
Expand Down Expand Up @@ -329,12 +325,7 @@ await crdtApi.CreateEntry(new Entry()

var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id));
crdtEntries.Should().BeEquivalentTo(fwdataEntries, SyncExclusions);
}

[Fact]
Expand Down Expand Up @@ -413,12 +404,7 @@ await crdtApi.CreateEntry(new Entry()

var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id));
crdtEntries.Should().BeEquivalentTo(fwdataEntries, SyncExclusions);
}

[Fact]
Expand All @@ -441,13 +427,9 @@ public async Task UpdatingAnEntryInEachProjectSyncsAcrossBoth()
var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
//todo the headword should be changed
options => SyncExclusions(options)
//todo the headwords should be changed
.For(e => e.Components).Exclude(c => c.ComponentHeadword)
.For(e => e.ComplexForms).Exclude(c => c.Id)
.For(e => e.ComplexForms).Exclude(c => c.ComponentHeadword));
}

Expand Down Expand Up @@ -512,12 +494,7 @@ public async Task AddingASenseToAnEntryInEachProjectSyncsAcrossBoth()

var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync();
var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync();
crdtEntries.Should().BeEquivalentTo(fwdataEntries,
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.Id)
.For(e => e.ComplexForms).Exclude(c => c.Id));
crdtEntries.Should().BeEquivalentTo(fwdataEntries, SyncExclusions);
}

[Fact]
Expand Down
Loading