From 324a04487bcdbe1cd3def3bffda1721097eb3ec9 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Tue, 9 Feb 2016 13:39:15 -0600 Subject: [PATCH] cleanup feature service error handling --- core/client/app/services/feature.js | 13 ++- .../integration/services/feature-test.js | 101 ++++++++++++++---- .../unit/validators/tag-settings-test.js | 1 - 3 files changed, 91 insertions(+), 24 deletions(-) diff --git a/core/client/app/services/feature.js b/core/client/app/services/feature.js index fd5f45a57c..307ffbb1d8 100644 --- a/core/client/app/services/feature.js +++ b/core/client/app/services/feature.js @@ -8,6 +8,8 @@ const { set } = Ember; +const EmberError = Ember.Error; + export function feature(name) { return computed(`config.${name}`, `labs.${name}`, { get() { @@ -22,7 +24,7 @@ export function feature(name) { }); }, set(key, value) { - this.update(key, value).then((savedValue) => { + return this.update(key, value).then((savedValue) => { return savedValue; }); } @@ -76,8 +78,13 @@ export default Service.extend({ this.set('_settings', savedSettings); resolve(this._parseLabs(savedSettings).get(key)); }).catch((errors) => { - this.get('notifications').showErrors(errors); - settings.rollbackAttributes(); + if (errors) { // model.save errors, show notifications + this.get('notifications').showErrors(errors); + settings.rollbackAttributes(); + } else { + settings.rollbackAttributes(); + throw new EmberError(`Validation of the feature service settings model failed when updating labs.`); + } resolve(this._parseLabs(settings)[key]); }); }).catch(reject); diff --git a/core/client/tests/integration/services/feature-test.js b/core/client/tests/integration/services/feature-test.js index 0e93a8d9fa..e2fb54e7e7 100644 --- a/core/client/tests/integration/services/feature-test.js +++ b/core/client/tests/integration/services/feature-test.js @@ -6,30 +6,43 @@ import Pretender from 'pretender'; import wait from 'ember-test-helpers/wait'; import FeatureService, {feature} from 'ghost/services/feature'; import Ember from 'ember'; +import { errorOverride, errorReset } from 'ghost/tests/helpers/adapter-error'; const {merge, run} = Ember; +const EmberError = Ember.Error; + +function stubSettings(server, labs, validSave = true, validSettings = true) { + let settings = [ + { + id: '1', + type: 'blog', + key: 'labs', + value: JSON.stringify(labs) + } + ]; + + if (validSettings) { + settings.push({ + id: '2', + type: 'blog', + key: 'postsPerPage', + value: 1 + }); + } -function stubSettings(server, labs) { server.get('/ghost/api/v0.1/settings/', function () { - return [200, {'Content-Type': 'application/json'}, JSON.stringify({settings: [ - { - id: '1', - type: 'blog', - key: 'labs', - value: JSON.stringify(labs) - }, - // postsPerPage is needed to satisfy the validation - { - id: '2', - type: 'blog', - key: 'postsPerPage', - value: 1 - } - ]})]; + return [200, {'Content-Type': 'application/json'}, JSON.stringify({settings})]; }); server.put('/ghost/api/v0.1/settings/', function (request) { - return [200, {'Content-Type': 'application/json'}, request.requestBody]; + let statusCode = (validSave) ? 200 : 400; + let response = (validSave) ? request.requestBody : JSON.stringify({ + errors: [{ + message: 'Test Error' + }] + }); + + return [statusCode, {'Content-Type': 'application/json'}, response]; }); } @@ -182,7 +195,6 @@ describeModule( return wait().then(() => { expect(server.handlers[1].numberOfCalls).to.equal(1); - // TODO: failing because service.update only sets values on service.get('testFlag').then((testFlag) => { expect(testFlag).to.be.true; done(); @@ -190,7 +202,56 @@ describeModule( }); }); - it('notifies for server errors'); - it('notifies for validation errors'); + it('notifies for server errors', function (done) { + stubSettings(server, {testFlag: false}, false); + addTestFlag(); + + let service = this.subject(); + + run(() => { + service.get('testFlag').then((testFlag) => { + expect(testFlag).to.be.false; + }); + }); + + run(() => { + service.set('testFlag', true); + }); + + return wait().then(() => { + expect(server.handlers[1].numberOfCalls).to.equal(1); + + expect(service.get('notifications.notifications').length).to.equal(1); + + service.get('testFlag').then((testFlag) => { + expect(testFlag).to.be.false; + done(); + }); + }); + }); + + it('notifies for validation errors', function (done) { + stubSettings(server, {testFlag: false}, true, false); + addTestFlag(); + + let service = this.subject(); + + run(() => { + service.get('testFlag').then((testFlag) => { + expect(testFlag).to.be.false; + }); + }); + + run(() => { + expect(() => { + service.set('testFlag', true); + }, EmberError, 'Threw validation error'); + }); + + service.get('testFlag').then((testFlag) => { + expect(testFlag).to.be.false; + done(); + }); + }); } ); diff --git a/core/client/tests/unit/validators/tag-settings-test.js b/core/client/tests/unit/validators/tag-settings-test.js index 6aa1f72a6f..01b26beeed 100644 --- a/core/client/tests/unit/validators/tag-settings-test.js +++ b/core/client/tests/unit/validators/tag-settings-test.js @@ -112,7 +112,6 @@ describe('Unit: Validator: tag-settings', function () { nameErrors = tag.get('errors').errorsFor('name').get(0); expect(nameErrors.attribute, 'errors.name.attribute').to.equal('name'); expect(nameErrors.message, 'errors.name.message').to.equal('Tag names can\'t start with commas.'); - expect(tag.get('errors.length')).to.equal(1); expect(passed, 'passed').to.be.false; expect(tag.get('hasValidated'), 'hasValidated').to.include('name');