Make Sense.PartOfSpeech an object instead of a string

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

@@ -206,7 +206,7 @@
         }
         await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",
             "Revert WritingSystem",
             async () =>
             {
                 var updateProxy = new UpdateWritingSystemProxy(lcmWritingSystem, this)
                 {
@@ -568,14 +568,15 @@
     private Sense FromLexSense(ILexSense sense)
     {
         var enWs = GetWritingSystemHandle("en");
+        var pos = sense.MorphoSyntaxAnalysisRA?.GetPartOfSpeech();
         var s =  new Sense
         {
             Id = sense.Guid,
             EntryId = sense.Entry.Guid,
             Gloss = FromLcmMultiString(sense.Gloss),
             Definition = FromLcmMultiString(sense.Definition),
-            PartOfSpeech = sense.MorphoSyntaxAnalysisRA?.GetPartOfSpeech()?.Name.get_String(enWs).Text ?? "",
-            PartOfSpeechId = sense.MorphoSyntaxAnalysisRA?.GetPartOfSpeech()?.Guid,
+            PartOfSpeech = pos is null ? null : FromLcmPartOfSpeech(pos),
+            PartOfSpeechId = pos?.Guid,
             SemanticDomains = sense.SemanticDomainsRC.Select(FromLcmSemanticDomain).ToList(),
             ExampleSentences = sense.ExamplesOS.Select(sentence => FromLexExampleSentence(sense.Guid, sentence)).ToList()
         };
@@ -887,8 +888,10 @@
         var lexSense = LexSenseFactory.Create(sense.Id);
         InsertSense(lexEntry, lexSense, between);
         var msa = new SandboxGenericMSA() { MsaType = lexSense.GetDesiredMsaType() };
-        if (sense.PartOfSpeechId.HasValue && PartOfSpeechRepository.TryGetObject(sense.PartOfSpeechId.Value, out var pos))
+        if (sense.PartOfSpeechId.HasValue)
         {
+            var found = PartOfSpeechRepository.TryGetObject(sense.PartOfSpeechId.Value, out var pos);
+            if (!found) throw new InvalidOperationException($"Part of speech must exist when creating a sense (could not find GUID {sense.PartOfSpeechId.Value})");
             msa.MainPOS = pos;
         }
         lexSense.SandboxMSA = msa;

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

@@ -25,7 +25,7 @@
         set => throw new NotImplementedException();
     }
 
-    public override string PartOfSpeech
+    public override PartOfSpeech? PartOfSpeech
     {
         get => throw new NotImplementedException();
         set { }
@@ -118,7 +118,7 @@
         get =>
             new UpdateListProxy<ExampleSentence>(
                 sentence => lexboxLcmApi.CreateExampleSentence(sense, sentence),
                 sentence => lexboxLcmApi.DeleteExampleSentence(sense.Owner.Guid, Id, sentence.Id),
                 i => new UpdateExampleSentenceProxy(sense.ExamplesOS[i], lexboxLcmApi),
                 sense.ExamplesOS.Count
             );

backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs

@@ -76,6 +76,7 @@ private async Task WorkaroundMissingWritingSystems()
     }
 
     [Fact]
+    [Trait("Category", "Integration")]
     public async Task DryRunImport_MakesNoChanges()
     {
         await WorkaroundMissingWritingSystems();
@@ -86,6 +87,7 @@ public async Task DryRunImport_MakesNoChanges()
     }
 
     [Fact]
+    [Trait("Category", "Integration")]
     public async Task DryRunImport_MakesTheSameChangesAsImport()
     {
         var dryRunSyncResult = await _syncService.SyncDryRun(_crdtApi, _fwDataApi);
@@ -94,6 +96,7 @@ public async Task DryRunImport_MakesTheSameChangesAsImport()
     }
 
     [Fact]
+    [Trait("Category", "Integration")]
     public async Task DryRunSync_MakesNoChanges()
     {
         await BypassImport();
@@ -106,6 +109,7 @@ public async Task DryRunSync_MakesNoChanges()
 
     [Fact]
     [Trait("Category", "Slow")]
+    [Trait("Category", "Integration")]
     public async Task DryRunSync_MakesTheSameChangesAsSync()
     {
         //syncing requires querying entries, which fails if there are no writing systems, so we import those first
@@ -120,6 +124,7 @@ public async Task DryRunSync_MakesTheSameChangesAsSync()
     }
 
     [Fact]
+    [Trait("Category", "Integration")]
     public async Task FirstSena3SyncJustDoesAnSync()
     {
         _fwDataApi.EntryCount.Should().BeGreaterThan(1000,
@@ -137,6 +142,7 @@ public async Task FirstSena3SyncJustDoesAnSync()
 
     [Fact]
     [Trait("Category", "Slow")]
+    [Trait("Category", "Integration")]
     public async Task SyncWithoutImport_CrdtShouldMatchFwdata()
     {
         await BypassImport();
@@ -152,6 +158,7 @@ public async Task SyncWithoutImport_CrdtShouldMatchFwdata()
     }
 
     [Fact]
+    [Trait("Category", "Integration")]
     public async Task SecondSena3SyncDoesNothing()
     {
         await _syncService.Sync(_crdtApi, _fwDataApi);

backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs

@@ -42,7 +42,7 @@ public void SenseDiffShouldUpdateAllFields()
         var senseDiffToUpdate = SenseSync.SenseDiffToUpdate(before, after);
         ArgumentNullException.ThrowIfNull(senseDiffToUpdate);
         senseDiffToUpdate.Apply(before);
-        before.Should().BeEquivalentTo(after, options => options.Excluding(x => x.Id).Excluding(x => x.EntryId).Excluding(x => x.DeletedAt).Excluding(x => x.ExampleSentences).Excluding(x => x.SemanticDomains));
+        before.Should().BeEquivalentTo(after, options => options.Excluding(x => x.Id).Excluding(x => x.EntryId).Excluding(x => x.DeletedAt).Excluding(x => x.ExampleSentences).Excluding(x => x.SemanticDomains).Excluding(x => x.PartOfSpeech));
     }
 
     [Fact]

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

@@ -37,13 +37,4 @@ public void NewChangeIPatchDoc_ThrowsForRemoveAtIndex()
         var act = () => new JsonPatchChange<Entry>(Guid.NewGuid(), patch);
         act.Should().Throw<NotSupportedException>();
     }
-
-    [Fact]
-    public void NewPatchDoc_ThrowsForIndexBasedPath()
-    {
-        var patch = new JsonPatchDocument<Entry>();
-        patch.Replace(entry => entry.Senses[0].PartOfSpeech, "noun");
-        var act = () => new JsonPatchChange<Entry>(Guid.NewGuid(), patch);
-        act.Should().Throw<NotSupportedException>();
-    }
 }

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

@@ -201,21 +201,23 @@
         Annotations: 
           Relational:ColumnType: jsonb
       Order (double) Required
-      PartOfSpeech (string) Required
-      PartOfSpeechId (Guid?)
+      PartOfSpeechId (Guid?) FK Index
       SemanticDomains (IList<SemanticDomain>) Required
         Annotations: 
           Relational:ColumnType: jsonb
       SnapshotId (no field, Guid?) Shadow FK Index
     Navigations: 
       ExampleSentences (IList<ExampleSentence>) Collection ToDependent ExampleSentence
+      PartOfSpeech (PartOfSpeech) ToPrincipal PartOfSpeech
     Keys: 
       Id PK
     Foreign keys: 
       Sense {'EntryId'} -> Entry {'Id'} Required Cascade ToDependent: Senses
+      Sense {'PartOfSpeechId'} -> PartOfSpeech {'Id'} ClientSetNull ToPrincipal: PartOfSpeech
       Sense {'SnapshotId'} -> ObjectSnapshot {'Id'} Unique SetNull
     Indexes: 
       EntryId
+      PartOfSpeechId
       SnapshotId Unique
     Annotations: 
       DiscriminatorProperty: 

backend/FwLite/LcmCrdt.Tests/JsonPatchSenseRewriteTests.cs

@@ -11,14 +11,20 @@ public class JsonPatchSenseRewriteTests
 {
     private JsonPatchDocument<MiniLcm.Models.Sense> _patchDocument = new() { Options = new JsonSerializerOptions(JsonSerializerDefaults.Web) };
 
-    private Sense _sense = new Sense()
+    private Sense _sense = MakeSense("test");
+
+    private static Sense MakeSense(string name)
     {
-        Id = Guid.NewGuid(),
-        EntryId = Guid.NewGuid(),
-        PartOfSpeechId = Guid.NewGuid(),
-        PartOfSpeech = "test",
-        SemanticDomains = [new SemanticDomain() { Id = Guid.NewGuid(), Code = "test", Name = new MultiString() }],
-    };
+        var pos = new PartOfSpeech() { Id = Guid.NewGuid(), Name = {{ "en", name }} };
+        return new Sense()
+        {
+            Id = Guid.NewGuid(),
+            EntryId = Guid.NewGuid(),
+            PartOfSpeech = pos,
+            PartOfSpeechId = pos.Id,
+            SemanticDomains = [new SemanticDomain() { Id = Guid.NewGuid(), Code = "test", Name = new MultiString() }],
+        };
+    }
 
     [Fact]
     public void RewritePartOfSpeechChangesIntoSetPartOfSpeechChange()

backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs

@@ -15,8 +15,7 @@ public CreateSenseChange(Sense sense, Guid entryId) : base(sense.Id == Guid.Empt
         Definition = sense.Definition;
         SemanticDomains = sense.SemanticDomains;
         Gloss = sense.Gloss;
-        PartOfSpeech = sense.PartOfSpeech;
-        PartOfSpeechId = sense.PartOfSpeechId;
+        PartOfSpeechId = sense.PartOfSpeech?.Id ?? sense.PartOfSpeechId;
     }
 
     [JsonConstructor]
@@ -29,7 +28,6 @@ private CreateSenseChange(Guid entityId, Guid entryId) : base(entityId)
     public double Order { get; set; }
     public MultiString? Definition { get; set; }
     public MultiString? Gloss { get; set; }
-    public string? PartOfSpeech { get; set; }
     public Guid? PartOfSpeechId { get; set; }
     public IList<SemanticDomain>? SemanticDomains { get; set; }
 
@@ -42,7 +40,6 @@ public override async ValueTask<Sense> NewEntity(Commit commit, ChangeContext co
             Order = Order,
             Definition = Definition ?? new MultiString(),
             Gloss = Gloss ?? new MultiString(),
-            PartOfSpeech = PartOfSpeech ?? string.Empty,
             PartOfSpeechId = PartOfSpeechId,
             SemanticDomains = SemanticDomains ?? [],
             DeletedAt = await context.IsObjectDeleted(EntryId) ? commit.DateTime : (DateTime?)null

backend/FwLite/LcmCrdt/Changes/SetPartOfSpeechChange.cs

@@ -12,18 +12,17 @@ public override async ValueTask ApplyChange(Sense entity, ChangeContext context)
         if (PartOfSpeechId is null)
         {
             entity.PartOfSpeechId = null;
-            entity.PartOfSpeech = string.Empty;
             return;
         }
 
         var partOfSpeech = await context.GetCurrent<PartOfSpeech>(PartOfSpeechId.Value);
         if (partOfSpeech is null or { DeletedAt: not null })
         {
             entity.PartOfSpeechId = null;
-            entity.PartOfSpeech = string.Empty;
+            entity.PartOfSpeech = null;
             return;
         }
         entity.PartOfSpeechId = partOfSpeech.Id;
-        entity.PartOfSpeech = partOfSpeech.Name["en"];
+        entity.PartOfSpeech = partOfSpeech;
     }
 }

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

@@ -16,7 +16,7 @@
 
 namespace LcmCrdt;
 
 public class CrdtMiniLcmApi(DataModel dataModel, CurrentProjectService projectService, LcmCrdtDbContext dbContext, MiniLcmValidators validators) : IMiniLcmApi
 {
     private Guid ClientId { get; } = projectService.ProjectData.ClientId;
     public ProjectData ProjectData => projectService.ProjectData;
@@ -276,6 +276,7 @@
         queryable = queryable
             .LoadWith(e => e.Senses)
             .ThenLoad(s => s.ExampleSentences)
+            .LoadWith(e => e.Senses).ThenLoad(s => s.PartOfSpeech)
             .LoadWith(e => e.ComplexForms)
             .LoadWith(e => e.Components)
             .AsQueryable()
@@ -295,6 +296,7 @@
         var entry = await Entries.AsTracking(false)
             .LoadWith(e => e.Senses)
             .ThenLoad(s => s.ExampleSentences)
+            .LoadWith(e => e.Senses).ThenLoad(s => s.PartOfSpeech)
             .LoadWith(e => e.ComplexForms)
             .LoadWith(e => e.Components)
             .AsQueryable()
@@ -321,16 +323,15 @@
     public async Task BulkCreateEntries(IAsyncEnumerable<Entry> entries)
     {
         var semanticDomains = await SemanticDomains.ToDictionaryAsync(sd => sd.Id, sd => sd);
-        var partsOfSpeech = await PartsOfSpeech.ToDictionaryAsync(p => p.Id, p => p);
         await dataModel.AddChanges(ClientId,
             entries.ToBlockingEnumerable()
-                .SelectMany(entry => CreateEntryChanges(entry, semanticDomains, partsOfSpeech))
+                .SelectMany(entry => CreateEntryChanges(entry, semanticDomains))
                 //force entries to be created first, this avoids issues where references are created before the entry is created
                 .OrderBy(c => c is CreateEntryChange ? 0 : 1)
         );
     }
 
-    private IEnumerable<IChange> CreateEntryChanges(Entry entry, Dictionary<Guid, SemanticDomain> semanticDomains, Dictionary<Guid, PartOfSpeech> partsOfSpeech)
+    private IEnumerable<IChange> CreateEntryChanges(Entry entry, Dictionary<Guid, SemanticDomain> semanticDomains)
     {
         yield return new CreateEntryChange(entry);
 
@@ -350,11 +351,6 @@
                 .Select(sd => semanticDomains.TryGetValue(sd.Id, out var selectedSd) ? selectedSd : null)
                 .OfType<MiniLcm.Models.SemanticDomain>()
                 .ToList();
-            if (sense.PartOfSpeechId is not null && partsOfSpeech.TryGetValue(sense.PartOfSpeechId.Value, out var partOfSpeech))
-            {
-                sense.PartOfSpeechId = partOfSpeech.Id;
-                sense.PartOfSpeech = partOfSpeech.Name["en"] ?? string.Empty;
-            }
             if (sense.Order != default) // we don't anticipate this being necessary, so we'll be strict for now
                 throw new InvalidOperationException("Order should not be provided when creating a sense");
             sense.Order = i++;
@@ -387,7 +383,7 @@
         ]);
         return await GetEntry(entry.Id) ?? throw new NullReferenceException();
 
         async IAsyncEnumerable<AddEntryComponentChange> ToComplexFormComponents(IList<ComplexFormComponent> complexFormComponents)
         {
             foreach (var complexFormComponent in complexFormComponents)
             {
@@ -471,12 +467,6 @@
         sense.SemanticDomains = await SemanticDomains
             .Where(sd => sense.SemanticDomains.Select(s => s.Id).Contains(sd.Id))
             .ToListAsync();
-        if (sense.PartOfSpeechId is not null)
-        {
-            var partOfSpeech = await PartsOfSpeech.FirstOrDefaultAsync(p => p.Id == sense.PartOfSpeechId);
-            sense.PartOfSpeechId = partOfSpeech?.Id;
-            sense.PartOfSpeech = partOfSpeech?.Name["en"] ?? string.Empty;
-        }
 
         yield return new CreateSenseChange(sense, entryId);
         foreach (var change in sense.ExampleSentences.Select(sentence =>
@@ -491,6 +481,7 @@
         var entry = await Entries.AsTracking(false)
             .LoadWith(e => e.Senses)
             .ThenLoad(s => s.ExampleSentences)
+            .LoadWith(e => e.Senses).ThenLoad(s => s.PartOfSpeech)
             .AsQueryable()
             .SingleOrDefaultAsync(e => e.Id == entryId);
         return entry?.Senses.FirstOrDefault(s => s.Id == id);
@@ -501,6 +492,9 @@
         if (sense.Order != default) // we don't anticipate this being necessary, so we'll be strict for now
             throw new InvalidOperationException("Order should not be provided when creating a sense");
 
+        if (sense.PartOfSpeechId.HasValue && await GetPartOfSpeech(sense.PartOfSpeechId.Value) is null)
+            throw new InvalidOperationException($"Part of speech must exist when creating a sense (could not find GUID {sense.PartOfSpeechId.Value})");
+
         sense.Order = await OrderPicker.PickOrder(Senses.Where(s => s.EntryId == entryId), between);
         await validators.ValidateAndThrow(sense);
         await dataModel.AddChanges(ClientId, await CreateSenseChanges(entryId, sense).ToArrayAsync());

backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs

@@ -20,14 +20,19 @@ public static async Task<Entry> EntryReadyForCreation(this AutoFaker autoFaker,
         foreach (var sense in entry.Senses)
         {
             sense.EntryId = entry.Id;
+            if (sense.PartOfSpeech is not null)
+            {
+                sense.PartOfSpeechId = sense.PartOfSpeech.Id;
+            }
             if (sense.PartOfSpeechId.HasValue)
             {
-                await api.CreatePartOfSpeech(new PartOfSpeech()
+                var pos = new PartOfSpeech()
                 {
                     Id = sense.PartOfSpeechId.Value,
-                    Predefined = false,
-                    Name = { { "en", sense.PartOfSpeech } }
-                });
+                    Name = { { "en", "generated pos" } }
+                };
+                await api.CreatePartOfSpeech(pos);
+                sense.PartOfSpeech = pos;
             }
             foreach (var senseSemanticDomain in sense.SemanticDomains)
             {

backend/FwLite/MiniLcm.Tests/BasicApiTestsBase.cs

@@ -359,13 +359,14 @@ public async Task CreateSense_WillCreateWithExistingDomains()
     }
 
     [Fact]
-    public async Task CreateSense_WontCreateMissingPartOfSpeech()
+    public async Task CreateSense_WillThrowExceptionWithMissingPartOfSpeech()
     {
         var senseId = Guid.NewGuid();
-        var createdSense = await Api.CreateSense(Entry1Id,
-            new Sense() { Id = senseId, PartOfSpeech = "test", PartOfSpeechId = Guid.NewGuid(), });
-        createdSense.Id.Should().Be(senseId);
-        createdSense.PartOfSpeechId.Should().BeNull("because the part of speech does not exist (or was deleted)");
+        var partOfSpeechId = Guid.NewGuid();
+        var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
+            await Api.CreateSense(Entry1Id, new Sense() { Id = senseId, PartOfSpeech = null, PartOfSpeechId = partOfSpeechId, })
+        );
+        exception.Should().NotBeNull("because the part of speech does not exist (or was deleted)");
     }
 
     [Fact]
@@ -377,7 +378,7 @@ public async Task CreateSense_WillCreateWthExistingPartOfSpeech()
         var partOfSpeech = await Api.GetPartsOfSpeech().SingleOrDefaultAsync(pos => pos.Id == partOfSpeechId);
         ArgumentNullException.ThrowIfNull(partOfSpeech);
         var createdSense = await Api.CreateSense(Entry1Id,
-            new Sense() { Id = senseId, PartOfSpeech = "test", PartOfSpeechId = partOfSpeechId, });
+            new Sense() { Id = senseId, PartOfSpeech = partOfSpeech, PartOfSpeechId = partOfSpeechId, });
         createdSense.Id.Should().Be(senseId);
         createdSense.PartOfSpeechId.Should().Be(partOfSpeechId, "because the part of speech does exist");
     }
@@ -400,7 +401,6 @@ public async Task UpdateSensePartOfSpeech()
             {
                 new Sense()
                 {
-                    PartOfSpeech = "test",
                     Definition = new MultiString
                     {
                         Values =
@@ -414,9 +414,10 @@ public async Task UpdateSensePartOfSpeech()
         var updatedSense = await Api.UpdateSense(entry.Id,
             entry.Senses[0].Id,
             new UpdateObjectInput<Sense>()
-                .Set(e => e.PartOfSpeech, "updated")//should be ignored
+                .Set(e => e.PartOfSpeech, new PartOfSpeech() { Id = Guid.NewGuid(), Name = {{"en","updated"}} }) // should be ignored
                 .Set(e => e.PartOfSpeechId, partOfSpeechId));
-        updatedSense.PartOfSpeech.Should().Be("Adverb");
+        updatedSense.PartOfSpeech.Should().NotBeNull();
+        updatedSense.PartOfSpeech.Name.Should().BeEquivalentTo(new MultiString() {{"en","Adverb"}});
         updatedSense.PartOfSpeechId.Should().Be(partOfSpeechId);
     }
 

backend/FwLite/MiniLcm.Tests/CreateEntryTestsBase.cs

@@ -26,9 +26,7 @@ public async Task CanCreateEntry_AutoFaker()
         var createdEntry = await Api.CreateEntry(entry);
         createdEntry.Should().BeEquivalentTo(entry, options => options
             .For(e => e.Components).Exclude(e => e.Id)
-            .For(e => e.ComplexForms).Exclude(e => e.Id)
-            //predefined is always true in fwdata bridge, so we need to exclude it for now
-            .For(e => e.Senses).For(s => s.SemanticDomains).Exclude(s => s.Predefined));
+            .For(e => e.ComplexForms).Exclude(e => e.Id));
     }
 
     [Fact]