diff --git a/app/server/lib/Triggers.ts b/app/server/lib/Triggers.ts index 313cedf4..561e4eee 100644 --- a/app/server/lib/Triggers.ts +++ b/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) { - return false; - } else if (!trigger.isReadyColRef) { + if (!trigger.isReadyColRef) { // User hasn't configured a column, so all records are considered ready immediately readyBefore = recordDelta.existedBefore; } else { diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 8311050c..3b242402 100644 --- a/test/server/lib/DocApi.ts +++ b/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,34 +3969,41 @@ function testDocApi() { } }); - it("delivers expected payloads from combinations of changes, with retrying and batching", - 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); - - // For some reason B is turned into Numeric even when given bools + 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', 'Table1', 'B', {type: 'Bool'}], + ['ModifyColumn', tableId, isReadyColumn, {type: 'Bool'}], ], chimpy); - // Make a webhook for every combination of event types const subscribeResponses = []; const webhookIds: Record = {}; - for (const eventTypes of [ - ["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); + + 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. + const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; + const docId = await userApi.newDoc({name: 'testdoc'}, ws1); + const doc = userApi.getDocAPI(docId); + + // Make a webhook for every combination of event types + const {subscribeResponses, webhookIds} = await createWebhooks({ + docId, tableId: 'Table1', isReadyColumn: "B", + eventTypesSet: [ + ["add"], + ["update"], + ["add", "update"], + ] + }); // 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 () {