0

In my MVC application, I normally have ViewModel for such actions as Edit, mainly for populating selection lists for the properties to modify. For example my Model looks like this:

class MyModel{
    public int? NavPropID {get; set;}
    public AnotherModel NavProp {get; set;}
    ...
}

Now I was wondering, how should I design my ViewModel to reference the NavProp.

 class MyViewModel{
    public int? NavPropID {get; set;}
    public AnotherModel NavProp {get; set;}
    ...
 }

Should I use both NavPropID and NavProp, or should I use only one (which?) in the ViewModel?

I wanted to use only NavProp at first, because it feels more natural to me (I feel it hides the database implementation detail and it is how I do in WPF and MVVM). But then in my Edit view, I have something like this:

@Html.DropDownListFor(model => model.NavProp, 
                         new SelectList(Model.AllAnotherModels.Select(
                           i => new {Item = i, Text = i.Name}), "Item","Text"))

In the postback of Edit action, I can see that NavProp is not correctly bonded by the binder because the attemptedValue is of type string and the value is the class name of NavProp (which I guess means it uses ToString() method to post back to the controller). How can I make it work for the post back action?

I then tried to only have NavPropID in the ViewModel. But there are two problems: 1) it means I have to load the actual NavProp in the controller before I use AutoMapper to map back from ViewModel to Model. I feel that this is beyond the responsibility of a controller and 2) even if I load the actual property in the controller, I have some problem later when I am updating by calling DBSet.Attach().

So what is the best practice for the ViewModel to reference a navigational property in the Model?

UPDATE: Here is my post back function of the Edit action. It is generic I think so I didn't paste it in the first place.

[HttpPost]
public ActionResult Edit(MyViewModel vm)
{
   if (ModelState.IsValid)     
   {
      ... // I use Automapper to map back to Model and then 
          // use UnitOfWork and Repository to update the Model
   }
}

And my ModelState.IsValid is false so it can't proceed. As I mentioned, I then checked the ModelState (a dictionary) and found out that NavProp is invalid and the attemptedValue is the class name while I think it should be the actual value of the property.

tete
  • 4,859
  • 11
  • 50
  • 81
  • I can't see the elusive `Edit` action of which you speak.... where is that? – Luke Oct 06 '15 at 12:57
  • @Coulton, please see my update of the original post – tete Oct 06 '15 at 13:08
  • You do not need `AnotherModel NavProp` in your view model. And you should not because of `AnotherModel` contains any properties with validation attributes you model will be invalid (all you are binding to is `NavPropID`). Your view model should however have a `SelectList` property. And you do not need `NavProp` in the data model to save it - the only property which is saved to the database table is `NavPropID` –  Oct 07 '15 at 03:46
  • @StephenMuecke I guess what you mean is I should only have `NavPropID` in my `ViewModel`, and in the post back of the action `Edit`, the controller is responsible to find the `AnotherModel` entity with passed `NavPropID`? As I mentioned I tried that too but something is wrong with the `Attach` function call when I save the changes (the `ModelState.IsValid` is true though). I'll test a bit more tomorrow. – tete Oct 07 '15 at 11:25
  • Yes, you should only have `NavPropID`. but if you saving `MyModel` to the database, then you should not really need to find the `AnotherModel` entity since its the value of `NavPropID` that is written to the database. –  Oct 07 '15 at 11:29
  • @StephenMuecke do you mean I should leave the `NavProp` property of the generated `MyModel` (I create one `MyModel` object in the post back and use `AutoMapper` to map the properties from passed `MyViewModel` to generated `MyModel`) empty and then call the `DbContext` save routine? I mentioned I had some problem with `Attach` function but it might not be relevant to this question. I'll test it tomorrow. Thank you for your answer. – tete Oct 07 '15 at 16:08
  • @StephenMuecke I believe your suggestion works: I only have `NavPropID` in my `ViewModel` and not to lookup and assign the actual navigation property in the controller before calling `Update`. So only the `NavPropID` is posted back and that's enough to update the database. Unless of course I need to use the navigation property later in the same function. Thanks! – tete Oct 08 '15 at 06:47

1 Answers1

1

In terms of having your NavPropID within the base view model, we need to establish whether it's appropriate to flatten your view model down.

If the view model is for a page that displays a single navigation properties' information, I would be tempted to have the following:

// Domain model
public class MyModel
{
    public int? NavPropID { get; set; }
    public NavProp NavProp { get; set; }
    ...
}

// Nav prop domain model
public class NavProp
{
    public int? NavPropID { get; set; }

    // Another property of your navigation property
    public string AnotherProperty { get; set; }
}

// Flat view model containing all your navigation properties' properties
public class MyViewModel
{
    public int? NavPropID { get; set; }
    public string AnotherProperty { get; set; }
}

If you wanted you could introduce another view model to represent your NavProp item - but it's overkill if you are happy to flatten it:

// Domain model
public class MyModel
{
    public int? NavPropID {get; set;}
    public NavProp NavProp {get; set;}
    ...
}

// Nav prop domain model
public class NavProp
{
    public int? NavPropID {get; set;}

    // Another property of your navigation property
    public string AnotherPropery {get; set;}
}

// View model representing your navigation property
public class NavPropViewModel 
{
    public int? NavPropID { get; set; }
    public string AnotherProperty { get; set; }
}

// Main view model
public class MyViewModel
{
    // Use another view model that represents your NavProp
    public NavPropViewModel NavProp { get; set; }
    ...
}

I would say that it is definitely the responsibility of the controller to map from the objects received from your business layer to your view models and vice versa.

To make this cleaner, I would introduce DTOs to map to which can then be passed up to your business layer. Check out general comments on this post.

Update

Here's how you would create your form to post back to MyViewModel with a NavPropViewModel field:

Example View:

@model MyViewModel
@using (Html.BeginForm())
{
    @Html.LabelFor(x => x.NavProp.AnotherProperty)
    @Html.TextBoxFor(x => x.NavProp.AnotherProperty)
}

Example controller:

[HttpPost]
public ActionResult Edit(MyViewModel vm)
{
    // Get the posted value of AnotherProperty
    var postedValue = vm.NavProp.AnotherProperty;
}
Community
  • 1
  • 1
Luke
  • 22,826
  • 31
  • 110
  • 193
  • Thank you for your answer. I have something unclear: 1) (a minor one) why in your `NavProp` class, the `NavPropID` is nullable? I have `NavPropID` is `MyModel` and `MyViewModel` as nullable because the navigational property actually can be empty. 2) I understand the point of having DTO. But does it mean that in the post back function, it is absolutely impossible to pass back a complex class object (in my case `NavProp` which is of type `AnotherModel`), but only generic values such as int and string? – tete Oct 06 '15 at 13:53
  • I did that because your example had `NavPropID` as nullable. If you will never be passing a null then change `int?` as appropriate. It could be valid in your code for all I know :P – Luke Oct 06 '15 at 13:55
  • Your second point isn't true, you can easily pass back a complex object. Let me show you in the answer. – Luke Oct 06 '15 at 13:58
  • Coulton, I'll test it tomorrow. My connection to my development machine is really unstable from home. – tete Oct 07 '15 at 11:26