Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Notifier] [BlueSky] Change the value returned as the message ID#59742
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
alexandre-daubois commentedFeb 10, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This snippet can be added to publicfunctiontestUriIsSetAsMessageId(){$client =newMockHttpClient(function () {returnnewJsonMockResponse(['uri' =>'https://example.com','cid' =>'my_cid', ]); });$transport =self::createTransport($client);$message =$transport->send(newChatMessage('Hello!'));$this->assertSame('https://example.com',$message->getMessageId());} |
Alex, thanks a lot for creating the test for this 🙏 I only changed the values of the uri/cid to make them a bit more realistic because the BlueSky URI uses an uncommon format and it's better to test that too. Thanks. |
| @@ -5,6 +5,7 @@ CHANGELOG | |||
| --- | |||
| * Add option to attach a media | |||
| * [BC Break] Change the returned message ID from record's 'cid' to 'uri' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should be reverted as changelogs are auto-updated for bugfixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Only the main CHANGELOG. Here, we want to signify something unusual, I think it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fine for me as well, but I think it's worth noticing that we had similar discussions in the past and the conclusion was it's either a bugfix or a BC break, not both.
Thank you@javiereguiluz. |
6262b65 intosymfony:7.2Uh oh!
There was an error while loading.Please reload this page.
…al info (javiereguiluz)This PR was squashed before being merged into the 7.3 branch.Discussion----------[Notifier] [Bluesky] Return the record CID as additional info| Q | A| ------------- | ---| Branch? | 7.3| Bug fix? | no| New feature? | yes| Deprecations? | no| Issues | -| License | MITSome Bluesky API operations (like replies and quotes) require "strong references", which is the combination of the `uri` and the `cid` (Seehttps://docs.bsky.app/docs/advanced-guides/posts#replies-quote-posts-and-embeds).After the changes made in#59742, we should also return the `cid` as additional info.Commits-------85a073e [Notifier] [Bluesky] Return the record CID as additional info
Some BlueSky API actions require both the
cidand theuri(seehttps://docs.bsky.app/docs/tutorials/like-repost) so we might re-add thecidto SentMessage'sinfoin Symfony 7.3.