Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Notifier] [Mercure] Add options #54961

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 21 commits into from
Closed

[Notifier] [Mercure] Add options #54961

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3e816ff
Added body, icon, tag ,renotify to MercureOptions.php for Symfony UX …
May 17, 2024
6c7428a
Edit CHANGELOG.md
May 17, 2024
7ed8d75
make tests
May 17, 2024
265f00e
make fabbot happy :)
May 17, 2024
9d05e77
Merge branch 'symfony:7.1' into NotifierMercure
ernie76 May 20, 2024
c830aed
Update src/Symfony/Component/Notifier/Bridge/Mercure/CHANGELOG.md
ernie76 May 21, 2024
651a89f
add content to MercureOptions.php for Symfony UX Notify
May 22, 2024
b5eeda6
edit unit test for MercureTransportTest.php
May 22, 2024
f5f1526
chenge __tostring in MercureTransportTest.php back
May 24, 2024
52f41ec
delete empty lines
May 24, 2024
5fc0ac5
delete empty lines
May 24, 2024
4a66651
delete private HubInterface $hub;
May 24, 2024
182f5a9
delete private string $hubId;
May 29, 2024
9095164
make __construct multiline
May 29, 2024
0c2e231
Update src/Symfony/Component/Notifier/Bridge/Mercure/Tests/MercureOpt…
ernie76 May 29, 2024
e0c1a9a
Update src/Symfony/Component/Notifier/Bridge/Mercure/Tests/MercureOpt…
ernie76 May 29, 2024
89d13ba
update CHANGELOG
May 29, 2024
ac83946
Update src/Symfony/Component/Notifier/Bridge/Mercure/MercureTransport…
ernie76 May 29, 2024
30958e8
Update src/Symfony/Component/Notifier/Bridge/Mercure/MercureTransport…
ernie76 May 29, 2024
c3554d2
update CHANGELOG
Jun 26, 2024
d27eaf2
Update src/Symfony/Component/Notifier/Bridge/Mercure/CHANGELOG.md
ernie76 Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add content to MercureOptions.php for Symfony UX Notify 
add "mediaType" => "application/json" to MercureOptions.php
  • Loading branch information
Sven Scholz committed May 22, 2024
commit 651a89ff666e6a5e09fb598ba99a44154e5c7c2b
3 changes: 2 additions & 1 deletion 3 src/Symfony/Component/Notifier/Bridge/Mercure/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
CHANGELOG
=========

* Add body, icon, tag ,renotify to MercureOptions.php and MercureTransport.php for Symfony UX Notify
* Add content to MercureOptions.php for Symfony UX Notify
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a new 7.2 section and add only

 * Add `content` option

* Add "mediaType" => "application/json" to MercureOptions.php

5.3
---
Expand Down
37 changes: 8 additions & 29 deletions 37 src/Symfony/Component/Notifier/Bridge/Mercure/MercureOptions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,19 @@ final class MercureOptions implements MessageOptionsInterface
private ?string $id;
private ?string $type;
private ?int $retry;
private ?string $body;
private ?string $icon;
private ?string $tag;
private bool $renotify;
private ?array $content;

/**
* @param string|string[]|null $topics
*/
public function __construct(string|array|null $topics = null, bool $private = false, ?string $id = null, ?string $type = null, ?int $retry = null, ?string $body = null, ?string $icon = null, ?string $tag = null, ?bool $renotify = false)
public function __construct(string|array|null $topics = null, bool $private = false, ?string $id = null, ?string $type = null, ?int $retry = null, ?array $content = null)
{
$this->topics = null !== $topics ? (array) $topics : null;
$this->private = $private;
$this->id = $id;
$this->type = $type;
$this->retry = $retry;
$this->body = $body;
$this->icon = $icon;
$this->tag = $tag;
$this->renotify = $renotify;
$this->content = $content;
}

/**
Expand Down Expand Up @@ -72,25 +66,11 @@ public function getRetry(): ?int
return $this->retry;
}

public function getBody(): ?string
public function getContent(): ?array
{
return $this->body;
return $this->content;
}

public function getIcon(): ?string
{
return $this->icon;
}

public function getTag(): ?string
{
return $this->tag;
}

public function isRenotify(): bool
{
return $this->renotify;
}

public function toArray(): array
{
Expand All @@ -100,15 +80,14 @@ public function toArray(): array
'id' => $this->id,
'type' => $this->type,
'retry' => $this->retry,
'body' => $this->body,
'icon' => $this->icon,
'tag' => $this->tag,
'renotify' => $this->renotify,
'content' => $this->content,
];
}

public function getRecipientId(): ?string
{
return null;
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

}
21 changes: 9 additions & 12 deletions 21 src/Symfony/Component/Notifier/Bridge/Mercure/MercureTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,25 @@
*/
final class MercureTransport extends AbstractTransport
{
private HubInterface $hub;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be reverted, we already had promoted properties

private string $hubId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed

private string|array $topics;

/**
* @param string|string[]|null $topics
*/
public function __construct(
private HubInterface $hub,
private string $hubId,
string|array|null $topics = null,
?HttpClientInterface $client = null,
?EventDispatcherInterface $dispatcher = null,
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep this

public function __construct(HubInterface $hub, string $hubId, string|array|null $topics = null, ?HttpClientInterface $client = null, ?EventDispatcherInterface $dispatcher = null)
{
$this->hub = $hub;
$this->hubId = $hubId;
$this->topics = $topics ?? 'https://symfony.com/notifier';

parent::__construct($client, $dispatcher);
}

public function __toString(): string
{
return sprintf('mercure://%s%s', $this->hubId, '?'.http_build_query(['topic' => $this->topics], '', '&'));
return sprintf('mercure://%s%s', $this->hubId, null !== $this->topics ? '?'.http_build_query(['topic' => $this->topics], '', '&') : '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why checking for null if constructor always sets a topic in that case?

}

public function supports(MessageInterface $message): bool
Expand Down Expand Up @@ -77,10 +76,8 @@ protected function doSend(MessageInterface $message): SentMessage
'@context' => 'https://www.w3.org/ns/activitystreams',
'type' => 'Announce',
'summary' => $message->getSubject(),
'body' => $options->getBody(),
'icon' => $options->getIcon(),
'tag' => $options->getTag(),
'renotify' => $options->isRenotify(),
"mediaType" => "application/json",
'content' => $options->getContent(),
ernie76 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@chapterjason chapterjason May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it fit correctly it should be serialized to json here. (Only the value of the content entry)

For now I think it is enough to just inject the Serializer and always encode to json. To keep this PR simple.

I would also add an test to cover this case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ernie76 What about this comment?

]), $options->isPrivate(), $options->getId(), $options->getType(), $options->getRetry());

try {
Expand Down
12 changes: 3 additions & 9 deletions 12 src/Symfony/Component/Notifier/Bridge/Mercure/Tests/MercureOptionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,21 @@ public function testConstructWithDefaults()
'id' => null,
'type' => null,
'retry' => null,
'body' => null,
'icon' => null,
'tag' => null,
'renotify' => false,
'content' => null
]);
}

public function testConstructWithParameters()
{
$options = (new MercureOptions('/topic/1', true, 'id', 'type', 1));
$options = (new MercureOptions('/topic/1', true, 'id', 'type', 1, null));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would provide a text here, rather than null


$this->assertSame($options->toArray(), [
'topics' => ['/topic/1'],
'private' => true,
'id' => 'id',
'type' => 'type',
'retry' => 1,
'body' => null,
'icon' => null,
'tag' => null,
'renotify' => false,
'content' => null
]);
}

Expand Down
6 changes: 3 additions & 3 deletions 6 src/Symfony/Component/Notifier/Bridge/Mercure/Tests/MercureTransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public function testSendWithMercureOptions()
{
$hub = new MockHub('https://foo.com/.well-known/mercure', new StaticTokenProvider('foo'), function (Update $update): string {
$this->assertSame(['/topic/1', '/topic/2'], $update->getTopics());
$this->assertSame('{"@context":"https:\/\/www.w3.org\/ns\/activitystreams","type":"Announce","summary":"subject","body":null,"icon":null,"tag":null,"renotify":false}', $update->getData());
$this->assertSame('{"@context":"https:\/\/www.w3.org\/ns\/activitystreams","type":"Announce","summary":"subject","content":null}', $update->getData());
$this->assertSame('id', $update->getId());
$this->assertSame('type', $update->getType());
$this->assertSame(1, $update->getRetry());
Expand All @@ -130,7 +130,7 @@ public function testSendWithMercureOptionsButWithoutOptionTopic()
{
$hub = new MockHub('https://foo.com/.well-known/mercure', new StaticTokenProvider('foo'), function (Update $update): string {
$this->assertSame(['https://symfony.com/notifier'], $update->getTopics());
$this->assertSame('{"@context":"https:\/\/www.w3.org\/ns\/activitystreams","type":"Announce","summary":"subject","body":null,"icon":null,"tag":null,"renotify":false}', $update->getData());
$this->assertSame('{"@context":"https:\/\/www.w3.org\/ns\/activitystreams","type":"Announce","summary":"subject","content":null}', $update->getData());
$this->assertSame('id', $update->getId());
$this->assertSame('type', $update->getType());
$this->assertSame(1, $update->getRetry());
Expand All @@ -146,7 +146,7 @@ public function testSendWithoutMercureOptions()
{
$hub = new MockHub('https://foo.com/.well-known/mercure', new StaticTokenProvider('foo'), function (Update $update): string {
$this->assertSame(['https://symfony.com/notifier'], $update->getTopics());
$this->assertSame('{"@context":"https:\/\/www.w3.org\/ns\/activitystreams","type":"Announce","summary":"subject","body":null,"icon":null,"tag":null,"renotify":false}', $update->getData());
$this->assertSame('{"@context":"https:\/\/www.w3.org\/ns\/activitystreams","type":"Announce","summary":"subject","content":null}', $update->getData());
$this->assertFalse($update->isPrivate());

return 'id';
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.