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.

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

[BUGFIX] Async relational validations resolve with fake positive #494

Merged
merged 8 commits into from
Apr 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 6 additions & 7 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ sudo: required
dist: trusty

cache:
yarn: true
directories:
- node_modules
- $HOME/.npm
- $HOME/.cache # includes bowers cache

addons:
Expand Down Expand Up @@ -44,17 +43,17 @@ matrix:
before_install:
- export DISPLAY=:99.0
- sh -e /etc/init.d/xvfb start
- npm config set spin false
- npm install -g npm@^3
- curl -o- -L https://yarnpkg.com/install.sh | bash
- export PATH=$HOME/.yarn/bin:$PATH
- npm install -g codeclimate-test-reporter
- npm install -g bower

install:
- npm install -g bower
- npm install
- yarn install --no-lockfile
- bower install

script:
- ember try:one $EMBER_TRY_SCENARIO --- ember test
- node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO test --skip-cleanup

after_script:
- codeclimate-test-reporter < coverage/lcov.info
34 changes: 23 additions & 11 deletions addon/validations/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ function createTopLevelPropsMixin(validatableAttrs) {
*/
__attrsResultCollection__: computed(...validatableAttrs.map((attr) => `attrs.${attr}`), function() {
return ResultCollection.create({
attribute: `Model:${this}`,
content: validatableAttrs.map((attr) => get(this, `attrs.${attr}`))
});
}).readOnly()
Expand Down Expand Up @@ -774,11 +775,11 @@ function resolveDebounce(resolve) {
* @param {Object} options
* @param {Array} options.on Only validate the given attributes. If empty, will validate over all validatable attribute
* @param {Array} options.excludes Exclude validation on the given attributes
* @param {Boolean} async If `false`, will get all validations and will error if an async validations is found.
* @param {Boolean} isAsync If `false`, will get all validations and will error if an async validations is found.
* If `true`, will get all validations and wrap them in a promise hash
* @return {Promise or Object} Promise if async is true, object if async is false
* @return {Promise or Object} Promise if isAsync is true, object if isAsync is false
*/
function validate(options = {}, async = true) {
function validate(options = {}, isAsync = true) {
let model = get(this, 'model');
let whiteList = makeArray(options.on);
let blackList = makeArray(options.excludes);
Expand All @@ -792,7 +793,7 @@ function validate(options = {}, async = true) {
let validationResult = get(this, `attrs.${name}`);

// If an async validation is found, throw an error
if (!async && get(validationResult, 'isAsync')) {
if (!isAsync && get(validationResult, 'isAsync')) {
throw new Error(`[ember-cp-validations] Synchronous validation failed due to ${name} being an async validation.`);
}

Expand All @@ -803,17 +804,21 @@ function validate(options = {}, async = true) {
}, []);

let validations = ResultCollection.create({
attribute: `Validate:${model}`,
content: validationResults
});

let resultObject = { model, validations };

if (async) {
if (get(validations, 'isAsync')) {
return RSVP.allSettled(makeArray(get(validations, '_promise'))).then(() => resultObject);
}

return Promise.resolve(resultObject);
if (isAsync) {
return Promise.resolve(get(validations, '_promise')).then(() => {
/*
NOTE: When dealing with belongsTo and hasMany relationships, there are cases
where we have to resolve the actual models and only then resolve all the underlying
validation promises. This is the reason that `validate` must be called recursively.
*/
return get(validations, 'isValidating') ? this.validate(options, isAsync) : resultObject;
});
}

return resultObject;
Expand Down Expand Up @@ -854,7 +859,14 @@ function validateAttribute(attribute, value) {

let result = { model, validations };

return Promise.resolve(get(validations, 'isAsync') ? get(validations, '_promise').then(() => result) : result);
return Promise.resolve(get(validations, '_promise')).then(() => {
/*
NOTE: When dealing with belongsTo and hasMany relationships, there are cases
where we have to resolve the actual models and only then resolve all the underlying
validation promises. This is the reason that `validateAttribute` must be called recursively.
*/
return get(validations, 'isValidating') ? this.validateAttribute(attribute, value) : result;
});
}

/**
Expand Down
14 changes: 12 additions & 2 deletions addon/validations/result-collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,20 @@ export default Ember.ArrayProxy.extend({
* @private
* @type {Promise}
*/
_promise: computed('content.@each._promise', cycleBreaker(function() {
return RSVP.allSettled(compact(flatten(this.getEach('_promise'))));
_promise: computed('content.@each._promise', '_contentValidations.@each._promise', cycleBreaker(function() {
let promises = [ this.get('_contentValidations').getEach('_promise'), this.getEach('_promise') ];
return RSVP.allSettled(compact(flatten(promises)));
})).readOnly(),

/**
* @property _contentValidations
* @type {Array}
* @private
*/
_contentValidations: computed('content.@each._validations', function() {
return emberArray(compact(this.getEach('_validations')));
}).readOnly(),

/**
* @property _contentValidators
* @type {Array}
Expand Down
12 changes: 9 additions & 3 deletions addon/validators/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,12 @@ const {
* @extends Base
*/
const BelongsTo = Base.extend({
validate(value) {
validate(value, ...args) {
if (value) {
if (isPromise(value)) {
return value.then((model) => model ? get(model, 'validations') : true);
return value.then((model) => this.validate(model, ...args));
}

return get(value, 'validations');
}

Expand All @@ -95,7 +96,12 @@ const BelongsTo = Base.extend({

BelongsTo.reopenClass({
getDependentsFor(attribute) {
return [ `model.${attribute}.isDeleted` ];
return [
`model.${attribute}.isDeleted`,
`model.${attribute}.content.isDeleted`,
`model.${attribute}.validations`,
`model.${attribute}.content.validations`
];
}
});

Expand Down
13 changes: 10 additions & 3 deletions addon/validators/has-many.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@ import { isPromise } from 'ember-cp-validations/utils/utils';
* @extends Base
*/
const HasMany = Base.extend({
validate(value) {
validate(value, ...args) {
if (value) {
if (isPromise(value)) {
return value.then((models) => models ? models.map((m) => m.get('validations')) : true);
return value.then((models) => this.validate(models, ...args));
}

return value.map((m) => m.get('validations'));
}

Expand All @@ -71,7 +72,13 @@ HasMany.reopenClass({
/*
The [email protected] must be added for older ember-data versions
*/
return [ `model.${attribute}.[]`, `model.${attribute}[email protected]`, `model.${attribute}[email protected]` ];
return [
`model.${attribute}.[]`,
`model.${attribute}[email protected]`,
`model.${attribute}[email protected]`,
`model.${attribute}[email protected]`,
`model.${attribute}[email protected]`
];
}
});

Expand Down
5 changes: 1 addition & 4 deletions bower.json
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
{
"name": "ember-cp-validations",
"dependencies": {
"jquery": "~2.2.4",
"jquery": "~2.1.4",
"ember": "2.10.0",
"ember-cli-shims": "~0.1.3",
"moment": ">= 2.8.0",
"moment-timezone": ">= 0.1.0",
"font-awesome": "~4.5.0"
},
"devDependencies": {
"ember-qunit-notifications": "0.1.0"
}
}
2 changes: 2 additions & 0 deletions tests/dummy/app/models/order-selection-question.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const Validations = buildValidations({
validator('presence', true)
]
}
}, {
debounce: 10
});

export default DS.Model.extend(Validations, {
Expand Down
98 changes: 98 additions & 0 deletions tests/integration/validations/model-relationships-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,101 @@ test('presence on empty DS.PromiseArray', function(assert) {
assert.equal(friends.get('isValid'), false);
assert.equal(friends.get('message'), 'This field can\'t be blank');
});

test('debounce should work across nested HasMany relationships', function(assert) {
this.register('validator:presence', PresenceValidator);
this.register('validator:has-many', HasManyValidator);

let done = assert.async();

let FriendValidations = buildValidations({
name: validator('presence', { presence: true, debounce: 50 })
});

let friend = setupObject(this, Ember.Object.extend(FriendValidations));
let user = setupObject(this, Ember.Object.extend(HasManyValidations), {
friends: new Ember.RSVP.Promise((resolve) => {
resolve([ friend ]);
})
});

user.validate().then(({ validations }) => {
assert.equal(friend.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(user.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValid'), false, 'User should not be valid');

friend.set('name', 'Offir');
return user.validate();

}).then(({ validations }) => {
assert.equal(friend.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(user.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValid'), true, 'User should be valid');
done();
});
});

test('debounce should work across nested BelongsTo relationships', function(assert) {
this.register('validator:presence', PresenceValidator);
this.register('validator:belongs-to', BelongsToValidator);

let done = assert.async();

let FriendValidations = buildValidations({
name: validator('presence', { presence: true, debounce: 50 })
});

let friend = setupObject(this, Ember.Object.extend(FriendValidations));

let user = setupObject(this, Ember.Object.extend(BelongsToValidations), {
friend: new Ember.RSVP.Promise((resolve) => {
resolve(friend);
})
});

user.validate().then(({ validations }) => {
assert.equal(friend.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(user.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValid'), false, 'User should not be valid');

friend.set('name', 'Offir');
return user.validate();

}).then(({ validations }) => {
assert.equal(friend.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(user.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValid'), true, 'User should be valid');
done();
});
});

test('Validations should work across two-way BelongsTo relationships', function(assert) {
this.register('validator:presence', PresenceValidator);
this.register('validator:belongs-to', BelongsToValidator);

let done = assert.async();

let user2 = setupObject(this, Ember.Object.extend(BelongsToValidations));

let user = setupObject(this, Ember.Object.extend(BelongsToValidations), {
friend: new Ember.RSVP.Promise((resolve) => {
resolve(user2);
})
});

user2.set('friend', new Ember.RSVP.Promise((resolve) => {
resolve(user);
}));

user.validate().then(({ validations }) => {
assert.equal(user.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(user2.get('validations.isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValidating'), false, 'All promises should be resolved');
assert.equal(validations.get('isValid'), true, 'User should be valid');
done();
});
});
Loading