[Notifier] Fix thread key in GoogleChat bridge #57384
Conversation
|
Please target 7.2 branch for your PR as this is a new feature. thanks |
Hello, thanks for your reply. This actually is not a new feature, since the thread key in query parameters has no effect now on the new Google Chat API. Shouldn't this qualifies as a bug and the fix be backported to 5.4? |
|
It introduces new code to handle the new threadKey . So this should go into 7.2 with the deprecation and the deprecation layer be removed in 8.0. Someone who needs this should upgrade to 7.2 then |
|
I agree that this should go in 7.2. |
a40bbd6 to
0942a43
Compare
There was a problem hiding this comment.
Thanks for catching this deprecation in the Google API.
This PR keeps the backward compatibility regarding the threadKey declaration (in DSN) and the GoogleChatOptions::setThreadKey() method.
I think this PR does too many things. We could just modify the HTTP request, and leave the options and methods untouched.
src/Symfony/Component/Notifier/Bridge/GoogleChat/GoogleChatOptions.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/GoogleChat/GoogleChatOptions.php
Outdated
Show resolved
Hide resolved
0942a43 to
634528e
Compare
Thanks for your time reviewing this change. I thought this was a good opportunity to update the options, but I guess this made too much noise around the actual fix. |
|
Thanks for simplifying this PR. I think we can now consider it as a bugfix to guarantee compatibility with the Google API in the future and merge in 5.4 @symfony/mergers? |
src/Symfony/Component/Notifier/Bridge/GoogleChat/GoogleChatTransport.php
Outdated
Show resolved
Hide resolved
634528e to
e8b6a2d
Compare
There was a problem hiding this comment.
Works for me as a bug fix for 5.4.
@romain-jacquart Can you remove the CHANGELOG change and rebase on 5.4?
Google Chat API has deprecated the use of `threadKey` query parameter in favor of `thread.threadKey` in request's body.
e8b6a2d to
e792b16
Compare
The branch has been rebased on 5.4 and CHANGELOG changes have been removed 👍. The rebase has introduced a slight change regarding a variable's name ( Thanks! |
|
Thank you @romain-jacquart. |
Hello,
Google Chat API has deprecated the use of
threadKeyquery parameter in favor ofthread.threadKeyin request's body.In order for the thread key value to not be ignored, a new query parameter
messageReplyOptionhas been added which controls the way the key is handled.I chose not to expose the values used by this new parameter as I wanted to focus this PR on the actual fix.
To do so, I've forced the value of
messageReplyOptiontoREPLY_MESSAGE_FALLBACK_TO_NEW_THREADwhich replicates the original behavior of this bridge.