Fix broken webhooks

[lunny]

Fix #29689
This PR makes the default http method of webhook as POST instead of GET

[wxiaoguang]

How does this change fix the bug? newDefaultRequest uses POST by default by a fallthrough.

[wxiaoguang]

And I can see JS error on the "add webhook" page (regression of #29211)

WebHookEditor.js:26 Uncaught TypeError: Cannot read properties of null (reading 'value')
    at updateContentType (WebHookEditor.js:26:58)
    at initCompWebHookEditor (WebHookEditor.js:29:3)
    at HTMLDocument.<anonymous> (index.js:109:24)

  const updateContentType = function () {
    const visible = document.getElementById('http_method').value === 'POST';  // <--------- HERE
    toggleElem(document.getElementById('content_type').parentNode.parentNode, visible);
  };

Maybe something is caused by the refactoring, while something is already broken before the refactoring.

Update: the JS error has been fixed by #29692

[lunny]

How does this change fix the bug? newDefaultRequest uses POST by default by a fallthrough.

I have tested in production env, it fixed. I haven't looked into the logic.

[wxiaoguang]

I think the real bug is here:

[wxiaoguang]

I have tested in production env, it fixed. I haven't looked into the logic.

I guess you only tested WeCom(WechatWork)

But the underlying bug affects almost all webhooks.

[lunny]

I have tested in production env, it fixed. I haven't looked into the logic.

I guess you only tested WeCom(WechatWork)

But the underlying bug affects almost all webhooks.

The w.HTTPMethod should be POST but now it's GET and the database field http_method is empty. Some webhooks types accept GET like feishu/lark, so they haven't been affected.

[wxiaoguang]

I have tested in production env, it fixed. I haven't looked into the logic.

I guess you only tested WeCom(WechatWork)
But the underlying bug affects almost all webhooks.

The w.HTTPMethod should be POST but now it's GET and the database field http_method is empty. Some webhooks types accept GET like feishu/lark, so they haven't been affected.

I think the logic should be checked carefully. Do all these webhooks accept "GET"? Should it default the empty HTTP method to POST (just like newDefaultRequest)? Or even ignore the HTTPMethod, always use "POST" for the "JSON body"?

[lunny]

I have tested in production env, it fixed. I haven't looked into the logic.

I guess you only tested WeCom(WechatWork)
But the underlying bug affects almost all webhooks.

The w.HTTPMethod should be POST but now it's GET and the database field http_method is empty. Some webhooks types accept GET like feishu/lark, so they haven't been affected.

I think the logic should be checked carefully. Do all these webhooks accept "GET"? Should it default the empty HTTP method to POST (just like newDefaultRequest)?

Maybe we need to fill HTTPMethod clearly for every webhookParams. At present, Gitea, matrix filled the HTTPMethod when creating.

[wxiaoguang]

But I do not see why it should use "GET" method for a "JSON body"

[silverwind]

Please don't use GET with body, unless the other end requires it. Either move params to query or make it a POST.

[lunny]

Please don't use GET with body, unless the other end requires it. Either move params to query or make it a POST.

I believe #29145 accidently make the default methods as GET, we are talking about how to fix it.

[lunny]

But I do not see why it should use "GET" method for a "JSON body"

I just pointed out that feishu/lark hasn't been affected by the POST -> GET change because they support GET. But I don't mean it should be GET.

[wxiaoguang]

But I do not see why it should use "GET" method for a "JSON body"

I just pointed out that feishu/lark hasn't been affected by the POST -> GET change because they support GET. But I don't mean it should be GET.

I was talking about #29690 (comment) : after the feishu/lark comment, it is for the Maybe we need to fill HTTPMethod clearly for every webhookParams. At present, Gitea, matrix filled the HTTPMethod when creating, my suggestion is use "POST" for "JSON body" in the "newJSONRequest" (if it is the old behavior)

[lunny]

But I do not see why it should use "GET" method for a "JSON body"

I just pointed out that feishu/lark hasn't been affected by the POST -> GET change because they support GET. But I don't mean it should be GET.

I was talking about #29690 (comment) : after the feishu/lark comment, it is for the Maybe we need to fill HTTPMethod clearly for every webhookParams. At present, Gitea, matrix filled the HTTPMethod when creating, my suggestion is use "POST" for "JSON body" in the "newJSONRequest" (if it is the old behavior)

6052ecf

[jessielw]

Can confirm that latest nightly broke discord webhooks as well