From 35309b863c4fc16aa7bcdcb1b16e9ee489d9a3d0 Mon Sep 17 00:00:00 2001 From: kjac Date: Fri, 20 Jun 2014 10:56:29 +0200 Subject: [PATCH 1/5] Fixing the unit tests The call to DisposableTimer.DebugDuration() in ArchetypeValueConverter.ConvertDataToSource() expects the propertyType parameter to be not null, but the implementation of ArchetypeValueConverter.ConvertDataToSource() allows the propertyType parameter to be null. This breaks some of the unit tests. I've added a null check in the call to DisposableTimer.DebugDuration() to counter this issue. --- .../PropertyConverters/ArchetypeValueConverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Umbraco/Umbraco.Archetype/PropertyConverters/ArchetypeValueConverter.cs b/app/Umbraco/Umbraco.Archetype/PropertyConverters/ArchetypeValueConverter.cs index 32f56cf..ad99f1c 100644 --- a/app/Umbraco/Umbraco.Archetype/PropertyConverters/ArchetypeValueConverter.cs +++ b/app/Umbraco/Umbraco.Archetype/PropertyConverters/ArchetypeValueConverter.cs @@ -34,7 +34,7 @@ namespace Archetype.PropertyConverters if (!sourceString.DetectIsJson()) return defaultValue; - using (var timer = DisposableTimer.DebugDuration(string.Format("ConvertDataToSource ({0})", propertyType.PropertyTypeAlias))) + using (var timer = DisposableTimer.DebugDuration(string.Format("ConvertDataToSource ({0})", propertyType != null ? propertyType.PropertyTypeAlias : "null"))) { var archetype = ArchetypeHelper.Instance.DeserializeJsonToArchetype(sourceString, (propertyType != null ? propertyType.DataTypeId : -1), From 2f195dd588878d65aae35c645e9b5355fe29a543 Mon Sep 17 00:00:00 2001 From: kjac Date: Fri, 20 Jun 2014 10:58:21 +0200 Subject: [PATCH 2/5] Add default value overload to GetValue() Fix issue #142 --- .../Archetype.Tests/Models/FieldsetTests.cs | 29 +++++++++++++++++++ .../Models/ArchetypeFieldsetModel.cs | 28 ++++++++++++++++-- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs b/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs index e1bbc0d..ca9e169 100644 --- a/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs +++ b/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs @@ -6,6 +6,7 @@ using System.Text; using System.Threading.Tasks; using Archetype.PropertyConverters; using NUnit.Framework; +using Archetype.Tests.Serialization; namespace Archetype.Tests.Models { @@ -34,6 +35,17 @@ namespace Archetype.Tests.Models Assert.That(propertyValue == "Box 1 Title"); } + [Test] + public void Can_Get_Fieldset_Property_Default_Value_By_Alias() { + var converter = new ArchetypeValueConverter(); + var result = (Archetype.Models.ArchetypeModel)converter.ConvertDataToSource(null, _sampleJson, false); + + var fieldset = result.Fieldsets.First(); + var propertyValue = fieldset.GetValue("noSuchProperty", "noSuchProperty default value"); + + Assert.That(propertyValue == "noSuchProperty default value"); + } + [Test] public void Can_Convert_Property_Value_Types() { @@ -47,6 +59,23 @@ namespace Archetype.Tests.Models Assert.That(fieldset.GetValue("blurb") == "A blurb here"); } + [Test] + public void Can_Convert_Property_Default_Value_Types() { + var converter = new ArchetypeValueConverter(); + var result = (Archetype.Models.ArchetypeModel)converter.ConvertDataToSource(null, _sampleJson, false); + + var fieldset = result.Fieldsets.First(); + + Assert.That(fieldset.GetValue("noSuchInteger", 1234) == 1234); + Assert.That(fieldset.GetValue("noSuchBoolean", true) == true); + Assert.That(fieldset.GetValue("noSuchBoolean", false) == false); + Assert.That(fieldset.GetValue("noSuchString", "noSuchString default value") == "noSuchString default value"); + + var contactDetails = fieldset.GetValue("noSuchContact", new ContactDetails { Address = "some street, some city", Name = "someone" }); + Assert.That(contactDetails.Address == "some street, some city"); + Assert.That(contactDetails.Name == "someone"); + } + [Test] public void Returns_String_When_No_Type_Specified() { diff --git a/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs b/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs index 52d2c9b..f8b3edf 100644 --- a/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs +++ b/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs @@ -29,12 +29,36 @@ namespace Archetype.Models { var property = GetProperty(propertyAlias); - if (property == null || property.Value == null || string.IsNullOrEmpty(property.Value.ToString())) - return default(T); + if (IsEmptyProperty(property)) + { + return default(T); + } return property.GetValue(); } + // issue 142: support default T value supplied by caller + // this code would look nicer if the two GetValue() methods had one common implementation. + // however, this would require GetValue(string propertyAlias) to call the common implementation + // with a default(T) value, which could in theory result in a performance hit, if T for some reason + // is costly to instantiate. + public T GetValue(string propertyAlias, T defaultValue) + { + var property = GetProperty(propertyAlias); + + if (IsEmptyProperty(property)) + { + return defaultValue; + } + + return property.GetValue(); + } + + private bool IsEmptyProperty(ArchetypePropertyModel property) + { + return (property == null || property.Value == null || string.IsNullOrEmpty(property.Value.ToString())); + } + public bool HasProperty(string propertyAlias) { return GetProperty(propertyAlias) != null; From 16a73e0c82d848252ea9bee06fade8a7e7ce5ef4 Mon Sep 17 00:00:00 2001 From: kjac Date: Fri, 20 Jun 2014 11:06:33 +0200 Subject: [PATCH 3/5] Fix code formatting --- .../Archetype.Tests/Models/FieldsetTests.cs | 44 ++++++++++--------- .../Models/ArchetypeFieldsetModel.cs | 36 +++++++-------- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs b/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs index ca9e169..d699500 100644 --- a/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs +++ b/app/Umbraco/Archetype.Tests/Models/FieldsetTests.cs @@ -35,16 +35,17 @@ namespace Archetype.Tests.Models Assert.That(propertyValue == "Box 1 Title"); } - [Test] - public void Can_Get_Fieldset_Property_Default_Value_By_Alias() { - var converter = new ArchetypeValueConverter(); - var result = (Archetype.Models.ArchetypeModel)converter.ConvertDataToSource(null, _sampleJson, false); + [Test] + public void Can_Get_Fieldset_Property_Default_Value_By_Alias() + { + var converter = new ArchetypeValueConverter(); + var result = (Archetype.Models.ArchetypeModel)converter.ConvertDataToSource(null, _sampleJson, false); - var fieldset = result.Fieldsets.First(); - var propertyValue = fieldset.GetValue("noSuchProperty", "noSuchProperty default value"); + var fieldset = result.Fieldsets.First(); + var propertyValue = fieldset.GetValue("noSuchProperty", "noSuchProperty default value"); - Assert.That(propertyValue == "noSuchProperty default value"); - } + Assert.That(propertyValue == "noSuchProperty default value"); + } [Test] public void Can_Convert_Property_Value_Types() @@ -59,22 +60,23 @@ namespace Archetype.Tests.Models Assert.That(fieldset.GetValue("blurb") == "A blurb here"); } - [Test] - public void Can_Convert_Property_Default_Value_Types() { - var converter = new ArchetypeValueConverter(); - var result = (Archetype.Models.ArchetypeModel)converter.ConvertDataToSource(null, _sampleJson, false); + [Test] + public void Can_Convert_Property_Default_Value_Types() + { + var converter = new ArchetypeValueConverter(); + var result = (Archetype.Models.ArchetypeModel)converter.ConvertDataToSource(null, _sampleJson, false); - var fieldset = result.Fieldsets.First(); + var fieldset = result.Fieldsets.First(); - Assert.That(fieldset.GetValue("noSuchInteger", 1234) == 1234); - Assert.That(fieldset.GetValue("noSuchBoolean", true) == true); - Assert.That(fieldset.GetValue("noSuchBoolean", false) == false); - Assert.That(fieldset.GetValue("noSuchString", "noSuchString default value") == "noSuchString default value"); + Assert.That(fieldset.GetValue("noSuchInteger", 1234) == 1234); + Assert.That(fieldset.GetValue("noSuchBoolean", true) == true); + Assert.That(fieldset.GetValue("noSuchBoolean", false) == false); + Assert.That(fieldset.GetValue("noSuchString", "noSuchString default value") == "noSuchString default value"); - var contactDetails = fieldset.GetValue("noSuchContact", new ContactDetails { Address = "some street, some city", Name = "someone" }); - Assert.That(contactDetails.Address == "some street, some city"); - Assert.That(contactDetails.Name == "someone"); - } + var contactDetails = fieldset.GetValue("noSuchContact", new ContactDetails { Address = "some street, some city", Name = "someone" }); + Assert.That(contactDetails.Address == "some street, some city"); + Assert.That(contactDetails.Name == "someone"); + } [Test] public void Returns_String_When_No_Type_Specified() diff --git a/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs b/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs index f8b3edf..2026dbd 100644 --- a/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs +++ b/app/Umbraco/Umbraco.Archetype/Models/ArchetypeFieldsetModel.cs @@ -29,35 +29,35 @@ namespace Archetype.Models { var property = GetProperty(propertyAlias); - if (IsEmptyProperty(property)) - { - return default(T); - } + if (IsEmptyProperty(property)) + { + return default(T); + } return property.GetValue(); } - // issue 142: support default T value supplied by caller - // this code would look nicer if the two GetValue() methods had one common implementation. - // however, this would require GetValue(string propertyAlias) to call the common implementation - // with a default(T) value, which could in theory result in a performance hit, if T for some reason - // is costly to instantiate. - public T GetValue(string propertyAlias, T defaultValue) + // issue 142: support default T value supplied by caller + // this code would look nicer if the two GetValue() methods had one common implementation. + // however, this would require GetValue(string propertyAlias) to call the common implementation + // with a default(T) value, which could in theory result in a performance hit, if T for some reason + // is costly to instantiate. + public T GetValue(string propertyAlias, T defaultValue) { var property = GetProperty(propertyAlias); - if (IsEmptyProperty(property)) - { - return defaultValue; - } + if (IsEmptyProperty(property)) + { + return defaultValue; + } return property.GetValue(); } - private bool IsEmptyProperty(ArchetypePropertyModel property) - { - return (property == null || property.Value == null || string.IsNullOrEmpty(property.Value.ToString())); - } + private bool IsEmptyProperty(ArchetypePropertyModel property) + { + return (property == null || property.Value == null || string.IsNullOrEmpty(property.Value.ToString())); + } public bool HasProperty(string propertyAlias) { From f99ef697db589763f8975f5ef0b9ca145dbf3b59 Mon Sep 17 00:00:00 2001 From: Kevin Giszewski Date: Wed, 25 Jun 2014 08:34:54 -0400 Subject: [PATCH 4/5] Fix 'hide property label' --- app/less/archetype.less | 4 ++++ app/views/archetype.default.html | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/less/archetype.less b/app/less/archetype.less index 0fa2209..9436837 100644 --- a/app/less/archetype.less +++ b/app/less/archetype.less @@ -4,6 +4,10 @@ .dropdown-menu { top: auto; } + + .controls-no-label { + padding: 10px; + } } .archetypeEditor fieldset{ diff --git a/app/views/archetype.default.html b/app/views/archetype.default.html index 13238df..328c300 100644 --- a/app/views/archetype.default.html +++ b/app/views/archetype.default.html @@ -29,14 +29,14 @@
-