Skip to content
9 changes: 9 additions & 0 deletions workspaces/announcements/.changeset/shy-hairs-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@backstage-community/plugin-announcements-backend': minor
'@backstage-community/plugin-announcements': minor
---

Fixed handling of `active` prop in NewAnnouncementBanner
Extended signal and notification on update when the annoucencement is activated
Updated `EditAnnoucementPage` to navigate to root path as the announcement creation page
Updated `EditAnnoucementPage` alertApi on success to be transient
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('createRouter', () => {
category: undefined,
max: undefined,
offset: undefined,
active: undefined,
active: false,
sortBy: 'created_at', // Default sortBy
order: 'desc', // Default order
current: undefined,
Expand Down Expand Up @@ -161,7 +161,7 @@ describe('createRouter', () => {
category: undefined,
max: undefined,
offset: undefined,
active: undefined,
active: false,
sortBy: 'created_at',
order: 'asc',
});
Expand Down Expand Up @@ -214,7 +214,7 @@ describe('createRouter', () => {
category: undefined,
max: undefined,
offset: undefined,
active: undefined,
active: false,
sortBy: 'created_at',
order: 'desc',
current: undefined,
Expand Down Expand Up @@ -255,7 +255,7 @@ describe('createRouter', () => {
category: undefined,
max: undefined,
offset: undefined,
active: undefined,
active: false,
sortBy: 'created_at',
order: 'desc',
current: undefined,
Expand Down Expand Up @@ -284,7 +284,7 @@ describe('createRouter', () => {
category: undefined,
max: undefined,
offset: undefined,
active: undefined,
active: false,
sortBy: 'created_at',
order: 'desc',
current: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type GetAnnouncementsQueryParams = {
category?: string;
page?: number;
max?: number;
active?: boolean;
active?: string;
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to keep it as a boolean

Suggested change
active?: string;
active?: boolean;

Copy link
Contributor Author

@matdtr matdtr Sep 22, 2025

Choose a reason for hiding this comment

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

Hi @gaelgoth, i do agree but on the GET path the backend receives the value as string that’s why the switch of type.

if kept as boolean it would fail as reported here #5235.

Let me know how you would like to address it, if revert the type or fix the tests.

PS.
Sorry maintainers not sure why all you got tagged in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Aaah, I didn’t realize it was the endpoint query, which is always a string with express. That makes sense, let’s treat it as a string for the "external" type contract in the http query and update the tests accordingly.

I think the rebase ended up modifying some existing commits. Can you please fix this. Or totally fine to just spin up a fresh PR instead 👍🏾

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, created a new PR #5519. Will close this

sortby?: 'created_at' | 'start_at';
order?: 'asc' | 'desc';
current?: boolean;
Expand Down Expand Up @@ -133,7 +133,7 @@ export async function createRouter(
category,
max,
offset: page ? (page - 1) * (max ?? 10) : undefined,
active,
active: active === 'true',
sortBy: ['created_at', 'start_at'].includes(sortby)
? sortby
: 'created_at',
Expand Down Expand Up @@ -314,6 +314,15 @@ export async function createRouter(
});
}

if (!initialAnnouncement.active && active) {
await signalAnnouncement(announcement, signals);
const announcementNotificationsEnabled =
req.body?.sendNotification === true;
if (announcementNotificationsEnabled) {
await sendAnnouncementNotification(announcement, notifications);
}
}

return res.status(200).json(announcement);
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ import { EditAnnouncementPage } from './EditAnnouncementPage';

jest.mock('../AnnouncementForm');

const mockNavigate = jest.fn();
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useNavigate: () => mockNavigate,
}));

jest.mock('@backstage-community/plugin-announcements-react', () => ({
...jest.requireActual('@backstage-community/plugin-announcements-react'),
useAnnouncementsTranslation: () => ({ t: (key: string) => key }),
Expand Down Expand Up @@ -109,6 +115,7 @@ describe('EditAnnouncementPage', () => {
beforeEach(() => {
jest.restoreAllMocks();
jest.useRealTimers();
mockNavigate.mockClear();
});

it('renders loading state', async () => {
Expand Down Expand Up @@ -236,7 +243,10 @@ describe('EditAnnouncementPage', () => {
expect(alertApiMock.post).toHaveBeenCalledWith({
message: 'editAnnouncementPage.updatedMessage',
severity: 'success',
display: 'transient',
});

expect(mockNavigate).toHaveBeenCalledWith('/announcements');
});
});

Expand Down Expand Up @@ -301,7 +311,10 @@ describe('EditAnnouncementPage', () => {
expect(alertApiMock.post).toHaveBeenCalledWith({
message: 'editAnnouncementPage.updatedMessage',
severity: 'success',
display: 'transient',
});

expect(mockNavigate).toHaveBeenCalledWith('/announcements');
});
});

Expand Down Expand Up @@ -362,7 +375,10 @@ describe('EditAnnouncementPage', () => {
message:
'editAnnouncementPageupdatedMessage editAnnouncementPage.updatedMessageWithNewCategory updated-category.',
severity: 'success',
display: 'transient',
});

expect(mockNavigate).toHaveBeenCalledWith('/announcements');
});
});

Expand Down Expand Up @@ -417,6 +433,8 @@ describe('EditAnnouncementPage', () => {
}),
);
});

expect(mockNavigate).not.toHaveBeenCalled();
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Page, Header, Content, Progress } from '@backstage/core-components';
import {
alertApiRef,
useApi,
useRouteRef,
useRouteRefParams,
} from '@backstage/core-plugin-api';
import { AnnouncementForm } from '../AnnouncementForm';
Expand All @@ -31,6 +32,8 @@ import {
useCategories,
} from '@backstage-community/plugin-announcements-react';
import { Alert } from '@material-ui/lab';
import { rootRouteRef } from '../../routes';
import { useNavigate } from 'react-router-dom';

type EditAnnouncementPageProps = {
themeId: string;
Expand All @@ -47,6 +50,8 @@ export const EditAnnouncementPage = (props: EditAnnouncementPageProps) => {
);
const { categories } = useCategories();
const { t } = useAnnouncementsTranslation();
const navigate = useNavigate();
const rootPage = useRouteRef(rootRouteRef);

let title = props.title;
let content: ReactNode = <Progress />;
Expand Down Expand Up @@ -76,7 +81,12 @@ export const EditAnnouncementPage = (props: EditAnnouncementPageProps) => {
}

await announcementsApi.updateAnnouncement(id, request);
alertApi.post({ message: updateMsg, severity: 'success' });
alertApi.post({
message: updateMsg,
severity: 'success',
display: 'transient',
});
navigate(rootPage());
} catch (err) {
alertApi.post({ message: (err as Error).message, severity: 'error' });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ export const NewAnnouncementBanner = (props: NewAnnouncementBannerProps) => {
active,
current,
});

const lastSeen = announcementsApi.lastSeenDate();

const { lastSignal } = useSignal<AnnouncementSignal>(
Expand Down