Correct handling of step values of zero. (#18532)

Author: AndyButland

src/Umbraco.Core/PropertyEditors/Validation/ValidationHelper.cs

@@ -19,6 +19,11 @@ public static bool IsValueValidForStep(decimal value, decimal min, decimal step)
             return true; // Outside of the range, so we expect another validator will have picked this up.
         }
 
+        if (step == 0)
+        {
+            return true; // A step of zero would trigger a divide by zero error in evaluating. So we always pass validation for zero, as effectively any step value is valid.
+        }
+
         return (value - min) % step == 0;
     }
 }

tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DecimalValueEditorTests.cs

@@ -132,11 +132,12 @@ public void Validates_Is_Less_Than_Or_Equal_To_Configured_Max(object value, bool
         }
     }
 
-    [TestCase(1.4, false)]
-    [TestCase(1.5, true)]
-    public void Validates_Matches_Configured_Step(object value, bool expectedSuccess)
+    [TestCase(0.2, 1.4, false)]
+    [TestCase(0.2, 1.5, true)]
+    [TestCase(0.0, 1.4, true)] // A step of zero would trigger a divide by zero error in evaluating. So we always pass validation for zero, as effectively any step value is valid.
+    public void Validates_Matches_Configured_Step(double step, object value, bool expectedSuccess)
     {
-        var editor = CreateValueEditor();
+        var editor = CreateValueEditor(step: step);
         var result = editor.Validate(value, false, null, PropertyValidationContext.Empty());
         if (expectedSuccess)
         {
@@ -164,7 +165,7 @@ public void Validates_Matches_Configured_Step(object value, bool expectedSuccess
         return CreateValueEditor().ToEditor(property.Object);
     }
 
-    private static DecimalPropertyEditor.DecimalPropertyValueEditor CreateValueEditor()
+    private static DecimalPropertyEditor.DecimalPropertyValueEditor CreateValueEditor(double step = 0.2)
     {
         var localizedTextServiceMock = new Mock<ILocalizedTextService>();
         localizedTextServiceMock.Setup(x => x.Localize(
@@ -184,7 +185,7 @@ private static DecimalPropertyEditor.DecimalPropertyValueEditor CreateValueEdito
             {
                 { "min", 1.1 },
                 { "max", 1.9 },
-                { "step", 0.2 }
+                { "step", step }
             }
         };
     }

tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/IntegerValueEditorTests.cs

@@ -132,11 +132,12 @@ public void Validates_Is_Less_Than_Or_Equal_To_Configured_Max(object value, bool
         }
     }
 
-    [TestCase(17, false)]
-    [TestCase(18, true)]
-    public void Validates_Matches_Configured_Step(object value, bool expectedSuccess)
+    [TestCase(2, 17, false)]
+    [TestCase(2, 18, true)]
+    [TestCase(0, 17, true)] // A step of zero would trigger a divide by zero error in evaluating. So we always pass validation for zero, as effectively any step value is valid.
+    public void Validates_Matches_Configured_Step(int step, object value, bool expectedSuccess)
     {
-        var editor = CreateValueEditor();
+        var editor = CreateValueEditor(step: step);
         var result = editor.Validate(value, false, null, PropertyValidationContext.Empty());
         if (expectedSuccess)
         {
@@ -164,7 +165,7 @@ public void Validates_Matches_Configured_Step(object value, bool expectedSuccess
         return CreateValueEditor().ToEditor(property.Object);
     }
 
-    private static IntegerPropertyEditor.IntegerPropertyValueEditor CreateValueEditor()
+    private static IntegerPropertyEditor.IntegerPropertyValueEditor CreateValueEditor(int step = 2)
     {
         var localizedTextServiceMock = new Mock<ILocalizedTextService>();
         localizedTextServiceMock.Setup(x => x.Localize(
@@ -184,7 +185,7 @@ private static IntegerPropertyEditor.IntegerPropertyValueEditor CreateValueEdito
             {
                 { "min", 10 },
                 { "max", 20 },
-                { "step", 2 }
+                { "step", step }
             }
         };
     }

tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/SliderValueEditorTests.cs

@@ -8,7 +8,6 @@
 using Umbraco.Cms.Core.Models.Editors;
 using Umbraco.Cms.Core.Models.Validation;
 using Umbraco.Cms.Core.PropertyEditors;
-using Umbraco.Cms.Core.Serialization;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Strings;
 using Umbraco.Cms.Infrastructure.Serialization;
@@ -206,17 +205,18 @@ public void Validates_Is_Less_Than_Or_Equal_To_Configured_Max(decimal from, deci
         }
     }
 
-    [TestCase(1.3, 1.7, true)]
-    [TestCase(1.4, 1.7, false)]
-    [TestCase(1.3, 1.6, false)]
-    public void Validates_Matches_Configured_Step(decimal from, decimal to, bool expectedSuccess)
+    [TestCase(0.2, 1.3, 1.7, true)]
+    [TestCase(0.2, 1.4, 1.7, false)]
+    [TestCase(0.2, 1.3, 1.6, false)]
+    [TestCase(0.0, 1.4, 1.7, true)] // A step of zero would trigger a divide by zero error in evaluating. So we always pass validation for zero, as effectively any step value is valid.
+    public void Validates_Matches_Configured_Step(decimal step, decimal from, decimal to, bool expectedSuccess)
     {
         var value = new JsonObject
         {
             { "from", from },
             { "to", to },
         };
-        var editor = CreateValueEditor();
+        var editor = CreateValueEditor(step: step);
         var result = editor.Validate(value, false, null, PropertyValidationContext.Empty());
         if (expectedSuccess)
         {
@@ -244,7 +244,7 @@ public void Validates_Matches_Configured_Step(decimal from, decimal to, bool exp
         return CreateValueEditor().ToEditor(property.Object);
     }
 
-    private static SliderPropertyEditor.SliderPropertyValueEditor CreateValueEditor(bool enableRange = true)
+    private static SliderPropertyEditor.SliderPropertyValueEditor CreateValueEditor(bool enableRange = true, decimal step = 0.2m)
     {
         var localizedTextServiceMock = new Mock<ILocalizedTextService>();
         localizedTextServiceMock.Setup(x => x.Localize(
@@ -265,7 +265,7 @@ private static SliderPropertyEditor.SliderPropertyValueEditor CreateValueEditor(
                 EnableRange = enableRange,
                 MinimumValue = 1.1m,
                 MaximumValue = 1.9m,
-                Step = 0.2m
+                Step = step
             },
         };
     }