Mvc: Support flagged enums for various scenarios

[tuespetre]

There are several times I would have like to have this functionality; sometimes when you have a user-facing system that involves lots of 'flag-setting', flagged enums can be much easier to manage than an explosion of boolean fields.

I have noticed that GetEnumSelectList throws an exception for flagged enums, and that model binding does not seem to properly account for flagged enums, nor do checkboxes and tag helpers behave as desired (at least as desired by myself...)

I would like to propose the following behaviors:

• During model binding, if there is an array of values for a given key and the binding target is a flagged enum, interpret each value of the array as a member of that enum and 'or' them all together instead of just taking the first value present in the array
• When determining the 'current values' for a flagged enum in the model, only consider those combined values that are explicitly defined for the enum (i.e. 3, 7, -1, etc.)
• During generation of checkbox controls and select list options, in order to determine whether they are selected or checked or not, check to see if the model value contains the flags represented by the value specified for the checkbox/option.
• During generation of select list items for an enum, don't throw an exception if the enum is flagged. Just proceed as normal

Please let me know any thoughts/concerns.

[dougbu]

The general concern is that [Flags] enums have wildly-varying semantics that cannot consistently be turned into a IEnumerable<SelectListItem>. Just for example, how should mask values (those w/ more than one bit set) in the enum be handled -- ignored, used to group the constituent bits, ...?

The system should work fine if you pass your own select list into the helpers. Please let us know the details if this is not the case.

[dougbu]

Hit "Comment" and then remembered my "should work fine" is probably not correct. A <select mutliple="multiple"> element for a [Flags] enum won't be bound correctly if the user selects more than one value. A set of checkboxes that an action or other user code or's together would work better.

[tuespetre]

Alright, so there are three areas of behavior surrounding [Flags] enums that could change:

1. Binding values to the model
   
   
   
   • Currently, if multiple values come in for a [Flags] enum, only the first value will be applied. I believe that the default binding behavior should instead be to 'or' the values together.
   
   
2. Generating an IEnumerable<SelectListItem> for a [Flags] enum
   
   
   
   • Currently, if you try to use GetEnumSelectList with a [Flags] enum, an exception is thrown. I believe that no exception should be thrown. I will go into the details of how the list should be generated below.
   
   
3. Determining the current values of a [Flags] enum model expression (applies to marking <option>s as selected and <input>s as checked)
   
   
   
   • Currently, you cannot successfully bind a set of flags to a set of select list options. Only the exact value contained in the model will be considered. I believe that, when comparing a [Flags] enum value to a select list option, the framework should consider whether the value of the option is a flag that is set within the current value of the model.
   
   
   • Currently, you cannot use either of the helpers for checkboxes with anything but a bool model value. I believe that this should be opened up to allow for [Flags] enum values, and that the 'current values' logic used for generating <select>s should then be applied when generating a <checkbox> when targeting a [Flags] enum.
   
   

The general concern is that [Flags] enums have wildly-varying semantics that cannot consistently be turned into a IEnumerable<SelectListItem>. Just for example, how should mask values (those w/ more than one bit set) in the enum be handled -- ignored, used to group the constituent bits, ...?

I think a sensible 'out of the box' behavior for generating an IEnumerable<SelectListItem> would be to generate one option per explicitly defined member of the enum, just like how it's done for non-flagged enums. Handle grouping just like it's done for other enums as well. This should meet the needs for typical scenarios.

In cases where a 'mask value' is defined, just go ahead and render it. Mark each applicable option selected: if the value is Five, and One, Two, Four, and Five are defined, render the four options, and mark the correct three as selected. If the developer needed to define a 'mask value' on the enum for some reason, but they did not want it to be included in the generated set of options, the framework could check for a DisplayAttribute on that member of the enum to see if it has the AutoGenerateField property set to false.

[dougbu]

/cc @Eilon @danroth27 for triage. Note this is really a request for at least three features: [Flags] handling in model binding, [Flags] handling in IHtmlHelper.GetEnumSelectList (), and [Flags] handling in DefaultHtmlGenerator.GetCurrentValues() / DefaultHtmlGenerator.UpdateSelectListItemsWithDefaultValue(). Other than not throwing, none of these extensions should require changes to our method signatures.

you cannot use either of the helpers for checkboxes with anything but a bool model value

@tuespetre the helpers that generate checkboxes are intentionally restricted to a bool model type; not sure what would be special about enum models. In any case, extensions there feel very separate from the rest of this issue.

[tuespetre]

@dougbu a checkbox could function similar to a select option when working with flagged enums, I guess you could say I'm requesting four features in total.

I'm already preparing a pull request since I know your team is very busy, and I know this may not make it into RTM if at all. I've got the model binding, the generation of select list items, and the binding of values to select options completed, with relevant tests. I have yet to start on the checkbox thing. Once I've got that done I'll see if I can split the changes out into multiple commits that each address a single area of concern so it will be easier for the team to pick and choose what to incorporate, if anything.

[tuespetre]

I've just submitted a PR with the broken-out commit for what I discern as the most important change that I have requested. The other changes can be easily done without by writing an EditorTemplate for the enum, but the model binding story has been the most awkward/frustrating in my experience.

[Eilon]

@tuespetre thank you for the PRs but at this time we are not planning to take this feature because we are working on stabilizing the RC2 release. We will revisit this in the next release.

[tuespetre]

Just popping back in to say:

• I still feel that the GetEnumSelectList should not be throwing with [Flags] enums. I think I understand why that was done, but it doesn't seem to be all that beneficial. If someone wants a <select multiple> with a generated list they should be able to just do it.
• I no longer believe the framework should _have_ to include 'proper' binding for [Flags] enums, for the following reasons:
  
  
  
  • Binding to entities: In general, I think most experienced developers are going to agree that you should be binding incoming values to some kind of view or form model and not your actual entities, so if an entity has a [Flags] enum property, there should ideally always be a chance to map from the view or form model to the entity.
  
  
  • Incoming values: I had forgotten about the ridiculously simple Aggregate LINQ method, which kind of eliminates any need for baked-in binding support and should suffice for virtually all applications' needs:
  
  

`entity.FlagsProperty = model.ArrayOfFlagsProperty.Aggregate((a, b) => a | b)`

• Outgoing values: For turning a 'proper' [Flags] enum value into an IEnumerable<TEnum> (for populating a view model or looping to create a checkbox list or similar), the following one-liner suffices:

`Enum.GetValues(typeof(MyFlagsEnum)).Cast<MyFlagsEnum>().Where(f => entity.FlagsProperty.HasFlag(f))`

This could be more terse if the framework offered a generic `GetValues` method:

`Enum.GetValues<MyFlagsEnum>().Where(f => entity.FlagsProperty.HasFlag(f))`

...and in a view, you would just loop through all of the `enum`'s values and compare it against the model with a simple `Contains()` or similar to determine whether to render a checked checkbox or whatever.

[Eilon]

Closing because we have no plans to support this.

[tuespetre]

@Eilon thanks! Would there be any possibility of still removing the throw from GetEnumSelectList when the Enum is a [Flags] enum?

[Eilon]

@tuespetre I believe that the current behavior is correct by throwing, because it doesn't make sense to produce a simple list of enum values for a Flags enum.

[tuespetre]

Ok, thanks 🐈

[los93sol]

I actually have a scenario where I need to be able to generate the select list using MVC, but using a separate front end for binding. In this scenario I believe it does make sense since it cuts me down to a very simple call to create the select list and I'm handling binding outside of MVC.