Ramesisiii Ramesisiii - 3 months ago 30
ASP.NET (C#) Question

Html.Hidden() is not binding the right values in foreach loop

I have a View that uses foreach loop to display the student list with a delete option. But the Html.hidden is not getting the correct student.ID as intended.

Here is my student class:

public class Student
{
public Guid ID { get; set; }
public string Name { get; set; }
}


My controller

public ActionResult Index()
{
var students = db.Students.OrderBy(s => s.ID).ToList();
return View(students);
}

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult DeleteStudent(Guid Id)
{
if (Id != null)
{
Student student = db.Students.Single(s => s.ID == Id);
if (student == null)
{
return View("Error");
}
db.Students.Remove(student);
db.SaveChanges();

return RedirectToAction("Index");
}
return View("Error");
}


My View

@model IEnumerable<MyProject.Models.Student>
@{ ViewBag.Title = "Student List"; }
<h2>@ViewBag.Title</h2>

<ul>
@foreach (var student in Model)
{
<li>
@Html.displayFor(modelItem => student.Name)
@using (Html.BeginForm("DeleteStudent", "MyController", FormMethod.Post))
{
@Html.AntiForgeryToken()
@Html.Hidden("Id", student.ID)
<input type="submit" value="Delete" class="btnSubmit">
}
</li>
}
</ul>


Say there are 3 students with ID's 1, 2 and 3 accordingly. But when I try to delete student 3, at the controller, the parameter
Guid Id
has the value 1, so the student with ID = 1 gets deleted.

In fact, no matter which student I delete,
Id
always takes the value of the first student on the list.

What am I doing wrong, or what's a better way to go about it?

Answer

Revised answer

Having a second look at your requirements, the @Html helpers are not really giving any benefit to you. Your basic setup is that you are generating a large number of forms, each designed simply to delete one item. The minimum requirement is some way of identifying the item. The problem at the moment is that @Html.Hidden generates both a name="ID" and id="ID" and duplicate IDs are not allowed and cannot be processed correctly.

There are several ways I can think of to fix this:

  1. Pass the id as a parameter to the form itself (my preferred solution)

e.g.

@using (Html.BeginForm("DeleteStudent", "MyController", FormMethod.Post, new {id=student.ID})){

The controller just accepts an id value as is parameter. This means you do not need a hidden field.

  1. Provide a single hidden value, via a manually generated hidden field

e.g.

<input name="ID" value="@student.ID" type="hidden"/>

The controller just accepts an id value as is parameter.

  1. Use the @Html.HiddenFor as described below, but the controller action takes a generic FormCollection parameter and you pull out the individually named element (probably the worst option now).

Original answer

As a rule, try to use @Html.xxxFor methods when you can. They then name items correctly for the postback.

As a second rule, you can't use foreach looks to render @Html.xxxFors as those methods actually interpret the expression and look for an index value. A foreach does not provide an index value, so you have to use a simple for loop instead, so that the expressions can look like Model[i].Property.

e.g.

for (int i = 0; i < Model.length; i++)
{
    @Html.EditorFor(x=>Model[i].Name)
}

The indexing present in the expression results in a name="Name[0]", name="Name[1]" etc in the input field.

Back to the question at-hand: you want a hidden field, for each form, so you should use the for loop and @Html.HiddenFor like this:

for (int i = 0; i < Model.length; i++)
{
    @using (Html.BeginForm("DeleteStudent", "MyController", FormMethod.Post))
    {
        @Html.AntiForgeryToken()
        @Html.HiddenFor(x=>Model[i].ID)
        <input type="submit" value="Delete" class="btnSubmit">
    }
}

Also, as mentioned in comment, your view model appears wrong. Generally you would use an IEnumerable or a IList like:

@model IEnumerable<MyProject.Models.Student>