Collection Posting and ModelState

Question

I have a problem in my MVC application that I'm not sure how to solve, or if I am going to do it wrong.

I have a controller / view that displays a list of elements in a grid with a checkmark and when the elements are sent to my controller, I would like to delete rows from my database based on the identifier that was passed.

The view looks something like this:

@for(int index = 0; index < Model.Items.Length; index++) { <td> @Html.HiddenFor(m => m[index].Id) @Html.CheckBoxFor(m => m[index].Delete) </td> } 

My controller takes the values:

 [HttpPost] public ActionResult Delete(DeleteItemsModel model) { if( !ModelState.IsValid ) { // ... } foreach( var id in model.Items.Where(i => i.Delete)) repo.Delete(id); } 

This script works great. Items are sent correctly with the identifier and flag to be deleted or not, and they are deleted properly. The problem I encountered is when my page fails validation. I need to get the items from the database again and send the data back to the view:

 if( !ModelState.IsValid ) { var order = repo.GetOrder(id); // map return View(Mapper.Map<Order, OrderModel>(order)); } 

Between the user receiving the list of items to delete and clicking the "Submit" button, it is possible that new items could be added. Now, when I pull out the data and send it back to the view, there may be new elements in the list.

Example problem:
I am doing an HTTP GET on my page, and there are two elements in my grid with identifiers 2 and 1. I select the first row (Id 2, sorted by last word), and then I click Submit. Validation on the page fails, and I return the view to the user. The grid now has three rows (3, 2, 1). In MVC, a checkmark will be set in the FIRST element (now with identifier 3). If the user does not verify this data, they can potentially delete the wrong thing.

Any ideas on how to fix this scenario or what should I do instead? Does anybody know how

Answer 1

Interest Ask. Let's first illustrate the problem with a simple example, because judging by the other answers, I'm not sure that everyone understands what the problem is.

Assume the following model:

 public class MyViewModel { public int Id { get; set; } public bool Delete { get; set; } } 

next controller:

 public class HomeController : Controller { public ActionResult Index() { // Initially we have 2 items in the database var model = new[] { new MyViewModel { Id = 2 }, new MyViewModel { Id = 1 } }; return View(model); } [HttpDelete] public ActionResult Index(MyViewModel[] model) { // simulate a validation error ModelState.AddModelError("", "some error occured"); if (!ModelState.IsValid) { // We refetch the items from the database except that // a new item was added in the beginning by some other user // in between var newModel = new[] { new MyViewModel { Id = 3 }, new MyViewModel { Id = 2 }, new MyViewModel { Id = 1 } }; return View(newModel); } // TODO: here we do the actual delete return RedirectToAction("Index"); } } 

and view:

 @model MyViewModel[] @Html.ValidationSummary() @using (Html.BeginForm()) { @Html.HttpMethodOverride(HttpVerbs.Delete) for (int i = 0; i < Model.Length; i++) { <div> @Html.HiddenFor(m => m[i].Id) @Html.CheckBoxFor(m => m[i].Delete) @Model[i].Id </div> } <button type="submit">Delete</button> } 

Here is what will happen:

The user proceeds to the Index action, selects the first item to delete, and clicks the Delete button. Here's what the view looks like before it presents the form:

The Delete action is called, and when the view is rendered again (because there was some validation error), the user is presented with the following:

See how the wrong item is pre-selected?

Why is this happening? Because the HTML helpers use the ModelState value in priority when binding instead of the model value, and this is by design.

So how to solve this problem? Reading the following Phil Haack blog post: http://haacked.com/archive/2008/10/23/model-binding-to-a-list.aspx

In his blog, he talks about Non-Sequential Indices and gives the following example:

 <form method="post" action="/Home/Create"> <input type="hidden" name="products.Index" value="cold" /> <input type="text" name="products[cold].Name" value="Beer" /> <input type="text" name="products[cold].Price" value="7.32" /> <input type="hidden" name="products.Index" value="123" /> <input type="text" name="products[123].Name" value="Chips" /> <input type="text" name="products[123].Price" value="2.23" /> <input type="hidden" name="products.Index" value="caliente" /> <input type="text" name="products[caliente].Name" value="Salsa" /> <input type="text" name="products[caliente].Price" value="1.23" /> <input type="submit" /> </form> 

See how we no longer use incremental indexes for input button names?

How do we apply this to our example?

Like this:

 @model MyViewModel[] @Html.ValidationSummary() @using (Html.BeginForm()) { @Html.HttpMethodOverride(HttpVerbs.Delete) for (int i = 0; i < Model.Length; i++) { <div> @Html.Hidden("index", Model[i].Id) @Html.Hidden("[" + Model[i].Id + "].Id", Model[i].Id) @Html.CheckBox("[" + Model[i].Id + "].Delete", Model[i].Delete) @Model[i].Id </div> } <button type="submit">Delete</button> } 

Now the problem is fixed. Or that? Have you seen the terrible mess that the presentation now represents? We posed one problem, but we introduced something absolutely disgusting in the view. I don't know about you, but when I look at it, I want to vomit.

So what can be done? We should read Steven Sanderson's blog post: http://blog.stevensanderson.com/2010/01/28/editing-a-variable-length-list-aspnet-mvc-2-style/ , in which he presents a very interesting Html.BeginCollectionItem custom helper, which is used as follows:

 <div class="editorRow"> <% using(Html.BeginCollectionItem("gifts")) { %> Item: <%= Html.TextBoxFor(x => x.Name) %> Value: $<%= Html.TextBoxFor(x => x.Price, new { size = 4 }) %> <% } %> </div> 

Notice how the form elements are wrapped in this helper?

What does this assistant do? It replaces consecutive indexes generated by strongly typed Guides and uses an additional hidden field to set this index on each iteration.

---

In this case, the problem appears only if you need to get fresh data from your database in the "Delete" action. If you rely on linking the model to rehydration, there will be no problems at all (except that if you have a model error, you will see a view with old data → which is probably not so problematic):

 [HttpDelete] public ActionResult Index(MyViewModel[] model) { // simulate a validation error ModelState.AddModelError("", "some error occured"); if (!ModelState.IsValid) { return View(model); } // TODO: here we do the actual delete return RedirectToAction("Index"); } 

Answer 2

A common solution to this problem is to use the Post-Redirect-Get pattern .

You can find an explanation with sample code for MVC here (along with a bunch of other good MVC tips). Scroll down to item 13 in the list for an explanation of PRG.