announcements: integration with notifications #4378 #4816

lavanya-sainik-ericsson · 2025-07-30T14:42:04Z

Hey, I just made a Pull Request!

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)
All your commits have a Signed-off-by line in the message. (more info)

lavanya-sainik-ericsson · 2025-07-30T14:47:39Z

I have to raise this new PR for issue #4378 with all the comments addressed from @kurtaking as the old PR #4793 had merge conflicts and it was not allowing me to resolve them locally. So I have to use the UI option "Discard 1 commit" & then create this new PR again. Kindly have a look at this PR and let me know if any comments / concerns

backstage-goalie · 2025-07-30T15:04:31Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage-community/plugin-announcements-backend	workspaces/announcements/plugins/announcements-backend	minor	`v0.9.0`

04kash · 2025-07-30T20:34:09Z

Looks like you need to run yarn install and commit your yarn.lock file to fix the failing CI

backstage-goalie · 2025-07-31T09:55:37Z

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

Signed-off-by: Lavanya Sainik <[email protected]>

…and dev app (backstage#4798) Signed-off-by: Christoph Jerolimov <[email protected]> Signed-off-by: Lavanya Sainik <[email protected]>

v1.40.2 version bump --------- Signed-off-by: Saif Chaudhry <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Saif Chaudhry <[email protected]> Signed-off-by: Lavanya Sainik <[email protected]>

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: Lavanya Sainik <[email protected]>

lavanya-sainik-ericsson · 2025-07-31T10:13:53Z

Hello @04kash and All,

Thanks for your comment. I have added the yarn.lock file . Looks like other issues are also solved related to build.

Kindly have a look and let me know if any further comments or concerns.

kurtaking · 2025-08-04T14:13:54Z

workspaces/announcements/plugins/announcements-backend/src/router.ts

+      title: 'New Announcement',
+      description: 'New announcement has been added',


Should we allow the user to specify the title and description via params?

kurtaking · 2025-08-04T14:17:40Z

workspaces/announcements/plugins/announcements-backend/src/router.ts

+  const notificationsConfig =
+    announcementNotificationOpts.config.getOptionalConfig(
+      'announcements.notification',
+    );
+
+  const notifications = announcementNotificationOpts.notifications;


Non-blocking - you could consider destructuring right here

const { config, notifications } = announcementNotificationOpts

not needed now based on @gaelgoth next comment. But do revert if nay concern.

workspaces/announcements/plugins/announcements-backend/src/router.ts

gaelgoth

It would be great if you could set up Notifications on the Backstage dev instance 🙏🏾. This will help with trying/testing notifications locally.

workspaces/announcements/plugins/announcements-backend/src/router.ts

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: Lavanya Sainik <[email protected]>

lavanya-sainik-ericsson · 2025-08-06T11:04:29Z

Hello @kurtaking , @gaelgoth

Thanks a lot for your comments. Have updated the PR with the comments included. Kindly review and do let me know if any further concerns or improvements.

Also @gaelgoth , could you please guide on how to set up the Backstage dev instance for you to test as you have mentioned in the comment ?
I am happy to do that but don't know link of instance and if you have any specific instructions as we have tested in our local backstage installation. So kindly guide here .

kurtaking

Hey @lavanya-sainik-ericsson, thank you for iterating through this. It's looking great. I've got another round of review for you.

kurtaking · 2025-08-07T01:15:23Z

workspaces/announcements/plugins/announcements-backend/src/router.ts

@@ -209,6 +211,9 @@ export async function createRouter(
          tags: validatedTags,
        });

+      // Send announcement notification
+      sendAnnouncementNotification(req, announcement.id, notifications);


The notifications plugin requires events. We need to put this next to await signalAnnouncement(announcement, signals).

Also, what happens if we call both signalAnnouncement and sendAnnouncementNotification. Does that cause two separate signals to go out?

I'm wondering if they have notifications enabled, we call sendAnnouncementNotification, else if they have have signals enabled, we call signalAnnouncement, else do nothing.

kurtaking · 2025-08-07T01:18:10Z

workspaces/announcements/plugins/announcements-backend/src/service/announcementNotification.ts

+export const sendAnnouncementNotification = (
+  request: any,
+  announcementId: string,
+  notifications?: NotificationService,
+) => {
+  const reqData: AnnouncementRequest = request.body;


sendAnnouncementNotification shouldn't need to know about the request object. Notice how you now have to import from the router now. This is a circular dependency and tightly couples sending an announcement notification to a request object.

By the time you call your function, you should already have a complete announcement that you can pass in. I highly recommend aligning this as closely to the signalAnnouncements fn as possible.

kurtaking · 2025-08-07T01:18:30Z

workspaces/announcements/plugins/announcements-backend/src/router.ts


-interface AnnouncementRequest {
+export interface AnnouncementRequest {


Revert this based on this thread.

kurtaking · 2025-08-07T01:22:25Z

workspaces/announcements/plugins/announcements-backend/src/service/announcementNotification.ts

+      title: `New Announcement "${reqData.title}"`,
+      description: reqData.excerpt,
+      link: `/announcements/view/${announcementId}`,


We will also get type safety back if the function is updated to take an announcement. Right now we are blindly trusting an any.

gaelgoth · 2025-08-07T06:57:55Z

Hello @kurtaking , @gaelgoth

Thanks a lot for your comments. Have updated the PR with the comments included. Kindly review and do let me know if any further concerns or improvements.

Also @gaelgoth , could you please guide on how to set up the Backstage dev instance for you to test as you have mentioned in the comment ? I am happy to do that but don't know link of instance and if you have any specific instructions as we have tested in our local backstage installation. So kindly guide here .

Hi @lavanya-sainik-ericsson,

You can follow the installation guide here: https://backstage.io/docs/notifications/#installation

Run the yarn add commands from the announcements root workspace cd workspaces/announcements, and then add the notifications backend and frontend as described in the documentation.

The signals backend is already installed, you can skip that step.

I did try the set up on my side, so far it's look good 😄

gaelgoth

I think it would be necessary to update the documentation to mention the prerequisites for Notifications

lavanya-sainik-ericsson requested review from kurtaking, gaelgoth and a team as code owners July 30, 2025 14:42

lavanya-sainik-ericsson requested a review from 04kash July 30, 2025 14:42

lavanya-sainik-ericsson mentioned this pull request Jul 30, 2025

announcements: integration with notifications and signals #4378 #4793

Closed

5 tasks

lavanya-sainik-ericsson force-pushed the announcement_notification_improvement branch from fa2d8c6 to 5d67d89 Compare July 30, 2025 15:04

lavanya-sainik-ericsson force-pushed the announcement_notification_improvement branch from 6ff401c to f83091c Compare July 31, 2025 09:51

lavanya-sainik-ericsson requested review from caugello, CryptoRodeo, ciiay, debsmita1, JessicaJHee, sachaudh, dvail and maknop as code owners July 31, 2025 09:51

lavanya-sainik-ericsson and others added 4 commits July 31, 2025 10:57

announcements: integration with notifications backstage#4378

ae52af2

Signed-off-by: Lavanya Sainik <[email protected]>

nexus-repository-manager: remove product theme from dev dependencies …

38e1aaf

…and dev app (backstage#4798) Signed-off-by: Christoph Jerolimov <[email protected]> Signed-off-by: Lavanya Sainik <[email protected]>

Version Packages (backstage#4811)

6fd4a54

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: Lavanya Sainik <[email protected]>

lavanya-sainik-ericsson force-pushed the announcement_notification_improvement branch from f83091c to 6fd4a54 Compare July 31, 2025 10:00

Merge branch 'backstage:main' into announcement_notification_improvement

5c204e2

kurtaking reviewed Aug 4, 2025

View reviewed changes

gaelgoth reviewed Aug 5, 2025

View reviewed changes

workspaces/announcements/plugins/announcements-backend/src/router.ts Outdated Show resolved Hide resolved

workspaces/announcements/plugins/announcements-backend/src/router.ts Outdated Show resolved Hide resolved

lavanya-sainik-ericsson and others added 2 commits August 6, 2025 11:37

Merge branch 'backstage:main' into announcement_notification_improvement

6f6f3e3

Version Packages (backstage#4811)

c48ede4

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: Lavanya Sainik <[email protected]>

kurtaking reviewed Aug 7, 2025

View reviewed changes

gaelgoth reviewed Aug 7, 2025

View reviewed changes

		title: 'New Announcement',
		description: 'New announcement has been added',


		interface AnnouncementRequest {
		export interface AnnouncementRequest {

announcements: integration with notifications #4378 #4816

Are you sure you want to change the base?

announcements: integration with notifications #4378 #4816

Uh oh!

Conversation

lavanya-sainik-ericsson commented Jul 30, 2025

Hey, I just made a Pull Request!

✔️ Checklist

Uh oh!

lavanya-sainik-ericsson commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

backstage-goalie bot commented Jul 30, 2025

Changed Packages

Uh oh!

04kash commented Jul 30, 2025

Uh oh!

backstage-goalie bot commented Jul 31, 2025

Uh oh!

lavanya-sainik-ericsson commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gaelgoth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lavanya-sainik-ericsson commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kurtaking left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaelgoth commented Aug 7, 2025

Uh oh!

gaelgoth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lavanya-sainik-ericsson commented Jul 30, 2025 •

edited

Loading

lavanya-sainik-ericsson commented Jul 31, 2025 •

edited

Loading

lavanya-sainik-ericsson commented Aug 6, 2025 •

edited

Loading