From 8adf21d71a221172235b034ff0d72b7e9fd67941 Mon Sep 17 00:00:00 2001 From: Mateusz Borowczyk Date: Thu, 14 Mar 2024 13:23:06 +0100 Subject: [PATCH] Update Notification Drawer to handle notifications without uri (Issue #5593, PR #5602) # Description * Short description here * closes #5593 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [x] Attached issue to pull request - [x] Changelog entry - [x] Code is clear and sufficiently documented - [x] Sufficient test cases (reproduces the bug/tests the requested feature) - [ ] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) --- .../5593-resolve-notification-issue.yml | 6 +++++ src/Slices/Notification/Core/Domain.ts | 2 +- src/Slices/Notification/Core/Mock.ts | 11 ++++++--- .../Notification/UI/Center/Page.test.tsx | 2 +- .../Notification/UI/Drawer/Drawer.test.tsx | 24 +++++++++++++++++-- src/Slices/Notification/UI/Drawer/Item.tsx | 2 +- 6 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/5593-resolve-notification-issue.yml diff --git a/changelogs/unreleased/5593-resolve-notification-issue.yml b/changelogs/unreleased/5593-resolve-notification-issue.yml new file mode 100644 index 000000000..fa5e0546e --- /dev/null +++ b/changelogs/unreleased/5593-resolve-notification-issue.yml @@ -0,0 +1,6 @@ +description: Update Notification Drawer to handle notifications without uri +issue-nr: 5593 +change-type: patch +destination-branches: [master, iso7] +sections: + minor-improvement: "{{description}}" diff --git a/src/Slices/Notification/Core/Domain.ts b/src/Slices/Notification/Core/Domain.ts index 9dd34a1ca..a17b1ba8f 100644 --- a/src/Slices/Notification/Core/Domain.ts +++ b/src/Slices/Notification/Core/Domain.ts @@ -16,7 +16,7 @@ export interface Notification extends Flags { title: string; message: string; severity: Severity; - uri: string; + uri: string | null; } export interface Flags { diff --git a/src/Slices/Notification/Core/Mock.ts b/src/Slices/Notification/Core/Mock.ts index 89c96f6a8..943367e93 100644 --- a/src/Slices/Notification/Core/Mock.ts +++ b/src/Slices/Notification/Core/Mock.ts @@ -26,17 +26,22 @@ export const read: Notification = { severity: "info", read: true, }; +export const withoutUri: Notification = { + ...unread, + id: "abcdefgh04", + uri: null, +}; export const response: Manifest["apiResponse"] = { - data: [unread, error, read], + data: [unread, error, read, withoutUri], metadata: Pagination.metadata, links: Pagination.links, }; export const data: Manifest["usedData"] = { - data: [unread, error, read], + data: [unread, error, read, withoutUri], metadata: Pagination.metadata, handlers: Pagination.handlers, }; -export const list: Notification[] = [unread, read, error]; +export const list: Notification[] = [unread, read, error, withoutUri]; diff --git a/src/Slices/Notification/UI/Center/Page.test.tsx b/src/Slices/Notification/UI/Center/Page.test.tsx index 3b714abb2..273a4fac6 100644 --- a/src/Slices/Notification/UI/Center/Page.test.tsx +++ b/src/Slices/Notification/UI/Center/Page.test.tsx @@ -59,7 +59,7 @@ test("Given Notification Center page Then fetches notifications", async () => { }); expect( screen.getAllByRole("listitem", { name: "NotificationItem" }), - ).toHaveLength(3); + ).toHaveLength(4); }); test("Given Notification Center page When user filters on severity Then executes correct request", async () => { diff --git a/src/Slices/Notification/UI/Drawer/Drawer.test.tsx b/src/Slices/Notification/UI/Drawer/Drawer.test.tsx index af28723ba..2fd66678b 100644 --- a/src/Slices/Notification/UI/Drawer/Drawer.test.tsx +++ b/src/Slices/Notification/UI/Drawer/Drawer.test.tsx @@ -101,7 +101,7 @@ test("Given Drawer Then a list of notifications are shown", async () => { expect( screen.getAllByRole("listitem", { name: "NotificationItem" }), - ).toHaveLength(3); + ).toHaveLength(4); }); test("Given Drawer When clicking on 'Clear all' Then all notifications are cleared", async () => { @@ -126,12 +126,14 @@ test("Given Drawer When clicking on 'Clear all' Then all notifications are clear updateRequest("abcdefgh01", { read: true, cleared: true }), updateRequest("abcdefgh02", { read: true, cleared: true }), updateRequest("abcdefgh03", { read: true, cleared: true }), + updateRequest("abcdefgh04", { read: true, cleared: true }), ]); await act(async () => { await apiHelper.resolve(Maybe.none()); await apiHelper.resolve(Maybe.none()); await apiHelper.resolve(Maybe.none()); + await apiHelper.resolve(Maybe.none()); }); expect(apiHelper.pendingRequests).toEqual([getAllRequest]); @@ -166,10 +168,12 @@ test("Given Drawer When user clicks on 'Read all' Then all notifications are rea expect(apiHelper.pendingRequests).toEqual([ updateRequest("abcdefgh01", { read: true }), updateRequest("abcdefgh02", { read: true }), + updateRequest("abcdefgh04", { read: true }), ]); await act(async () => { await apiHelper.resolve(Maybe.none()); await apiHelper.resolve(Maybe.none()); + await apiHelper.resolve(Maybe.none()); }); expect(apiHelper.pendingRequests).toEqual([getAllRequest]); @@ -182,6 +186,7 @@ test("Given Drawer When user clicks on 'Read all' Then all notifications are rea { ...Mock.unread, read: true }, { ...Mock.error, read: true }, Mock.read, + { ...Mock.withoutUri, read: true }, ], }), ); @@ -189,7 +194,7 @@ test("Given Drawer When user clicks on 'Read all' Then all notifications are rea expect( screen.getAllByRole("listitem", { name: "NotificationItem" }), - ).toHaveLength(3); + ).toHaveLength(4); }); test("Given Drawer When user clicks a notification Then it becomes read", async () => { @@ -243,6 +248,21 @@ test("Given Drawer When user clicks a notification with an uri then go to the ur ); }); +test("Given Drawer When user clicks a notification without an uri then nothing happens", async () => { + const { component, apiHelper, history } = setup(); + render(component); + await act(async () => { + await apiHelper.resolve(Either.right(Mock.response)); + }); + + const items = screen.getAllByRole("listitem", { name: "NotificationItem" }); + await act(async () => { + await userEvent.click(items[3]); + }); + + expect(history.location.pathname).toBe("/"); +}); + test("Given Drawer When user clicks a notification toggle with an uri then do not go to uri", async () => { const { component, apiHelper, history } = setup(); render(component); diff --git a/src/Slices/Notification/UI/Drawer/Item.tsx b/src/Slices/Notification/UI/Drawer/Item.tsx index 2ec2c8fb8..b53db0fd6 100644 --- a/src/Slices/Notification/UI/Drawer/Item.tsx +++ b/src/Slices/Notification/UI/Drawer/Item.tsx @@ -27,7 +27,7 @@ interface Props { export const Item: React.FC = ({ notification, onUpdate }) => { const { routeManager } = useContext(DependencyContext); const detailsLink: RouteKindWithId<"CompileDetails"> | undefined = - routeManager.getParamsFromUrl(notification.uri); + routeManager.getParamsFromUrl(notification.uri || ""); const navigate = useNavigateTo(); const onClick = (): void => {