MVC Controller not Binding for ViewModels

Question

I hope you can help me, I am quite new to MVC and Entity Framework and trying to get my head around it. I am using MVC4 and EF5.

Here is an overview of what I am trying to do...

I have two entities, Site and SiteType. Each Site can have multiple SiteTypes so they have a Many-to-Many relationship. I setup the database with a typical Many-to-Many relationship by creating 3 tables, Site, SiteType and SiteSiteType. SiteSiteType just contained 2 columns, the SiteId and the SiteType Id.

I updated the Entity Model and it worked perfectly creating my entities like this for Site...

public partial class Site
{
    public Site()
    {
        this.SiteTypes = new HashSet<SiteType>();
    }

    public int Id { get; set; }
    public string Name { get; set; }
    public string Address { get; set; }

    public virtual ICollection<SiteType> SiteTypes { get; set; }
}

And this for SiteType

public partial class SiteType
{
    public SiteType()
    {
        this.Sites = new HashSet<Site>();
    }    
    public int Id { get; set; }
    public string Name { get; set; }    
    public virtual ICollection<Site> Sites { get; set; }
}

Now I am using a strongly typed view, however instead of using Site as the type, i am using a custom ViewModel I created called SiteViewModel which looks like this...

public class SiteViewModel
{
    public Site Site { get; set; }
    public bool WasPosted { get; set; }
    public IEnumerable<SiteType> AvailableSiteTypes { get; set; }
    public IEnumerable<SiteType> SelectedSiteTypes { get; set; }
}

The reason for this is I want to use the CheckBoxList extension for MVC as described here

Now everything works when I am creating a new Site and adding SiteTypes to it, however when I tried to edit and existing record i started having problems.

Here is my edit GET method in the controller....

    public ActionResult Edit(int id = 0)
    {
        Site site = _repository.GetSiteByID(id);
        if (site == null)
        {
            return HttpNotFound();
        }

        var model = new SiteViewModel();
        model.AvailableSiteTypes = _repository.GetSiteTypes();
        model.SelectedSiteTypes = site.SiteTypes;
        model.Site = site;

        return View(model);
    }

And the post method originally looked like this....

    [HttpPost]
    public ActionResult Edit(SiteViewModel siteViewModel, int[] siteTypes)
    {

        if (ModelState.IsValid)
        {
            _repository.UpdateSite(siteViewModel.Site, siteTypes);
            _repository.Save();
            return RedirectToAction("Index");
        }

        siteViewModel.AvailableSiteTypes = _repository.GetSiteTypes();
        List<SiteType> selectedSiteTypes = _repository.GetSiteTypes(siteTypes).ToList();
        if (selectedSiteTypes.Count > 0)
        {
            siteViewModel.SelectedSiteTypes = selectedSiteTypes;
        }
        return View(siteViewModel);

    }

Now the problem I was having was that siteViewModel.Site was being populated properly from the form with the updated data, however the Id column was being set to "0" even though it was an existing record and not a new one. I have tried everything and I can't work out why. The only way I could get it working was to take in an "id" parameter and manually set it to the Site object like this...

HttpPost]
    public ActionResult Edit(int id, SiteViewModel siteViewModel, int[] siteTypes)
    {
        siteViewModel.Site.Id = id;
        ......

But it just feels really messy compared to how well everything else binds together.

Is this the only way to do this? Am I missing something?

Thanks a lot for taking the time to read my post. Sorry it it is too long, i didn't know how else to explain it...

Answer 1

It's never really a good idea to send actual entities to your view, your view models should only ever take the data it needs to present the view to the user. If you need to pass information about each Site to the view then create a separate view model or DTO and pass that instead e.g.

siteViewModel.Site = new SiteDto()
{
    Id = site.Id,
    Name = site.Name,
    ...
}

Entities have hidden properties which are used to map it back to the record in the database, and when you pass entities to/from views this information usually gets lost. The correct approach is to send the view all the data it needs, and no more. Then in your POST method, re-pull your entity and apply the changes from the view model.

You can use AutoMapper if you find yourself doing this all over the place.