MVC structure query

Question

Evening all, I'm chasing my tail trying to get the structure of a simple ASP.NET MVC correct. From the start I should say I'm completely new to ASP and MVC, I've used a bit C# before.

For examples sake let's say I'm trying to check two pieces of information e.g. a username and password that a user enters, against a database that stores all the user info. If they put in the correct credentials a summary of the user info is displayed, if not they are taken back (well visually never leave) the login page with a please try again message.

So I have my Home (Login page) view, a HomeController and a HomeIndexViewModel. Similarly I have an Account view, a AccountController and an AccountUserViewModel (there is also AccountIndexViewModel but that isn't really used).

The Home view 'takes in' (through it's controller) a HomeIndexVM as it's model:

@model ViewModels.HomeIndexViewModel

@using (Html.BeginForm("User", "Account", FormMethod.Post))
{
    if (@Model.PreviousAttempts)
    { 
        <p><b>Username or password were not recognised please try again.</b></p>
    }

    <p>Username: @Html.TextBoxFor(x => x.Username)</p>
    <p>Password: @Html.PasswordFor(x => x.Password)</p>
    <input id="btnLogin" type="submit" value="Login" />
}

HomeController:

public class HomeController : Controller
{
    public ActionResult Index(bool invalidLogin = false)
    {
        var vm = new HomeIndexViewModel() { Username = string.Empty, Password = string.Empty, PreviousAttempt = invalidLogin };

        return View(vm);
    }
}

And finally the HomeIndexViewModel.cs

public class HomeIndexViewModel
{        
    public string Username { get; set; }
    public string Password { get; set; }
    public bool PreviousAttempt { get; set; }
}

I think that is okay so far. Now on clicking the login button, it will post to Account, User.

public ActionResult User(UserLogin userLogin)
{
    if (!ModelState.IsValid)
        return RedirectToAction("Index", "Home", new { invalidLogin = true });

    // Match username and password against database
    User user = userLogin.IsValid(userLogin.Username, userLogin.Password);
    if (user != null)
    {
         return this.View(user);
    }

    return RedirectToAction("Index", "Home", new { invalidLogin = true });
}

There is a couple of things here, you can see the redirects back to the login page with the true flag to show the failed login message.

But more to my point, clearly it won't work as it takes a UserLogin as a parameter. This is a model object that contains:

public class UserLogin
{
    private NewDBSolution_v1Entities accountsDB = new NewDBSolution_v1Entities();

    [Required, MinLength(2)]
    public string Username { get; set; }

    [DataType(DataType.Password), Required]
    public string Password { get; set; }

    public bool PreviousAttempts { get; set; }

    public User IsValid(string username, string password) // Ideally use the local username and password properties rather than pass in as they are the same.
    {            
        // Match username and password against database and return full user info if match found, otherwise return null
    }
}

So what am I asking... well is it best practice for, in this case, the Account's User Action to take the HomeIndexViewModel, even although it's Home as opposed to Account related? Or should I pass in a model object as I had originally done, and use it to the validation (what I don't like about this is the validation is done in object that is passed from a different view if that makes sense?)

How do you best bundle information up from a view to pass to an Action? I realize VMs and Model objects the compiler doesn't care they are just classes but I'd like to get my separation of concerns correct.

Basically here all I need is the username and password from the Home view, should that be bundled in to a VM or a M?

It just seems to me there is potential for an awful lot of classes that are just slightly different, so why not make one and use it. I suppose that is where inheritance comes in, but do you gain much there if ever subclass just adds one different property?

Anyway I keep going in circles about the best way to structure. I did read somewhere that VMs should basically be a mask/adapter over the model such that the view only sees just what it needs. But these VMs don't have a model associated with them.

I'm rambling now, if anyone can make head or tail of this and give me a few pointers I'd be very grateful, thanks.

Answer 1

I think you might be going round in circles because you've created the login view in the HomeController. Login code is Account-related, so why not put this into the AccountController instead?

I tend to favour posting to the same action as you get from, if you follow the Post-Redirect-Get pattern, as you're trying to do here. I would move the login action into the AccountController. I would move the code that does the database check into a separate class, rather than keeping it in the model. Perhaps a membership provider or something like that. I would pass that provider into the Controller - this allows the Controller and the Model object to stop worrying about how to decide whether a user is valid or not. They can just ask the provider to tell them.

It's generally good practice to keep your entities (whether UI or domain) simple. They certainly should have database connection objects inside them.

Your Controller would look something like this:

public class AccountController : Controller
{
    private readonly IMembershipProvider membershipProvider;

    public AccountController(IMembershipProvider membershipProvider)
    {
        this.membershipProvider= membershipProvider;
    }

    public ActionResult Login()
    {
        var viewModel = new LoginViewModel();  
        return View(viewModel);
    }

    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult Login(LoginViewModel viewModel)
    {
        if (!ModelState.IsValid)
        {
            return View(viewModel);
        }

        var user = membershipProvider.Find(viewModel.Username, viewModel.Password);
        if (user != null)
        {
            membershipProvider.SignIn(user, true);
            return RedirectToAction("Index", "Home");
        }

        ModelState.AddModelError("Login", "Your credentials were not recognised. Please try again.");
        return View(viewModel);
    }
}

Because the Login action has the same name for both Get and Post, your view becomes simpler. You'd have a Login.cshtml view in the Views/Account folder like this:

@model ViewModels.LoginViewModel

@using (Html.BeginForm())
{
    @Html.AntiForgeryToken() // good practice to add this, ties in with the ValidateAntiForgeryToken attribute on the Post action
    @Html.ValidationSummary() // displays model errors if there are any

    <p>Username: @Html.TextBoxFor(x => x.Username)</p>
    <p>Password: @Html.PasswordFor(x => x.Password)</p>
    <input id="btnLogin" type="submit" value="Login" />
}

To answer this specific question:

How do you best bundle information up from a view to pass to an Action?

You let the default model binder take care of that if you can. It's very powerful and can handle most situations in regards to data being passed back from a form. All the model binder does is match name-value pairs from the form collection that is passed back when the form is posted to properties on your class. Provided your class has the string properties Username and Password the model binder will populate them. The actual class that you provide is irrelevant as far as the model binder is concerned. This would work equally well (but anyone else working on the project wouldn't thank you for it!):

public class Dave
{
    [Required, MinLength(2)]
    public string Username { get; set; }

    [DataType(DataType.Password), Required]
    public string Password { get; set; }
}

And then in the Post action:

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult Login(Dave dave)
{
    ...
}

Does that help?