From f16d9802d009c757c3131a8bb5f6dd5d0edfca95 Mon Sep 17 00:00:00 2001 From: Aileen Booker Date: Thu, 22 Feb 2024 12:51:41 -0400 Subject: [PATCH] Added ability to pass `minThreshold` for Milestone Slack notifications closes ENG-632 - This listens to a new property in the `milestones` config to set a minimum value of Milestones we wanna use the Slack notification service for --- .../services/slack-notifications/service.js | 9 ++-- .../slack-notifications/index.test.js | 8 +-- .../lib/SlackNotificationsService.js | 2 + .../test/SlackNotificationsService.test.js | 50 +++++++++++++++++-- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/ghost/core/core/server/services/slack-notifications/service.js b/ghost/core/core/server/services/slack-notifications/service.js index 85257e91e7..3f4ed85cb6 100644 --- a/ghost/core/core/server/services/slack-notifications/service.js +++ b/ghost/core/core/server/services/slack-notifications/service.js @@ -12,10 +12,11 @@ class SlackNotificationsServiceWrapper { * @param {string} deps.siteUrl * @param {boolean} deps.isEnabled * @param {URL} deps.webhookUrl + * @param {number} deps.minThreshold * * @returns {import('@tryghost/slack-notifications/lib/SlackNotificationsService')} */ - static create({siteUrl, isEnabled, webhookUrl}) { + static create({siteUrl, isEnabled, webhookUrl, minThreshold}) { const { SlackNotificationsService, SlackNotifications @@ -32,7 +33,8 @@ class SlackNotificationsServiceWrapper { logging, config: { isEnabled, - webhookUrl + webhookUrl, + minThreshold }, slackNotifications }); @@ -49,8 +51,9 @@ class SlackNotificationsServiceWrapper { const siteUrl = urlUtils.getSiteUrl(); const isEnabled = !!(hostSettings?.milestones?.enabled && hostSettings?.milestones?.url); const webhookUrl = hostSettings?.milestones?.url; + const minThreshold = hostSettings?.milestones?.minThreshold ? parseInt(hostSettings.milestones.minThreshold) : 0; - this.#api = SlackNotificationsServiceWrapper.create({siteUrl, isEnabled, webhookUrl}); + this.#api = SlackNotificationsServiceWrapper.create({siteUrl, isEnabled, webhookUrl, minThreshold}); this.#api.subscribeEvents(); } diff --git a/ghost/core/test/unit/server/services/slack-notifications/index.test.js b/ghost/core/test/unit/server/services/slack-notifications/index.test.js index 1d082d5e2b..8e91adb577 100644 --- a/ghost/core/test/unit/server/services/slack-notifications/index.test.js +++ b/ghost/core/test/unit/server/services/slack-notifications/index.test.js @@ -9,7 +9,7 @@ describe('Slack Notifications Service', function () { let scope; beforeEach(function () { - configUtils.set('hostSettings', {milestones: {enabled: true, url: 'https://testhooks.slack.com/'}}); + configUtils.set('hostSettings', {milestones: {enabled: true, url: 'https://testhooks.slack.com/', minThreshold: '100'}}); scope = nock('https://testhooks.slack.com/') .post('/') @@ -28,13 +28,13 @@ describe('Slack Notifications Service', function () { milestone: { type: 'arr', currency: 'usd', - name: 'arr-100-usd', - value: 100, + name: 'arr-1000-usd', + value: 1000, createdAt: new Date(), emailSentAt: new Date() }, meta: { - currentValue: 105 + currentValue: 1005 } })); diff --git a/ghost/slack-notifications/lib/SlackNotificationsService.js b/ghost/slack-notifications/lib/SlackNotificationsService.js index 0ab7bb4c80..73f26b6eec 100644 --- a/ghost/slack-notifications/lib/SlackNotificationsService.js +++ b/ghost/slack-notifications/lib/SlackNotificationsService.js @@ -27,6 +27,7 @@ const {MilestoneCreatedEvent} = require('@tryghost/milestones'); * @typedef {object} config * @prop {boolean} isEnabled * @prop {URL} webhookUrl + * @prop {number} minThreshold */ module.exports = class SlackNotificationsService { @@ -71,6 +72,7 @@ module.exports = class SlackNotificationsService { && event.data.milestone && this.#config.isEnabled && this.#config.webhookUrl + && this.#config.minThreshold < event.data.milestone.value ) { try { await this.#slackNotifications.notifyMilestoneReceived(event.data); diff --git a/ghost/slack-notifications/test/SlackNotificationsService.test.js b/ghost/slack-notifications/test/SlackNotificationsService.test.js index 1e90150da4..ecb17563cf 100644 --- a/ghost/slack-notifications/test/SlackNotificationsService.test.js +++ b/ghost/slack-notifications/test/SlackNotificationsService.test.js @@ -19,7 +19,8 @@ describe('SlackNotificationsService', function () { const config = { isEnabled: true, - webhookUrl: 'https://slack-webhook.example' + webhookUrl: 'https://slack-webhook.example', + minThreshold: 1000 }; beforeEach(function () { @@ -75,13 +76,13 @@ describe('SlackNotificationsService', function () { milestone: { id: new ObjectId().toHexString(), type: 'arr', - value: 1000, + value: 10000, currency: 'usd', createdAt: new Date(), emailSentAt: new Date() }, meta: { - currentValue: 1398 + currentValue: 13980 } })); @@ -118,6 +119,45 @@ describe('SlackNotificationsService', function () { assert(slackNotificationStub.callCount === 0); }); + it('does not send notification when milestone value below notification threshold', async function () { + service = new SlackNotificationsService({ + logging: { + warn: () => {}, + error: loggingSpy + }, + DomainEvents, + siteUrl: 'https://ghost.example', + config: { + isEnabled: false, + webhookUrl: 'https://slack-webhook.example' + }, + slackNotifications: { + notifyMilestoneReceived: slackNotificationStub + } + }); + + service.subscribeEvents(); + + DomainEvents.dispatch(MilestoneCreatedEvent.create({ + milestone: { + id: new ObjectId().toHexString(), + type: 'arr', + value: 1000, + currency: 'usd', + createdAt: new Date(), + emailSentAt: new Date() + }, + meta: { + currentValue: 1398 + } + })); + + await DomainEvents.allSettled(); + + assert(loggingSpy.callCount === 0); + assert(slackNotificationStub.callCount === 0); + }); + it('does not send notification when no url in hostSettings provided', async function () { service = new SlackNotificationsService({ logging: { @@ -166,8 +206,8 @@ describe('SlackNotificationsService', function () { DomainEvents.dispatch(MilestoneCreatedEvent.create({ milestone: { type: 'members', - name: 'members-100', - value: 100, + name: 'members-10000', + value: 10000, createdAt: new Date() } }));