1

I'm trying to pass my model back to a controller. I have included all fields of the model in the form but two parts of the model are getting lost once it makes it back to the controller, Employees and EmployeesInMultipleCompanies, which are both of type IList. I have verified that the fields are present when they are passed to the view, they just don't make it back to the controller.

.cshtml

@using (Html.BeginForm("PostEmail","Import",FormMethod.Post))
{
    @Html.AntiForgeryToken()
    @Html.ValidationSummary(true)

    <fieldset>
        <legend>EmailViewModel</legend>
        <p>
            <input type="email" name="EmailAddresses" value=" " required="required" />
            <span class="btn_orange"><a href="#" class="remove_field" >x</a></span>
        </p>
        <p>
            <span class="btn_orange"><a class="add_email_button" href="#">Add Another Email</a></span>
        </p>
        @Html.HiddenFor(m=> Model.Employees)
        @Html.HiddenFor(m=> Model.CompanyId)
        @Html.HiddenFor(m=> Model.CompanyName)
        @Html.HiddenFor(m=> Model.PayFrequency)
        @Html.HiddenFor(m=> Model.FirstPayPeriodBeginDate)
        @Html.HiddenFor(m=> Model.LastPayPeriodEndDate)
        @Html.HiddenFor(m=> Model.NumberPayPeriods)
        @Html.HiddenFor(m=> Model.ValidEmployees)
        @Html.HiddenFor(m=> Model.InvalidEmployees)
        @Html.HiddenFor(m=> Model.EmployeesInMultipleCompanies)
        @Html.HiddenFor(m=> Model.TotalEmployeeCount)
        <p>
            <input type="submit" value="Send Email" />
        </p>
    </fieldset>
}

<div>
    @Html.ActionLink("Cancel", "Continue", "Import")
</div>

<script type="text/javascript">
    $(document).ready(function () {
        $('body').on('click', '.remove_field', function () {
            $(this).closest('p').remove();
        });
        $('.add_email_button').closest('p').click(function () {
            var html = '<p><input type="email" required="required" name="EmailAddresses" /><span class="btn_orange"><a href="#" class="remove_field">x</a></span></p>';
            $(html).insertBefore($(this));
        });

        $(body).on('click', 'submit', function() {
            $('email').attr('required', true);
        });
    });
</script>

controller .cs

public ActionResult SendEmail(ImportViewModel model)
        {
            var editedEmployees = model.EmployeesInMultipleCompanies;
            var importModel = TempData["ImportModel"] as ImportViewModel;

            //for each employee who is in multiple companies, set user-chosen company id and the COHD related to that company
            var importService = new ImportService();
            importService.UpdateEmployeesInMultipleCompanies(editedEmployees, importModel.Employees);
            TempData["ImportModel"] = importModel;

            return this.RazorView("SendEmail", importModel);
        }


        [HttpPost]
        public ActionResult PostEmail(ImportViewModel model)
        {
            IEmailer emailer = new Emailer();
            emailer.SendEmail(model.EmailAddresses, model.Employees);
            var employees =
                model.Employees.Where(e => !string.IsNullOrWhiteSpace(e.ValidationErrorOrException)).ToList();
            var emailViewModel = new EmailViewModel(employees);
            return this.RazorView("Continue", emailViewModel);
        }
Antarr Byrd
  • 24,863
  • 33
  • 100
  • 188
  • 1
    I would consider not rendering them in a single hidden but rather rendering multiple inputs with the same name and different values. The DefaultModelBinder will see this and interpret it as a collection type and it should just work. Alternatively, you could create a custom model binder that split the values stored in an individual hidden. – xDaevax Sep 10 '14 at 17:51
  • Also be wary of TempData as it is using sessions under the covers (which is not necessarily bad unless you are in a web-farm) and should really only be used when redirecting: http://stackoverflow.com/questions/1500402/when-to-use-tempdata-vs-session-in-asp-net-mvc. Also many would consider using TempData in lieu of a strongly typed ViewModel / other persistence mechanism to be a code smell: http://stackoverflow.com/questions/386681/asp-net-mvc-tempdata-good-or-bad-practice. – xDaevax Sep 10 '14 at 18:17
  • @xDaevax thanks for the notice. I will consider it in the future, as for now its already there in server other view I'm just using it in mine. – Antarr Byrd Sep 10 '14 at 18:24

3 Answers3

2

Replace it with the following:

@foreach(var i = 0; i < Model.Employees.Count; i++)
{
  Html.Hiddenfor(m => m.Employees[i])
}

The same should be done for EmployeesInMultipleCompanies

Alex Art.
  • 8,711
  • 3
  • 29
  • 47
1

Values that are inserted to hidden fields are serialized to a string to do so.

IList<T> is not serializable because it is an interface and and therefore cannot be stored in a hidden field.

See this question for some more detail

How about you convert your IList<T> to a regular list?

Community
  • 1
  • 1
Trucktech
  • 328
  • 1
  • 11
  • this makes since but it has no problem serializing my EmailAddresses which is also IList – Antarr Byrd Sep 10 '14 at 17:39
  • even after changing to a regular list I get the same problem – Antarr Byrd Sep 10 '14 at 17:41
  • The IList should be fine since the DefaultModelBinder is smart and isn't serializing the IList, but the type argument `T` of the IList. The only time this could be an issue would be if the IList had a generic type constraint that was not a concrete type (abstract / interface). The question you refer to is unrelated to the ModelBinder. – xDaevax Sep 10 '14 at 17:55
  • I figured another way around it. I did not realize the the Employee data was already present in the TempData which is already in the controller. – Antarr Byrd Sep 10 '14 at 18:10
  • As others have stated using a `foreach` loop to create a hidden field for each item in the collection will work as well as long as the underlying type is also serializable. – Trucktech Sep 10 '14 at 18:25
  • The DefaultModelBinder will convert a generic `IList` to a `List` given T is a bind-able type. It seems like your answer is not correct. The issue isn't with the interface type, it is with the fact that the `HiddenFor` method will invoke the `ToString()` method on the `IList` object, which will put a value in the hidden field that is not usable on return to the controller. – xDaevax Sep 10 '14 at 19:21
1

The problem is not with the controller but rather the way the values are stored on the client. The DefaultModelBinder is very smart, but it is also sometimes not that smart. It can handle interfaces in certain situations, collections of complex types and many other things as long as the properties match up. For detailed information on the behavior, see this article: http://msdn.microsoft.com/en-us/magazine/hh781022.aspx.

If you debug your controller and inspect the values in Request.Form, you will see your data stored as a single value. If model binders could talk they would say:

The Request.Form["EmployeesInMultipleCompanies"] doesn't relate to the EmployeesInMultipleCompanies property of my model because my model meta data (ModelMetaDataProvider) tells me that I should be looking for a collection type (IList of an unknown generic type) and this is clearly a single string value so I will not bind this value.

To solve this, you have to give the DefaultModelBinder enough of a hint for it to do the work. One way of doing this is to render the hidden fields in such a way that they have an indexer associated with them. This is the clue the model binder needs to function the way you expect.

If you rendered both like so:

@for(var i = 0; i < Model.Employees.Count; i++)
{
  Html.Hiddenfor(m => m.Employees[i])
}
@for(var i = 0; i < Model.EmployeesInMultipleCompanies.Count; i++)
{
  Html.Hiddenfor(m => m.EmployeesInMultipleCompanies[i])
}

Then stepped through your debugger again, you would see this now in Request.Form:

Request.Form["EmployeesInMultipleCompanies[0]"]
Request.Form["EmployeesInMultipleCompanies[1]"]

etc...

Now the model binder can do work.

Alternatively, you could create a custom model binder that knows how to read the fields from a single input.


I have created a Gist to demonstrate the different types of possible results when doing this kind of ModelBinding with a generic IList here: https://gist.github.com/xDaevax/bd35b5d88fed03709d30

xDaevax
  • 2,012
  • 2
  • 25
  • 36
  • Are those supposed to be for loops not foreach? – Antarr Byrd Sep 10 '14 at 18:38
  • Does this approach still require me to use List instead of IList? – Antarr Byrd Sep 10 '14 at 18:42
  • @AntarrByrd No you shouldn't have to: http://stackoverflow.com/questions/653514/asp-net-mvc-model-binding-an-ilist-parameter <-- That question is old but the answer should still be relevant. – xDaevax Sep 10 '14 at 18:43
  • Its still not working for me right now. I will move on for now, thanks – Antarr Byrd Sep 10 '14 at 18:47
  • @AntarrByrd The only exception would be if the `EmployeesInMultipleCompanies` is a non-primitive type (user defined). If your IList is of `string` or `Guid` for example, you'll be fine. If it is something like IList, you'll potentially have an issue. If you would like to discuss this more, we could in chat. – xDaevax Sep 10 '14 at 18:56
  • @AntarrByrd I have added a Gist example to my answer to illustrate the behavior I'm describing. – xDaevax Sep 10 '14 at 19:16