MVC3 Master-Details Validation not Displaying

Question

I have an MVC3 page with an object (Header) that has data and a list of objects (Details) that I want to update on a single page. On the details object I have custom validation (IValidatableObject) that also needs to run.

This appears to generally be working as expected, validations are running and returning ValidationResults and if I put an @Html.ValidationSummary(false); on the page it displays those validations. However I don't want a list of validations at the top, but rather next to the item being validated i.e. Html.ValidationMessageFor which is on the page, but not displaying the relevant message. Is there something I'm missing? This is working on other pages (that don't have this Master-Details situation), so i'm thinking it is something about how I'm going about setting up the list of items to be updated or the editor template for the item?

Edit.cshtml (the Header-Details edit view)

@foreach (var d in Model.Details.OrderBy(d => d.DetailId))
{
   @Html.EditorFor(item => d, "Detail")
}

Detail.ascx (the Details Editor Template)

<%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<Detail>" %>

<tr>            
    <td>
        <%= Model.Name %>
        <%= Html.HiddenFor(model => model.DetailId) %>
    </td>
    <td class="colDescription">
        <%= Html.EditorFor(model => model.Description) %>
        <%= Html.ValidationMessageFor(model => model.Description) %>
    </td>
    <td class="colAmount">
        <%= Html.EditorFor(model => model.Amount) %>
        <%= Html.ValidationMessageFor(model => model.Amount) %>
    </td>
</tr>

Model is Entity Framework with Header that has Name and HeaderId and Detail has DetailId, HeaderId, Description and Amount

Controller Code:

public ActionResult Edit(Header header, FormCollection formCollection)
{
   if (formCollection["saveButton"] != null)
   {
      header = this.ProcessFormCollectionHeader(header, formCollection);
      if (ModelState.IsValid)
      {
         return new RedirectResult("~/saveNotification");
      }
      else
      {
         return View("Edit", header);
      }
   }
   else
   {
      return View("Edit", header);
   }
}

[I know controller code can be cleaned up a bit, just at this state as a result of trying to determine what is occuring here]

IValidatableObject implementation:

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
   if (this.Name.Length < 5) && (this.Amount > 10))
   {
      yield return new ValidationResult("Item must have sensible name to have Amount larger than 10.", new[] { "Amount" });
   }
}

Answer 1

I would recommend you to use real editor templates. The problem with your code is that you are writing a foreach loop inside your view to render the template which generates wrong names for the corresponding input fields. I guess that's the reason why you are doing some workarounds in your controller action to populate the model (header = this.ProcessFormCollectionHeader(header, formCollection);) instead of simply using the model binder to do the job.

So let me show you the correct way to achieve that.

Model:

public class Header
{
    public IEnumerable<Detail> Details { get; set; }
}

public class Detail : IValidatableObject
{
    public int DetailId { get; set; }
    public string Name { get; set; }
    public string Description { get; set; }
    public int Amount { get; set; }

    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        if ((this.Name ?? string.Empty).Length < 5 && this.Amount > 10)
        {
            yield return new ValidationResult(
                "Item must have sensible name to have Amount larger than 10.", 
                new[] { "Amount" }
            );
        }
    }
}

Controller:

public class HomeController : Controller
{
    public ActionResult Index()
    {
        var model = new Header
        {
            Details = Enumerable.Range(1, 5).Select(x => new Detail
            {
                DetailId = x,
                Name = "n" + x,
                Amount = 50
            }).OrderBy(d => d.DetailId)
        };
        return View(model);
    }

    [HttpPost]
    public ActionResult Index(Header model)
    {
        if (ModelState.IsValid)
        {
            return Redirect("~/saveNotification");
        }
        return View(model);
    }
}

View (~/Views/Home/Index.cshtml):

@model Header

@using (Html.BeginForm())
{
    <table>
        <thead>
            <tr>
                <th>Name</th>
                <th>Description</th>
                <th>Amount</th>
            </tr>
        </thead>
        <tbody>
            @Html.EditorFor(x => x.Details)
        </tbody>
    </table>
    <button type="submit">OK</button>
}

Editor template for the Detail type (~/Views/Shared/EditorTemplates/Detail.ascx or ~/Views/Shared/EditorTemplates/Detail.cshtml for Razor):

<%@ Control 
    Language="C#" 
    Inherits="System.Web.Mvc.ViewUserControl<MvcApplication1.Controllers.Detail>" 
%>

<tr>            
    <td>
        <%= Html.DisplayFor(model => model.Name) %>
        <%= Html.HiddenFor(model => model.DetailId) %>
        <%= Html.HiddenFor(model => model.Name) %>
    </td>
    <td class="colDescription">
        <%= Html.EditorFor(model => model.Description) %>
        <%= Html.ValidationMessageFor(model => model.Description) %>
    </td>
    <td class="colAmount">
        <%= Html.EditorFor(model => model.Amount) %>
        <%= Html.ValidationMessageFor(model => model.Amount) %>
    </td>
</tr>

Here are a couple of things that I did to improve your code:

• I performed the ordering of the Details collection by DetailId at the controller level. It's the controller's responsibility to prepare the view model for display. The view should not be doing this ordering. All that the view should do is display the data
• Thanks to the previous improvement I git rid of the foreach loop in the view that you were using to render the editor template and replaced it with a single @Html.EditorFor(x => x.Details) call. The way this works is that ASP.NET MVC detects that Details is a collection property (of type IEnumerable<Detail>) and it will automatically look for a custom editor templated inside the ~/Views/SomeController/EditorTemplates or ~/Views/Shared/EditorTemplates folders called Detail.ascx or Detail.cshtml (same name as the type of the collection). It will then render this template for each element of the collection so that you don't need to worry about it
• Thanks to the previous improvement, inside the [HttpPost] action you no longer need any ProcessFormCollectionHeader hacks. The header action argument will be correctly bound from the request data by the model binder
• Inside the Detail.ascx template I have replaced <%= Model.Name %> with <%= Html.DisplayFor(model => model.Name) %> in order to properly HTML encode the output and fill the XSS hole that was open on your site.
• Inside the Validate method I ensured that the Name property is not null before testing against its length. By the way in your example you only had an input field for the Description field inside the template and didn't have a corresponding input field for the Name property, so when the form is submitted this property will always be null. As a consequence I have added a corresponding hidden input field for it.