Skip to content

feat: dispatching AnnouncementPublished event - #1158

Open
madbob wants to merge 1 commit into
nextcloud:mainfrom
madbob:issue_1050
Open

feat: dispatching AnnouncementPublished event#1158
madbob wants to merge 1 commit into
nextcloud:mainfrom
madbob:issue_1050

Conversation

@madbob

@madbob madbob commented Sep 3, 2026

Copy link
Copy Markdown

Added an Event to notify other apps (both local than remote, using webhooks) about announcement publishing.

Ref #1050 (as the event can be listened by the mohamedsakhri/nextcloud-announcementbanner app and display the attached announcement as preferred).

Comment thread lib/NotificationQueueJob.php Outdated
@madbob
madbob force-pushed the issue_1050 branch 2 times, most recently from 580c1d6 to f25f0e9 Compare September 3, 2026 15:36
}

#[\Override]
public function getWebhookSerializable(): array {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer if it's only this instead of exposing an internal entity. Sorry didn't see that before.

So I would have the constructor of the event take those strings and integers instead and not have a getAnnouncement method

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Initially I had the same idea, then I saw other apps (e.g. in Mail) just dispatching the whole involved Entity.

Anyway, I would suggest to keep IWebhookCompatibleEvent implementation separated by the behavior of the event. Maybe, something like

public function getData() {
  return $this->serialize();
}

public function getWebhookSerializable() {
  return $this->serialize();
}

private function serialize() {
  return [
    'subject' => $this->announcement->getsubJect(),
    etc...
  ];
}

Just to avoid breaking all Listeners in the unlikely event IWebhookCompatibleEvent interface will change.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

(And I just realized I mistyped all entity's getters method... Not my best performance, today...)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or simply:

public function getWebhookSerializable() {
  return [
    'subject' => $this->announcement->getSubject(),
    etc...
  ];
}

Ne need to duplicate a getData method

Signed-off-by: Roberto Guido <info@madbob.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants