Add authorization header in webhooks stored in secrets table #941
Conversation
app/server/lib/DocApi.ts
Outdated
| if (url) { | ||
| await this._dbManager.updateWebhookUrl(webhookId, docId, url); | ||
| await this._dbManager.updateWebhookUrlAndAuth(webhookId, docId, url, authorization || ""); |
There was a problem hiding this comment.
Should authorization be set to empty string? Or should it be set to null to express its absence?
I'd suggest to remain consistant with the rest of the code and allow it to be null or undefined
Authorizarion wuth capital A and add it in header call only if there is one Co-authored-by: Florent <[email protected]>
|
@CamilleLegeron Probably worth to take a look at the failures, I see some of them are related to Webhooks. |
| @@ -4889,6 +4891,7 @@ function testDocApi() { | |||
|
|
|||
| const expectedFields = { | |||
| url: `${serving.url}/foo`, | |||
| authorization: '', | |||
There was a problem hiding this comment.
Maybe also add a specific tests to ensure that a filled authorization header is passed to the webhook consumer?
There was a problem hiding this comment.
Good remark, adding this test I find a bug when calling patch call : before when we used patch call to update URL without giving information of authorization it removed it
…d for webhooks when using patch call
|
Hi, I send you a notif @paulfitz to know if you can review this PR or maybe other person in Gristlabs |
Co-authored-by: Florent <[email protected]>
#827