#8640: Fixing consistency between different Enumeration Field flavors' data storage (#8789)

* Reworking EnumerationField's logic to store/retrieve its (selected) values

* Fixing exception when creating new item with CheckboxList flavor, adding more nullchecks and compactness

* Code styling in EnumerationFieldDriver

* Code styling in EnumerationField editor template

* Fixing that EnumerationFieldDriver and the EnumerationField editor template should read SelectedValues instead of Values directly

---------

Co-authored-by: Matteo Piovanelli <MatteoPiovanelli-Laser@users.noreply.github.com>

src/Orchard.Web/Modules/Orchard.Fields/Drivers/EnumerationFieldDriver.cs

@@ -4,46 +4,41 @@ using Orchard.ContentManagement.Handlers;
 using Orchard.Fields.Fields;
 using Orchard.Fields.Settings;
 using Orchard.Localization;
-using System;
-using System.Collections.Generic;
-using System.Linq;
 
 namespace Orchard.Fields.Drivers {
     public class EnumerationFieldDriver : ContentFieldDriver<EnumerationField> {
         public IOrchardServices Services { get; set; }
+
         private const string TemplateName = "Fields/Enumeration.Edit";
 
         public EnumerationFieldDriver(IOrchardServices services) {
             Services = services;
+
             T = NullLocalizer.Instance;
         }
 
         public Localizer T { get; set; }
 
-        private static string GetPrefix(ContentField field, ContentPart part) {
+        private static string GetPrefix(ContentField field, ContentPart part) =>
-            return part.PartDefinition.Name + "." + field.Name;
+            part.PartDefinition.Name + "." + field.Name;
-        }
 
-        private static string GetDifferentiator(EnumerationField field, ContentPart part) {
+        private static string GetDifferentiator(EnumerationField field) => field.Name;
-            return field.Name;
-        }
 
         protected override DriverResult Display(ContentPart part, EnumerationField field, string displayType, dynamic shapeHelper) {
-            return ContentShape("Fields_Enumeration", GetDifferentiator(field, part),
+            return ContentShape("Fields_Enumeration", GetDifferentiator(field), () => shapeHelper.Fields_Enumeration());
-                                () => shapeHelper.Fields_Enumeration());
         }
 
         protected override DriverResult Editor(ContentPart part, EnumerationField field, dynamic shapeHelper) {
-            return ContentShape("Fields_Enumeration_Edit", GetDifferentiator(field, part),
+            return ContentShape("Fields_Enumeration_Edit", GetDifferentiator(field), () => {
-                () => {
+                if (part.IsNew() && string.IsNullOrEmpty(field.Value)) {
-                    if (part.IsNew() && String.IsNullOrEmpty(field.Value)) {
+                    var settings = field.PartFieldDefinition.Settings.GetModel<EnumerationFieldSettings>();
-                        var settings = field.PartFieldDefinition.Settings.GetModel<EnumerationFieldSettings>();
+                    if (!string.IsNullOrWhiteSpace(settings.DefaultValue)) {
-                        if (!String.IsNullOrWhiteSpace(settings.DefaultValue)) {
+                        field.SelectedValues = new string[] { settings.DefaultValue };
-                            field.Value = settings.DefaultValue;
-                        }
                     }
-                    return shapeHelper.EditorTemplate(TemplateName: TemplateName, Model: field, Prefix: GetPrefix(field, part));
+                }
-                });
+
+                return shapeHelper.EditorTemplate(TemplateName: TemplateName, Model: field, Prefix: GetPrefix(field, part));
+            });
         }
 
         protected override DriverResult Editor(ContentPart part, EnumerationField field, IUpdateModel updater, dynamic shapeHelper) {

src/Orchard.Web/Modules/Orchard.Fields/Fields/EnumerationField.cs

@@ -7,28 +7,16 @@ namespace Orchard.Fields.Fields {
         private const char Separator = ';';
 
         public string Value {
-            get { return Storage.Get<string>(); }
+            get => Storage.Get<string>()?.Trim(Separator) ?? "";
-            set { Storage.Set(value ?? String.Empty); }
+            set => Storage.Set(string.IsNullOrWhiteSpace(value)
+                ? string.Empty
+                // It is now the responsibility of this field to (re-)add the separators.
+                : Separator + value.Trim(Separator) + Separator);
         }
 
         public string[] SelectedValues {
-            get {
+            get => Value?.Split(new[] { Separator }, StringSplitOptions.RemoveEmptyEntries) ?? new string[0];
-                var value = Value;
+            set => Value = value?.Length > 0 ? string.Join(Separator.ToString(), value) : "";
-                if(string.IsNullOrWhiteSpace(value)) {
-                    return new string[0];
-                }
-
-                return value.Split(new [] { Separator }, StringSplitOptions.RemoveEmptyEntries);
-            }
-
-            set {
-                if (value == null || value.Length == 0) {
-                    Value = String.Empty;
-                }
-                else {
-                    Value = Separator + string.Join(Separator.ToString(), value) + Separator;
-                }
-            }
         }
     }
 }

src/Orchard.Web/Modules/Orchard.Fields/Views/EditorTemplates/Fields/Enumeration.Edit.cshtml

@@ -1,22 +1,27 @@
 @model Orchard.Fields.Fields.EnumerationField
+
 @using Orchard.Fields.Settings;
+
 @{
     var settings = Model.PartFieldDefinition.Settings.GetModel<EnumerationFieldSettings>();
     string[] options = (!String.IsNullOrWhiteSpace(settings.Options)) ? settings.Options.Split(new string[] { System.Environment.NewLine }, StringSplitOptions.None) : new string[] { T("Select an option").ToString() };
 }
+
 <fieldset>
-    <label for="@Html.FieldIdFor(m => m.Value)" @if (settings.Required) { <text> class="required" </text>  }>@Model.DisplayName</label>
+    <label for="@Html.FieldIdFor(m => m.Value)" @if (settings.Required) { <text> class="required" </text> }>@Model.DisplayName</label>
     @switch (settings.ListMode) {
         case ListMode.Dropdown:
-            @Html.DropDownListFor(m => m.Value, new SelectList(options, Model.Value), settings.Required ? new { required = "required" } : null)
+            @Html.DropDownListFor(m => m.Value, new SelectList(options, Model.SelectedValues.FirstOrDefault()), settings.Required ? new { required = "required" } : null)
             break;
 
         case ListMode.Radiobutton:
             foreach (var option in options) {
                 if (string.IsNullOrWhiteSpace(option)) {
-                    <label>@Html.RadioButton("Value", "", string.IsNullOrWhiteSpace(Model.Value), settings.Required ? new { required = "required" } : null)<i>@T("unset")</i></label>            }
+                    <label>@Html.RadioButton("Value", "", string.IsNullOrWhiteSpace(Model.SelectedValues.FirstOrDefault()), settings.Required ? new { required = "required" } : null)<i>@T("unset")</i></label>
+                }
                 else {
-                    <label>@Html.RadioButton("Value", option, (option == Model.Value), settings.Required ? new { required = "required" } : null)@option</label>         }
+                    <label>@Html.RadioButton("Value", option, (option == Model.SelectedValues.FirstOrDefault()), settings.Required ? new { required = "required" } : null)@option</label>
+                }
             }
             break;