I’ve got a form with a dropdownlist in my MVC app. Now that I’m trying to add validation to the mix it seems that a dropdownlist fails validation no matter what it’s value is.
Without the validation it will allow the controller to work and redirect as planned. With the validation it does seem to allow the database changes to occur but ModelState.IsValid is false.
I’m stuck. Is this a known issue?
The error returned is:
The value ‘x’ is invalid.
Where ‘x’ is the numeric value of the current selection. The failure occurs no matter what the chosen value is.
Here is a workaround:
public ActionResult Create([Bind(Exclude="parent")] Page page)
{
page.parent = Convert.ToInt32(Request.Form["parent"]);
It seems a bit sketchy though. Is it good practice?
Did this automatically fail validation because the DropDownList’s value is intrinsically a string and the parent property is defined as an int in the DomainModel?
If that’s the case how do you bind a DropDownList’s value to an int?
try “page.parent” instead of “parent” as the name of the <select />
I suspect it can’t bind because it doesn’t find any value.
In short, it didn’t fail because of a conversion error. The first thing to look at is if the actual value is X, or did you just use that as a placeholder in your post text?
Show me the following:
~ The model your binding.
~ The viewmodel used to pass it into the view.
~ The markup used to render the dropdown.
It could simply be a naming issue. It’s a good practice to fully qualify your control names: DropDown(“Page.Parent” …) instead of just DropDown(“Parent”).
It sounds like the binder simply can’t tell what it’s supposed to be doing with the value and discards it, causing an error with Parent until you used exclude on it.
Here are the items you requested. 
It’s still a work in progress.
public class Page
{
private EntityRef<Page> _parent = default(EntityRef<Page>);
private EntitySet<Page> _children = new EntitySet<Page>();
public int ID { get; set; }
public string pageTitle { get; set; }
public string menuTitle { get; set; }
public string content { get; set; }
public int listOrder { get; set; }
public bool visible { get; set; }
public int parent { get; set; }
public DateTime? created { get; set; }
public DateTime? edited { get; set; }
public string createdBy { get; set; }
public string lastEditBy { get; set; }
public string linkInfo { get; set; }
public bool IsSelected { get; set; }
public Page Parent
{
// return the current entity
get { return this._parent.Entity; }
set { this._parent.Entity = value; }
}
public EntitySet<Page> Children
{
get { return this._children; }
set { this._children.Assign(value); }
}
public static Page Error404()
{
return (new Page
{
content = "<p>Page not found</p>",
pageTitle = "404. Page not found"
});
}
}
[AcceptVerbs(HttpVerbs.Get)]
public ActionResult Create()
{
ViewData["currentLink"] = "Create a new Page";
var pageSelectList = pageRepository.GetTop().ToList();
pageSelectList.Add(new Page
{
menuTitle = "None"
});
ViewData["pageList"] = new SelectList(pageSelectList.OrderBy(x => x.listOrder), "ID", "menuTitle");
return View();
}
<label for="parent">Child of:</label>
<%= Html.DropDownList("parent", (SelectList)ViewData["pageList"])%>
<%= Html.ValidationMessage("parent") %>
Ok, let’s try and make this debug friendly, and help separate concerns at the same time.
First, you need to abandon the use of ViewModel[“key”]. In order to catch potential errors and make the best use of c# types, we need to make a view model class to wrap what you are sending to the view.
public class PageCreateViewModel
{
public string CurrentLink { get; set; }
public SelectList PageSelectList { get; private set; }
public PageCreateViewModel(string currentLink, IEnumerable<Page> pageSelectListItems)
{
CurrentLink = currentLink;
PageSelectList = new SelectList(pageSelectListItems, “ID”, “menuTitle”);
}
}
This will type your model, instead of the non-typed key-value pairs in ViewModel. Now your action should look like this:
[AcceptVerbs(HttpVerbs.Get)]
public ActionResult Create()
{
return View(new PageCreateViewModel(
“Create a new Page”,
pageRepository.GetTopCOLOR=#000000[/COLOR].ToListCOLOR=#000000.OrderBy(x => x.listOrder)[/COLOR]
));
}
Personally, I’d make a method in your repository that returns exactly what you want and name it something like GetOrderedTop().
Next your view should be altered so that it uses ViewPage<Namespace.To.PageCreateViewModel> instead of your page class, or whatever it’s using now. Your Dropdown call should look like this now:
<label for=“parent”>Child of</label>
<%= Html.DropDownList(“parent”, Model.PageSelectList, “None”)%>
<%= Html.ValidationMessage(“parent”) %>
Unless I missed something, that should fill a box with “None” listed at the top. Now you should be able to put in some breakpoints and examine things better as everything is typed.
If your binding a Page class on post action, ensure that you include only the fields used (otherwise, ModelState will erroneously report errors).
Thanks Serenarules. I do intend to remove the ViewModel[“key”] items but in this case it affects the IsSelected boolean in the Nav controller and I just hadn’t worked out how to pass it back and forth yet.
It does put a “None” at the top but it also nulls the value. It would have to hold a value of zero or I can just check for null and make it zero in the [AcceptVerbs(HttpVerbs.Post)]
I’m not sure what you mean.
The listOrder property is also throwing a default ModelState error of “A value is required.” instead of “Please enter the list order for this menu item” which I have in my validation tests. If I change it to int? then my message occurs.
Could that have something to do with parent throwing its default error?
Maybe I need to come up with a better plan than throwing a RuleException error.
Is there an issue with this:
public class RuleException : Exception
{
public NameValueCollection Errors { get; private set; }
public RuleException(string key, string value)
{
Errors = new NameValueCollection { { key, value } };
}
public RuleException(NameValueCollection errors)
{
Errors = errors;
}
public void CopyToModelState(ModelStateDictionary modelState)
{
foreach (string key in Errors)
foreach (string value in Errors.GetValues(key))
modelState.AddModelError(String.Format("{0}", key), value);
}
}
I also get the same validation error “The value ‘x’ is invalid.” when using non-numerical characters in the listOrder field so I think it is a conversion problem.
There must be a way to bypass the built in validation when using your own.
If you’re running MVC 1.0 you’ll need to call SetModelValue in addition to AddModelError. You must do this also, if you ever call ModelState.Clear().
modelState.AddModelError(String.Format(“{0}”, key), value);
ModelState.SetModelValue(key, new ValueProviderResult(value, value, CultureInfo.CurrentCulture));
I see that this sets the textbox value to the validation error message. Why do I want this?
Oops, my bad. Replace “value” with whatever variable holds your model value. That was a cut and paste oversight.
Should I just abandon this idea of throwing an exception for validation errors or is that the way it should be handled?
It’s just not working as I expect it should and I don’t think something this simple should require writing a custom ModelBinder.
I’ll post my own thing in a little bit. Just got kinda busy here at home.
Thanks.
I don’t know if it’s my inexperience with MVC or if I just have a bad idea or poor design.
Neither, you’re doing fine. Like everything else one does, it kind of evolves as you go. You’ve stumbled on the toes of the DefaultModelBinder. This is good. It means you’ve tried something, and should now realise that there are times when custom validation, or custom binders, are needed.
Here’s what I do, albeit for MVC 2.0…
The first thing I did was create custom error classes:
public interface IValidationError
{
string Field { get; }
string Error { get; }
}
public interface IValidationErrors : ICollection
{
int Add(string field, string error);
int Add(IValidationError error);
bool IsValid { get; }
}
public class ValidationError : IValidationError
{
public string Field { get; private set; }
public string Error { get; private set; }
public ValidationError(string field, string error)
{
this.Field = field;
this.Error = error;
}
}
public class ValidationErrors : CollectionBase, IValidationErrors
{
public int Add(string field, string error)
{
return this.Add(new ValidationError(field, error));
}
public int Add(IValidationError error)
{
return this.List.Add(error);
}
public bool IsValid
{
get { return this.Count == 0; }
}
}
public abstract class BaseController : Controller
{
[NonAction]
protected void ConsumeError(string field, string error)
{
ModelState.AddModelError(field, error.Localize());
}
[NonAction]
protected void ConsumeError(IValidationError error)
{
ConsumeError(error.Field, error.Error);
}
[NonAction]
protected void ConsumeErrors(IValidationErrors errors)
{
ModelState.Clear();
foreach (IValidationError error in errors)
{
ConsumeError(error);
}
}
[NonAction]
protected void ConsumeException(Exception exception)
{
ConsumeError("_FORM", exception.Message);
}
}
We now have a custom validation error, a runner style collection, and a base controller class designed to clear the model state errors (this will clear your pesky conversion error), and allows us to insert our own messages into the model state. Typically, I do my validation in a service. Such a service interface might look like this:
public interface ICategoryService
{
IEnumerable<Category> GetAll();
Category GetById(int id);
IValidationErrors Add(Category entity);
IValidationErrors Edit(Category entity);
IValidationErrors Delete(Category entity);
IValidationErrors Validate(Category entity);
}
Since the Add, Edit and Delete methods call Validate, your actions can now look like this:
[Authorize(Roles = "Administrator")]
[AcceptVerbs(HttpVerbs.Get)]
public ActionResult Edit(int id)
{
return View(new CategoryEditViewModel()
{
Category = categoryService.GetById(id)
});
}
[Authorize(Roles = "Administrator")]
[AcceptVerbs(HttpVerbs.Post)]
[ValidateAntiForgeryToken()]
public ActionResult Edit(Category entity)
{
ConsumeErrors(categoryService.Add(entity));
if (ModelState.IsValid) return RedirectToAction("Manager");
return View(new CategoryEditViewModel()
{
Category = entity
});
}
Again, since you’re using MVC 1.0 you’ll need to adjust the ConsumeError method of the base controller to this:
protected void ConsumeError(string field, string error)
{
ModelState.AddModelError(field, error.Localize());
ModelState.SetModelValue(field, this.ValueProvider.GetValue(field));
}
With something like this in place, you can now validate anywhere you choose, exactly what you want to validate, and exactly how you want to validate it. You can even consume exceptions and place them in ModelState.
A bit long winded, but I hope it helps anyway.
Wowzers. That’s a lot to digest.
Thanks, brother. I’m missing something somewhere. Trying to track it down.
Hmm. It seems that when I test for any field it nulls it out causing an error at modelbinding. I can’t seem to get it to show the errors in the UI.
edit - Ah, I think I found the issue:
protected void ConsumeErrors(IValidationErrors errors)
{
ModelState.Clear();
foreach (IValidationError error in errors)
{
/*this line ---------->*/ ModelState.SetModelValue(error.Field, ValueProvider[error.Field]);
ConsumeError(error);
}
}