Skip to content
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.

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

Webhook trigger update by column #832

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
55bde5f
show column settings in webhooks and consolelog to know how to take i…
CamilleLegeron Jan 25, 2024
6403d7a
Merge branch 'main' into webhook-trigger-update-by-column
CamilleLegeron Jan 25, 2024
f497288
remove text typo
CamilleLegeron Jan 25, 2024
0f0dda3
add columnsIds param in webhook register in db
CamilleLegeron Feb 1, 2024
f736afc
take in account the columns to check before trigger action
CamilleLegeron Feb 1, 2024
d4d6fe8
remove columnIds if change tableID
CamilleLegeron Feb 1, 2024
62836eb
webhook translatable
CamilleLegeron Feb 2, 2024
3797649
add test for columnIds in webhooks
CamilleLegeron Feb 5, 2024
5819a57
delete samples bd
CamilleLegeron Feb 5, 2024
fd0da6d
add ignore eslint error
CamilleLegeron Feb 6, 2024
4c813e0
correct tests for webhookPage
CamilleLegeron Feb 6, 2024
dbc8498
add a delay before testing
CamilleLegeron Feb 6, 2024
8a878a5
Merge branch 'main' into webhook-trigger-update-by-column
CamilleLegeron Feb 7, 2024
706c8f7
fix forgotten log + typo + simplify if statement
CamilleLegeron Feb 14, 2024
9d802e0
update Triggers-ti.ts for APIs body validator
CamilleLegeron Feb 14, 2024
8a75bb9
add forgotten translation
CamilleLegeron Feb 14, 2024
3f5ebb3
Add test + fix to ignore empty columnId string
CamilleLegeron Feb 21, 2024
bd38d9b
delete accidently added samples
CamilleLegeron Feb 21, 2024
1a71169
rename migration 43 to 42
CamilleLegeron Feb 21, 2024
157adf7
Merge branch 'main' into webhook-trigger-update-by-column
CamilleLegeron Feb 21, 2024
24c1870
make test clearer
CamilleLegeron Feb 26, 2024
2a18e15
Merge branch 'main' into webhook-trigger-update-by-column
CamilleLegeron Mar 21, 2024
22ad13a
rename label for webhook columnIds
CamilleLegeron Mar 25, 2024
3226372
rename columnRefList to watchedColRefList
CamilleLegeron Mar 25, 2024
cca6541
remove white spaces and bring together comment and function
CamilleLegeron Mar 25, 2024
c73e695
isolate watchedColIds string in frontend
CamilleLegeron Mar 25, 2024
c5dfa4a
Merge branch 'main' into webhook-trigger-update-by-column
CamilleLegeron Mar 25, 2024
35e5b2e
rename columnIds
CamilleLegeron Mar 25, 2024
b19d169
adapt WebhookPage test
CamilleLegeron Mar 25, 2024
f6a9199
rename UIWebhookSummary type
CamilleLegeron Mar 25, 2024
f2e8ee8
add webhookPage test
CamilleLegeron Mar 25, 2024
b20d459
make watchedColIds optional
CamilleLegeron Mar 26, 2024
b4ab5ee
add translation
CamilleLegeron Mar 26, 2024
410dd28
add generic options in _grist_triggers table
CamilleLegeron Mar 26, 2024
e62a844
add migration
CamilleLegeron Mar 27, 2024
1aeca1b
remove migration
CamilleLegeron Mar 27, 2024
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
67 changes: 46 additions & 21 deletions app/client/ui/WebhookPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,33 +37,39 @@ const WEBHOOK_COLUMNS = [
id: 'vt_webhook_fc1',
colId: 'tableId',
type: 'Choice',
label: 'Table',
label: t('Table'),
// widgetOptions are configured later, since the choices depend
// on the user tables in the document.
},
{
id: 'vt_webhook_fc2',
colId: 'url',
type: 'Text',
label: 'URL',
label: t('URL'),
CamilleLegeron marked this conversation as resolved.
Show resolved Hide resolved
},
{
id: 'vt_webhook_fc3',
colId: 'eventTypes',
type: 'ChoiceList',
label: 'Event Types',
label: t('Event Types'),
widgetOptions: JSON.stringify({
widget: 'TextBox',
alignment: 'left',
choices: ['add', 'update'],
choiceOptions: {},
}),
},
{
id: 'vt_webhook_fc10',
colId: 'watchedColIdsText',
type: 'Text',
label: t('Filter for changes in these columns (semicolon-separated ids)'),
},
{
id: 'vt_webhook_fc4',
colId: 'enabled',
type: 'Bool',
label: 'Enabled',
label: t('Enabled'),
widgetOptions: JSON.stringify({
widget: 'Switch',
}),
Expand All @@ -72,31 +78,31 @@ const WEBHOOK_COLUMNS = [
id: 'vt_webhook_fc5',
colId: 'isReadyColumn',
type: 'Text',
label: 'Ready Column',
label: t('Ready Column'),
},
{
id: 'vt_webhook_fc6',
colId: 'webhookId',
type: 'Text',
label: 'Webhook Id',
label: t('Webhook Id'),
},
{
id: 'vt_webhook_fc7',
colId: 'name',
type: 'Text',
label: 'Name',
label: t('Name'),
},
{
id: 'vt_webhook_fc8',
colId: 'memo',
type: 'Text',
label: 'Memo',
label: t('Memo'),
},
{
id: 'vt_webhook_fc9',
colId: 'status',
type: 'Text',
label: 'Status',
label: t('Status'),
},
] as const;

Expand All @@ -107,8 +113,8 @@ const WEBHOOK_VIEW_FIELDS: Array<(typeof WEBHOOK_COLUMNS)[number]['colId']> = [
'name', 'memo',
'eventTypes', 'url',
'tableId', 'isReadyColumn',
'webhookId', 'enabled',
'status'
'watchedColIdsText', 'webhookId',
'enabled', 'status'
];

/**
Expand All @@ -127,9 +133,9 @@ class WebhookExternalTable implements IExternalTable {
public name = 'GristHidden_WebhookTable';
public initialActions = _prepareWebhookInitialActions(this.name);
public saveableFields = [
'tableId', 'url', 'eventTypes', 'enabled', 'name', 'memo', 'isReadyColumn',
'tableId', 'watchedColIdsText', 'url', 'eventTypes', 'enabled', 'name', 'memo', 'isReadyColumn',
];
public webhooks: ObservableArray<WebhookSummary> = observableArray<WebhookSummary>([]);
public webhooks: ObservableArray<UIWebhookSummary> = observableArray<UIWebhookSummary>([]);

public constructor(private _docApi: DocAPI) {
}
Expand All @@ -151,7 +157,7 @@ class WebhookExternalTable implements IExternalTable {
}
const colIds = new Set(getColIdsFromDocAction(d) || []);
if (colIds.has('webhookId') || colIds.has('status')) {
throw new Error(`Sorry, not all fields can be edited.`);
throw new Error(t(`Sorry, not all fields can be edited.`));
}
}
}
Expand All @@ -162,7 +168,7 @@ class WebhookExternalTable implements IExternalTable {
continue;
}
await this._removeWebhook(rec);
reportMessage(`Removed webhook.`);
reportMessage(t(`Removed webhook.`));
}
const updates = new Set(delta.updateRows);
const t2 = editor;
Expand Down Expand Up @@ -227,6 +233,7 @@ class WebhookExternalTable implements IExternalTable {
for (const webhook of webhooks) {
const values = _mapWebhookValues(webhook);
const rowId = rowMap.get(webhook.id);

if (rowId) {
toRemove.delete(rowId);
actions.push(
Expand Down Expand Up @@ -269,7 +276,12 @@ class WebhookExternalTable implements IExternalTable {
private _initalizeWebhookList(webhooks: WebhookSummary[]){

this.webhooks.removeAll();
this.webhooks.push(...webhooks);
this.webhooks.push(
...webhooks.map(webhook => {
const uiWebhook: UIWebhookSummary = {...webhook};
uiWebhook.fields.watchedColIdsText = webhook.fields.watchedColIds ? webhook.fields.watchedColIds.join(";") : "";
return uiWebhook;
}));
}

private _getErrorString(e: ApiError): string {
Expand Down Expand Up @@ -308,6 +320,9 @@ class WebhookExternalTable implements IExternalTable {
if (fields.eventTypes) {
fields.eventTypes = without(fields.eventTypes, 'L');
}
fields.watchedColIds = fields.watchedColIdsText
? fields.watchedColIdsText.split(";").filter((colId: string) => colId.trim() !== "")
: [];
return fields;
}
}
Expand Down Expand Up @@ -355,12 +370,12 @@ export class WebhookPage extends DisposableWithEvents {

public async reset() {
await this.docApi.flushWebhooks();
reportSuccess('Cleared webhook queue.');
reportSuccess(t('Cleared webhook queue.'));
}

public async resetSelected(id: string) {
await this.docApi.flushWebhook(id);
reportSuccess(`Cleared webhook ${id} queue.`);
reportSuccess(t(`Cleared webhook ${id} queue.`));
}
}

Expand Down Expand Up @@ -440,23 +455,33 @@ function _prepareWebhookInitialActions(tableId: string): DocAction[] {
/**
* Map a webhook summary to a webhook table raw record. The main
* difference is that `eventTypes` is tweaked to be in a cell format,
* and `status` is converted to a string.
* `status` is converted to a string,
* and `watchedColIdsText` is converted to list in a cell format.
*/
function _mapWebhookValues(webhookSummary: WebhookSummary): Partial<WebhookSchemaType> {
function _mapWebhookValues(webhookSummary: UIWebhookSummary): Partial<WebhookSchemaType> {
const fields = webhookSummary.fields;
const {eventTypes} = fields;
const {eventTypes, watchedColIdsText} = fields;
const watchedColIds = watchedColIdsText
? watchedColIdsText.split(";").filter(colId => colId.trim() !== "")
: [];
return {
...fields,
webhookId: webhookSummary.id,
status: JSON.stringify(webhookSummary.usage),
eventTypes: [GristObjCode.List, ...eventTypes],
watchedColIds: [GristObjCode.List, ...watchedColIds],
};
}

type WebhookSchemaType = {
[prop in keyof WebhookSummary['fields']]: WebhookSummary['fields'][prop]
} & {
eventTypes: [GristObjCode, ...unknown[]];
watchedColIds: [GristObjCode, ...unknown[]];
status: string;
webhookId: string;
}

type UIWebhookSummary = WebhookSummary & {
fields: {watchedColIdsText?: string;}
}
4 changes: 4 additions & 0 deletions app/common/Triggers-ti.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const WebhookFields = t.iface([], {
"url": "string",
"eventTypes": t.array(t.union(t.lit("add"), t.lit("update"))),
"tableId": "string",
"watchedColIds": t.opt(t.array("string")),
"enabled": t.opt("boolean"),
"isReadyColumn": t.opt(t.union("string", "null")),
"name": t.opt("string"),
Expand All @@ -29,6 +30,7 @@ export const WebhookStatus = t.union(t.lit('idle'), t.lit('sending'), t.lit('ret
export const WebhookSubscribe = t.iface([], {
"url": "string",
"eventTypes": t.array(t.union(t.lit("add"), t.lit("update"))),
"watchedColIds": t.opt(t.array("string")),
"enabled": t.opt("boolean"),
"isReadyColumn": t.opt(t.union("string", "null")),
"name": t.opt("string"),
Expand All @@ -47,6 +49,7 @@ export const WebhookSummary = t.iface([], {
"eventTypes": t.array("string"),
"isReadyColumn": t.union("string", "null"),
"tableId": "string",
"watchedColIds": t.opt(t.array("string")),
"enabled": "boolean",
"name": "string",
"memo": "string",
Expand All @@ -63,6 +66,7 @@ export const WebhookPatch = t.iface([], {
"url": t.opt("string"),
"eventTypes": t.opt(t.array(t.union(t.lit("add"), t.lit("update")))),
"tableId": t.opt("string"),
"watchedColIds": t.opt(t.array("string")),
"enabled": t.opt("boolean"),
"isReadyColumn": t.opt(t.union("string", "null")),
"name": t.opt("string"),
Expand Down
4 changes: 4 additions & 0 deletions app/common/Triggers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface WebhookFields {
url: string;
eventTypes: Array<"add"|"update">;
tableId: string;
watchedColIds?: string[];
enabled?: boolean;
isReadyColumn?: string|null;
name?: string;
Expand All @@ -26,6 +27,7 @@ export type WebhookStatus = 'idle'|'sending'|'retrying'|'postponed'|'error'|'inv
export interface WebhookSubscribe {
url: string;
eventTypes: Array<"add"|"update">;
watchedColIds?: string[];
enabled?: boolean;
isReadyColumn?: string|null;
name?: string;
Expand All @@ -44,6 +46,7 @@ export interface WebhookSummary {
eventTypes: string[];
isReadyColumn: string|null;
tableId: string;
watchedColIds?: string[];
enabled: boolean;
name: string;
memo: string;
Expand All @@ -63,6 +66,7 @@ export interface WebhookPatch {
url?: string;
eventTypes?: Array<"add"|"update">;
tableId?: string;
watchedColIds?: string[];
enabled?: boolean;
isReadyColumn?: string|null;
name?: string;
Expand Down
6 changes: 5 additions & 1 deletion app/common/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { GristObjCode } from "app/plugin/GristData";

// tslint:disable:object-literal-key-quotes

export const SCHEMA_VERSION = 41;
export const SCHEMA_VERSION = 42;

export const schema = {

Expand Down Expand Up @@ -167,6 +167,8 @@ export const schema = {
label : "Text",
memo : "Text",
enabled : "Bool",
watchedColRefList : "RefList:_grist_Tables_column",
options : "Text",
},

"_grist_ACLRules": {
Expand Down Expand Up @@ -388,6 +390,8 @@ export interface SchemaTypes {
label: string;
memo: string;
enabled: boolean;
watchedColRefList: [GristObjCode.List, ...number[]]|null;
options: string;
};

"_grist_ACLRules": {
Expand Down
20 changes: 18 additions & 2 deletions app/server/lib/DocApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ export class DocWorkerApi {
const tablesTable = activeDoc.docData!.getMetaTable("_grist_Tables");
const trigger = webhookId ? activeDoc.triggers.getWebhookTriggerRecord(webhookId) : undefined;
let currentTableId = trigger ? tablesTable.getValue(trigger.tableRef, 'tableId')! : undefined;
const {url, eventTypes, isReadyColumn, name} = webhook;
const {url, eventTypes, watchedColIds, isReadyColumn, name} = webhook;
const tableId = await getRealTableId(req.params.tableId || webhook.tableId, {metaTables});

const fields: Partial<SchemaTypes['_grist_Triggers']> = {};
Expand All @@ -397,6 +397,23 @@ export class DocWorkerApi {
}

if (tableId !== undefined) {
if (watchedColIds) {
if (tableId !== currentTableId && currentTableId) {
// if the tableId changed, we need to reset the watchedColIds
fields.watchedColRefList = [GristObjCode.List];
} else {
if (!tableId) {
throw new ApiError(`Cannot find columns "${watchedColIds}" because table is not known`, 404);
}
fields.watchedColRefList = [GristObjCode.List, ...watchedColIds
.filter(colId => colId.trim() !== "")
.map(
colId => { return colIdToReference(metaTables, tableId, colId.trim().replace(/^\$/, '')); }
)];
}
} else {
fields.watchedColRefList = [GristObjCode.List];
}
fields.tableRef = tableIdToRef(metaTables, tableId);
currentTableId = tableId;
}
Expand Down Expand Up @@ -898,7 +915,6 @@ export class DocWorkerApi {
const docId = activeDoc.docName;
const webhookId = req.params.webhookId;
const {fields, url} = await getWebhookSettings(activeDoc, req, webhookId, req.body);

if (fields.enabled === false) {
await activeDoc.triggers.clearSingleWebhookQueue(webhookId);
}
Expand Down
31 changes: 30 additions & 1 deletion app/server/lib/Triggers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ export class DocTriggers {
// Webhook might have been deleted in the mean time.
continue;
}
const decodedWatchedColRefList = decodeObject(t.watchedColRefList) as number[] || [];
// Report some basic info and usage stats.
const entry: WebhookSummary = {
// Id of the webhook
Expand All @@ -288,6 +289,7 @@ export class DocTriggers {
// Other fields used to register this webhook.
eventTypes: decodeObject(t.eventTypes) as string[],
isReadyColumn: getColId(t.isReadyColRef) ?? null,
watchedColIds: decodedWatchedColRefList.map((columnRef) => getColId(columnRef)),
tableId: getTableId(t.tableRef) ?? null,
// For future use - for now every webhook is enabled.
enabled: t.enabled,
Expand Down Expand Up @@ -509,6 +511,21 @@ export class DocTriggers {
}
}

if (trigger.watchedColRefList) {
for (const colRef of trigger.watchedColRefList.slice(1)) {
if (!this._validateColId(colRef as number, trigger.tableRef)) {
// column does not belong to table, let's ignore trigger and log stats
for (const action of webhookActions) {
const colId = this._getColId(colRef as number); // no validation
const tableId = this._getTableId(trigger.tableRef);
const error = `column is not valid: colId ${colId} does not belong to ${tableId}`;
this._stats.logInvalid(action.id, error).catch(e => log.error("Webhook stats failed to log", e));
}
continue;
}
}
}

// TODO: would be worth checking that the trigger's fields are valid (ie: eventTypes, url,
// ...) as there's no guarantee that they are.

Expand Down Expand Up @@ -585,9 +602,21 @@ export class DocTriggers {
}
}

const colIdsToCheck: Array<string> = [];
if (trigger.watchedColRefList) {
for (const colRef of trigger.watchedColRefList.slice(1)) {
colIdsToCheck.push(this._getColId(colRef as number)!);
}
}

let eventType: EventType;
if (readyBefore) {
eventType = "update";
// check if any of the columns to check were changed to consider this an update
if (colIdsToCheck.length === 0 || colIdsToCheck.some(colId => tableDelta.columnDeltas[colId]?.[rowId])) {
eventType = "update";
} else {
return false;
}
// If we allow subscribing to deletion in the future
// if (recordDelta.existedAfter) {
// eventType = "update";
Expand Down
Loading
Loading