MVC Html.HiddenFor in a loop passing model back to Controller

Question

I am looping through an IEnumerable of my model:

@model IEnumerable<Testing.Models.ProductItem>
@{
    ViewBag.Title = "Buy Products";
}

<div class="row">
    @foreach (var product in Model)
    {
        using (Html.BeginForm())
        {
            @Html.HiddenFor(Model => product)
            ... More Controls and stuff...
            <input type="submit" value="Add To Kart" class="btn btn-info">

        }

    }
</div>

and on submit I want to pass the selected instance of my model back to my controller:

    [HttpPost]
    public ActionResult Index(ProductItem product)
    {
        ... Do Stuff ...
        return View();
    }

However I have tried a few things but always seem to be getting null passed into the controller... Please could someone please help point me in the right direction?

EDIT

I dont actually need to the full model instance as I can get this within the controller from the ID - so I have tried the following:

@model IEnumerable<Testing.Models.ProductItem>
@{
    ViewBag.Title = "Buy Products";
}

<div class="row">
    @foreach (var product in Model)
    {
        using (Html.BeginForm())
        {
            @Html.HiddenFor(Model => product.ID)
            @Html.TextBox("qty", "1", htmlAttributes: new { @style = "width: 30px;" })

            ... More Controls and stuff...

            <input type="submit" value="Add To Kart" class="btn btn-info">

        }

    }
</div>

which posts to the controller:

[HttpPost]
public ActionResult Index([Bind(Include = "ID")] int? ID, [Bind(Include = "qty")] int? qty)
{
    return null;
}

The textbox is not part of the model as it is user input - this value is passed nicely into the actions parameter, however I am still getting a null for the ID in the HiddenFor control. Is this to do with the naming of the control? I dont seem to be able to add a name to the HiddenFor control.

I know this puts a different light on the original question but I am hoping you may still be able to help.

I take the note about the BeginForm being inside the loop - creating for each item in the list... Is there an easy alternative to this (note I haven't tried anything yet).

There are multiple issues with your code. First you cannot bind a complex object to form control (you need to bind each property if the model). And have a form for each item in the collection makes no sense - you can only submit one form at a time and in any case you would be generating inputs that have no relationship to your model — , Jul 31 '17 at 00:19
Either generate your form controls in a single form using a `for` loop or `EditorTemplate` and submit it all in one action (refer [this answer](http://stackoverflow.com/questions/30094047/html-table-to-ado-net-datatable/30094943#30094943)) or if your wanting to update only one item at a time, use ajax or have a link that redirects your to a page for editing that item — , Jul 31 '17 at 00:22
Thanks both... In light of that I have updated the question a little bit to passing the ID instead of the full model. If you were able to take a look and let me know your thoughts now would be great. — CJH, Jul 31 '17 at 08:19
You need to read the previous comments and the link I gave you. Your view makes no sense, and your generating control with `name="product.ID", but your model does not contain a complex property named `product` (nor does the parameters in your method). And you should remove those pointless `[Bind]` attributes — , Jul 31 '17 at 08:23

Brandon Osborne · Answer 1 · 2017-07-31T12:32:56.743

2

It sounds like you're trying to use HiddenFor on a complex type and that won't work. You'll need to use a property of ProductItem like ProductId or something like that, which will most likely be an int or Guid.

Now that you have cleared up the complex binding to a simple field, you'll notice that your name is being set to product.id and that is why it is always null in your controller. You can't override the name attribute with Hidden for, so you'll want to change your code to:

@foreach (var product in Model)
{
    using (Html.BeginForm())
    {
        @Html.Hidden("ID", product.ID)
        @Html.TextBox("qty", "1", htmlAttributes: new { @style = "width: 30px;" })

        <input type="submit" value = "Add To Kart" class="btn btn-info">
    }

}

edited Jul 31 '17 at 12:32

answered Jul 31 '17 at 00:18

Brandon Osborne

835
1
12
26

Thanks codepros... In light of that I have updated the question a little bit to passing the ID instead of the full model. If you were able to take a look and let me know your thoughts now would be great. – CJH Jul 31 '17 at 08:19
Heya @CJH, HiddenFor is going to set the name and id automatically. You can override ID, but you can't easily override the name. So, to get the functionality that you want you'll want to use Html.Hidden, for example: using (Html.BeginForm()) { Html.Hidden("ID", product.ID) Html.TextBox("qty", "1", htmlAttributes: new { style = "width: 30px;" }) } – Brandon Osborne Jul 31 '17 at 12:29

score 1 · Answer 2 · answered Jul 31 '17 at 19:10

1

I have managed to arrive at my desired functionality (rightly or wrongly) with the following:

@model List<ShoppingKartTest.Models.ProductItem>
@{
    ViewBag.Title = "Buy Products";
}



@foreach (var item in Model)
{
    using (Html.BeginForm())
    {

       <input type="hidden" value="@item.ID" name="ID" />
       @Html.TextBox("qty", "1", new { @style = "width: 30px;" })


       <input type="submit" value="Add To Kart" class="btn btn-info">
    }
}

Which correctly submits the Hidden ID and the contents of the Textbox to the Controller Action:

[HttpPost]
public ActionResult Index(int ID, int qty)
{
    //... Do stuff with parameters...
    return View();
}

I would be interested to hear any comments on this. I know that I was told above that I shouldn't have my BeginForm within the loop... But it just works for me.

answered Jul 31 '17 at 19:10

CJH

1,266
2
27
61

There's nothing "wrong" with this code, per se. It renders pretty much exactly as the code that I gave you early this morning. Purists are not arguing that your approach won't work, but that it is ugly and goes against modern best practices / isn't scalable. Your code is needlessly rendering lots of form open and close tags bloating your page. A more correct and modern approach would be to have a single Html.BeginForm or Ajax.BeginForm and have your controller accept the 'add to cart' clicks through AJAX. – Brandon Osborne Jul 31 '17 at 20:54
Thanks for the feedback codepros... I do appreciate it. My current project is never going to see a production environment and is to be used purely as a functional example so I may just get away with it... This time... – CJH Jul 31 '17 at 22:08
1

There is a lot that is bad with this code. You not 2-way binding to a model, you do not get validation and it will not work correctly if you had to return the view. Why even use MVC if your going to write code that negates all its benefits. – Jul 31 '17 at 23:32
While I concur with most of what Stephen says, for whatever reason this is not what the OP wants. He's obviously new to MVC and just wants it to function. Multiple people have said it isn't the best approach, but obviously, he doesn't care about that, but perhaps more important than that is the fact that you don't know his requirements. He obviously wants it out the door and doesn't particularly care about quality. I suppose that's what makes he and I management. – Brandon Osborne Aug 01 '17 at 02:26
Thanks both for your comments... I am very new to MVC and am finding it sometimes difficult to buck the trend from event driven development (as yo could probably tell). I did take a look through your link Stephen however time is against me on this and I would prefer to 'get it working' then if I have time, revisit to get it working "correctly", if I am able to transfer your answer into my question (Remember, I am a beginner!!!)... If I do then I will certainly update this post with my results. Coderpros, I am not sure whether you were defending my position or the opposite there...? But Thanks! – CJH Aug 01 '17 at 08:46
@CJH Defending & pretty much saying exactly what you did. Sometimes it's more important to get it done than to get it done right. Developers tend to be a little myopic and don't quite get the concept of a proof of concept and the importance of getting things done on time over being done perfectly. – Brandon Osborne Aug 02 '17 at 20:16
I hear you brother... Thanks coderpros! Nail on the head there. – CJH Aug 02 '17 at 21:08

score -1 · Answer 3 · answered Jul 31 '17 at 13:02

Instead of Model => product.Id, try p=> product.Id

   @model IEnumerable<Testing.Models.ProductItem>
   @{
       ViewBag.Title = "Buy Products";
    }

    <div class="row">
    using (Html.BeginForm())
    {
       @foreach (var product in Model)
       {
             @Html.HiddenFor(p => product.ID)
             @Html.TextBox("qty", "1", htmlAttributes: new { @style = "width: 
           30px;" })

            ... More Controls and stuff...         

        }
        <input type="submit" value="Add To Kart" class="btn btn-info">
    }
    </div>

That would still render hidden input with a name of product.ID which means that his controller would never pick up the value (based on how it is presently written). — Brandon Osborne, Jul 31 '17 at 17:43

MVC Html.HiddenFor in a loop passing model back to Controller

3 Answers3