Collection property is posted with all null values for properties

Question

Using the view specified below I am able to get all my properties in just fine, and the Team property contains the right number of items, but all of those items' properties are just null...

View

@using (Html.BeginForm("BuildTeam", "Team", FormMethod.Post, new { crmtItem = Model, team = Model.Team }))
{
    @Html.HiddenFor(m => m.Id);
    @Html.HiddenFor(m => m.ProjectTitle);
    @Html.HiddenFor(m => m.OpportunityStatus);
    @Html.HiddenFor(m => m.ProjectManager);
    @Html.HiddenFor(m => m.ProjectNumber);
    @Html.HiddenFor(m => m.Sector);
    @Html.HiddenFor(m => m.Service);
    @Html.HiddenFor(m => m.Stage);
    for (int i = 0; i < Model.Team.Count(); i++)
    {
        @Html.HiddenFor(m => m.Team[i].DisplayName)
        @Html.HiddenFor(m => m.Team[i].UserName)
    }
    <a class="btn btn-default btn-sm" onclick="location.href='@Url.Action("Index", new { filter= Model.Stage })'"><span class="glyphicon glyphicon-circle-arrow-left"></span></a>
    <a class="btn btn-default btn-sm" href="@Model.WorkspaceUrl"><span class="glyphicon glyphicon-globe"> Workspace</span></a>

    <button type="submit" class="btn btn-default btn-sm pull-right">
        <span class="glyphicon glyphicon-user" /> Team
    </button>
}

ViewModel

public class CRMTItemViewModel
{
    public int Id { get; set; }
    public string ProjectTitle { get; set; }
    public string ProjectManager { get; set; }
    public string BidNoBid { get; set; }
    public string OpportunityStatus { get; set; }
    public string Stage { get; set; }
    public string Sector { get; set; }
    public string Service { get; set; }
    public int? CRMTNumber { get; set; }
    public int? ProjectNumber { get; set; }
    public string WorkspaceUrl { get; set; }
    public List<SharePointUser> Team { get; set; }
}

SharePointUser

public class SharePointUser
{
    public string FirstName { get; internal set; }
    public string LastName { get; internal set; }
    public string UserName { get; internal set; }
    public string DisplayName { get; internal set; }
    public string ImagePath { get; internal set; }
    public byte[] ImageBytes { get; internal set; }
    public Guid GUID { get; internal set; }
}

Controller

[HttpPost]
public ActionResult BuildTeam(CRMTItemViewModel crmtItem, List<SharePointUser> team)
{

    return View(crmtItem);
}

I originally only had the crmtItem paramater in the Controller, but it still failed so I tried splitting it out but without any success.

Really aprpeciate any guidance on this.

Start by removing the `List team` parameter - you values will be bound to the `Team` of `CRMTItemViewModel`. What is the point of this view - all it contains are hidden inputs? — , Mar 07 '18 at 06:17
And you must remove the `new { crmtItem = Model, team = Model.Team }` from your `BeginForm()` - look at the `action` attribute of your `
` element your generating to understand why that can never work — , Mar 07 '18 at 06:18
@StephenMuecke The view displays the items (not included for simplicity), but I have a button which I want to call a controller action into which I pass the item — Bassie, Mar 07 '18 at 06:18
Have a look on https://stackoverflow.com/questions/3866716/what-does-html-hiddenfor-do. He used `@Html.HiddenFor(Model.Id);` — DTeuchert, Mar 07 '18 at 06:21
@StephenMuecke I removed the route object and teams parameter but still getting the same result — Bassie, Mar 07 '18 at 06:22
@DTeuchert I know I can pass the Id in and make another call to the DB, but that seems like I could end up making too many db calls - isnt it just simpler to pass the object directly? — Bassie, Mar 07 '18 at 06:22
If you are not editing anything then DO NOT include a whole lot of inputs - a malicious user could change those to whatever they want. Just pass back the ID and get the object again — , Mar 07 '18 at 06:23
Despite what you may think - your actually degrading performance - your executing all the code to generate all that extra html, then sending all that extra html across the wire and then sending it all back again unchanged — , Mar 07 '18 at 06:24
@StephenMuecke Thanks stephen, that is really helpful. I will just pass the id back then! — Bassie, Mar 07 '18 at 06:25
In any case, the `DefaultModelBinder` cannot bind your collection properties because they all have `internal` setters (they need to be `public`) — , Mar 07 '18 at 06:25
Correct performance is the keychain. So you should only care obout the id in the hidden fields. That was what I suggest — DTeuchert, Mar 07 '18 at 06:28

score 1 · Accepted Answer · answered Mar 07 '18 at 06:55

The DefaultModelBinder cannot set the value of your SharePointUser properties because they have internal setters. You need to make then public

public class SharePointUser
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    ....

In addition, your should remove the new { crmtItem = Model, team = Model.Team } route values in your BeginForm() method. If you inspect the action attribute of your <form> element you will see that its generating

../Team/BuildTeam?crmtItem=yourAssembly.CRMTItemViewModel&team=System.Collections....

which could never bind to your model anyway. In addition, you do not need the second List<SharePointUser> team parameter in your POST method since (afetr removing the internal keywords), the values will be bound to the Team property of crmtItem.

Having said that, what your doing by generating all those hidden inputs is just degrading performance. Generating all that extra html, sending it all to the client and then sending it all back again is almost certainly going to be slower that getting the model again from the database, and you are now allowing malicious users to alter the values in the form which may result in other problems. If you are concerned about making another database call (should be < 2 milliseconds), you can always cache it on the server (say in Session).

Just post back the Id value of the model, which you can do by including it as a route value (no hidden inputs are required)

@using (Html.BeginForm("BuildTeam", "Team", FormMethod.Post, new { id Model.Id}))

and change the POST method to

[HttpPost]
public ActionResult BuildTeam(int id)

although you may need to change that signature if you also have a [HttpGet] method with that same signature

DTeuchert · Answer 2 · 2018-03-07T06:38:44.473

0

To summarize the comments and in the aspect of performance. For your hidden fields. Try to use @Html.HiddenFor(Model.Id); instead.

When you need to store your result in your database at the end, you also only need to use the id of the object for the db query.

(Hints on: http://www.stackoverflow.com/questions/3866716/what-does-html-hiddenfor-do)

edited Mar 07 '18 at 06:38

answered Mar 07 '18 at 06:36

DTeuchert

406
7
14

This is not an answer to the question (which is why the properties are not bound) - its just a comment on better practice which you already have made in the comments! – Mar 07 '18 at 06:38
@Bassie: You already defined your view model correctly at the beginning of the file? `@Model CRMTItemViewModel` – DTeuchert Mar 07 '18 at 06:43
@DTeuchert Thanks for your comments. Yes the view references `CRMTItemViewModel` – Bassie Mar 07 '18 at 08:10

Collection property is posted with all null values for properties

2 Answers2