Fixed webhooks not firing for internal integrations

ref https://linear.app/ghost/issue/AP-598 We want to make sure that webhooks for internal integrations are fired even when the custom integrations limit has been hit. In order to test this we've had to update the fixtureManager to allow passing the integration type when inserting fixture webhooks into the db.
TryGhost · Nov 21, 2024 · 3265e25 · 3265e25
1 parent 263c493
commit 3265e25
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 8 deletions.
diff --git a/ghost/core/core/server/services/webhooks/WebhookTrigger.js b/ghost/core/core/server/services/webhooks/WebhookTrigger.js
@@ -27,9 +27,18 @@ class WebhookTrigger {
             const overLimit = await this.limitService.checkWouldGoOverLimit('customIntegrations');
 
             if (overLimit) {
-                logging.info(`Skipping all webhooks for event ${event}. The "customIntegrations" plan limit is enabled.`);
+                logging.info(`Skipping all non-internal webhooks for event ${event}. The "customIntegrations" plan limit is enabled.`);
+                const result = await this.models
+                    .Webhook
+                    .findAllByEvent(event, {
+                        context: {internal: true},
+                        withRelated: ['integration']
+                    });
+
                 return {
-                    models: []
+                    models: result?.models?.filter((model) => {
+                        return model.related('integration')?.get('type') === 'internal';
+                    }) || []
                 };
             }
         }

diff --git a/ghost/core/test/e2e-webhooks/__snapshots__/site.test.js.snap b/ghost/core/test/e2e-webhooks/__snapshots__/site.test.js.snap
@@ -11,3 +11,15 @@ Object {
 `;
 
 exports[`site.* events site.changed event is triggered 2: [body] 1`] = `Object {}`;
+
+exports[`site.* events site.changed event is triggered, custom integrations are limited but we have an internal webhook 1: [headers] 1`] = `
+Object {
+  "accept-encoding": "gzip, deflate, br",
+  "content-length": Any<Number>,
+  "content-type": "application/json",
+  "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
+  "user-agent": StringMatching /Ghost\\\\/\\\\d\\+\\\\\\.\\\\d\\+\\\\\\.\\\\d\\+\\\\s\\\\\\(https:\\\\/\\\\/github\\.com\\\\/TryGhost\\\\/Ghost\\\\\\)/,
+}
+`;
+
+exports[`site.* events site.changed event is triggered, custom integrations are limited but we have an internal webhook 2: [body] 1`] = `Object {}`;
diff --git a/ghost/core/test/e2e-webhooks/site.test.js b/ghost/core/test/e2e-webhooks/site.test.js
@@ -87,4 +87,40 @@ describe('site.* events', function () {
             throw new Error('The webhook should not have been sent.');
         }
     });
+
+    it('site.changed event is triggered, custom integrations are limited but we have an internal webhook', async function () {
+        const webhookURL = 'https://test-webhook-receiver.com/site-changed';
+        await webhookMockReceiver.mock(webhookURL);
+        await fixtureManager.insertWebhook({
+            event: 'site.changed',
+            url: webhookURL,
+            integrationType: 'internal'
+        });
+
+        mockManager.mockLimitService('customIntegrations', {
+            isLimited: true,
+            wouldGoOverLimit: true
+        });
+
+        await adminAPIAgent
+            .post('posts/')
+            .body({
+                posts: [{
+                    title: 'webhookz',
+                    status: 'published',
+                    mobiledoc: fixtureManager.get('posts', 1).mobiledoc
+                }]
+            })
+            .expectStatus(201);
+
+        await webhookMockReceiver.receivedRequest();
+
+        webhookMockReceiver
+            .matchHeaderSnapshot({
+                'content-version': anyContentVersion,
+                'content-length': anyNumber,
+                'user-agent': anyGhostAgent
+            })
+            .matchBodySnapshot();
+    });
 });
diff --git a/ghost/core/test/unit/server/services/webhooks/trigger.test.js b/ghost/core/test/unit/server/services/webhooks/trigger.test.js
@@ -61,23 +61,27 @@ describe('Webhook Service', function () {
 
         it('does not trigger payload handler when there are hooks registered for an event, but the custom integrations limit is active', async function () {
             const webhookModel = {
-                get: sinon.stub()
+                get: sinon.stub(),
+                related: sinon.stub()
             };
 
             webhookModel.get
                 .withArgs('event').returns(WEBHOOK_EVENT)
                 .withArgs('target_url').returns(WEBHOOK_TARGET_URL);
 
+            webhookModel.related
+                .withArgs('integration').returns(null);
+
             models.Webhook.findAllByEvent
-                .withArgs(WEBHOOK_EVENT, {context: {internal: true}})
+                .withArgs(WEBHOOK_EVENT, {context: {internal: true}, withRelated: ['integration']})
                 .resolves({models: [webhookModel]});
 
             limitService.isLimited.withArgs('customIntegrations').returns(true);
             limitService.checkWouldGoOverLimit.withArgs('customIntegrations').resolves(true);
 
             await webhookTrigger.trigger(WEBHOOK_EVENT);
 
-            assert.equal(models.Webhook.findAllByEvent.called, false);
+            assert.equal(models.Webhook.findAllByEvent.called, true);
             assert.equal(payload.called, false);
             assert.equal(request.called, false);
         });

diff --git a/ghost/core/test/utils/e2e-framework.js b/ghost/core/test/utils/e2e-framework.js
@@ -409,10 +409,12 @@ const getAgentsWithFrontend = async () => {
     };
 };
 
-const insertWebhook = ({event, url}) => {
+const insertWebhook = ({event, url, integrationType = undefined}) => {
     return fixtureUtils.fixtures.insertWebhook({
         event: event,
         target_url: url
+    }, {
+        integrationType
     });
 };
 

diff --git a/ghost/core/test/utils/fixture-utils.js b/ghost/core/test/utils/fixture-utils.js
@@ -426,10 +426,16 @@ const fixtures = {
         }));
     },
 
-    insertWebhook: function (attributes) {
+    insertWebhook: function (attributes, options = {}) {
+        let integration = DataGenerator.forKnex.integrations[0];
+        if (options.integrationType) {
+            integration = DataGenerator.forKnex.integrations.find(
+                props => props.type === options.integrationType
+            );
+        }
         const webhook = DataGenerator.forKnex.createWebhook(
             Object.assign(attributes, {
-                integration_id: DataGenerator.forKnex.integrations[0].id
+                integration_id: integration.id
             })
         );