How to save MVC DRY controllers for Edit-> Save-> ValidateFail

Question

I have a Manage User event that accepts an optional user id and displays a user edit screen. There is manageUserViewModel on this screen.

There are some dependencies on my Manage page - for example, PageTitle, which method to send, etc.

If I checked the failure, I need to show the control screen again, but this time, using the view model that was passed to the same method.

The supply of these dependencies in the failure scenario is not very dry.

How to do dependency repetition? I tried putting them in a separate method, but that doesn't seem right.

public ActionResult Manage(Guid? UserID) { User user = this._UserLogic.GetUser(UserID); ViewBag.Title = "User List"; ViewBag.OnSubmit = "Save"; ManageUserViewModel uvm = Mapper.Map<User, ManageUserViewModel>(user); return View("Manage", uvm); } [AcceptVerbs("POST")] public ActionResult Save(ManageUserViewModel uvm) { User user = this._UserLogic.GetUser(uvm.UserID); if (!ModelState.IsValid) // This is not very DRY!!! ViewBag.Title = "Manage User"; ViewBag.OnSubmit = "Save"; return View("Manage", uvm); } Mapper.Map<ManageUserViewModel, User>(uvm, user ); this._UserLogic.SaveUser(user); return RedirectToAction("Manage", new { UserID = user.ID }); } 

Answer 1

I think you misunderstood DRY. DRY does not mean "NEVER repeat yourself", it means that you should not repeat yourself when it makes sense not to do this.

Different views have different requirements, and creating a complex structure to avoid repeating yourself violates other best practices such as KISS and SRP.

SOLID is interesting in that the principle of common responsibility often contradicts "Do not repeat yourself", and you need to come up with a balance. In most cases, DRY loses because SRP is much more important.

It seems to me that you have code that processes several duties only in order to avoid writing such code more than once. I do not agree with this because each submission has different responsibilities and different requirements.

I would suggest simply creating separate actions, views, and controller models for each action, especially if validation requirements are important to them. There may be a few things you can do (for example, using partial views or editor templates) to reduce repetition, but overall don't add a lot of complexity to avoid repetition.

Answer 2

You can add the “User Manager” and “Save” OnSubmit headers as properties in the ManageUserViewModel. This means that you did not have to add them to the ViewBag every time you called Save.

You can also create a ManageUserService, which can be responsible for AutoMapper mappings and save the user.

After that, the code will look like this:

 public ActionResult Manage(Guid? UserID) { var uvm = _userService.GetById(UserId); return View("Manage", uvm); } [AcceptVerbs("POST")] public ActionResult Save(ManageUserViewModel uvm) { if (!ModelState.IsValid) { return View("Save", uvm); } _userService.Save(uvm); return RedirectToAction("Manage", new { UserID = uvm.ID }); } 

Just put the CRUD logic and the AutoMapping function in a class called UserService, and an instance of which can be inserted using the control inverse function in your controller.

If you do not want to hard-code your string values ​​in the view model itself, you can add values ​​to the ApplicationResources file and reference them from the view model.

Answer 3

You will need to find a way to store this information between requests, which means either sending it between the server and the client, or storing it on the server. Saving it on the server means something like a session, but for me it is a little hard. You can add it to your ViewModel as @Ryan Spears suggested. For me this is a bit wrong, polluting the ViewModel with what can be considered metadata. But this is just an opinion, and I will not discredit his answer, because it is valid. Another possibility is to simply add additional fields to the list of parameters of the action method itself and use hidden fields.

 [AcceptVerbs("POST")] public ActionResult Save(ManageUserViewModel uvm, string title, string onSubmit) { ... } 

In the add form:

 <input type="hidden" name="title" value="@ViewBag.Title" /> <input type="hidden" name="onSubmit" value="@ViewBag.OnSubmit" /> 

This is essentially the same concept and solution as adding them to the ViewModel, except that in reality they are not part of the ViewModel.

Answer 4

You can use RedirectToAction() and then export and import your tempdata (to support ModelState) if you are worried about 3 lines.

Personally, I would find it more readable if you saved the logic in the POST version of the method, since you are doing something a little different from the GET method, so don't repeat yourself. Perhaps you could save the two ViewBag variables that you have in the view, and then there is no repetition at all.

As a side note: [HttpPost] now replaces [AcceptVerbs]

Answer 5

We came up with another solution that I thought I would share.

This is based on a view model that contains information about what actions it can do, but we believe that the controller should specify these (i.e., control which actions use different links), because we have cases when view models reuse used in actions. EG, the case when you can edit a template or an instance of something while editing - the user interface is the same, the only difference is the actions that you send to / cancel.

We distracted the part of the presentation model, which contains the properties associated with the data, and the presentation model, which contains the other things we need to render the view. We call the object only for the DTO object - this is not a true dto, because it contains validation attributes.

We believe that we will be able to reuse these DTOs in the future for ajax or even XML requests - it can continue to validate DRY.

In any case - here is a sample code, we are happy with it (for now) and hope that it will help others.

  [HttpGet] [ValidateInput(false)] public virtual ActionResult ManageUser(ManageUserDTO dto, bool PopulateFromObject = true) { User user = this._UserLogic.GetUser(dto.UserID); if (PopulateFromObject) Mapper.Map<User, ManageUserDTO>(user, dto); ManageUserViewModel vm = new ManageUserViewModel() { DTO = dto, PageTitle = Captions.GetCaption("pageTitle_EditUser"), OnSubmit = GetSubmitEventData(this.ControllerName, "SaveUser"), OnCancel = GetCancelEventData(this.ControllerName, "ListUsers"), }; return View("ManageUser", vm); } [HttpPost] public virtual ActionResult SaveUser(ManageUserViewModel vm) { User user = this._UserLogic.GetUser(vm.DTO.UserID); if (!ModelState.IsValid) { return ManageUser(vm.DTO, false); } Mapper.Map<ManageUserDTO, User>(vm.DTO, user); this._UserLogic.SaveUser(user); TempData.AddSuccess(Captions.GetCaption("message_UserSavedSuccessfuly")); return RedirectToAction("ManageUser", new { UserID = user.ID }); } 

The binder model sets any URI variables in dto in the get action. My logic level will return a new User object if getUserByID (null) is called.