Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00
Code Issues Activity

Removed method complexity in integrations API controller

Browse source 
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-10/tackle-integrationsjs

- The controller code is not meant to contain complex business logic.
- Added a test case checking 'PUT' endpoint for integrations to ensure
proper 'NotFound' handling. Found that previous implemenation was
buggy - threw a 500 as 'models.Integration.NotFoundError' that was removed
in previous commit didn't catch a needed error.
This commit is contained in:
Naz 2021-09-16 14:05:48 +03:00
parent 08b7fbc73f
commit 4744349381
4 changed files with 84 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
core/server/api/canary/integrations.js
View file

@ -1,6 +1,12 @@
const i18n = require('../../../shared/i18n');
const errors = require('@tryghost/errors');
const models = require('../../models');
const getIntegrationsServiceInstance = require('../../services/integrations/integrations-service');
const integrationsService = getIntegrationsServiceInstance({
IntegrationModel: models.Integration,
ApiKeyModel: models.ApiKey
});
module.exports = {
docName: 'integrations',
@ -76,36 +82,7 @@ module.exports = {
}
},
query({data, options}) {
if (options.keyid) {
return models.ApiKey.findOne({id: options.keyid})
.then(async (model) => {
if (!model) {
throw new errors.NotFoundError({
message: i18n.t('errors.api.resource.resourceNotFound', {
resource: 'ApiKey'
})
});
}
try {
await models.ApiKey.refreshSecret(model.toJSON(), Object.assign({}, options, {id: options.keyid}));
return models.Integration.findOne({id: options.id}, {
withRelated: ['api_keys', 'webhooks']
});
} catch (err) {
throw new errors.GhostError({
err: err
});
}
});
}
return models.Integration.edit(data, Object.assign(options, {require: true}))
.catch(models.Integration.NotFoundError, () => {
throw new errors.NotFoundError({
message: i18n.t('errors.api.resource.resourceNotFound', {
resource: 'Integration'
})
});
});
return integrationsService.edit(data, options);
}
},
add: {

37
core/server/api/v3/integrations.js
View file

@ -1,6 +1,12 @@
const i18n = require('../../../shared/i18n');
const errors = require('@tryghost/errors');
const models = require('../../models');
const getIntegrationsServiceInstance = require('../../services/integrations/integrations-service');
const integrationsService = getIntegrationsServiceInstance({
IntegrationModel: models.Integration,
ApiKeyModel: models.ApiKey
});
module.exports = {
docName: 'integrations',
@ -76,36 +82,7 @@ module.exports = {
}
},
query({data, options}) {
if (options.keyid) {
return models.ApiKey.findOne({id: options.keyid})
.then(async (model) => {
if (!model) {
throw new errors.NotFoundError({
message: i18n.t('errors.api.resource.resourceNotFound', {
resource: 'ApiKey'
})
});
}
try {
await models.ApiKey.refreshSecret(model.toJSON(), Object.assign({}, options, {id: options.keyid}));
return models.Integration.findOne({id: options.id}, {
withRelated: ['api_keys', 'webhooks']
});
} catch (err) {
throw new errors.GhostError({
err: err
});
}
});
}
return models.Integration.edit(data, Object.assign(options, {require: true}))
.catch(models.Integration.NotFoundError, () => {
throw new errors.NotFoundError({
message: i18n.t('errors.api.resource.resourceNotFound', {
resource: 'Integration'
})
});
});
return integrationsService.edit(data, options);
}
},
add: {

61
core/server/services/integrations/integrations-service.js Normal file
View file

@ -0,0 +1,61 @@
const {NotFoundError, GhostError} = require('@tryghost/errors');
const tpl = require('@tryghost/tpl');
const messages = {
notFound: '{resource} not found.'
};
class IntegrationsService {
constructor({IntegrationModel, ApiKeyModel}) {
this.IntegrationModel = IntegrationModel;
this.ApiKeyModel = ApiKeyModel;
}
async edit(data, options) {
if (options.keyid) {
const model = await this.ApiKeyModel.findOne({id: options.keyid});
if (!model) {
throw new NotFoundError({
message: tpl(messages.notFound, {
resource: 'ApiKey'
})
});
}
try {
await this.ApiKeyModel.refreshSecret(model.toJSON(), Object.assign({}, options, {id: options.keyid}));
return await this.IntegrationModel.findOne({id: options.id}, {
withRelated: ['api_keys', 'webhooks']
});
} catch (err) {
throw new GhostError({
err: err
});
}
}
try {
return await this.IntegrationModel.edit(data, Object.assign(options, {require: true}));
} catch (error) {
if (error.message === 'NotFound' || error.message === 'EmptyResponse') {
throw new NotFoundError({
message: tpl(messages.notFound, {
resource: 'Integration'
})
});
}
throw error;
}
}
}
/**
* @returns {IntegrationsService} instance of the PostsService
*/
const getIntegrationsServiceInstance = ({IntegrationModel, ApiKeyModel}) => {
return new IntegrationsService({IntegrationModel, ApiKeyModel});
};
module.exports = getIntegrationsServiceInstance;

9
test/api-acceptance/admin/integrations.test.js
View file

@ -331,5 +331,14 @@ describe('Integrations API', function () {
await request.get(localUtils.API.getApiQuery(`integrations/${createdIntegration.id}/`))
.set('Origin', config.get('url'))
.expect(404);
const editRes = await request.put(localUtils.API.getApiQuery(`integrations/${createdIntegration.id}/`))
.send({
integrations: [createdIntegration]
})
.set('Origin', config.get('url'))
.expect(404);
editRes.body.errors[0].context.should.eql('Integration not found.');
});
});