feature(webhook): Add remove event type #1762
Conversation
|
@paulfitz What do you think about my workaround ? It's okay for you ? (See "Proposed solution") If is okay, i can add the unit test and set this merge request as ready |
| 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
3 participants
Context
It adds the possibility to register webhook for deleted rows.
Proposed solution
I add the event type "removed", uncomment some existing work about it.
I need to add "workaround" because TableColValues seems to be "created" from Database query.
But, it not works for deleted rows because it no longer exists on database, so I need to append the data from TableDelta for each removed row.
TableDelta contains old values of the deleted rows.
Feel free to purpose any other better solution for this.
Related issues
Has this been tested?
Screenshots / Screencasts