Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Commit

Permalink
use ember-ajax in place of ember-data's networking (#283)
Browse files Browse the repository at this point in the history
closes #7014
- uses the `AjaxServiceSupport` mixin from `ember-ajax` to replace Ember Data's internal `ajax` method with our own ajax service
- normalizes all error handling to use `ember-ajax` style errors
- default to the `application/vnd.api+json` content-type so that we don't have a mix of urlencoded and plain JSON content
- fix `normalizeErrorResponse` in our `ajax` service so that it doesn't add an empty `errors` array to payloads
  • Loading branch information
kevinansfield authored and acburdine committed Sep 26, 2016
1 parent 3afc43e commit 87e1c5a
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 52 deletions.
18 changes: 2 additions & 16 deletions app/adapters/base.js
Expand Up @@ -2,20 +2,16 @@ import injectService from 'ember-service/inject';
import RESTAdapter from 'ember-data/adapters/rest';
import ghostPaths from 'ghost-admin/utils/ghost-paths';
import DataAdapterMixin from 'ember-simple-auth/mixins/data-adapter-mixin';
import config from 'ghost-admin/config/environment';
import AjaxServiceSupport from 'ember-ajax/mixins/ajax-support';

export default RESTAdapter.extend(DataAdapterMixin, {
export default RESTAdapter.extend(DataAdapterMixin, AjaxServiceSupport, {
authorizer: 'authorizer:oauth2',

host: window.location.origin,
namespace: ghostPaths().apiRoot.slice(1),

session: injectService(),

headers: {
'X-Ghost-Version': config.APP.version
},

shouldBackgroundReloadRecord() {
return false;
},
Expand All @@ -40,15 +36,5 @@ export default RESTAdapter.extend(DataAdapterMixin, {
}

return url;
},

handleResponse(status) {
if (status === 401) {
if (this.get('session.isAuthenticated')) {
this.get('session').invalidate();
}
}

return this._super(...arguments);
}
});
15 changes: 7 additions & 8 deletions app/components/modals/new-subscriber.js
@@ -1,5 +1,6 @@
import {A as emberA} from 'ember-array/utils';
import ModalComponent from 'ghost-admin/components/modals/base';
import {isInvalidError} from 'ember-ajax/errors';

export default ModalComponent.extend({
actions: {
Expand All @@ -20,16 +21,14 @@ export default ModalComponent.extend({
// TODO: server-side validation errors should be serialized
// properly so that errors are added to the model's errors
// property
if (error && error.isAdapterError) {
if (error && isInvalidError(error)) {
let [firstError] = error.errors;
let {message, errorType} = firstError;
let {message} = firstError;

if (errorType === 'ValidationError') {
if (message && message.match(/email/i)) {
this.get('model.errors').add('email', message);
this.get('model.hasValidated').pushObject('email');
return;
}
if (message && message.match(/email/i)) {
this.get('model.errors').add('email', message);
this.get('model.hasValidated').pushObject('email');
return;
}
}

Expand Down
5 changes: 2 additions & 3 deletions app/mirage/config/authentication.js
@@ -1,6 +1,5 @@
/* jscs:disable requireCamelCaseOrUpperCaseIdentifiers */
import Mirage from 'ember-cli-mirage';
import $ from 'jquery';
import {isBlank} from 'ember-utils';

export default function mockAuthentication(server) {
Expand All @@ -15,7 +14,7 @@ export default function mockAuthentication(server) {

server.post('/authentication/passwordreset', function (db, request) {
// jscs:disable requireObjectDestructuring
let {passwordreset} = $.deparam(request.requestBody);
let {passwordreset} = JSON.parse(request.requestBody);
let email = passwordreset[0].email;
// jscs:enable requireObjectDestructuring

Expand All @@ -40,7 +39,7 @@ export default function mockAuthentication(server) {
/* Setup ---------------------------------------------------------------- */

server.post('/authentication/setup', function (db, request) {
let [attrs] = $.deparam(request.requestBody).setup;
let [attrs] = JSON.parse(request.requestBody).setup;
let [role] = db.roles.where({name: 'Owner'});
let user;

Expand Down
5 changes: 2 additions & 3 deletions app/mixins/editor-base-controller.js
Expand Up @@ -15,6 +15,7 @@ import {task, timeout} from 'ember-concurrency';
import PostModel from 'ghost-admin/models/post';
import boundOneWay from 'ghost-admin/utils/bound-one-way';
import {isVersionMismatchError} from 'ghost-admin/services/ajax';
import {isInvalidError} from 'ember-ajax/errors';

const {resolve} = RSVP;

Expand Down Expand Up @@ -450,9 +451,7 @@ export default Mixin.create({
});
}).catch((error) => {
// re-throw if we have a general server error
// TODO: use isValidationError(error) once we have
// ember-ajax/ember-data integration
if (error && error.errors && error.errors[0].errorType !== 'ValidationError') {
if (error && !isInvalidError(error)) {
this.toggleProperty('submitting');
this.send('error', error);
return;
Expand Down
6 changes: 4 additions & 2 deletions app/serializers/user.js
Expand Up @@ -26,8 +26,10 @@ export default ApplicationSerializer.extend(EmbeddedRecordsMixin, {
let root = this.keyForAttribute(primaryModelClass.modelName);
let pluralizedRoot = pluralize(primaryModelClass.modelName);

payload[root] = payload[pluralizedRoot][0];
delete payload[pluralizedRoot];
if (payload[pluralizedRoot]) {
payload[root] = payload[pluralizedRoot][0];
delete payload[pluralizedRoot];
}

return this._super(...arguments);
}
Expand Down
59 changes: 42 additions & 17 deletions app/services/ajax.js
Expand Up @@ -2,10 +2,20 @@ import get from 'ember-metal/get';
import computed from 'ember-computed';
import injectService from 'ember-service/inject';
import {isEmberArray} from 'ember-array/utils';
import {isNone} from 'ember-utils';
import AjaxService from 'ember-ajax/services/ajax';
import {AjaxError, isAjaxError} from 'ember-ajax/errors';
import config from 'ghost-admin/config/environment';

const JSONContentType = 'application/json';

function isJSONContentType(header) {
if (isNone(header)) {
return false;
}
return header.indexOf(JSONContentType) === 0;
}

/* Version mismatch error */

export function VersionMismatchError(errors) {
Expand All @@ -17,8 +27,6 @@ VersionMismatchError.prototype = Object.create(AjaxError.prototype);
export function isVersionMismatchError(errorOrStatus, payload) {
if (isAjaxError(errorOrStatus)) {
return errorOrStatus instanceof VersionMismatchError;
} else if (errorOrStatus && get(errorOrStatus, 'isAdapterError')) {
return get(errorOrStatus, 'errors.firstObject.errorType') === 'VersionMismatchError';
} else {
return get(payload || {}, 'errors.firstObject.errorType') === 'VersionMismatchError';
}
Expand Down Expand Up @@ -81,8 +89,6 @@ MaintenanceError.prototype = Object.create(AjaxError.prototype);
export function isMaintenanceError(errorOrStatus) {
if (isAjaxError(errorOrStatus)) {
return errorOrStatus instanceof MaintenanceError;
} else if (errorOrStatus && get(errorOrStatus, 'isAdapterError')) {
return get(errorOrStatus, 'errors.firstObject.errorType') === 'Maintenance';
} else {
return errorOrStatus === 503;
}
Expand All @@ -99,8 +105,6 @@ ThemeValidationError.prototype = Object.create(AjaxError.prototype);
export function isThemeValidationError(errorOrStatus, payload) {
if (isAjaxError(errorOrStatus)) {
return errorOrStatus instanceof ThemeValidationError;
} else if (errorOrStatus && get(errorOrStatus, 'isAdapterError')) {
return get(errorOrStatus, 'errors.firstObject.errorType') === 'ThemeValidationError';
} else {
return get(payload || {}, 'errors.firstObject.errorType') === 'ThemeValidationError';
}
Expand All @@ -116,6 +120,7 @@ export default AjaxService.extend({
let headers = {};

headers['X-Ghost-Version'] = config.APP.version;
headers['Content-Type'] = `${JSONContentType}; charset=utf-8`;

if (session.get('isAuthenticated')) {
session.authorize('authorizer:oauth2', (headerName, headerValue) => {
Expand All @@ -126,6 +131,18 @@ export default AjaxService.extend({
return headers;
}).volatile(),

// ember-ajax recognises `application/vnd.api+json` as a JSON-API request
// and formats appropriately, we want to handle `application/json` the same
_makeRequest(url, hash) {
if (isJSONContentType(hash.headers['Content-Type']) && hash.type !== 'GET') {
if (typeof hash.data === 'object') {
hash.data = JSON.stringify(hash.data);
}
}

return this._super(...arguments);
},

handleResponse(status, headers, payload) {
if (this.isVersionMismatchError(status, headers, payload)) {
return new VersionMismatchError(payload.errors);
Expand All @@ -141,24 +158,32 @@ export default AjaxService.extend({
return new ThemeValidationError(payload.errors);
}

// TODO: we may want to check that we are hitting our own API before
// logging the user out due to a 401 response
if (this.isUnauthorizedError(status, headers, payload) && this.get('session.isAuthenticated')) {
this.get('session').invalidate();
}

return this._super(...arguments);
},

normalizeErrorResponse(status, headers, payload) {
if (payload && typeof payload === 'object') {
payload.errors = payload.error || payload.errors || payload.message || undefined;

if (!isEmberArray(payload.errors)) {
payload.errors = [payload.errors];
}
let errors = payload.error || payload.errors || payload.message || undefined;

payload.errors = payload.errors.map(function(error) {
if (typeof error === 'string') {
return {message: error};
} else {
return error;
if (errors) {
if (!isEmberArray(errors)) {
errors = [errors];
}
});

payload.errors = errors.map(function(error) {
if (typeof error === 'string') {
return {message: error};
} else {
return error;
}
});
}
}

return this._super(status, headers, payload);
Expand Down
2 changes: 1 addition & 1 deletion app/services/notifications.js
Expand Up @@ -95,7 +95,7 @@ export default Service.extend({
return this.get('upgradeStatus').maintenanceAlert();
}

// loop over Ember Data / ember-ajax errors object
// loop over ember-ajax errors object
if (resp && isEmberArray(resp.errors)) {
return resp.errors.forEach((error) => {
this._showAPIError(error, options);
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/signup-test.js
Expand Up @@ -115,7 +115,7 @@ describe('Acceptance: Signup', function() {
});

server.post('/authentication/invitation/', function (db, request) {
let params = $.deparam(request.requestBody);
let params = JSON.parse(request.requestBody);
expect(params.invitation[0].name).to.equal('Test User');
expect(params.invitation[0].email).to.equal('kevin+test2@ghost.org');
expect(params.invitation[0].password).to.equal('ValidPassword');
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/team-test.js
Expand Up @@ -685,7 +685,7 @@ describe('Acceptance: Team', function () {
andThen(() => {
// hits the endpoint
let [lastRequest] = server.pretender.handledRequests.slice(-1);
let params = $.deparam(lastRequest.requestBody);
let params = JSON.parse(lastRequest.requestBody);

expect(lastRequest.url, 'password request URL')
.to.match(/\/users\/password/);
Expand Down

0 comments on commit 87e1c5a

Please sign in to comment.