From 6a057fad990a71d138c571bf15d08e18f418f04f Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Fri, 22 Nov 2019 14:20:32 +0000 Subject: [PATCH] Added /emails/:id/retry/ endpoint for retrying failed emails (#11410) We want to allow admin users to trigger a retry of failed emails without having to go through the unpublish/republish dance. - fixed resource identifier in email permissions migration so email permissions are added correctly - added new email permissions migration so that beta releases can be upgraded without rollback (will be a no-op for any non-beta upgrades) - added `/emails/:id/retry/` canary Admin API endpoint - follows same URL pattern as theme activation - only triggers mega service retry endpoint if the email has a `'failed'` status --- core/server/api/canary/email.js | 29 ++++++++++- .../canary/utils/serializers/output/emails.js | 4 ++ .../versions/3.1/06-add-email-permissions.js | 2 +- .../3.1/09-add-further-email-permissions.js | 51 +++++++++++++++++++ .../server/data/schema/fixtures/fixtures.json | 12 ++++- core/server/translations/en.json | 3 +- core/server/web/api/canary/admin/routes.js | 1 + core/test/unit/data/schema/integrity_spec.js | 2 +- 8 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 core/server/data/migrations/versions/3.1/09-add-further-email-permissions.js diff --git a/core/server/api/canary/email.js b/core/server/api/canary/email.js index ba2eba66863f..9dad02b5065f 100644 --- a/core/server/api/canary/email.js +++ b/core/server/api/canary/email.js @@ -1,5 +1,6 @@ const models = require('../../models'); const common = require('../../lib/common'); +const megaService = require('../../services/mega'); module.exports = { docName: 'emails', @@ -22,12 +23,38 @@ module.exports = { .then((model) => { if (!model) { throw new common.errors.NotFoundError({ - message: common.i18n.t('errors.api.email.emailNotFound') + message: common.i18n.t('errors.models.email.emailNotFound') }); } return model.toJSON(frame.options); }); } + }, + + retry: { + data: [ + 'id' + ], + permissions: true, + query(frame) { + return models.Email.findOne(frame.data, frame.options) + .then(async (model) => { + if (!model) { + throw new common.errors.NotFoundError({ + message: common.i18n.t('errors.models.email.emailNotFound') + }); + } + + if (model.get('status') !== 'failed') { + throw new common.errors.IncorrectUsageError({ + message: common.i18n.t('errors.models.email.retryNotAllowed') + }); + } + + const result = await megaService.mega.retryFailedEmail(model); + return result.toJSON(frame.options); + }); + } } }; diff --git a/core/server/api/canary/utils/serializers/output/emails.js b/core/server/api/canary/utils/serializers/output/emails.js index 6c5c2fb37178..c44c4b5f5e86 100644 --- a/core/server/api/canary/utils/serializers/output/emails.js +++ b/core/server/api/canary/utils/serializers/output/emails.js @@ -3,5 +3,9 @@ module.exports = { frame.response = { emails: [email] }; + }, + + get retry() { + return this.read; } }; diff --git a/core/server/data/migrations/versions/3.1/06-add-email-permissions.js b/core/server/data/migrations/versions/3.1/06-add-email-permissions.js index f270c03b1650..cc46ec1ba644 100644 --- a/core/server/data/migrations/versions/3.1/06-add-email-permissions.js +++ b/core/server/data/migrations/versions/3.1/06-add-email-permissions.js @@ -3,7 +3,7 @@ const utils = require('../../../schema/fixtures/utils'); const permissions = require('../../../../services/permissions'); const logging = require('../../../../lib/common/logging'); -const resources = ['emails']; +const resources = ['email']; const _private = {}; _private.getPermissions = function getPermissions(resource) { diff --git a/core/server/data/migrations/versions/3.1/09-add-further-email-permissions.js b/core/server/data/migrations/versions/3.1/09-add-further-email-permissions.js new file mode 100644 index 000000000000..cc46ec1ba644 --- /dev/null +++ b/core/server/data/migrations/versions/3.1/09-add-further-email-permissions.js @@ -0,0 +1,51 @@ +const _ = require('lodash'); +const utils = require('../../../schema/fixtures/utils'); +const permissions = require('../../../../services/permissions'); +const logging = require('../../../../lib/common/logging'); + +const resources = ['email']; +const _private = {}; + +_private.getPermissions = function getPermissions(resource) { + return utils.findModelFixtures('Permission', {object_type: resource}); +}; + +_private.printResult = function printResult(result, message) { + if (result.done === result.expected) { + logging.info(message); + } else { + logging.warn(`(${result.done}/${result.expected}) ${message}`); + } +}; + +module.exports.config = { + transaction: true +}; + +module.exports.up = (options) => { + const localOptions = _.merge({ + context: {internal: true} + }, options); + + return Promise.map(resources, (resource) => { + const modelToAdd = _private.getPermissions(resource); + + return utils.addFixturesForModel(modelToAdd, localOptions) + .then(result => _private.printResult(result, `Adding permissions fixtures for ${resource}`)) + .then(() => permissions.init(localOptions)); + }); +}; + +module.exports.down = (options) => { + const localOptions = _.merge({ + context: {internal: true} + }, options); + + return Promise.map(resources, (resource) => { + const modelToRemove = _private.getPermissions(resource); + + // permission model automatically cleans up permissions_roles on .destroy() + return utils.removeFixturesForModel(modelToRemove, localOptions) + .then(result => _private.printResult(result, `Removing permissions fixtures for ${resource}s`)); + }); +}; diff --git a/core/server/data/schema/fixtures/fixtures.json b/core/server/data/schema/fixtures/fixtures.json index 7eedbb426619..cea8e8863b87 100644 --- a/core/server/data/schema/fixtures/fixtures.json +++ b/core/server/data/schema/fixtures/fixtures.json @@ -374,9 +374,19 @@ "object_type": "email_preview" }, { - "name": "Email", + "name": "Browse emails", + "action_type": "browse", + "object_type": "email" + }, + { + "name": "Read emails", "action_type": "read", "object_type": "email" + }, + { + "name": "Retry emails", + "action_type": "retry", + "object_type": "email" } ] }, diff --git a/core/server/translations/en.json b/core/server/translations/en.json index fb475e312c7d..d4910365e705 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -256,7 +256,8 @@ "apiKeyNotFound": "API Key not found" }, "email": { - "emailNotFound": "Email not found." + "emailNotFound": "Email not found.", + "retryNotAllowed": "Only failed emails can be retried" }, "base": { "index": { diff --git a/core/server/web/api/canary/admin/routes.js b/core/server/web/api/canary/admin/routes.js index ec83d9d8f8af..2d10cb96e5c1 100644 --- a/core/server/web/api/canary/admin/routes.js +++ b/core/server/web/api/canary/admin/routes.js @@ -221,6 +221,7 @@ module.exports = function apiRoutes() { // ## Emails router.get('/emails/:id', mw.authAdminApi, http(apiCanary.emails.read)); + router.put('/emails/:id/retry', mw.authAdminApi, http(apiCanary.emails.retry)); return router; }; diff --git a/core/test/unit/data/schema/integrity_spec.js b/core/test/unit/data/schema/integrity_spec.js index a26fede900ed..470de5d54152 100644 --- a/core/test/unit/data/schema/integrity_spec.js +++ b/core/test/unit/data/schema/integrity_spec.js @@ -20,7 +20,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating const currentSchemaHash = '773f8f6cd4267f50aec6af8c8b1edbd2'; - const currentFixturesHash = 'b1787330f042f3954d73c43aa8bfa915'; + const currentFixturesHash = '1a0f96fa1d8b976d663eb06719be031c'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, // and the values above will need updating as confirmation