Checkbox checked value lost if !ModelState.IsValid

Question

My approach here may be entirely wrong, but I'm slowly learning MVC... I have a form whereby a user must select a number of (or no) modules, based on this model:

public class MyProductModule
{
    public string ModuleName { get; set; }
    public bool Checked { get; set; }
}

public class ProductRequest
{

    public ProductRequest()
    {
        Modules = LoadModules();
    }

    public static List<MyProductModule> LoadModules()
    {
        return new List<MyProductModule>()
        {
            new MyProductModule() { ModuleName = "Module One", Checked = false },
            new MyProductModule() { ModuleName = "Module Two", Checked = false },
            new MyProductModule() { ModuleName = "Module Three", Checked = false }
        };
    }

    [Required]
    [EmailAddress]
    public string Email { get; set; }

    [DisplayName("MyProduct Modules")]
    public List<MyProductModule> Modules { get; set; }
}

A check box list is rendered to display each module:

@for (int i = 0; i < Model.Modules.Count; i++)
{
    @Html.CheckBoxFor(m => m.Modules[i].Checked)
    @Html.HiddenFor(m => m.Modules[i].ModuleName)
    @Html.LabelFor(m => m.Modules[i].Checked, Model.Modules[i].ModuleName)
}

Finally, the form is processed like this:

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult ProcessRequest(ProductRequest qd)
{
    if (!ModelState.IsValid)
    {
        return View("Index", qd);
    }
    else
    {
        // check email domains
        List<string> badDomains = new List<string>();
        badDomains.Add("gmail");
        badDomains.Add("yahoo");
        badDomains.Add("hotmail");
        foreach (string s in badDomains)
        {
            if (qd.Email.Contains(string.Format("@{0}.", s)))
            {
                ModelState.AddModelError(string.Empty, string.Format("Please use your work email address.", s));
            }
        }

        if (!ModelState.IsValid)
        {
            return View("Index", qd);
        }
        else
        {
            // process
        }
    }
}

Everything is working perfectly, unless for some reason my server-side validation fails, and the model is sent back (return View("Index", qd);). At that point, the checkbox list mysteriously changes from this:

[x] Module One
[ ] Module Two
[ ] Module Three

...to this:

[ ] Module One
[ ] Module Two
[ ] Module Three

All of the checkbox values are lost. If I examine the raw posted data in Firebug, I see that for some reason both true AND false are posted for the checked boxes "checked" value:

The View returns error 500 (null reference) when it attempts to read properties from the Model — EvilDr, Dec 19 '16 at 13:44
Do you need to return to the Index view? What about just `return View()`? — Grizzly, Dec 19 '16 at 13:50
Returns a 404 (the URL has quite a few parameters in there). — EvilDr, Dec 19 '16 at 13:55
What about changing to `@Html.Label(Model.Modules[i].ModuleName)`? — Grizzly, Dec 19 '16 at 14:13
hmmm what about changing this `@Html.LabelFor(m => m.Modules[i].Checked, Model.Modules[i].ModuleName)` to this `@Html.LabelFor(m => m.Modules[i].Checked, m.Modules[i].ModuleName)`? — Grizzly, Dec 19 '16 at 15:01
See http://stackoverflow.com/questions/2697299/asp-net-mvc-why-is-html-checkbox-generating-an-additional-hidden-input for the reason why your postback includes Modules[0].Checked twice. — Dean Goodman, Dec 19 '16 at 18:32
The link above explains the reason for a checked checkbox generated using `CheckBoxFor()` returns both `true` and `false`, but what you claiming is not possible (if the 1st checkbox is checked when you submit, it will be checked when you return the view) unless there is other code you have not shown us causing the issue. As a side note, your constructor that is calling a static method is bad practice, and you should remove that, and instead set the value of `Modules` in the GET method - `ProductRequest model = new ProductRequest(){ Modules = // populate it here }; return View(model);` — , Dec 19 '16 at 21:56
@StephenMuecke. Good advice - thank you. Out of interest, seeing as you're the C# maestro around here lately, can you advise on any training material (books) for "best practice"? — EvilDr, Dec 20 '16 at 10:38
Sorry, its been a while since I read a programming book :) - and why did you accept an answer that has nothing to do with your question? — , Dec 20 '16 at 10:41
Using the accepted answer did actually work. However, the tutorial I previously followed recommended not using an Editor Template for a single-use scenario. I therefore worked late last night creating a sample solution for you guys to try and spot my mistake. Annoyingly, the sample solution worked perfectly, so I then stripped my actual solution down to basics, and added each model property back in until the glitch re-occurred. Bizarrely, the glitch did not occur, so the only difference was a Clean/Rebuild based on yesterday's code. I'm confused by it nonetheless, but at least its working — EvilDr, Dec 20 '16 at 14:18
Using an `EditorTemplate` is a good option, but it is no different at all from using a `for` loop, and it certainly does not _solve your issue_. The code you have in your question works perfectly well and does not produce the behavior you claim. — , Dec 20 '16 at 23:51

FBO · Accepted Answer · 2016-12-19T18:25:51.017

On the server side, your variable qd contains the values you posted from view : For each MyProductModule you post only the Checked member.

So when you use return View("Index", qd); you give to the view only the value you have : Checked member.

If you want to have the ModuleName member, you have to post it alongside to the Checked member

@Html.LabelFor(model => model.Modules, htmlAttributes: new { @class = "required" })
@for (int i = 0; i < Model.Modules.Count; i++)
{
   @Html.CheckBoxFor(m => m.Modules[i].Checked)
   @Html.HiddenFor(m => m.Modules[i].ModuleName)
   @Html.LabelFor(m => m.Modules[i].Checked, Model.Modules[i].ModuleName)
}

Ok, I can't reproduce your error, but I can suggest another way to write it : using an editor template

Instead of the for loop, use @Html.EditorFor(m => m.Modules) You will have something like

@using (Html.BeginForm("ProcessRequest", "Home"))
{
    @Html.AntiForgeryToken()
    @Html.TextBoxFor(m => m.Email)
    @Html.EditorFor(m => m.Modules)
    <input type="submit" value="send" />
}

Then create a EditorTemplates folder and a new view named MyProductModule.cshtml

MyProductModule.cshtml :

@model MyNamespace.MyProductModule

@Html.CheckBoxFor(m => m.Checked)
@Html.HiddenFor(m => m.ModuleName)
@Html.LabelFor(m => m.Checked, Model.ModuleName)

Thanks. Although the label names do persist, the "checked" property is lost, so all boxes become unchecked if the model is returned in the event of me doing AddModelError — EvilDr, Dec 19 '16 at 17:23
Strangely, if I examine the raw post data, I see this: `Modules[2].Checked=true, Modules[2].Checked=false, Modules[2].ModuleName=Module Three`. Why would the Checked property appear twice with different values? — EvilDr, Dec 19 '16 at 17:30
Could you post your exact cshtml please ? I use this exact code and it works well — FBO, Dec 19 '16 at 17:38
I've updated the question. The code is EXACTLY the same as my own — EvilDr, Dec 19 '16 at 17:48

Checkbox checked value lost if !ModelState.IsValid

1 Answers1

Linked