loopbackio / loopback-datasource-juggler

Connect Loopback to various Data Sources
http://www.loopback.io
Other
278 stars 365 forks source link

Make value inside of validation messages optional #504

Closed fabien closed 7 years ago

fabien commented 9 years ago

I would like to propose to make the value inside of validation messages optional; essentially this would allow you to go back to the message format as it was before https://github.com/strongloop/loopback-datasource-juggler/commit/9b759c95ac9155e23f1b52efd6d8a46d417f0997 landed.

@bajtos since you landed this, any objections to a PR? Suggestions on how to disable it? Globally? Per model?

bajtos commented 9 years ago

I would like to propose to make the value inside of validation messages optional

Could you please elaborate more on the reasons why you would like that? Is there any way how to improve the current implementation to meet your needs instead of disabling it?

Suggestions on how to disable it? Globally? Per model?

It really depends on why you want to disable it. One possible option is to expose a global configuration property on loopback.ValidationError.

fabien commented 9 years ago

I prefer the old way because it's less verbose, avoids clutter. While returning the value makes sense in the strict API workflow, it's kind of redundant when doing actual UI related work. Usually you'll have the values still on display in your UI (think AJAX submit), and listing errors with values returned from the server then becomes a bit visually distracting/verbose/layout-breaking. Especially if you hook up the errors to be displayed inline.

Finally, I was able to integrate http://validatejs.org/ for my client-side work, and (by coincidence) its output closely resembled that of Juggler before this PR. Before, it would not matter if the errors were pure client-side, or sent from the server - they were equally formatted.

It might sound like a lousy reason, but still - I have never seen any validation lib include the value like is done right now.

bajtos commented 9 years ago

So you only want to remove the property values, but still keep the list of invalid properties with the validation errors? Somehow I thought you wanted to get rid of the latter too.

I am totally fine with adding a flag to disable 9b759c9 as long as the flag is enabled by default to preserve BC. Perhaps loopback.ValidationError.includeValues or a more generic loopback.ValidationError.verbose?

Nice to have: allow developers to configure this flag via server/config.json, e.g. by setting a top-level property { "verboseValidationErrors": false }.

@ritch @raymondfeng @bitmage thoughts?

bitmage commented 9 years ago

Sounds great to me. Default verbose, but tune it down if it bothers you.

On Mar 13, 2015, at 2:39 AM, Miroslav Bajtoš notifications@github.com wrote:

So you only want to remove the property values, but still keep the list of invalid properties with the validation errors? Somehow I thought you wanted to get rid of the latter too.

I am totally fine with adding a flag to disable 9b759c9 as long as the flag is enabled by default to preserve BC. Perhaps loopback.ValidationError.includeValues or a more generic loopback.ValidationError.verbose?

Nice to have: allow developers to configure this flag via server/config.json, e.g. by setting a top-level property { "verboseValidationErrors": false }.

@ritch @raymondfeng @bitmage thoughts?

— Reply to this email directly or view it on GitHub.