Problem
I’m a beginner to web programming and just started a MVC project from scratch. Because this will become a large project eventually, I would like to make sure that I’m doing things kind of right from the beginning.
The architecture is the following: ASP.NET 4.6, MVC 5, EF 6, Identity 2.0. I’m using EF Database First approach and Bootstrap 3.3.5. The solution is divided into 3 projects: Data (where I keep my .edmx and model classes), Resources (where I keep strings for localization purposes -and eventually images-), and Web (with my controllers, views, etc).
I’m going to point out a couple of examples in my code where I’m not sure about my approach. I have a navigation bar with an “Administration” link and two submenu links, “Users” and “Roles”.
Users
When a user clicks on “Users”, I’d like to show a table with four columns:
- Username
- Roles (string with the names of all roles assigned)
- Assign Role (button that will take you to another form)
- Remove Role (button that will take you to another form)
This is what the UserIndex.cshtml view looks like:
@model List<MySolution.Data.DAL.ApplicationUser>
@{
ViewBag.Title = Resources.Users;
}
<h2>@Resources.Users</h2>
<hr />
<table class="table table-striped table-hover ">
<thead>
<tr>
<th>@Resources.User</th>
<th>@Resources.Roles</th>
<th></th>
<th></th>
</tr>
</thead>
<tbody>
@foreach (var user in Model)
{
<tr>
<td>
@user.UserName
</td>
<td>
@user.DisplayRoles()
</td>
<td>
@using (Html.BeginForm("UserAssignRole", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Get, new { @class = "form-horizontal", role = "form" }))
{
@Html.AntiForgeryToken()
@Html.HiddenFor(m => m.Where(u => u.Id.Equals(user.Id)).FirstOrDefault().UserName)
<input type="submit" value="@Resources.AssignRole" class="btn btn-default btn-sm" />
}
</td>
<td>
@using (Html.BeginForm("UserRemoveRole", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Get, new { @class = "form-horizontal", role = "form" }))
{
@Html.AntiForgeryToken()
@Html.HiddenFor(m => m.Where(u => u.Id.Equals(user.Id)).FirstOrDefault().UserName)
<input type="submit" value="@Resources.RemoveRole" class="btn btn-default btn-sm" />
}
</td>
</tr>
}
</tbody>
</table>
I added a DisplayRoles() method to my ApplicationUser class that returns a string with the list of assigned roles separated by commas, so that I can plug it directly into the user table in my view. I’m not sure at all about this approach; it does work, but putting logic like that in my model just seems kind of weird. I just haven’t figured a better way to do this.
Then, on my controller, I have the following:
//
// GET: /Admin/UserIndex
[Authorize(Roles = "Admin")]
public ActionResult UserIndex()
{
var users = context.Users.ToList();
return View(users);
}
//
// GET: /Admin/UserAssignRole
[HttpGet]
//[ValidateAntiForgeryToken]
public ActionResult UserAssignRole(UserAssignRoleViewModel vm)
{
ViewBag.Username = vm.Username;
ViewBag.Roles = context.Roles.OrderBy(r => r.Name).ToList().Select(rr => new SelectListItem { Value = rr.Name.ToString(), Text = rr.Name }).ToList();
return View("UserAssignRole");
}
//
// POST: /Admin/UserAssignRole
[HttpPost]
//[ValidateAntiForgeryToken]
[ActionName("UserAssignRole")]
public ActionResult UserAssignRolePost(UserAssignRoleViewModel vm)
{
ApplicationUser user = context.Users.Where(u => u.UserName.Equals(vm.Username, StringComparison.CurrentCultureIgnoreCase)).FirstOrDefault();
this.UserManager.AddToRole(user.Id, vm.Role);
return RedirectToAction("UserIndex");
}
With my UserAssignRoleViewModel looking like this:
/// <summary>
/// ViewsAdminUserAssignRole.cshtml
/// </summary>
public class UserAssignRoleViewModel
{
[Display(Name = "Username", ResourceType = typeof(Resources))]
public string Username { get; set; }
[Display(Name = "Role", ResourceType = typeof(Resources))]
public string Role { get; set; }
}
And the UserAssignRole view being this:
@model UserAssignRoleViewModel
@{
ViewBag.Title = Resources.AssignRole;
}
<h2>@Resources.AssignRole</h2>
<hr />
<div class="row">
<div class="col-md-8">
<section id="assignRoleForm">
@using (Html.BeginForm("UserAssignRole", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Post, new { @class = "form-horizontal", role = "form" }))
{
@Html.AntiForgeryToken()
@Html.ValidationSummary(true, "", new { @class = "text-danger" })
<div class="form-group">
@Html.LabelFor(m => m.Username, new { @class = "col-md-2 control-label" })
<div class="col-md-10">
@Html.TextBoxFor(m => m.Username, new { @class = "form-control" , @readonly = "readonly" })
@Html.ValidationMessageFor(m => m.Username, "", new { @class = "text-danger" })
</div>
</div>
<div class="form-group">
@Html.LabelFor(m => m.Role, new { @class = "col-md-2 control-label" })
<div class="col-md-10">
@Html.DropDownListFor(m => m.Role, (IEnumerable<SelectListItem>)ViewBag.Roles, Resources.DropdownSelect, new { @class = "form-control" })
</div>
</div>
<div class="form-group">
<div class="col-md-offset-2 col-md-10">
<input type="submit" value="@Resources.Assign" class="btn btn-default" />
</div>
</div>
}
</section>
</div>
</div>
I especially am not sure about the way that I use my controller actions, and how I’m calling them from my forms. And does it make sense to have a Get and Post method for the same action, or should I be doing something else?
Roles
The “Roles” section is very similar, with a table with three columns:
- Name
- Edit (button that will take you to another form to rename the role)
- Delete button (button that will show a modal asking for verification)
On top of the table, there’s a separate button allowing the user to add a new role.
Here’s my RoleIndex.cshtml view.
@model IEnumerable<Microsoft.AspNet.Identity.EntityFramework.IdentityRole>
@{
ViewBag.Title = Resources.Roles;
}
<h2>@Resources.Roles</h2>
<hr />
@using (Html.BeginForm("RoleCreate", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Get, new { @class = "form-horizontal", role = "form" }))
{
<input type="submit" value="@Resources.CreateRole" class="btn btn-default btn-sm" />
}
<hr />
<table class="table table-striped table-hover ">
<thead>
<tr>
<th>@Resources.Role</th>
<th></th>
<th></th>
</tr>
</thead>
<tbody>
@foreach (var role in Model)
{
<tr>
<td>
@role.Name
</td>
<td>
@using (Html.BeginForm("RoleEdit", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Get, new { @class = "form-horizontal", role = "form" }))
{
@Html.AntiForgeryToken()
@Html.HiddenFor(m => m.Where(r => r.Id.Equals(role.Id)).FirstOrDefault().Name)
<input type="submit" value="@Resources.Edit" class="btn btn-default btn-sm" />
}
</td>
<td>
<input type="submit" value="@Resources.Delete" class="btn btn-default btn-sm" data-toggle="modal" data-target="#confirm-delete"/>
<div class="modal fade" id="confirm-delete" tabindex="-1" role="dialog" aria-labelledby="myModalLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
@Resources.DeleteRole
</div>
<div class="modal-body">
@Resources.AreYouSureYouWantToDelete
</div>
<div class="modal-footer">
@using (Html.BeginForm("RoleDelete", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Post, new { @class = "form-horizontal", role = "form" }))
{
@Html.AntiForgeryToken()
@Html.HiddenFor(m => m.Where(r => r.Id.Equals(role.Id)).FirstOrDefault().Name)
<button type="button" class="btn btn-default" data-dismiss="modal">@Resources.Cancel</button>
<input type="submit" value="@Resources.Delete" class="btn btn-danger btn-ok" />
}
</div>
</div>
</div>
</div>
</td>
</tr>
}
</tbody>
</table>
Here’s my RoleCreateViewModel
/// <summary>
/// ViewsAdminRoleCreate.cshtml
/// </summary>
public class RoleCreateViewModel
{
[Required]
[Display(Name = "Name", ResourceType = typeof(Resources))]
public string Name { get; set; }
}
and RoleCreate actions
//
// GET: /Admin/RoleCreate
[HttpGet]
[Authorize(Roles = "Admin")]
public ActionResult RoleCreate()
{
return View();
}
//
// POST: /Admin/RoleCreate
[HttpPost]
[Authorize(Roles = "Admin")]
public ActionResult RoleCreate(RoleCreateViewModel vm)
{
context.Roles.Add(new IdentityRole()
{
Name = vm.Name
});
context.SaveChanges();
ViewBag.ResultMessage = Resources.RoleCreatedSuccessfully;
return RedirectToAction("RoleIndex");
}
and RoleCreate.cshtml view
@model RoleCreateViewModel
@{
ViewBag.Title = Resources.CreateRole;
}
<h2>@Resources.CreateRole</h2>
<hr />
<div class="row">
<div class="col-md-8">
<section id="createRoleForm">
@using (Html.BeginForm("RoleCreate", "Admin", new { ReturnUrl = ViewBag.ReturnUrl }, FormMethod.Post, new { @class = "form-horizontal", role = "form" }))
{
@Html.AntiForgeryToken()
@Html.ValidationSummary(true)
<div class="form-group">
@Html.LabelFor(m => m.Name, new { @class = "col-md-2 control-label" })
<div class="col-md-10">
@Html.TextBoxFor(m => m.Name, new { @class = "form-control" })
@Html.ValidationMessageFor(m => m.Name, "", new { @class = "text-danger" })
</div>
</div>
<div class="form-group">
<div class="col-md-offset-2 col-md-10">
<input type="submit" value="@Resources.Save" class="btn btn-default" />
</div>
</div>
}
</section>
</div>
</div>
Please do critique.
Solution
Well, there’s a lot there. I’ll give some feedback on the Users bit.
I wouldn’t call DisplayRoles in cshtml either. I would use a view model for that page. It would have an int, UserId, and 2 strings, UserName and UserRoles, and the page would use a list or ienumerable of that view model. Then in your Get for the Index, create the view model from each user and build up the collection. Pretty straightforward using LINQ.
For the 2 buttons in your table, can’t you just use ActionLink? Yes it makes sense to have Get and Post for same action. But your Get for assigning roles would just take the user Id. Your Post would accept your view model, then you don’t have to change the name in order to make it unique.