[Notifier] [Bluesky] Allow to attach website preview card #59667
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
| 'uri' => $options['external']['uri'], | ||
| 'title' => $options['external']['title'], | ||
| 'description' => $options['external']['description'], | ||
| 'thumb' => current($uploadedMedia)['image'], |
There was a problem hiding this comment.
We reuse the same method as for adding media, but we can only use one image in our case, so we take the first image that comes along.
There was a problem hiding this comment.
Can we avoid current() and take the first one instead?
| 'thumb' => current($uploadedMedia)['image'], | |
| 'thumb' => $uploadedMedia[array_key_first($uploadedMedia)]['image'], |
| @@ -274,9 +274,12 @@ public function testParseFacetsUrlWithTrickyRegex() | |||
| $this->assertEquals($expected, $this->parseFacets($input)); | |||
| } | |||
|
|
|||
| public function testWithMedia() | |||
| /** | |||
| * @dataProvider sendMessageWithEmbedDataProvider | |||
There was a problem hiding this comment.
Using dataProvider to add website card preview to previous test.
31b4cae to
de0c847
Compare
src/Symfony/Component/Notifier/Bridge/Bluesky/BlueskyTransport.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Bluesky/BlueskyTransport.php
Outdated
Show resolved
Hide resolved
1804405 to
72a2a2f
Compare
| 'uri' => $options['external']['uri'], | ||
| 'title' => $options['external']['title'], | ||
| 'description' => $options['external']['description'], | ||
| 'thumb' => current($uploadedMedia)['image'], |
There was a problem hiding this comment.
Can we avoid current() and take the first one instead?
| 'thumb' => current($uploadedMedia)['image'], | |
| 'thumb' => $uploadedMedia[array_key_first($uploadedMedia)]['image'], |
72a2a2f to
61a04ff
Compare
Hello and thanks for your positive feedback :) I actually tested the code to make sure it was OK on the Bluesky side, and the screenshot in the commentary is from the test in question. |
|
Thank you. Let's wait for another maintainer to give their review. If they also like it they will merge it. |
61a04ff to
ce977ee
Compare
| // Add website preview card to the message | ||
| $options = (new BlueskyOptions()) | ||
| ->attachCard('https://example.com', new File('image.jpg')) | ||
| // ... |
There was a problem hiding this comment.
Can you also describe attachMedia() as this the only other possible option.
There was a problem hiding this comment.
I've added the method and commented on it, as it's not possible to use both of them at the same time.
ce977ee to
15ded98
Compare
|
Thank you @ppoulpe. |
This addition will allow us to add a preview card to a link:
