Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Remove use of RequiredValidator from Mutable Model Binder #2526

Closed
Eilon opened this issue May 8, 2015 · 6 comments
Closed

Remove use of RequiredValidator from Mutable Model Binder #2526

Eilon opened this issue May 8, 2015 · 6 comments

Comments

@Eilon
Copy link
Member

Eilon commented May 8, 2015

We should just ignore this case and let whatever happens, happen.

@Eilon Eilon added the bug label May 8, 2015
@Eilon Eilon added this to the 6.0.0-beta5 milestone May 8, 2015
@dougbu
Copy link
Member

dougbu commented May 21, 2015

Had three cases when we opened this bug:

  1. Property in model type has an associated [Required] attribute that strangely doesn't actually validate. Submission leaves the property null.
  2. Property in model type has associated [BindRequired] and [Required] attributes. [Required] attribute strangely does not actually validate. Submission does not include data for the property.
  3. Property in model type has associated [BindRequired] attribute but no [Required] attribute. Submission does not include data for the property.

In all cases, we previously provided a fallback error message.

However case 1 no longer exists because the fallback code existed only in model binding. @rynowak removed the [Required] special cases from model binding. Validation code has no equivalent fallback so 👯

We have a number of other bugs and design change requests out related to cases 2 and 3. For now i.e. for this bug, I will just remove the fallback message from case 2. If there is no [Required] attribute (case 3), model binding will continue to provide its own error for the missing property data.

tl;dr; wherever I said "[Required] attribute" above, I meant "validator with IsRequired == true".

@dougbu
Copy link
Member

dougbu commented May 22, 2015

Postponing based on offline conversations. Will discuss and decide what to do about the mentioned cluster of bugs touching on how [BindRequired] works.

@dougbu dougbu modified the milestones: 6.0.0-beta6, 6.0.0-beta5 May 22, 2015
@Eilon Eilon changed the title Remove fallback behavior for Required Validator that doesn't add an error message Remove use of RequiredValidator from Mutable Model Binder Jun 5, 2015
@Eilon
Copy link
Member Author

Eilon commented Jun 5, 2015

To be clear, in the Mutable Model Binder where it does some fancy stuff to create an error message for a "not provided" value on the wire, just put in our own custom error message always.

In the far future, we can look at some way to customize this.

@Eilon
Copy link
Member Author

Eilon commented Jun 5, 2015

The error message should be: A value for the '{0}' property was not provided.

Copied from #2493

@Eilon
Copy link
Member Author

Eilon commented Jun 5, 2015

The future work is captured in #2303.

dougbu added a commit that referenced this issue Jun 5, 2015
…indRequired]`

- only use MVC error message when `[BindRequired]` is violated
- update that error message to more clearly describe the problem
- enable all tests skipped due to dupe bug #2493
- update expectations of a few tests using the old messages

nits:
- rename `ModelBinding_MissingRequiredMember` to `ModelBinding_MissingBindRequiredMember`
- remove `<param>` description of removed `requiredValidator` parameter
- remove unused `MutableObjectModelBinderTest.GetRequiredValidator()`
@dougbu
Copy link
Member

dougbu commented Jun 6, 2015

eefa582

@dougbu dougbu closed this as completed Jun 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants