Skip to content

feature(webhook): Add remove event type #1762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feature(webhook): Add remove event type #1762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/client/ui/WebhookPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const WEBHOOK_COLUMNS = [
widgetOptions: JSON.stringify({
widget: 'TextBox',
alignment: 'left',
choices: ['add', 'update'],
choices: ['add', 'update', 'remove'],
choiceOptions: {},
}),
},
Expand Down
6 changes: 3 additions & 3 deletions app/common/Triggers-ti.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const Webhook = t.iface([], {
export const WebhookFields = t.iface([], {
"url": "string",
"authorization": t.opt("string"),
"eventTypes": t.array(t.union(t.lit("add"), t.lit("update"))),
"eventTypes": t.array(t.union(t.lit("add"), t.lit("update"), t.lit("remove"))),
"tableId": "string",
"watchedColIds": t.opt(t.array("string")),
"enabled": t.opt("boolean"),
Expand All @@ -31,7 +31,7 @@ export const WebhookStatus = t.union(t.lit('idle'), t.lit('sending'), t.lit('ret
export const WebhookSubscribe = t.iface([], {
"url": "string",
"authorization": t.opt("string"),
"eventTypes": t.array(t.union(t.lit("add"), t.lit("update"))),
"eventTypes": t.array(t.union(t.lit("add"), t.lit("update"), t.lit("remove"))),
"watchedColIds": t.opt(t.array("string")),
"enabled": t.opt("boolean"),
"isReadyColumn": t.opt(t.union("string", "null")),
Expand Down Expand Up @@ -68,7 +68,7 @@ export const WebhookUpdate = t.iface([], {
export const WebhookPatch = t.iface([], {
"url": t.opt("string"),
"authorization": t.opt("string"),
"eventTypes": t.opt(t.array(t.union(t.lit("add"), t.lit("update")))),
"eventTypes": t.opt(t.array(t.union(t.lit("add"), t.lit("update"), t.lit("remove")))),
"tableId": t.opt("string"),
"watchedColIds": t.opt(t.array("string")),
"enabled": t.opt("boolean"),
Expand Down
6 changes: 3 additions & 3 deletions app/common/Triggers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export interface Webhook {
export interface WebhookFields {
url: string;
authorization?: string;
eventTypes: Array<"add"|"update">;
eventTypes: Array<"add"|"update"|"remove">;
tableId: string;
watchedColIds?: string[];
enabled?: boolean;
Expand All @@ -28,7 +28,7 @@ export type WebhookStatus = 'idle'|'sending'|'retrying'|'postponed'|'error'|'inv
export interface WebhookSubscribe {
url: string;
authorization?: string;
eventTypes: Array<"add"|"update">;
eventTypes: Array<"add"|"update"|"remove">;
watchedColIds?: string[];
enabled?: boolean;
isReadyColumn?: string|null;
Expand Down Expand Up @@ -68,7 +68,7 @@ export interface WebhookUpdate {
export interface WebhookPatch {
url?: string;
authorization?: string;
eventTypes?: Array<"add"|"update">;
eventTypes?: Array<"add"|"update"|"remove">;
tableId?: string;
watchedColIds?: string[];
enabled?: boolean;
Expand Down
48 changes: 38 additions & 10 deletions app/server/lib/Triggers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ interface WebHookEvent {
id: string;
}

export const allowedEventTypes = StringUnion("add", "update");
export const allowedEventTypes = StringUnion("add", "update", "remove");

type EventType = typeof allowedEventTypes.type;

Expand Down Expand Up @@ -485,9 +485,8 @@ export class DocTriggers {
tableDelta.addRows.forEach(id =>
recordDeltas.set(id, {existedBefore: false, existedAfter: true}));

// If we allow subscribing to deletion in the future
// delta.removeRows.forEach(id =>
// recordDeltas.set(id, {existedBefore: true, existedAfter: false}));
tableDelta.removeRows.forEach(id =>
recordDeltas.set(id, {existedBefore: true, existedAfter: false}));

return recordDeltas;
}
Expand All @@ -497,6 +496,9 @@ export class DocTriggers {
tableDataAction: TableDataAction,
) {
const bulkColValues = fromTableDataAction(tableDataAction);
// HACK: bulkColValues don't include removed data because the data no longer exist on the Database
// but tableDataAction is got from Database query
this._appendRemovedRowsIntoTableColValues(tableDelta, bulkColValues);
Copy link
Collaborator

@fflorent fflorent Sep 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand your intent, you would like the remove event to give the data of the deleted record, right?

I am not sure this would be necessary.
In comparison, DELETE operations in REST normally don't pass any body to the request (not allowed by the browsers), but the ID through the request path.

As you are in the context of the webhook, I would just pass the ID (if I remember correctly in the body) and nothing else.

What do you think?

EDIT: Also probably worth to take a look at what other products' webhooks do, to confirm or contradict what I say above.

Copy link

@chobeat chobeat Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello. I've just casually stumbled into this PR out of curiosity because I would need it for my own use case. For what I need, the body would be really useful and the comparison with REST doesnt' stand, I think.

In REST, you can request the resource to know the content and then deleted it. With this trigger, the resource will be gone once you send the ID to the webhook and you don't know what was there.

In my specific case, I use an email as a primary key throughout the system and if a row is deleted in grist, I would need to delete it elsewhere in the system. The webhook would be very useful, but it should send me the email to know what to do with this info. Only the ID would be meaningless.

I hope this comment was useful and not off-topic.

Copy link
Contributor Author

@mrdev023 mrdev023 Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fflorent Sorry for the late reply, from my memory, i don't have access to the ID too but maybe i recheck Friday to confirm that.

@chobeat No, your comment is welcome.

@fflorent What do you think about the comment of @chobeat ?


const meta = {numTriggers: triggers.length, numRecords: bulkColValues.id.length};
this._log(`Processing triggers`, meta);
Expand Down Expand Up @@ -632,12 +634,12 @@ export class DocTriggers {
} else {
return false;
}
// If we allow subscribing to deletion in the future
// if (recordDelta.existedAfter) {
// eventType = "update";
// } else {
// eventType = "remove";
// }

if (recordDelta.existedAfter) {
eventType = "update";
} else {
eventType = "remove";
}
} else {
eventType = "add";
}
Expand Down Expand Up @@ -876,6 +878,32 @@ export class DocTriggers {
}
return false;
}

/**
* Use this to append removed rows in TableColValues data from current tableDelta.
* This method modify bulkColValues parameter directly.
*/
private _appendRemovedRowsIntoTableColValues(tableDelta: TableDelta, bulkColValues: TableColValues) {
for (const removedRow of tableDelta.removeRows) {
for (const key in bulkColValues) {
if (key === "id") {
bulkColValues.id.push(removedRow);
} else {
const columnDelta = tableDelta.columnDeltas[key];
const cellDelta = columnDelta[removedRow];
const [previousValue, ] = cellDelta;

// previousValue is always include on a array for typescript type workaround
// See CellDelta type declaration comment
if (previousValue instanceof Array) {
bulkColValues[key].push(previousValue[0]);
} else {
bulkColValues[key].push(previousValue);
}
}
}
}
}
}

export function isUrlAllowed(urlString: string) {
Expand Down
16 changes: 8 additions & 8 deletions test/server/lib/DocApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3914,7 +3914,7 @@ function testDocApi(settings: {
await oldSubscribeCheck({eventTypes: 0}, 400, /url is missing/, /eventTypes is not an array/);
await oldSubscribeCheck({eventTypes: []}, 400, /url is missing/);
await oldSubscribeCheck({eventTypes: [], url: "https://example.com"}, 400, /eventTypes must be a non-empty array/);
await oldSubscribeCheck({eventTypes: ["foo"], url: "https://example.com"}, 400, /eventTypes\[0] is none of "add", "update"/);
await oldSubscribeCheck({eventTypes: ["foo"], url: "https://example.com"}, 400, /eventTypes\[0] is none of "add", "update", "remove"/);
await oldSubscribeCheck({eventTypes: ["add"]}, 400, /url is missing/);
await oldSubscribeCheck({eventTypes: ["add"], url: "https://evil.com"}, 403, /Provided url is forbidden/);
await oldSubscribeCheck({eventTypes: ["add"], url: "http://example.com"}, 403, /Provided url is forbidden/); // not https
Expand All @@ -3935,7 +3935,7 @@ function testDocApi(settings: {
400, /eventTypes must be a non-empty array/);
await postWebhookCheck({webhooks:[{fields: {tableId: "Table1", eventTypes: ["foo"],
url: "https://example.com"}}]},
400, /eventTypes\[0] is none of "add", "update"/);
400, /eventTypes\[0] is none of "add", "update", "remove"/);
await postWebhookCheck({webhooks:[{fields: {tableId: "Table1", eventTypes: ["add"]}}]},
400, /url is missing/);
await postWebhookCheck({webhooks:[{fields: {tableId: "Table1", eventTypes: ["add"],
Expand Down Expand Up @@ -4411,7 +4411,7 @@ function testDocApi(settings: {
const {data, status} = await axios.post(
`${serverUrl}/api/docs/${docId}/tables/${options?.tableId ?? 'Table1'}/_subscribe`,
{
eventTypes: options?.eventTypes ?? ['add', 'update'],
eventTypes: options?.eventTypes ?? ['add', 'update', 'remove'],
url: `${serving.url}/${endpoint}`,
isReadyColumn: options?.isReadyColumn === undefined ? 'B' : options?.isReadyColumn,
...pick(options, 'name', 'memo', 'enabled', 'watchedColIds'),
Expand Down Expand Up @@ -4993,7 +4993,7 @@ function testDocApi(settings: {

// Webhook with only one watchedColId.
const webhook1 = await autoSubscribe('200', docId, {
watchedColIds: ['A'], eventTypes: ['add', 'update']
watchedColIds: ['A'], eventTypes: ['add', 'update', 'remove']
});
successCalled.reset();
// Create record, that will call the webhook.
Expand Down Expand Up @@ -5046,7 +5046,7 @@ function testDocApi(settings: {
url: `${serving.url}/200`,
authorization: '',
unsubscribeKey: first.unsubscribeKey,
eventTypes: ['add', 'update'],
eventTypes: ['add', 'update', 'remove'],
enabled: true,
isReadyColumn: 'B',
tableId: 'Table1',
Expand All @@ -5065,7 +5065,7 @@ function testDocApi(settings: {
url: `${serving.url}/404`,
authorization: '',
unsubscribeKey: second.unsubscribeKey,
eventTypes: ['add', 'update'],
eventTypes: ['add', 'update', 'remove'],
enabled: true,
isReadyColumn: 'B',
tableId: 'Table1',
Expand Down Expand Up @@ -5511,9 +5511,9 @@ function testDocApi(settings: {
await check({tableId: 'Santa'}, 404, `Table not found "Santa"`);
await check({tableId: 'Table2', isReadyColumn: 'Foo', watchedColIds: []}, 200);

await check({eventTypes: ['add', 'update']}, 200);
await check({eventTypes: ['add', 'update', 'remove']}, 200);
await check({eventTypes: []}, 400, "eventTypes must be a non-empty array");
await check({eventTypes: ["foo"]}, 400, /eventTypes\[0] is none of "add", "update"/);
await check({eventTypes: ["foo"]}, 400, /eventTypes\[0] is none of "add", "update", "remove"/);

await check({isReadyColumn: null}, 200);
await check({isReadyColumn: "bar"}, 404, `Column not found "bar"`);
Expand Down
2 changes: 1 addition & 1 deletion test/server/lib/Webhooks-Proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe('Webhooks-Proxy', function () {
const {data, status} = await axios.post(
`${serverUrl}/api/docs/${docId}/tables/${options?.tableId ?? 'Table1'}/_subscribe`,
{
eventTypes: options?.eventTypes ?? ['add', 'update'],
eventTypes: options?.eventTypes ?? ['add', 'update', 'remove'],
url: `${serving.url}/${endpoint}`,
isReadyColumn: options?.isReadyColumn === undefined ? 'B' : options?.isReadyColumn
}, chimpy
Expand Down
Loading