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.
This commit is contained in:
Naz
parent
c11e79765e
commit
478eb6ead6
@ -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();
|
||||
|
||||
|
||||
@ -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",
|
||||
|
||||
@ -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 = {
|
||||
|
||||
@ -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<Object>} 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();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -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",
|
||||
|
||||
@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user