2

I Have the following in my razor view

                @foreach (var group in Model.QuestionList.GroupBy(x => x.AreaName))
            {
                <h4>@group.Key</h4>

                for (int i = 0; i < Model.QuestionList.Count(x => x.AreaName == group.Key); i++)
                {

                <div class="form-group">
                    <div class="row">
                        <div class="col-md-4">
                            @Html.DisplayFor(x => Model.QuestionList[i].Question)
                        </div>
                        <div class="col-md-2">
                            @Html.HiddenFor(x => Model.QuestionList[i].StnAssureQuestionId)
                            @Html.DropDownListFor(model => model.QuestionList[i].Score, new SelectList(Model.QuestionList[i].Scores, "ScoreId", "ScoreNum", 0), "Please Select", new { @class = "form-control" })
                            @Html.ValidationMessageFor(model => model.QuestionList[i].Score, "", new { @class = "text-danger" })
                        </div>
                        <div class="col-md-4">
                            @Html.EditorFor(x => Model.QuestionList[i].Comments, new { htmlAttributes = new { @class = "form-control" } })
                            @Html.ValidationMessageFor(model => model.QuestionList[i].Comments, "", new { @class = "text-danger" })
                        </div>
                    </div>
                </div>
                }
            }

I want to be able to display all the objects in QuestionList but group them by AreaName, which is the group key.

The current code displays the title of the first group then the questions in that group but after that all it does is display the next group name followed by the same questions then again for all the group.

It's a no brainer I'm sure but I'm still not skilled enough to spot it.

Phill Sanders
  • 487
  • 2
  • 10
  • 30

4 Answers4

6

You might be able to get by with something like this, but I'd take other's advice about creating a specific view model for this view also.

@foreach (var group in Model.QuestionList.GroupBy(x => x.AreaName))
{
    var questionList = group.ToList();
    <h4>@group.Key</h4>
    for (int i = 0; i < questionList.Count(); i++)
    {
        <div class="form-group">
            <div class="row">
                <div class="col-md-4">
                    @Html.DisplayFor(x => questionList[i].Question)
                </div>
                <div class="col-md-2">
                    @Html.HiddenFor(x => questionList[i].StnAssureQuestionId)
                    @Html.DropDownListFor(model => questionList[i].Score, new SelectList(questionList[i].Scores, "ScoreId", "ScoreNum", questionList[i].Score), "Please Select", new { @class = "form-control" })
                    @Html.ValidationMessageFor(model => questionList[i].Score, "", new { @class = "text-danger" })
                </div>
                <div class="col-md-4">
                    @Html.EditorFor(x => questionList[i].Comments, new { htmlAttributes = new { @class = "form-control" } })
                    @Html.ValidationMessageFor(model => questionList[i].Comments, "", new { @class = "text-danger" })
                </div>
            </div>
        </div>
    }
}            

Dot Net Fiddle Example

JamieD77
  • 13,796
  • 1
  • 17
  • 27
  • the only problem with binding are you sure that `@Html.DropDownListFor(model => questionList[i].Score` will bind correct on `POST`? – teo van kot Jan 14 '16 at 15:47
  • @teovankot all depends on the model you're binding to and the names you use – JamieD77 Jan 14 '16 at 16:21
  • Thanks @JamieD77 I have no problem with the post side that works fine, I would just like to group the items for diaply purposes only. The solution above doesnt work sadly as I get the following intellisense error: Cant apply Indexing with [] to an to an expression of type System.Linq.IGrouping on all the questionList[i] – Phill Sanders Jan 14 '16 at 16:26
  • @PhillSanders sorry about that.. you have to cast the group to something besides IGrouping.. i updated the answer – JamieD77 Jan 14 '16 at 16:53
  • Thanks @jamieD77 that sorts it and buys me some time to get the other suggested solution implemented – Phill Sanders Jan 14 '16 at 16:59
  • @PhillSanders, Despite what you think, this will not bind to your model when you submit. Each group its creating multiple collections with indexers starting at zero so only the first group will bind. Using a linq query in a view is bad practice. This should be done in the controller using a model to represent your structure similar to KevDev's answer. –  Jan 14 '16 at 20:51
  • @StephenMuecke i dont see how Kevs answer would help if you use foreach either.. Dont think I've ever tried editing a list inside a list before so I'm not real sure how it works.. Maybe using editor templates is the way to go along with a model like Kev suggests – JamieD77 Jan 14 '16 at 20:58
  • @JamieD77 You cant use a `foreach` loop (it needs to be a `for` loop so the controls are correcty named with indexers, or better, custom `EditorTemplates` for the model) –  Jan 14 '16 at 21:00
  • @StephenMuecke yes I discovered that this evening, would be able to point me in the direction of an example of the solution you the others are suggesting as I'm afraid I can't quite get my head round it. Thanks – Phill Sanders Jan 14 '16 at 22:49
  • You need a view model something along the lines of KevDev's answer and populate it on the controller. Unfortunately that answer is incomplete (and incorrectly states to use a `foreach` loop) so I'll add one (but give me an hour or so) –  Jan 14 '16 at 22:53
3

You should not be using complex queries in your view, and while the JamieD77's answer will solve the issue of correctly displaying the items, it will fail to bind to your model when you submit.

If you inspect the html you generating you will see that for each group you have inputs such as

<input type="hidden" name="questionList[0].StnAssureQuestionId" ... />
<input type="hidden" name="questionList[1].StnAssureQuestionId" ... />

but the DefaultModelBinder requires collection indexers to start at zero and be consecutive so when binding, it will correctly bind the inputs in the first group, but ignore those in all other groups because the indexers starts back at zero.

As always start with view models to represent waht you want to display/edit (What is ViewModel in MVC?). In this case I'm assuming the SelectList options associated with Score are common across all Questions

public class QuestionnaireVM
{
  public IEnumerable<QuestionGroupVM> Groups { get; set; }
  public SelectList Scores { get; set; }
}
public class QuestionGroupVM
{
  public string Name { get; set; }
  public IEnumerable<QuestionVM> Questions { get; set; }
}
public class QuestionVM
{
  public int ID { get; set; }
  public string Question { get; set; }
  public int Score { get; set; }
  public string Comments { get; set; }
}

While you could use nested loops in the view

for (int i = 0; i < Model.Groups.Count; i++)
{
  <h4>Model.Groups[i].Name</h4>
  for (int j = 0; j < Model.Groups[i].Count; j++)
  {
    @Html.DisplayFor(m => m.Groups[i].Questions[j].Question)

a better solution is to use EditorTemplates which give you a reusable component (and you would not have to change the collection properties to IList<T>). Note I have omitted <div> elements to keep it simple.

In /Views/Shared/EditorTemplates/QuestionVM.cshtml

@model QuestionVM
@Html.DisplayFor(m => m.Question)
@Html.HiddenFor(m => m.ID)
@Html.DropDownListFor(m => m.Score, (SelectList)ViewData["scores"], "Please Select", new { @class = "form-control" })
@Html.ValidationMessageFor(m => m.Score, "", new { @class = "text-danger" })
@Html.EditorFor(m => m.Comments, new { htmlAttributes = new { @class = "form-control" } })
@Html.ValidationMessageFor(m => m.Comments, "", new { @class = "text-danger" })

In /Views/Shared/EditorTemplates/QuestionGroupVM.cshtml

@model QuestionGroupVM
<h4>@Html.DisplayFor(m => m.Name)</h4>
@Html.EditorFor(m => m.Questions, new { scores = ViewData["scores"] })

and the main view would be

@model QuestionnaireVM
@using (Html.BeginForm())
{
  @Html.EditorFor(m => m.Groups, new { scores = Model.Scores })
  <input type="submit" value="Save" />
}

Then in the get method, project your data model to the view model, for example

QuestionnaireVM model = new QuestionnaireVM
{
  Groups = db.Questions.GroupBy(x => x.AreaName).Select(x => new QuestionGroupVM
  {
    Name = x.Key,
    Questions = x.Select(y => new QuestionVM
    {
      ID = y.StnAssureQuestionId,
      Question = y.Question,
      Score = y.Score,
      Comments = y.Comments
    }
  },
  Scores = new SelectList(.....)
};
return View(model);

and the signature of the POST method would be

public ActionResult Edit(QuestionnaireVM model)

Side note: You do not currently have an input for the group name property which means if you needed to return the view because ModelState was invalid, you would need to run the query again, so consider adding @Html.HiddenFor(m => m.Name) to the QuestionGroupVM.cshtml template (and of course if you do retur the view, you also need to reassign the SelectList property.

Community
  • 1
  • 1
2

When you displaying your questions you should work with your group object (grouped collection) but not with the initial collection.

I mean you should change your

Model.QuestionList[i]

To

group.Select(x => x.QuestionList)[i]

Anyway your solution really messy It's better to do such grouping on server side.

teo van kot
  • 12,350
  • 10
  • 38
  • 70
  • Thanks, Ive tried replacing as suggested but all I get are intelisense errors saying that a definiotion for QuestionList isnt in the viewModel – Phill Sanders Jan 14 '16 at 14:55
  • @PhillSanders i'm sorry but it's not enougth just replace. But i hope you got the idea. I will show full exaple if i have a bit more time. – teo van kot Jan 14 '16 at 15:07
  • Thanks, I understand the principle behind your answer, but sadly dont know where to place it to get it to work – Phill Sanders Jan 14 '16 at 15:10
1

Please consider that a View should be completely agnostic to the business logic implemented by the service layer. View is just a dummy presentation mechanism which grabs data served by a Controller through a ViewModel and displays the data. It is the basic of the MVC architecture and my strong recommendation is that following the architecture is itself a very good reason to go the right way.

That being said the view you have is getting messy. Consider reconstructing it as something like this:

public class QuestionDataViewModel
{
    public List<QuestionData> Data { get; set; }
}

public class QuestionData
{
    public string AreaName { get; set; }
    public List<Question> QuestionList { get; set; }
}

public class Question
{
    public int StnAssureQuestionId { get; set; }
    public int Score { get; set; }
    public IEnumerable<SelectListItem> Scores { get; set; }
    public List<Comment> Comments { get; set; }
}

Construct this server side and just render through simple razor foreach loops. This will not only benefit you with cleaner code but will also help avoid the model binding pain you are about to run into when you post the form back with your current implementation.

KevDev
  • 336
  • 1
  • 8