From 478eb6ead6fe2d6d9e1b9df64935e286003f6c42 Mon Sep 17 00:00:00 2001 From: Naz Date: Mon, 30 Jan 2023 17:57:02 +0800 Subject: [PATCH] Limited integrations triggering version mismatch emails refs https://github.com/TryGhost/Toolbox/issues/500 refs https://ghost.notion.site/Data-Types-e5dc54dd0078443f9afd6b2abda443c4 - There current notification logic for incompatible integrations did not take into account the source of the trigger, which might have been causing emails to instance owners that did not ever set up custom integration - so they had nothing to fix. - The "internal" and "core" integrations are maintained/controlled by the Ghost team, so there should never be a notification going out to the instance owner about possible incompatibility in the code they do not control. - Along with changed updated the unit test threshold in the packages that were touched to 100%. As that's the standard for all new packages. --- .../lib/api-version-compatibility-service.js | 13 +- .../package.json | 2 +- .../api-version-compatibility-service.test.js | 113 +++++++++++++++++- .../lib/version-notifications-data-service.js | 7 +- .../package.json | 2 +- .../version-notificatons-data-service.test.js | 19 ++- 6 files changed, 144 insertions(+), 12 deletions(-) diff --git a/ghost/api-version-compatibility-service/lib/api-version-compatibility-service.js b/ghost/api-version-compatibility-service/lib/api-version-compatibility-service.js index 67bb9b0a2f..14c4cfc576 100644 --- a/ghost/api-version-compatibility-service/lib/api-version-compatibility-service.js +++ b/ghost/api-version-compatibility-service/lib/api-version-compatibility-service.js @@ -41,7 +41,18 @@ class APIVersionCompatibilityService { */ async handleMismatch({acceptVersion, contentVersion, apiKeyValue, apiKeyType, requestURL, userAgent = ''}) { if (!await this.versionNotificationsDataService.fetchNotification(acceptVersion)) { - const integrationName = await this.versionNotificationsDataService.getIntegrationName(apiKeyValue, apiKeyType); + const { + name: integrationName, + type: integrationType + } = await this.versionNotificationsDataService.getIntegration(apiKeyValue, apiKeyType); + + // @NOTE: "internal" or "core" integrations (https://ghost.notion.site/Data-Types-e5dc54dd0078443f9afd6b2abda443c4) + // are maintained by Ghost team, so there is no sense notifying the instance owner about it's incompatibility. + // The other two integration types: "builtin" and "custom", is when we want to notify about incompatibility. + if (['internal', 'core'].includes(integrationType)) { + return; + } + const trimmedUseAgent = userAgent.split('/')[0]; const emails = await this.versionNotificationsDataService.getNotificationEmails(); diff --git a/ghost/api-version-compatibility-service/package.json b/ghost/api-version-compatibility-service/package.json index b25b2f55f7..322c2b280a 100644 --- a/ghost/api-version-compatibility-service/package.json +++ b/ghost/api-version-compatibility-service/package.json @@ -7,7 +7,7 @@ "main": "index.js", "scripts": { "dev": "echo \"Implement me!\"", - "test:unit": "NODE_ENV=testing c8 --all --check-coverage --reporter text --reporter cobertura mocha './test/**/*.test.js'", + "test:unit": "NODE_ENV=testing c8 --all --check-coverage --100 --reporter text --reporter cobertura mocha './test/**/*.test.js'", "test": "yarn test:unit", "lint:code": "eslint *.js lib/ --ext .js --cache", "lint": "yarn lint:code && yarn lint:test", diff --git a/ghost/api-version-compatibility-service/test/api-version-compatibility-service.test.js b/ghost/api-version-compatibility-service/test/api-version-compatibility-service.test.js index 5b7223bf12..21ace5388e 100644 --- a/ghost/api-version-compatibility-service/test/api-version-compatibility-service.test.js +++ b/ghost/api-version-compatibility-service/test/api-version-compatibility-service.test.js @@ -40,7 +40,10 @@ describe('APIVersionCompatibilityService', function () { .resolves({ relations: { integration: { - get: () => 'Elaborate Fox' + toJSON: () => ({ + name: 'Elaborate Fox', + type: 'custom' + }) } } }) @@ -181,6 +184,114 @@ describe('APIVersionCompatibilityService', function () { assert.equal(sendEmail.calledTwice, false); }); + it('Does NOT send an email to the instance owner when "internal" or "core" integration triggered version mismatch', async function () { + const sendEmail = sinon.spy(); + const findOneStub = sinon.stub(); + findOneStub + .withArgs({ + id: 'secret_core' + }, { + withRelated: ['integration'] + }) + .resolves({ + relations: { + integration: { + toJSON: () => ({ + name: 'Core Integration', + type: 'core' + }) + } + } + }); + findOneStub + .withArgs({ + id: 'secrete_internal' + }, { + withRelated: ['integration'] + }) + .resolves({ + relations: { + integration: { + toJSON: () => ({ + name: 'Internal Integration', + type: 'internal' + }) + } + } + }); + findOneStub + .withArgs({ + secret: 'custom_key_id' + }, { + withRelated: ['integration'] + }) + .resolves({ + relations: { + integration: { + toJSON: () => ({ + name: 'Custom Integration', + type: 'custom' + }) + } + } + }); + + ApiKeyModel = { + findOne: findOneStub + }; + + settingsService = { + read: sinon.stub().resolves({ + version_notifications: { + value: JSON.stringify([]) + } + }), + edit: sinon.stub().resolves() + }; + + const compatibilityService = new APIVersionCompatibilityService({ + sendEmail, + ApiKeyModel, + UserModel, + settingsService, + getSiteUrl, + getSiteTitle + }); + + await compatibilityService.handleMismatch({ + acceptVersion: 'v4.5', + contentVersion: 'v5.1', + userAgent: 'GhostAdminSDK/2.4.0', + requestURL: '', + apiKeyValue: 'secret_core', + apiKeyType: 'admin' + }); + + assert.equal(sendEmail.called, false); + + await compatibilityService.handleMismatch({ + acceptVersion: 'v4.5', + contentVersion: 'v5.1', + userAgent: 'GhostAdminSDK/2.4.0', + requestURL: 'does not matter', + apiKeyValue: 'secrete_internal', + apiKeyType: 'admin' + }); + + assert.equal(sendEmail.called, false); + + await compatibilityService.handleMismatch({ + acceptVersion: 'v4.5', + contentVersion: 'v5.1', + userAgent: 'GhostContentSDK/2.4.0', + requestURL: 'https://amazeballsghostsite.com/ghost/api/admin/posts/dew023d9203se4', + apiKeyValue: 'custom_key_id', + apiKeyType: 'content' + }); + + assert.equal(sendEmail.called, true); + }); + it('Does send multiple emails to the instance owners when previously unhandled accept-version header mismatch is detected', async function () { const sendEmail = sinon.spy(); UserModel = { diff --git a/ghost/version-notifications-data-service/lib/version-notifications-data-service.js b/ghost/version-notifications-data-service/lib/version-notifications-data-service.js index 245d118fe9..fe73d74380 100644 --- a/ghost/version-notifications-data-service/lib/version-notifications-data-service.js +++ b/ghost/version-notifications-data-service/lib/version-notifications-data-service.js @@ -53,12 +53,13 @@ class VersionNotificationsDataService { } /** + * This method is for internal use only. * * @param {String} key - api key identification value, it's "secret" in case of Content API key and "id" for Admin API * @param {String} type - one of "content" or "admin" values - * @returns + * @returns {Promise} Integration JSON object */ - async getIntegrationName(key, type) { + async getIntegration(key, type) { let queryOptions = null; if (type === 'content') { @@ -69,7 +70,7 @@ class VersionNotificationsDataService { const apiKey = await this.ApiKeyModel.findOne(queryOptions, {withRelated: ['integration']}); - return apiKey.relations.integration.get('name'); + return apiKey.relations.integration.toJSON(); } } diff --git a/ghost/version-notifications-data-service/package.json b/ghost/version-notifications-data-service/package.json index 8f82532e76..c7c6042cb5 100644 --- a/ghost/version-notifications-data-service/package.json +++ b/ghost/version-notifications-data-service/package.json @@ -7,7 +7,7 @@ "main": "index.js", "scripts": { "dev": "echo \"Implement me!\"", - "test:unit": "NODE_ENV=testing c8 --all --check-coverage --reporter text --reporter cobertura mocha './test/**/*.test.js'", + "test:unit": "NODE_ENV=testing c8 --all --check-coverage --100 --reporter text --reporter cobertura mocha './test/**/*.test.js'", "test": "yarn test:unit", "lint:code": "eslint *.js lib/ --ext .js --cache", "lint": "yarn lint:code && yarn lint:test", diff --git a/ghost/version-notifications-data-service/test/version-notificatons-data-service.test.js b/ghost/version-notifications-data-service/test/version-notificatons-data-service.test.js index 90109d0aa5..91a361c4aa 100644 --- a/ghost/version-notifications-data-service/test/version-notificatons-data-service.test.js +++ b/ghost/version-notifications-data-service/test/version-notificatons-data-service.test.js @@ -123,7 +123,7 @@ describe('Version Notification Data Service', function () { }); }); - describe('getIntegrationName', function () { + describe('getIntegration', function () { it('Queries for Content API key when such is provided', async function () { const ApiKeyModel = { findOne: sinon @@ -136,7 +136,10 @@ describe('Version Notification Data Service', function () { .resolves({ relations: { integration: { - get: () => 'live fast die young' + toJSON: () => ({ + name: 'live fast die young', + type: 'custom' + }) } } }) @@ -148,9 +151,10 @@ describe('Version Notification Data Service', function () { settingsService: {} }); - const integrationName = await versionNotificationsDataService.getIntegrationName('super_secret', 'content'); + const {name: integrationName, type: integrationType} = await versionNotificationsDataService.getIntegration('super_secret', 'content'); assert.equal(integrationName, 'live fast die young'); + assert.equal(integrationType, 'custom'); }); it('Queries for Admin API key when such is provided', async function () { @@ -164,8 +168,12 @@ describe('Version Notification Data Service', function () { }) .resolves({ relations: { + integration: { - get: () => 'Tri Hita Karana' + toJSON: () => ({ + name: 'Tri Hita Karana', + type: 'core' + }) } } }) @@ -177,9 +185,10 @@ describe('Version Notification Data Service', function () { settingsService: {} }); - const integrationName = await versionNotificationsDataService.getIntegrationName('key_id', 'admin'); + const {name: integrationName, type: integrationType} = await versionNotificationsDataService.getIntegration('key_id', 'admin'); assert.equal(integrationName, 'Tri Hita Karana'); + assert.equal(integrationType, 'core'); }); }); });