Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Feature] Warning Validators #226

Merged
merged 5 commits into from
Jun 27, 2016
Merged

[Feature] Warning Validators #226

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a8e37f
Initial implementation of warnings
offirgolan Jun 23, 2016
198f493
isAsync should depend on entire content
offirgolan Jun 23, 2016
8bc874b
isValidating should be dependent on entire content
offirgolan Jun 25, 2016
41480d6
Fixes
offirgolan Jun 27, 2016
869f2ee
Remove has-many comment
offirgolan Jun 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions addon/validations/factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,14 @@ function createTopLevelPropsMixin(validatableAttrs) {
return get(this, 'messages.0');
})).readOnly(),

warnings: computed(...validatableAttrs.map(attr => `attrs.${attr}.@each.warning`), function () {
return emberArray(flatten(validatableAttrs.map(attr => get(this, `attrs.${attr}.warning`)))).compact();
}).readOnly(),

warning: computed('warning.[]', cycleBreaker(function () {
return get(this, 'warning.0');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is .0 the right thing to use here? I've always used firstObject.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same code that is the errors and error CPs. I believe I tried firstObject but it errored on me. Ill give it another shot though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem - definitely not required, I just haven't seen this syntax before.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firstObject ideally

})).readOnly(),

errors: computed(...validatableAttrs.map(attr => `attrs.${attr}.@each.errors`), function () {
return emberArray(flatten(validatableAttrs.map(attr => get(this, `attrs.${attr}.errors`)))).compact();
}).readOnly(),
Expand Down
1 change: 1 addition & 0 deletions addon/validations/internal-result-object.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export default Ember.Object.extend({
isNotValidating: not('isValidating'),
isInvalid: not('isValid'),
isTruelyValid: and('isNotValidating', 'isValid'),
isWarning: computed.bool('_validator.isWarning').readOnly(),

isAsync: computed('_promise', function () {
const promise = get(this, '_promise');
Expand Down
103 changes: 82 additions & 21 deletions addon/validations/result-collection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export default Ember.Object.extend({
* @readOnly
* @type {Ember.ComputedProperty | Boolean}
*/
isValid: computed('content.@each.isValid', cycleBreaker(function () {
return get(this, 'content').isEvery('isValid', true);
isValid: computed('_errorContent.@each.isValid', cycleBreaker(function () {
return get(this, '_errorContent').isEvery('isValid', true);
}, true)),

/**
Expand Down Expand Up @@ -116,8 +116,8 @@ export default Ember.Object.extend({
* @readOnly
* @type {Ember.ComputedProperty | Boolean}
*/
isTruelyValid: computed('content.@each.isTruelyValid', cycleBreaker(function () {
return get(this, 'content').isEvery('isTruelyValid', true);
isTruelyValid: computed('_errorContent.@each.isTruelyValid', cycleBreaker(function () {
return get(this, '_errorContent').isEvery('isTruelyValid', true);
}, true)),

/**
Expand All @@ -136,8 +136,8 @@ export default Ember.Object.extend({
* @readOnly
* @type {Ember.ComputedProperty | Boolean}
*/
isDirty: computed('content.@each.isDirty', cycleBreaker(function () {
return !get(this, 'content').isEvery('isDirty', false);
isDirty: computed('_errorContent.@each.isDirty', cycleBreaker(function () {
return !get(this, '_errorContent').isEvery('isDirty', false);
}, false)),

/**
Expand Down Expand Up @@ -171,8 +171,8 @@ export default Ember.Object.extend({
* @readOnly
* @type {Ember.ComputedProperty | Array}
*/
messages: computed('content.@each.messages', cycleBreaker(function () {
const messages = flatten(get(this, 'content').getEach('messages'));
messages: computed('_errorContent.@each.messages', cycleBreaker(function () {
const messages = flatten(get(this, '_errorContent').getEach('messages'));

return uniq(compact(messages));
})),
Expand All @@ -194,6 +194,41 @@ export default Ember.Object.extend({
return get(this, 'messages.0');
})),

/**
* A collection of all {{#crossLink "Error"}}Warnings{{/crossLink}} on the object in question.
* Each warning object includes the warning message and it's associated attribute name.
*
* ```javascript
* // Example
* get(user, 'validations.warnings')
* get(user, 'validations.attrs.username.warnings')
* ```
*
* @property warnings
* @readOnly
* @type {Ember.ComputedProperty | Array}
*/
warnings: computed('attribute', '_warningContent.@each.errors', cycleBreaker(function () {
return this._computeErrorCollection(get(this, '_warningContent'));
})),

/**
* An alias to the first {{#crossLink "Warning"}}{{/crossLink}} in the warnings collection.
*
* ```javascript
* // Example
* get(user, 'validations.warning')
* get(user, 'validations.attrs.username.warning')
* ```
*
* @property warning
* @readOnly
* @type {Ember.ComputedProperty | Error}
*/
warning: computed('warnings.[]', cycleBreaker(function () {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firstObject

return get(this, 'warnings.0');
})),

/**
* A collection of all {{#crossLink "Error"}}Errors{{/crossLink}} on the object in question.
* Each error object includes the error message and it's associated attribute name.
Expand All @@ -208,18 +243,8 @@ export default Ember.Object.extend({
* @readOnly
* @type {Ember.ComputedProperty | Array}
*/
errors: computed('attribute', 'content.@each.errors', cycleBreaker(function () {
const attribute = get(this, 'attribute');
let errors = flatten(get(this, 'content').getEach('errors'));

errors = uniq(compact(errors));
errors.forEach(e => {
if(e.get('attribute') !== attribute) {
e.set('parentAttribute', attribute);
}
});

return errors;
errors: computed('attribute', '_errorContent.@each.errors', cycleBreaker(function () {
return this._computeErrorCollection(get(this, '_errorContent'));
})),

/**
Expand Down Expand Up @@ -303,7 +328,43 @@ export default Ember.Object.extend({
* @type {Ember.ComputedProperty}
* @private
*/
_contentValidators: computed.mapBy('content', '_validator').readOnly(),
_contentValidators: computed.mapBy('_errorContent', '_validator').readOnly(),

/**
* @property _errorContent
* @type {Ember.ComputedProperty}
* @private
*/
_errorContent: computed.filter('content', function(result) {
return get(result, 'isWarning') !== true;
}).readOnly(),

/**
* @property _warningContent
* @type {Ember.ComputedProperty}
* @private
*/
_warningContent: computed.filterBy('content', 'isWarning', true).readOnly(),


/**
* @method _computeErrorCollection
* @return {Object}
* @private
*/
_computeErrorCollection(content) {
const attribute = get(this, 'attribute');
let errors = flatten(content.getEach('errors'));

errors = uniq(compact(errors));
errors.forEach(e => {
if(e.get('attribute') !== attribute) {
e.set('parentAttribute', attribute);
}
});

return errors;
},

/**
* Used by the `options` property to create a hash from the `content` that is grouped by validator type.
Expand Down
9 changes: 8 additions & 1 deletion addon/validations/result.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ export default Ember.Object.extend({
*/
isDirty: readOnly('_validations.isDirty'),

/**
* @property isWarning
* @readOnly
* @type {Ember.ComputedProperty}
*/
isWarning: readOnly('_validations.isWarning'),

/**
* @property message
* @readOnly
Expand Down Expand Up @@ -134,7 +141,7 @@ export default Ember.Object.extend({
* @private
* @type {Result}
*/
_validations: computed('model', 'attribute', '_promise', function () {
_validations: computed('model', 'attribute', '_promise', '_validator', function () {
return InternalResultObject.create(getProperties(this, ['model', 'attribute', '_promise', '_validator']));
}),

Expand Down
9 changes: 8 additions & 1 deletion addon/validators/base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import { unwrapString } from 'ember-cp-validations/utils/utils';
const {
get,
set,
isNone
isNone,
computed
} = Ember;

const assign = Ember.assign || Ember.merge;
Expand Down Expand Up @@ -64,6 +65,12 @@ const Base = Ember.Object.extend({
*/
errorMessages: null,

/**
* @property isWarning
* @type {Boolean}
*/
isWarning: computed.bool('_cachedOptions.warning').readOnly(),

/**
* Validator type
* @property _type
Expand Down
6 changes: 5 additions & 1 deletion tests/dummy/app/components/validated-input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ export default Ember.Component.extend({
isValid: computed.and('hasContent', 'validation.isValid', 'notValidating'),
isInvalid: computed.oneWay('validation.isInvalid'),
showErrorClass: computed.and('notValidating', 'showMessage', 'hasContent', 'validation'),
showMessage: computed('validation.isDirty', 'isInvalid', 'didValidate', function() {
showErrorMessage: computed('validation.isDirty', 'isInvalid', 'didValidate', function() {
return (this.get('validation.isDirty') || this.get('didValidate')) && this.get('isInvalid');
}),

showWarningMessage: computed('validation.isDirty', 'isValid', 'didValidate', function() {
return (this.get('validation.isDirty') || this.get('didValidate')) && this.get('isValid');
})
});
7 changes: 6 additions & 1 deletion tests/dummy/app/models/user.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@ var Validations = buildValidations({
validator('presence', true),
validator('length', {
min: 4,
max: 8
max: 10
}),
validator('format', {
regex: /^(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{4,8}$/,
message: '{description} must include at least one upper case letter, one lower case letter, and a number'
}),
validator('length', {
warning: true,
min: 6,
message: 'What kind of weak password is that?'
})
]
},
Expand Down
12 changes: 10 additions & 2 deletions tests/dummy/app/styles/app.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,20 @@ body {
font-size: 12px;
}

.validated-input .input-error .error {
.validated-input .input-error .error,
.validated-input .input-error .warning {
padding: 8px 5px 0 0;
color: rgb(255, 65, 31);
text-align: left;
}

.validated-input .input-error .error {
color: rgb(255, 65, 31);
}

.validated-input .input-error .warning {
color: rgb(255, 165, 31);
}

.demo a.show-code {
cursor: pointer;
}
Expand Down
12 changes: 9 additions & 3 deletions tests/dummy/app/templates/components/validated-input.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@
{{/if}}

<div class="input-error">
{{#if showMessage}}
{{#if showErrorMessage}}
<div class="error">
{{v-get model valuePath 'message'}}
{{get (v-get model valuePath 'error') 'message'}}
</div>
{{/if}}

{{#if showWarningMessage}}
<div class="warning">
{{get (v-get model valuePath 'warning') 'message'}}
</div>
{{/if}}
Copy link
Collaborator

@blimmer blimmer Jun 25, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we expose all the messages for an attribute somehow? Since this is new functionality, I wonder if messages should return all messages, including warnings. I could see wanting to do something like this for my flavor of validated input:

{{#each (v-get model valuePath 'messages') as |msg|}}
  <div class="message {{if msg.isWarning 'warning'}}">
     {{msg}}
  </div>
{{/each}}

That way I don't have to have two blocks and reduce boilerplate templating. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My theory was:

  1. Not to break current API since there could be other addons that are dependent that the messages array will return a collection of strings that are required.
  2. I wanted to separate out warning and errors because errors are whats important. Warnings are optional and should affect the state of the model in any way.

We could release 3.0 with API changes and think of a better way to handle this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course - I blanked that messages are strings on the current API. Definitely don't want to break that.

Though warnings are optional, it seems like anytime someone took the time to write a validation for a warning, they'd want to expose it to the user, likely just with another class to style differently.

</div>
</div>
</div>