Completely ignored disabled webhooks (#800)

app/server/lib/Triggers.ts

@@ -174,7 +174,7 @@ export class DocTriggers {
     const triggersTable = docData.getMetaTable("_grist_Triggers");
     const getTableId = docData.getMetaTable("_grist_Tables").getRowPropFunc("tableId");
 
-    const triggersByTableRef = _.groupBy(triggersTable.getRecords(), "tableRef");
+    const triggersByTableRef = _.groupBy(triggersTable.getRecords().filter(t => t.enabled), "tableRef");
     const triggersByTableId: Array<[string, Trigger[]]> = [];
 
     // First we need a list of columns which must be included in full in the action summary
@@ -545,9 +545,7 @@ export class DocTriggers {
     tableDelta: TableDelta,
   ): boolean {
     let readyBefore: boolean;
-    if (!trigger.enabled) {
+    if (!trigger.isReadyColRef) {
-      return false;
-    } else if (!trigger.isReadyColRef) {
       // User hasn't configured a column, so all records are considered ready immediately
       readyBefore = recordDelta.existedBefore;
     } else {

test/server/lib/DocApi.ts

@@ -18,7 +18,7 @@ import {delayAbort} from 'app/server/lib/serverUtils';
 import axios, {AxiosRequestConfig, AxiosResponse} from 'axios';
 import {delay} from 'bluebird';
 import {assert} from 'chai';
-import * as express from 'express';
+import express from 'express';
 import FormData from 'form-data';
 import * as fse from 'fs-extra';
 import * as _ from 'lodash';
@@ -3307,7 +3307,6 @@ function testDocApi() {
   });
 
   describe('webhooks related endpoints', async function () {
-
       /*
         Regression test for old _subscribe endpoint. /docs/{did}/webhooks should be used instead to subscribe
       */
@@ -3503,8 +3502,6 @@ function testDocApi() {
       assert.equal(webhookList.length, 3);
     });
 
-
-
     it("POST /docs/{did}/tables/{tid}/_unsubscribe validates inputs for editors", async function () {
 
       const subscribeResponse = await subscribeWebhook();
@@ -3563,6 +3560,7 @@ function testDocApi() {
       assert.equal(accessResp.status, 200);
       await flushAuth();
     });
+
   });
 
   describe("Daily API Limit", () => {
@@ -3836,6 +3834,7 @@ function testDocApi() {
       eventTypes?: string[],
       name?: string,
       memo?: string,
+      enabled?: boolean,
     }) {
       // Subscribe helper that returns a method to unsubscribe.
       const {data, status} = await axios.post(
@@ -3844,7 +3843,7 @@ function testDocApi() {
           eventTypes: options?.eventTypes ?? ['add', 'update'],
           url: `${serving.url}/${endpoint}`,
           isReadyColumn: options?.isReadyColumn === undefined ? 'B' : options?.isReadyColumn,
-          ...pick(options, 'name', 'memo'),
+          ...pick(options, 'name', 'memo', 'enabled'),
         }, chimpy
       );
       assert.equal(status, 200);
@@ -3970,6 +3969,25 @@ function testDocApi() {
         }
       });
 
+      async function createWebhooks({docId, tableId, eventTypesSet, isReadyColumn, enabled}:
+        {docId: string, tableId: string, eventTypesSet: string[][], isReadyColumn: string, enabled?: boolean}
+      ) {
+        // Ensure the isReady column is a Boolean
+        await axios.post(`${serverUrl}/api/docs/${docId}/apply`, [
+          ['ModifyColumn', tableId, isReadyColumn, {type: 'Bool'}],
+        ], chimpy);
+
+        const subscribeResponses = [];
+        const webhookIds: Record<string, string> = {};
+
+        for (const eventTypes of eventTypesSet) {
+          const data = await subscribe(String(eventTypes), docId, {tableId, eventTypes, isReadyColumn, enabled});
+          subscribeResponses.push(data);
+          webhookIds[data.webhookId] = String(eventTypes);
+        }
+        return {subscribeResponses, webhookIds};
+      }
+
       it("delivers expected payloads from combinations of changes, with retrying and batching",
         async function () {
         // Create a test document.
@@ -3977,27 +3995,15 @@ function testDocApi() {
         const docId = await userApi.newDoc({name: 'testdoc'}, ws1);
         const doc = userApi.getDocAPI(docId);
 
-        // For some reason B is turned into Numeric even when given bools
-        await axios.post(`${serverUrl}/api/docs/${docId}/apply`, [
-          ['ModifyColumn', 'Table1', 'B', {type: 'Bool'}],
-        ], chimpy);
-
         // Make a webhook for every combination of event types
-        const subscribeResponses = [];
+        const {subscribeResponses, webhookIds} = await createWebhooks({
-        const webhookIds: Record<string, string> = {};
+          docId, tableId: 'Table1', isReadyColumn: "B",
-        for (const eventTypes of [
+          eventTypesSet: [
             ["add"],
             ["update"],
             ["add", "update"],
-        ]) {
+          ]
-          const {data, status} = await axios.post(
+        });
-            `${serverUrl}/api/docs/${docId}/tables/Table1/_subscribe`,
-            {eventTypes, url: `${serving.url}/${eventTypes}`, isReadyColumn: "B"}, chimpy
-          );
-          assert.equal(status, 200);
-          subscribeResponses.push(data);
-          webhookIds[data.webhookId] = String(eventTypes);
-        }
 
         // Add and update some rows, trigger some events
         // Values of A where B is true and thus the record is ready are [1, 4, 7, 8]
@@ -4101,6 +4107,41 @@ function testDocApi() {
         );
 
       });
+
+      [{
+        itMsg: "doesn't trigger webhook that has been disabled",
+        enabled: false,
+      }, {
+        itMsg: "does trigger webhook that has been enable",
+        enabled: true,
+      }].forEach((ctx) => {
+
+        it(ctx.itMsg, async function () {
+          // Create a test document.
+          const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id;
+          const docId = await userApi.newDoc({name: 'testdoc'}, ws1);
+          const doc = userApi.getDocAPI(docId);
+
+          await createWebhooks({
+            docId, tableId: 'Table1', isReadyColumn: "B", eventTypesSet: [ ["add"] ], enabled: ctx.enabled
+          });
+
+          await doc.addRows("Table1", {
+            A: [42],
+            B: [true]
+          });
+
+          const queueRedisCalls = redisCalls.filter(args => args[1] === "webhook-queue-" + docId);
+          const redisPushIndex = queueRedisCalls.findIndex(args => args[0] === "rpush");
+
+          if (ctx.enabled) {
+            assert.isAbove(redisPushIndex, 0, "Should have pushed events to the redis queue");
+          } else {
+            assert.equal(redisPushIndex, -1, "Should not have pushed any events to the redis queue");
+          }
+        });
+
+      });
     });
 
     describe("/webhooks endpoint", function () {