1

I have a string array called Groups containing the following two strings

  1. Group Alpha
  2. Group Bravo

Controller

public ActionResult Task(TaskModel model)
{

    return PartialView(model);
}

Index view which invokes my Task action

<div id="update">

    @using (Ajax.BeginForm("Task", "Home", new AjaxOptions { UpdateTargetId = "update", InsertionMode = InsertionMode.Replace}))
    {

        <input value="Group Alpha" name="Groups"/>
        <input value="Group Bravo" name="Groups"/>

        <button type="submit">Submit</button>
    }

</div>

My task view

@model MvcApplication4.Models.TaskModel

<div id="container">

        @if (Model.Groups != null)
        {

            foreach (var group in Model.Groups)
            {

                @Html.TextBox("Groups", group, new { @id = String.Empty })                                                         

            }
        } 
</div>

my task model

public class TaskModel : IValidatableObject
{
    public Guid TaskId { get; set; }
    public String Name { get; set; }

    public string[] Groups { get; set; }

    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {

        yield break;
    }
}

This only outputs Group Alpha Group Alpha. Wheres the problem??

Sorry for not being clear before

Example project replicating the issue https://www.dropbox.com/s/s2o59cu00am3eov/MvcApplication4.zip

heymega
  • 9,215
  • 8
  • 42
  • 61
  • Can you add some demo code for `Model.Groups`? – Patrick Hofman Mar 27 '14 at 16:09
  • 5
    Do you think you are the first person to use `@Html.TextBox` in a `foreach` loop? That is very unlikely, so it's more likely the bug is in your code. Are you sure you're calling the correct overload? Can you show how you fill `Model.Groups`? Can you print `@Html.Raw(group)`? – CodeCaster Mar 27 '14 at 16:09
  • 3
    Odds are that whatever `IEnumerable` you're using is written such that it mutates its results as it is iterated, one common example being closing over the loop variable pre C# 5.0, although there are certainly other ways of making really evil iterators like this. – Servy Mar 27 '14 at 16:10
  • 1
    @JoachimIsaksson The issue of closing over the loop variable wouldn't be in the code shown; it would be in the implementation of `Groups`. The code here is going to be evaluating any lambdas within the body of the loop. – Servy Mar 27 '14 at 16:11
  • @Servy Ok, shows my Razor knowledge ;) Correct problem, wrong place to fix then. – Joachim Isaksson Mar 27 '14 at 16:12
  • @JoachimIsaksson It may or may not be the exact same problem. That's *possible*, but there are certainly other ways of creating iterators that mutate themselves or even already yielded items as you iterate them. There's just no real way of knowing what is wrong without seeing the relevant code. – Servy Mar 27 '14 at 16:14
  • @CodeCaster - I'm calling this overload TextBox(this HtmlHelper htmlHelper, string name, object value, object htmlAttributes); which is correct. I can output the values correctly if I use normal html – heymega Mar 27 '14 at 16:16
  • Again, please show how you assign the Groups property. I've copy-pasted your code in a new MVC 3 project and using `return View(new TaskModel { Groups = new string[] { "A", "B" } });` it just works. – CodeCaster Mar 27 '14 at 16:30
  • @CodeCaster I've updated my answer. It just binds the values in using the default model binder. I've checked this and the binder has bound the groups in correctly – heymega Mar 27 '14 at 16:40
  • 1
    You can _say_ that you saw it work, but I don't believe that, because for me it works like intended. Please update your question to _show_ the code that actually returns this model to the view. Try with a hardcoded model like me. We really can't do anything with your question as-is. – CodeCaster Mar 27 '14 at 16:44
  • 1
    @heymega Given that the problem is very clearly not with the code that you have shown, *please* show us the code that we're asking to see, rather than just telling us that "it's right". Clearly it's not, or *something* isn't right somewhere else. The code shown doesn't replicate your problem though. – Servy Mar 27 '14 at 16:45
  • Ive added a link to a project that replicates the issue – heymega Mar 27 '14 at 16:57
  • 1
    @heymega You should be providing enough information within the question itself to be able to replicate the problem. Adding a link to your entire project is not appropriate. Condense your program into a minimal example that is still capable of replicating the problem but that doesn't contain code irrelevant to the problem at hand. – Servy Mar 27 '14 at 17:01
  • @Servy Ive pasted my code as well lol The project ive put up is just enough to replicate the issue. Its not the entire project lol – heymega Mar 27 '14 at 17:03

2 Answers2

3

You POST some input fields named Groups. In the action method processing that POST, you render a partial that uses @Html.TextBox("Groups", ...).

@Html.TextBox uses the ModelState to fill the value with the previously POSTed value, based on input element name. In this case it takes the first Groups value it finds in the POST, being "Group Alpha".

Clear the ModelState (or just the Groups key) before rendering your partial view.

This:

ModelState.Clear();
ModelState.Remove("Groups");

return PartialView(model);

Works.

Next time try showing all relevant code up front, so we don't need so many comments asking you to. :-)

CodeCaster
  • 147,647
  • 23
  • 218
  • 272
  • Ahh I actually had this in another action :( Thanks for your help but i was going as fast as I could. Its company code so I have to refactor it first :( – heymega Mar 27 '14 at 17:12
  • @CodeCasterv - Isnt this a defect though? Why would it base it on input element name? Its perfectly legal to have multiple elements with the same name – heymega Mar 27 '14 at 17:15
  • That's valid (and you can perfectly read the values from code), but this isn't PHP and it's not how you're supposed to process lists of values for the same property. See [Submitting form elements with the same name](http://stackoverflow.com/questions/437955/submitting-form-elements-with-the-same-name) on how to achieve the same behavior using MVC. – CodeCaster Mar 27 '14 at 17:16
  • 1
    I totally get where youre coming from but if that's the case, the defaultmodelbinder shouldnt bind collections based on same name as mentioned here - http://haacked.com/archive/2008/10/23/model-binding-to-a-list.aspx/ Thanks for your help though it was stressing me out :) – heymega Mar 27 '14 at 17:26
-1

This is a closure problem I think. Try this code:

<div id="container">

    @if (Model.Groups != null)
    {
        foreach (var group in Model.Groups)
        {
            var currentGroup = group;
            @Html.TextBox("Groups", currentGroup, new {@id = String.Empty});
        }
    } 
</div>
Haney
  • 32,775
  • 8
  • 59
  • 68
  • He's *not* closing over that variable in the code shown, and even if he were, it wouldn't produce the indicated problem (there's no way that it would result in the first item being printed twice; at most it could result in the last item being printed twice). The problem is in code that's somewhere else. – Servy Mar 27 '14 at 16:38
  • @Servy - can you explain how he's not closing over the variable? – Haney Mar 27 '14 at 16:50
  • Can you explain how he *is* closing over the variable, given that there's no anonymous method, which is the only way of creating a closure in C#? – Servy Mar 27 '14 at 16:53
  • I assume it is in the `@Html.TextBox` method, since it is not visible in his code. That, or the Razor compilation. I'm not entirely sure, which is why I said *I think* and not *I know*. Just trying to help. – Haney Mar 27 '14 at 17:03