fix: handle tph during bulk save collection fixup

Author: ascott18

.vscode/settings.json

@@ -1,3 +1,9 @@
 {
-    "dotnet.defaultSolution": "Coalesce.sln"
-}
+    "dotnet.defaultSolution": "Coalesce.sln",
+    
+    "editor.formatOnPaste": false,
+    "editor.formatOnSave": true,
+    "editor.formatOnType": true,
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
+  }
+  

CHANGELOG.md

@@ -1,26 +1,22 @@
-# 5.3.5
+# 5.3.6
 
-## Fixes
+- Fix error in bulk saves where models that belong to inheritance hierarchies would get skipped over while performing collection fixup.
+
+# 5.3.5
 
 - Fix error in c-select-many-to-many when the near side is part of an inheritance hierarchy.
 
 # 5.3.4
 
-## Fixes
-
 - Adjust DateTimeOffset handling to support Postgres restrictions of not allowing non-zero offsets.
 
 # 5.3.3
 
-## Fixes
-
 - Add model validation error when ForeignKeyAttribute references a navigation prop that doesn't exist.
 - Fix code gen performance regression introduced in 5.3.2
 
 # 5.3.2
 
-## Fixes
-
 - Fix error in .NET 9 thrown by vite development middleware if the HTTPS cert directory doesn't exist.
 - Fix `ViewModel.$load` and `ListViewModel.$load` not properly working with `.useResponseCaching()`.
 - Entities that own multiple one-to-one relationships should no longer throw errors when generating.
@@ -30,8 +26,6 @@
 
 # 5.3.1
 
-## Fixes
-
 - Fix error thrown by `CoalesceApiDescriptionProvider` when a custom method on a model or service has an explicitly `[Inject]`ed data source parameter.
 
 # 5.3.0

src/IntelliTect.Coalesce.Tests/TargetClasses/TestDbContext/AbstractModel.cs

@@ -1,4 +1,6 @@
 using IntelliTect.Coalesce.DataAnnotations;
+using IntelliTect.Coalesce.Tests.TargetClasses.TestDbContext;
+using System.Collections.Generic;
 
 namespace IntelliTect.Coalesce.Tests.TargetClasses
 {
@@ -10,12 +12,32 @@ public abstract class AbstractModel
     {
         public int Id { get; set; }
 
-        public string Discriminatior { get; set; }
+        public string Discriminator { get; set; }
+
+        [ManyToMany("People")]
+        public List<AbstractModelPerson> AbstractModelPeople { get; set; }
+    }
+
+    [Edit(PermissionLevel = SecurityPermissionLevels.DenyAll)]
+    public class AbstractImpl1 : AbstractModel
+    {
+        public string Impl1OnlyField { get; set; }
     }
 
     [Edit(PermissionLevel = SecurityPermissionLevels.DenyAll)]
-    public class AbstractImpl : AbstractModel
+    public class AbstractImpl2 : AbstractModel
     {
-        public string ImplOnlyField { get; set; }
+        public string Impl2OnlyField { get; set; }
+    }
+
+    public class AbstractModelPerson
+    {
+        public int Id { get; set; }
+
+        public int PersonId { get; set; }
+        public Person Person { get; set; }
+
+        public int AbstractModelId { get; set; }
+        public AbstractModel AbstractModel { get; set; }
     }
 }

src/IntelliTect.Coalesce.Tests/TargetClasses/TestDbContext/TestDbContext.cs

@@ -24,7 +24,9 @@ public class AppDbContext : DbContext
         public DbSet<Test> Tests { get; set; }
 
         public DbSet<AbstractModel> AbstractModels { get; set; }
-        public DbSet<AbstractImpl> AbstractImpls { get; set; }
+        public DbSet<AbstractModelPerson> AbstractModelPeople { get; set; }
+        public DbSet<AbstractImpl1> AbstractImpl1s { get; set; }
+        public DbSet<AbstractImpl2> AbstractImpl2s { get; set; }
 
         public DbSet<EnumPk> EnumPks { get; set; }
         public DbSet<ZipCode> ZipCodes { get; set; }
@@ -60,8 +62,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             base.OnModelCreating(modelBuilder);
 
             modelBuilder.Entity<AbstractModel>()
-                .HasDiscriminator(b => b.Discriminatior)
-                .HasValue<AbstractImpl>("impl");
+                .HasDiscriminator(b => b.Discriminator)
+                .HasValue<AbstractImpl1>("impl1")
+                .HasValue<AbstractImpl2>("impl2");
         }
     }
 

src/IntelliTect.Coalesce.Tests/Tests/TypeDefinition/ClassViewModelTests.cs

@@ -44,7 +44,7 @@ public void DefaultOrderBy_UsesNavigationDirectlyWhenOrderingByUnorderableRefNav
         }
 
         [Theory]
-        [ClassViewModelData(typeof(AbstractImpl))]
+        [ClassViewModelData(typeof(AbstractImpl1))]
         public void GetAttribute_RespectsInheritance(ClassViewModelData data)
         {
             var vm = data.ClassViewModel;

src/coalesce-vue/src/viewmodel.ts

@@ -744,7 +744,12 @@ export abstract class ViewModel<
             if (
               !principal &&
               collection?.$parent instanceof ViewModel &&
-              collection.$metadata == prop.inverseNavigation
+              (collection.$metadata == prop.inverseNavigation ||
+                // Handle inheritance hierarchies (i.e. TPH):
+                // The collection is the same collection, but has a different metadata instance
+                // so direct equality won't work.
+                (collection.$metadata.name == prop.inverseNavigation?.name &&
+                  collection.$metadata.type == prop.inverseNavigation.type))
             ) {
               // The reference navigation property has no value,
               // and the foreign key has no value,

src/coalesce-vue/test/viewmodel.spec.ts

@@ -37,6 +37,7 @@ import {
   ProductViewModel,
   TestViewModel,
   ZipCodeViewModel,
+  AbstractImpl1ViewModel,
 } from "../../test-targets/viewmodels.g";
 import {
   StudentViewModel,
@@ -1402,6 +1403,52 @@ describe("ViewModel", () => {
 
       bulkSaveEndpoint.destroy();
     });
+
+    test("creation of TPH entity with many-to-many that references base type", async () => {
+      const bulkSaveEndpoint = mockEndpoint(
+        "/AbstractImpl1/bulkSave",
+        vitest.fn((req) => ({
+          wasSuccessful: true,
+          object: null,
+        }))
+      );
+
+      // This tests the fact that the metadata instance of
+      // `AbstractImpl1ViewModel.abstractModelPeople.$metadata` is the one owned by `AbstractImpl1`'s metadata,
+      // but the metadata instance of `AbstractModelPerson.$metadata.props.abstractModel.inverseNavigation`
+      // is the copy of the same collection that is owned by `AbstractModel`'s metadata.
+      //
+      // The collection navigation fixup logic in bulk saves has to allow for this type to be different.
+
+      const person = new PersonViewModel();
+      person.$loadCleanData({ personId: 1, name: "bob" });
+      const vm = new AbstractImpl1ViewModel({ impl1OnlyField: "fieldval" });
+      vm.abstractModelPeople.push({ person });
+      await vm.$bulkSave();
+
+      expect(JSON.parse(bulkSaveEndpoint.mock.calls[0][0].data)).toMatchObject({
+        items: [
+          {
+            action: "save",
+            type: "AbstractImpl1",
+            data: { id: null, impl1OnlyField: "fieldval" },
+            refs: { id: vm.$stableId },
+            root: true,
+          },
+          {
+            action: "save",
+            type: "AbstractModelPerson",
+            data: { id: null },
+            refs: {
+              id: vm.abstractModelPeople[0].$stableId,
+              abstractModelId: vm.$stableId,
+            },
+          },
+        ],
+      });
+
+      bulkSaveEndpoint.destroy();
+    });
   });
 
   describe("autoSave", () => {