fix: fix a few scenarios and add a few test around foreign keys witho…

…ut reference nav props
IntelliTect · Oct 10, 2023 · 6997a5a · 6997a5a
1 parent c12c0da
commit 6997a5a
Show file tree

Hide file tree

Showing 8 changed files with 109 additions and 51 deletions.
diff --git a/src/IntelliTect.Coalesce.CodeGeneration.Vue/Generators/Scripts/TsMetadata.cs b/src/IntelliTect.Coalesce.CodeGeneration.Vue/Generators/Scripts/TsMetadata.cs
@@ -246,7 +246,7 @@ private void WriteClassPropertyMetadata(TypeScriptCodeBuilder b, ClassViewModel
                     case PropertyRole.CollectionNavigation:
                         // TS Type: "ModelCollectionNavigationProperty"
                         b.StringProp("role", "collectionNavigation");
-                        b.Line($"get foreignKey() {{ return {GetClassMetadataRef(prop.Object)}.props.{prop.InverseProperty.ForeignKeyProperty.JsVariable} as ForeignKeyProperty }},");
+                        b.Line($"get foreignKey() {{ return {GetClassMetadataRef(prop.Object)}.props.{prop.ForeignKeyProperty.JsVariable} as ForeignKeyProperty }},");
 
                         if (prop.InverseProperty != null)
                         {

diff --git a/src/IntelliTect.Coalesce.Tests/TargetClasses/TestDbContext/ComplexModel.cs b/src/IntelliTect.Coalesce.Tests/TargetClasses/TestDbContext/ComplexModel.cs
@@ -25,7 +25,7 @@ public class ComplexModel
         /// This configuration *will* be picked up by EF conventions.
         /// </summary>
         [ForeignKey(nameof(ComplexModelDependent.ParentId))]
-        public ICollection<ComplexModelDependent> Children { get; set; }
+        public ICollection<ComplexModelDependent> ChildrenWithoutRefNavProp { get; set; }
 
         public int SingleTestId { get; set; }
         public Test SingleTest { get; set; }

diff --git a/src/IntelliTect.Coalesce.Tests/Tests/TypeDefinition/PropertyViewModelTests.cs b/src/IntelliTect.Coalesce.Tests/Tests/TypeDefinition/PropertyViewModelTests.cs
@@ -134,7 +134,7 @@ public void IsBool_CorrectForBoolProperties(ClassViewModelData data)
             }
         }
 
-        [Theory, ClassViewModelData(typeof(ComplexModel))]
+        [Theory, ClassViewModelData<ComplexModel>]
         public void DateType_IsCorrect(ClassViewModelData data)
         {
             ClassViewModel vm = data;
@@ -151,8 +151,8 @@ public void DateType_IsCorrect(ClassViewModelData data)
         }
 
         [Theory]
-        [PropertyViewModelData(typeof(ComplexModel), nameof(ComplexModel.SingleTestId))]
-        [PropertyViewModelData(typeof(ComplexModelDependent), nameof(ComplexModelDependent.ParentId))]
+        [PropertyViewModelData<ComplexModel>(nameof(ComplexModel.SingleTestId))]
+        [PropertyViewModelData<ComplexModelDependent>(nameof(ComplexModelDependent.ParentId))]
         public void IsForeignKey_IsCorrect(PropertyViewModelData data)
         {
             PropertyViewModel vm = data;
@@ -161,5 +161,16 @@ public void IsForeignKey_IsCorrect(PropertyViewModelData data)
             Assert.NotNull(vm.ForeignKeyPrincipalType);
             Assert.Equal(PropertyRole.ForeignKey, vm.Role);
         }
+
+        [Theory]
+        [PropertyViewModelData<ComplexModel>(nameof(ComplexModel.Tests), nameof(Test.ComplexModelId))]
+        [PropertyViewModelData<ComplexModel>(nameof(ComplexModel.ChildrenWithoutRefNavProp), nameof(ComplexModelDependent.ParentId))]
+        public void Role_IsCollectionNavigation_IsCorrect(PropertyViewModelData data, string fkName)
+        {
+            PropertyViewModel vm = data;
+
+            Assert.Equal(PropertyRole.CollectionNavigation, vm.Role);
+            Assert.Equal(fkName, vm.ForeignKeyProperty.Name);
+        }
     }
 }
diff --git a/src/IntelliTect.Coalesce.Tests/Util/ClassViewModelDataAttribute.cs b/src/IntelliTect.Coalesce.Tests/Util/ClassViewModelDataAttribute.cs
@@ -34,31 +34,10 @@ public override IEnumerable<object[]> GetData(MethodInfo testMethod)
         }
     }
 
-    internal class PropertyViewModelDataAttribute : Xunit.Sdk.DataAttribute
+    internal class ClassViewModelDataAttribute<T> : ClassViewModelDataAttribute
     {
-        private readonly Type targetClass;
-        private readonly string propName;
-        private readonly object[] inlineData;
-
-        protected bool reflection = true;
-        protected bool symbol = true;
-
-        public PropertyViewModelDataAttribute(Type targetClass, string propName, params object[] additionalInlineData)
+        public ClassViewModelDataAttribute(params object[] additionalInlineData) : base(typeof(T), additionalInlineData)
         {
-            this.targetClass = targetClass;
-            this.propName = propName;
-            this.inlineData = additionalInlineData;
-        }
-
-        public override IEnumerable<object[]> GetData(MethodInfo testMethod)
-        {
-            if (reflection) yield return new[] {
-                new PropertyViewModelData(targetClass, propName, typeof(ReflectionClassViewModel))
-            }.Concat(inlineData).ToArray();
-
-            if (symbol) yield return new[] {
-                new PropertyViewModelData(targetClass, propName, typeof(SymbolClassViewModel))
-            }.Concat(inlineData).ToArray();
         }
     }
 

diff --git a/src/IntelliTect.Coalesce.Tests/Util/PropertyViewModelDataAttribute.cs b/src/IntelliTect.Coalesce.Tests/Util/PropertyViewModelDataAttribute.cs
@@ -0,0 +1,43 @@
+using IntelliTect.Coalesce.TypeDefinition;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+
+namespace IntelliTect.Coalesce.Tests.Util
+{
+    internal class PropertyViewModelDataAttribute : Xunit.Sdk.DataAttribute
+    {
+        private readonly Type targetClass;
+        private readonly string propName;
+        private readonly object[] inlineData;
+
+        protected bool reflection = true;
+        protected bool symbol = true;
+
+        public PropertyViewModelDataAttribute(Type targetClass, string propName, params object[] additionalInlineData)
+        {
+            this.targetClass = targetClass;
+            this.propName = propName;
+            this.inlineData = additionalInlineData;
+        }
+
+        public override IEnumerable<object[]> GetData(MethodInfo testMethod)
+        {
+            if (reflection) yield return new[] {
+                new PropertyViewModelData(targetClass, propName, typeof(ReflectionClassViewModel))
+            }.Concat(inlineData).ToArray();
+
+            if (symbol) yield return new[] {
+                new PropertyViewModelData(targetClass, propName, typeof(SymbolClassViewModel))
+            }.Concat(inlineData).ToArray();
+        }
+    }
+
+    internal class PropertyViewModelDataAttribute<T> : PropertyViewModelDataAttribute
+    {
+        public PropertyViewModelDataAttribute(string propName, params object[] additionalInlineData) : base(typeof(T), propName, additionalInlineData)
+        {
+        }
+    }
+}
diff --git a/src/IntelliTect.Coalesce/TypeDefinition/PropertyViewModel.cs b/src/IntelliTect.Coalesce/TypeDefinition/PropertyViewModel.cs
@@ -234,7 +234,7 @@ propName is null
         public bool IsClientWritable => this switch
         {
             { Role: PropertyRole.ReferenceNavigation } => ForeignKeyProperty!.IsClientWritable,
-            { Role: PropertyRole.CollectionNavigation } => InverseProperty!.IsClientWritable,
+            { Role: PropertyRole.CollectionNavigation } => InverseProperty?.IsClientWritable == true,
             { IsAutoGeneratedPrimaryKey: true } => false,
             _ => IsClientSerializable,
         };
@@ -474,36 +474,53 @@ public DatabaseGeneratedOption DatabaseGenerated
         }
 
         /// <summary>
-        /// If this is a reference navigation property, returns the property that holds the foreign key. 
+        /// If this is a navigation property, returns the property that holds the foreign key. 
         /// </summary>
         public PropertyViewModel? ForeignKeyProperty
         {
             get
             {
-                // ForeignKeyProperty only has meaning on reference navigation props.
-                // If this prop isn't a POCO, that definitely isn't true.
-                if (!Type.IsPOCO) return null;
-
                 // Types/props that aren't DB mapped don't have properties that have relational meaning.
                 // EffectiveParent used here to correctly handle properties on base classes -
                 // we need to know that the class that is ultimately used is DB mapped.
                 if (!IsDbMapped || !EffectiveParent.IsDbMappedType) return null;
 
-                var name =
-                    // Use the foreign key attribute
-                    this.GetAttributeValue<ForeignKeyAttribute>(a => a.Name)
+                PropertyViewModel? prop = null;
+                if (Type.IsCollection)
+                {
+                    // `this` may be a collection navigation prop
 
-                    // Use the ForeignKey Attribute on the key property if it is there.
-                    ?? EffectiveParent.Properties.SingleOrDefault(p => Name == p.GetAttributeValue<ForeignKeyAttribute>(a => a.Name))?.Name
+                    if (InverseProperty?.ForeignKeyProperty is { } fk) return fk;
 
-                    // See if this is a one-to-one using the parent's key
-                    // Look up the other object and check the key
-                    ?? (Object?.IsOneToOne ?? false ? EffectiveParent.PrimaryKey?.Name : null)
+                    // Handle [ForeignKeyAttribute] on collection navigations
+                    // for relationships that lack a reference navigation property.
+                    var name = this.GetAttributeValue<ForeignKeyAttribute>(a => a.Name);
+                    if (name is not null)
+                    {
+                        prop = PureType.ClassViewModel?.PropertyByName(name);
+                    }
+                }
+                else if (Type.IsPOCO)
+                {
+                    // `this` may be a reference navigation prop
 
-                    // Look for a property that follows convention.
-                    ?? Name + ConventionalIdSuffix;
+                    var name =
+                        // Use the foreign key attribute
+                        this.GetAttributeValue<ForeignKeyAttribute>(a => a.Name)
+
+                        // Use the ForeignKey Attribute on the key property if it is there.
+                        ?? EffectiveParent.Properties.SingleOrDefault(p => Name == p.GetAttributeValue<ForeignKeyAttribute>(a => a.Name))?.Name
+
+                        // See if this is a one-to-one using the parent's key
+                        // Look up the other object and check the key
+                        ?? (Object?.IsOneToOne ?? false ? EffectiveParent.PrimaryKey?.Name : null)
+
+                        // Look for a property that follows convention.
+                        ?? Name + ConventionalIdSuffix;
+
+                    prop = EffectiveParent.PropertyByName(name);
+                }
 
-                var prop = EffectiveParent.PropertyByName(name);
                 if (prop == null || !prop.Type.IsValidKeyType || !prop.IsDbMapped)
                 {
                     return null;
@@ -573,9 +590,10 @@ public ClassViewModel? ForeignKeyPrincipalType
                     .OfType<PropertyViewModel>()
                     .FirstOrDefault(p => 
                         p.Type.IsCollection && 
-                        p.GetAttributeValue<ForeignKeyAttribute>(a => a.Name) == this.Name
+                        p.GetAttributeValue<ForeignKeyAttribute>(a => a.Name) == this.Name &&
+                        p.EffectiveParent.IsDbMappedType
                     )
-                    ?.Parent;
+                    ?.EffectiveParent;
             }
         }
 
@@ -781,7 +799,10 @@ public PropertyRole Role
                 var obj = Object;
                 if (obj != null && obj.IsDbMappedType)
                 {
-                    if (Type.IsCollection && obj.PrimaryKey != null && InverseProperty != null)
+                    if (Type.IsCollection && 
+                        obj.PrimaryKey != null && 
+                        (InverseProperty != null || HasAttribute<ForeignKeyAttribute>())
+                    )
                     {
                         return PropertyRole.CollectionNavigation;
                     }

diff --git a/src/IntelliTect.Coalesce/Validation/ValidateContext.cs b/src/IntelliTect.Coalesce/Validation/ValidateContext.cs
@@ -111,8 +111,12 @@ public static ValidationHelper Validate(ReflectionRepository repository)
                             }
                             if (prop.Role == PropertyRole.CollectionNavigation)
                             {
-                                assert.IsTrue(prop.InverseProperty!.IsPOCO, "The inverse property of a collection navigation should reference the corresponding reference navigation on the other side of the relationship.");
-                                assert.IsNotNull(prop.InverseProperty.ForeignKeyProperty, "Could not find the foreign key of the referenced inverse property");
+                                if (prop.InverseProperty != null)
+                                {
+                                    assert.IsTrue(prop.InverseProperty.IsPOCO, "The inverse property of a collection navigation should reference the corresponding reference navigation on the other side of the relationship.");
+                                }
+
+                                assert.IsNotNull(prop.ForeignKeyProperty, "Could not find the foreign key of the navigation property");
                             }
                         }
                         if (prop.IsManytoManyCollection &&

diff --git a/src/coalesce-vue-vuetify2/src/components/input/c-list-filters.vue b/src/coalesce-vue-vuetify2/src/components/input/c-list-filters.vue
@@ -80,7 +80,7 @@
               :for="
                 filter.propMeta.role == 'primaryKey'
                   ? list.$metadata.name
-                  : filter.propMeta.navigationProp ??
+                  : filter.propMeta.navigationProp ||
                     filter.propMeta.principalType
               "
               clearable