From 95909ae7f4a147ab8feaf11139806e92cb2f03e0 Mon Sep 17 00:00:00 2001 From: Yassine Bounekhla Date: Tue, 4 Jun 2024 19:14:25 -0400 Subject: [PATCH] add support for access list notifications --- .../src/Navigation/NavigationItem.test.tsx | 4 +- .../src/Navigation/NavigationItem.tsx | 4 +- .../src/Notifications/Notification.story.tsx | 2 + .../src/Notifications/Notification.tsx | 92 ++++---- .../src/Notifications/Notifications.test.tsx | 122 +++++++++++ .../src/Notifications/Notifications.tsx | 201 ++++++++++++++++-- .../notificationContentFactory.tsx | 5 +- .../Notifications/Notifications.test.tsx | 127 ----------- .../TopBar/Notifications/Notifications.tsx | 175 --------------- .../src/TopBar/Notifications/index.ts | 19 -- .../teleport/src/TopBar/TopBar.story.tsx | 8 +- .../teleport/src/TopBar/TopBar.test.tsx | 76 +++++-- .../src/services/notifications/types.ts | 24 ++- .../src/services/storageService/types.ts | 2 + .../src/stores/storeNotifications.test.ts | 24 +-- .../teleport/src/stores/storeNotifications.ts | 146 +++++++++++-- 16 files changed, 596 insertions(+), 435 deletions(-) create mode 100644 web/packages/teleport/src/Notifications/Notifications.test.tsx delete mode 100644 web/packages/teleport/src/TopBar/Notifications/Notifications.test.tsx delete mode 100644 web/packages/teleport/src/TopBar/Notifications/Notifications.tsx delete mode 100644 web/packages/teleport/src/TopBar/Notifications/index.ts diff --git a/web/packages/teleport/src/Navigation/NavigationItem.test.tsx b/web/packages/teleport/src/Navigation/NavigationItem.test.tsx index fbeb92bbd4e2c..6f812da3eba84 100644 --- a/web/packages/teleport/src/Navigation/NavigationItem.test.tsx +++ b/web/packages/teleport/src/Navigation/NavigationItem.test.tsx @@ -36,7 +36,7 @@ import { NavigationCategory } from 'teleport/Navigation/categories'; import { NavigationItem } from 'teleport/Navigation/NavigationItem'; import { NavigationItemSize } from 'teleport/Navigation/common'; import { makeUserContext } from 'teleport/services/user'; -import { NotificationKind } from 'teleport/stores/storeNotifications'; +import { LocalNotificationKind } from 'teleport/services/notifications'; class MockUserFeature implements TeleportFeature { category = NavigationCategory.Resources; @@ -142,7 +142,7 @@ describe('navigation items', () => { ctx.storeNotifications.setNotifications([ { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'banana', route: '', }, diff --git a/web/packages/teleport/src/Navigation/NavigationItem.tsx b/web/packages/teleport/src/Navigation/NavigationItem.tsx index 861de1b49805a..a95c7e78491ba 100644 --- a/web/packages/teleport/src/Navigation/NavigationItem.tsx +++ b/web/packages/teleport/src/Navigation/NavigationItem.tsx @@ -32,10 +32,10 @@ import { } from 'teleport/Navigation/common'; import useStickyClusterId from 'teleport/useStickyClusterId'; import { storageService } from 'teleport/services/storageService'; +import { LocalNotificationKind } from 'teleport/services/notifications'; import { useTeleport } from 'teleport'; import { NavTitle, RecommendationStatus } from 'teleport/types'; -import { NotificationKind } from 'teleport/stores/storeNotifications'; import type { TeleportFeature, @@ -196,7 +196,7 @@ export function NavigationItem(props: NavigationItemProps) { function renderHighlightFeature(featureName: NavTitle): JSX.Element { if (featureName === NavTitle.AccessLists) { const hasNotifications = ctx.storeNotifications.hasNotificationsByKind( - NotificationKind.AccessList + LocalNotificationKind.AccessList ); if (hasNotifications) { diff --git a/web/packages/teleport/src/Notifications/Notification.story.tsx b/web/packages/teleport/src/Notifications/Notification.story.tsx index 810b16352508e..39f0b0fcc8f74 100644 --- a/web/packages/teleport/src/Notifications/Notification.story.tsx +++ b/web/packages/teleport/src/Notifications/Notification.story.tsx @@ -69,6 +69,8 @@ export const NotificationTypes = () => { notification={notification} key={notification.id} closeNotificationsList={() => null} + markNotificationAsClicked={() => null} + removeNotification={() => null} /> ); })} diff --git a/web/packages/teleport/src/Notifications/Notification.tsx b/web/packages/teleport/src/Notifications/Notification.tsx index 8b6badf76d88f..8398b2b01442d 100644 --- a/web/packages/teleport/src/Notifications/Notification.tsx +++ b/web/packages/teleport/src/Notifications/Notification.tsx @@ -48,21 +48,23 @@ import useStickyClusterId from 'teleport/useStickyClusterId'; import { useTeleport } from '..'; import { NotificationContent } from './notificationContentFactory'; - import { View } from './Notifications'; export function Notification({ notification, view = 'All', closeNotificationsList, + removeNotification, + markNotificationAsClicked, }: { notification: NotificationType; view?: View; closeNotificationsList: () => void; + removeNotification: (notificationId: string) => void; + markNotificationAsClicked: (notificationId: string) => void; }) { const ctx = useTeleport(); const { clusterId } = useStickyClusterId(); - const [clicked, setClicked] = useState(notification.clicked); const content = ctx.notificationContentFactory(notification); @@ -73,29 +75,38 @@ export function Notification({ notificationState: NotificationState.CLICKED, }) .then(res => { - setClicked(true); + markNotificationAsClicked(notification.id); return res; }) ); const [hideNotificationAttempt, hideNotification] = useAsync(() => { - return ctx.notificationService.upsertNotificationState(clusterId, { - notificationId: notification.id, - notificationState: NotificationState.DISMISSED, - }); + return ctx.notificationService + .upsertNotificationState(clusterId, { + notificationId: notification.id, + notificationState: NotificationState.DISMISSED, + }) + .then(() => { + removeNotification(notification.id); + }); }); + + function onMarkAsClicked() { + if (notification.localNotification) { + ctx.storeNotifications.markNotificationAsClicked(notification.id); + markNotificationAsClicked(notification.id); + return; + } + markAsClicked(); + } + // Whether to show the text content dialog. This is only ever used for user-created notifications which only contain informational text // and don't redirect to any page. const [showTextContentDialog, setShowTextContentDialog] = useState(false); // If the notification is unsupported or hidden, or if the view is "Unread" and the notification has been read, // it should not be shown. - if ( - !content || - hideNotificationAttempt.status === 'success' || - hideNotificationAttempt.status === 'processing' || - (view === 'Unread' && clicked) - ) { + if (!content || (view === 'Unread' && notification.clicked)) { return null; } @@ -119,7 +130,6 @@ export function Notification({ const formattedDate = formatDate(notification.createdDate); function onNotificationClick(e: React.MouseEvent) { - markAsClicked(); // Prevents this from being triggered when the user is just clicking away from // an open "mark as read/hide this notification" menu popover. if (e.currentTarget.contains(e.target as HTMLElement)) { @@ -127,21 +137,19 @@ export function Notification({ setShowTextContentDialog(true); return; } + onMarkAsClicked(); closeNotificationsList(); history.push(content.redirectRoute); } } const isClicked = - clicked || - markAsClickedAttempt.status === 'processing' || - (markAsClickedAttempt.status === 'success' && - markAsClickedAttempt.data.notificationState === - NotificationState.CLICKED); + notification.clicked || markAsClickedAttempt.status === 'processing'; return ( <> {content.title} {content.kind === 'redirect' && content.QuickAction && ( - + )} {hideNotificationAttempt.status === 'error' && ( @@ -174,30 +182,34 @@ export function Notification({ )} - {formattedDate} - - {!isClicked && ( + {!content?.hideDate && ( + {formattedDate} + )} + {!notification.localNotification && ( + + {!isClicked && ( + + Mark as read + + )} - Mark as read + Hide this notification - )} - - Hide this notification - - + + )} diff --git a/web/packages/teleport/src/Notifications/Notifications.test.tsx b/web/packages/teleport/src/Notifications/Notifications.test.tsx new file mode 100644 index 0000000000000..35f4dbb778ca4 --- /dev/null +++ b/web/packages/teleport/src/Notifications/Notifications.test.tsx @@ -0,0 +1,122 @@ +/** + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import React from 'react'; +import { subMinutes, subSeconds } from 'date-fns'; +import { createMemoryHistory } from 'history'; +import { Router } from 'react-router'; +import { render, screen } from 'design/utils/testing'; + +import { createTeleportContext } from 'teleport/mocks/contexts'; +import { LayoutContextProvider } from 'teleport/Main/LayoutContext'; + +import { FeaturesContextProvider } from 'teleport/FeaturesContext'; +import { getOSSFeatures } from 'teleport/features'; +import TeleportContextProvider from 'teleport/TeleportContextProvider'; +import TeleportContext from 'teleport/teleportContext'; +import { NotificationSubKind } from 'teleport/services/notifications'; + +import { Notifications } from './Notifications'; + +test('notification bell with notifications', async () => { + const ctx = createTeleportContext(); + + jest.spyOn(ctx.notificationService, 'fetchNotifications').mockResolvedValue({ + nextKey: '', + userLastSeenNotification: subMinutes(Date.now(), 12), // 12 minutes ago + notifications: [ + { + id: '1', + title: 'Example notification 1', + subKind: NotificationSubKind.UserCreatedInformational, + createdDate: subSeconds(Date.now(), 15), // 15 seconds ago + clicked: false, + labels: [ + { + name: 'text-content', + value: 'This is the text content of the notification.', + }, + ], + }, + { + id: '2', + title: 'Example notification 2', + subKind: NotificationSubKind.UserCreatedInformational, + createdDate: subSeconds(Date.now(), 50), // 50 seconds ago + clicked: false, + labels: [ + { + name: 'text-content', + value: 'This is the text content of the notification.', + }, + ], + }, + ], + }); + + jest + .spyOn(ctx.notificationService, 'upsertLastSeenNotificationTime') + .mockResolvedValue({ + time: new Date(), + }); + + render(renderNotifications(ctx)); + + await screen.findByTestId('tb-notifications-badge'); + + expect(screen.getByTestId('tb-notifications')).toBeInTheDocument(); + + // Expect there to be 2 notifications. + expect(screen.getByTestId('tb-notifications-badge')).toHaveTextContent('2'); + expect(screen.queryAllByTestId('notification-item')).toHaveLength(2); +}); + +test('notification bell with no notifications', async () => { + const ctx = createTeleportContext(); + jest.spyOn(ctx.notificationService, 'fetchNotifications').mockResolvedValue({ + nextKey: '', + userLastSeenNotification: subMinutes(Date.now(), 12), // 12 minutes ago + notifications: [], + }); + + jest + .spyOn(ctx.notificationService, 'upsertLastSeenNotificationTime') + .mockResolvedValue({ + time: new Date(), + }); + + render(renderNotifications(ctx)); + + await screen.findByText(/you currently have no notifications/i); + + expect(screen.queryByTestId('notification-item')).not.toBeInTheDocument(); +}); + +const renderNotifications = (ctx: TeleportContext) => { + return ( + + + + + + + + + + ); +}; diff --git a/web/packages/teleport/src/Notifications/Notifications.tsx b/web/packages/teleport/src/Notifications/Notifications.tsx index 709484b462be5..70e73bbd6a471 100644 --- a/web/packages/teleport/src/Notifications/Notifications.tsx +++ b/web/packages/teleport/src/Notifications/Notifications.tsx @@ -17,7 +17,7 @@ */ import React, { useState, useEffect, useCallback } from 'react'; -import { isBefore } from 'date-fns'; +import { isBefore, isAfter, formatDistanceToNowStrict } from 'date-fns'; import styled from 'styled-components'; import { Alert, Box, Flex, Indicator, Text } from 'design'; @@ -32,12 +32,25 @@ import { } from 'shared/hooks/useInfiniteScroll'; import { IGNORE_CLICK_CLASSNAME } from 'shared/hooks/useRefClickOutside/useRefClickOutside'; +import { useStore } from 'shared/libs/stores'; + import { useTeleport } from 'teleport'; import useStickyClusterId from 'teleport/useStickyClusterId'; import { Dropdown } from 'teleport/components/Dropdown'; import { ButtonIconContainer } from 'teleport/TopBar/Shared'; +import { + LocalNotificationGroupedKind, + LocalNotificationKind, + Notification as NotificationType, +} from 'teleport/services/notifications'; + +import { + Notification as AccessListNotification, + LocalNotificationStates, +} from 'teleport/stores/storeNotifications'; + import { Notification } from './Notification'; const PAGE_SIZE = 15; @@ -47,10 +60,27 @@ const logger = Logger.create('Notifications'); export function Notifications({ iconSize = 24 }: { iconSize?: number }) { const ctx = useTeleport(); const { clusterId } = useStickyClusterId(); - + const store = useStore(ctx.storeNotifications); const [userLastSeenNotification, setUserLastSeenNotification] = useState(); + const [combinedNotifications, setCombinedNotifications] = useState< + NotificationType[] + >([]); + + // Access list review reminder notifications aren't currently supported by the native notifications + // system, and so they won't be returned by the prior request. They are instead generated by the frontend + // and stored in a local store on the frontend. We retrieve these separately and then combine them with + // the "real" notifications from the backend. + const localNotifications = React.useMemo( + () => + accessListStoreNotificationsToNotifications( + store.getNotifications(), + store.getNotificationStates() + ), + [store.state] + ); + const { resources: notifications, fetch, @@ -67,6 +97,7 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { }); setUserLastSeenNotification(response.userLastSeenNotification); + return { agents: response.notifications, startKey: response.nextKey, @@ -81,6 +112,10 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { fetch(); }, []); + useEffect(() => { + setCombinedNotifications([...localNotifications, ...notifications]); + }, [localNotifications, notifications]); + const { setTrigger } = useInfiniteScroll({ fetch, }); @@ -94,6 +129,12 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { if (!open) { setOpen(true); + if (localNotifications.length) { + store.markNotificationsAsSeen( + localNotifications.map(notif => notif.id) + ); + } + if (notifications.length) { const latestNotificationTime = notifications[0].createdDate; // If the current userLastSeenNotification is already set to the most recent notification's time, don't do anything. @@ -121,14 +162,40 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { } } - const unseenNotifsCount = notifications.filter(notif => - isBefore(userLastSeenNotification, notif.createdDate) - ).length; + const unseenNotifsCount = combinedNotifications.filter(notif => { + if (notif.localNotification) { + const seenNotifications = store.getNotificationStates().seen; + + return !seenNotifications.includes(notif.id); + } + + return isBefore(userLastSeenNotification, notif.createdDate); + }).length; + + function removeNotification(notificationId: string) { + const notificationsCopy = [...combinedNotifications]; + const index = notificationsCopy.findIndex( + notif => notif.id == notificationId + ); + notificationsCopy.splice(index, 1); + + setCombinedNotifications(notificationsCopy); + } + + function markNotificationAsClicked(notificationId: string) { + const newNotifications = combinedNotifications.map(notification => { + return notification.id === notificationId + ? { ...notification, clicked: true } + : notification; + }); + + setCombinedNotifications(newNotifications); + } return ( {unseenNotifsCount > 0 && ( - + {unseenNotifsCount >= 9 ? '9+' : unseenNotifsCount} )} @@ -156,25 +223,30 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { - +
{attempt.status === 'failed' && ( Could not load notifications: {attempt.statusText} )} - {attempt.status === 'success' && notifications.length === 0 && ( + {attempt.status === 'success' && combinedNotifications.length === 0 && ( )} <> - {!!notifications.length && - notifications.map(notif => ( + {!!combinedNotifications.length && + combinedNotifications.map(notif => ( setOpen(false)} + markNotificationAsClicked={markNotificationAsClicked} + removeNotification={removeNotification} /> ))} {open &&
} @@ -276,6 +348,111 @@ function EmptyState() { ); } +/** accessListStoreNotificationsToNotifications converts a list of access list notifications from the notifications store into the primary + * Notification type used by the notifications list. + */ +function accessListStoreNotificationsToNotifications( + accessListNotifs: AccessListNotification[], + notificationStates: LocalNotificationStates +): NotificationType[] { + const today = new Date(); + + /** dueNotifications are the notifications for access lists which are due for review in the future. */ + const dueNotifications = accessListNotifs + .filter(notif => isAfter(notif.date, today)) + // Sort by earliest dates. + .sort((a, b) => { + return a.date.getTime() - b.date.getTime(); + }); + + /** overdueNotifications are the notifications for access lists which are overdue for review. */ + const overdueNotifications = accessListNotifs + .filter(notif => isBefore(notif.date, today)) + .sort((a, b) => { + return a.date.getTime() - b.date.getTime(); + }); + + const processedDueNotifications: NotificationType[] = []; + const processedOverdueNotifications: NotificationType[] = []; + + // If there are 2 or less access list notifications due for review, then we will return a notification per access list. + // If there are more than 2, then we return one "grouped" notification for all of them. This is to prevent clutter in the notifications list + // in case there are many access lists due for review. + if (dueNotifications.length <= 2) { + // Process and add them to the final processed array. + dueNotifications.forEach(notif => { + const numDays = formatDistanceToNowStrict(notif.date); + const titleText = `Access list '${notif.item.resourceName}' needs your review within ${numDays}.`; + + processedDueNotifications.push({ + localNotification: true, + title: titleText, + id: notif.id, + subKind: LocalNotificationKind.AccessList, + clicked: notif.clicked, + createdDate: today, + labels: [{ name: 'redirect-route', value: notif.item.route }], + }); + }); + } else { + const mostUrgentNotif = dueNotifications[0]; + const numDays = formatDistanceToNowStrict(mostUrgentNotif.date); + const titleText = `${dueNotifications.length} of your access lists require review, the most urgent of which is due in ${numDays}.`; + + // The ID for this combined notification is --. + const id = `${dueNotifications[0].id}-${dueNotifications[dueNotifications.length - 1].id}-${dueNotifications.length}`; + + const clicked = notificationStates.clicked.includes(id); + + processedDueNotifications.push({ + localNotification: true, + title: titleText, + id, + subKind: LocalNotificationGroupedKind.AccessListGrouping, + clicked, + createdDate: today, + labels: [], + }); + } + + if (overdueNotifications.length <= 2) { + // Process and add them to the final processed array. + overdueNotifications.forEach(notif => { + const numDays = formatDistanceToNowStrict(notif.date); + const titleText = `Your review of access list '${notif.item.resourceName}' is overdue by ${numDays}.`; + + processedOverdueNotifications.push({ + localNotification: true, + title: titleText, + id: notif.id, + subKind: LocalNotificationKind.AccessList, + clicked: notif.clicked, + createdDate: today, + labels: [{ name: 'redirect-route', value: notif.item.route }], + }); + }); + } else { + const titleText = `${overdueNotifications.length} of your access lists are overdue for review.`; + + // The ID for this combined notification is --. + const id = `${overdueNotifications[0].id}-${overdueNotifications[overdueNotifications.length - 1].id}-${overdueNotifications.length}`; + + const clicked = notificationStates.clicked.includes(id); + + processedOverdueNotifications.push({ + localNotification: true, + title: titleText, + id, + subKind: LocalNotificationGroupedKind.AccessListGrouping, + clicked, + createdDate: today, + labels: [], + }); + } + + return [...processedOverdueNotifications, ...processedDueNotifications]; +} + const NotificationsDropdown = styled(Dropdown)` width: 450px; padding: 0px; diff --git a/web/packages/teleport/src/Notifications/notificationContentFactory.tsx b/web/packages/teleport/src/Notifications/notificationContentFactory.tsx index ca26ed4056f75..bebf8ae862bf8 100644 --- a/web/packages/teleport/src/Notifications/notificationContentFactory.tsx +++ b/web/packages/teleport/src/Notifications/notificationContentFactory.tsx @@ -62,6 +62,7 @@ export function notificationContentFactory({ }; break; } + default: return null; } @@ -76,6 +77,8 @@ type NotificationContentBase = { type: 'success' | 'success-alt' | 'informational' | 'warning' | 'failure'; /** icon is the icon to render for this notification. This should be an icon from `design/Icon`. */ icon: React.FC; + /** hideDate is whether to not display how old the notification is in the top right corner of the notification. */ + hideDate?: boolean; }; /** For notifications that are clickable and redirect you to a page, and may also optionally include a quick action. */ @@ -87,7 +90,7 @@ type NotificationContentRedirect = NotificationContentBase & { QuickAction?: (props: QuickActionProps) => JSX.Element; }; -export type QuickActionProps = { markAsClicked: () => Promise }; +export type QuickActionProps = { markAsClicked: () => void }; /** For notifications that only contain text and are not interactive in any other way. This is used for user-created notifications. */ type NotificationContentText = NotificationContentBase & { diff --git a/web/packages/teleport/src/TopBar/Notifications/Notifications.test.tsx b/web/packages/teleport/src/TopBar/Notifications/Notifications.test.tsx deleted file mode 100644 index 8d67bc1f86ddc..0000000000000 --- a/web/packages/teleport/src/TopBar/Notifications/Notifications.test.tsx +++ /dev/null @@ -1,127 +0,0 @@ -/** - * Teleport - * Copyright (C) 2023 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import React from 'react'; -import { MemoryRouter } from 'react-router'; -import { render, screen } from 'design/utils/testing'; - -import { createTeleportContext } from 'teleport/mocks/contexts'; -import { LayoutContextProvider } from 'teleport/Main/LayoutContext'; -import { ContextProvider } from 'teleport'; - -import { - StoreNotifications, - NotificationKind, -} from 'teleport/stores/storeNotifications'; - -import { Notifications } from './Notifications'; - -beforeAll(() => { - jest.useFakeTimers(); - jest.setSystemTime(new Date('2023-01-20')); -}); - -afterAll(() => { - jest.useRealTimers(); -}); - -test('due dates and overdue dates', async () => { - const ctx = createTeleportContext(); - const store = new StoreNotifications(); - - store.setNotifications([ - { - item: { - kind: NotificationKind.AccessList, - resourceName: 'carrot', - route: '', - }, - id: '1', - date: new Date('2023-01-25'), - }, - // overdue 10 days - { - item: { - kind: NotificationKind.AccessList, - resourceName: 'carrot', - route: '', - }, - id: '2', - date: new Date('2023-01-10'), - }, - // overdue month - { - item: { - kind: NotificationKind.AccessList, - resourceName: 'carrot', - route: '', - }, - id: '3', - date: new Date('2022-12-20'), - }, - ]); - - ctx.storeNotifications = store; - - render( - - - - - - - - ); - - // no need to click on button for render. - // it's already in the dom but hidden. - - expect(screen.queryAllByTestId('note-item')).toHaveLength(3); - - expect( - screen.getByText(/overdue for a review 10 days ago/i) - ).toBeInTheDocument(); - - expect( - screen.getByText(/overdue for a review about 1 month ago/i) - ).toBeInTheDocument(); - - expect( - screen.getByText(/needs your review within 5 days/i) - ).toBeInTheDocument(); -}); - -test('no notes', async () => { - const ctx = createTeleportContext(); - - render( - - - - - - - - ); - - // no need to click on button for render. - // it's already in the dom but hidden. - - expect(screen.queryByTestId('note-item')).not.toBeInTheDocument(); - expect(screen.getByText(/no notifications/i)).toBeInTheDocument(); -}); diff --git a/web/packages/teleport/src/TopBar/Notifications/Notifications.tsx b/web/packages/teleport/src/TopBar/Notifications/Notifications.tsx deleted file mode 100644 index 7d45bb9f353fe..0000000000000 --- a/web/packages/teleport/src/TopBar/Notifications/Notifications.tsx +++ /dev/null @@ -1,175 +0,0 @@ -/** - * Teleport - * Copyright (C) 2023 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import React, { useState } from 'react'; -import { formatDistanceToNow } from 'date-fns'; -import styled from 'styled-components'; -import { Text } from 'design'; - -import { Notification as NotificationIcon, UserList } from 'design/Icon'; -import { useRefClickOutside } from 'shared/hooks/useRefClickOutside'; -import { useStore } from 'shared/libs/stores'; -import { assertUnreachable } from 'shared/utils/assertUnreachable'; -import { HoverTooltip } from 'shared/components/ToolTip'; - -import { - Dropdown, - DropdownItem, - DropdownItemButton, - DropdownItemIcon, - STARTING_TRANSITION_DELAY, - INCREMENT_TRANSITION_DELAY, - DropdownItemLink, -} from 'teleport/components/Dropdown'; -import useTeleport from 'teleport/useTeleport'; -import { - Notification, - NotificationKind, -} from 'teleport/stores/storeNotifications'; - -import { ButtonIconContainer } from '../Shared'; - -export function Notifications({ iconSize = 24 }: { iconSize?: number }) { - const ctx = useTeleport(); - useStore(ctx.storeNotifications); - - const notices = ctx.storeNotifications.getNotifications(); - - const [open, setOpen] = useState(false); - - const ref = useRefClickOutside({ open, setOpen }); - - let transitionDelay = STARTING_TRANSITION_DELAY; - const items = notices.map(notice => { - const currentTransitionDelay = transitionDelay; - transitionDelay += INCREMENT_TRANSITION_DELAY; - - return ( - - setOpen(false)} /> - - ); - }); - - return ( - - - setOpen(!open)} - data-testid="tb-note-button" - > - {items.length > 0 && } - - - - - {items.length ? ( - items - ) : ( - - No notifications - - )} - - - - ); -} - -function NotificationItem({ - notice, - close, -}: { - notice: Notification; - close(): void; -}) { - const today = new Date(); - const numDays = formatDistanceToNow(notice.date); - - let dueText; - if (notice.date <= today) { - dueText = `was overdue for a review ${numDays} ago`; - } else { - dueText = `needs your review within ${numDays}`; - } - switch (notice.item.kind) { - case NotificationKind.AccessList: - return ( - - - - - - - Access list {notice.item.resourceName} {dueText}. - - - - ); - default: - assertUnreachable(notice.item.kind); - } -} - -const NotificationButtonContainer = styled.div` - position: relative; - height: 100%; -`; - -const AttentionDot = styled.div` - position: absolute; - width: 7px; - height: 7px; - border-radius: 100px; - background-color: ${p => p.theme.colors.buttons.warning.default}; - top: 10px; - right: 15px; - @media screen and (min-width: ${p => p.theme.breakpoints.large}px) { - top: 20px; - right: 25px; - } -`; - -const NotificationItemButton = styled(DropdownItemButton)` - align-items: flex-start; - line-height: 20px; -`; - -const NotificationLink = styled(DropdownItemLink)` - padding: 0; - z-index: 999; -`; diff --git a/web/packages/teleport/src/TopBar/Notifications/index.ts b/web/packages/teleport/src/TopBar/Notifications/index.ts deleted file mode 100644 index 904c210616f13..0000000000000 --- a/web/packages/teleport/src/TopBar/Notifications/index.ts +++ /dev/null @@ -1,19 +0,0 @@ -/** - * Teleport - * Copyright (C) 2023 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -export { Notifications } from './Notifications'; diff --git a/web/packages/teleport/src/TopBar/TopBar.story.tsx b/web/packages/teleport/src/TopBar/TopBar.story.tsx index 5125b7d2c5f8b..18c633db54f03 100644 --- a/web/packages/teleport/src/TopBar/TopBar.story.tsx +++ b/web/packages/teleport/src/TopBar/TopBar.story.tsx @@ -24,9 +24,9 @@ import { FeaturesContextProvider } from 'teleport/FeaturesContext'; import { getOSSFeatures } from 'teleport/features'; import TeleportContext from 'teleport/teleportContext'; import { makeUserContext } from 'teleport/services/user'; +import { LocalNotificationKind } from 'teleport/services/notifications'; import TeleportContextProvider from 'teleport/TeleportContextProvider'; import { LayoutContextProvider } from 'teleport/Main/LayoutContext'; -import { NotificationKind } from 'teleport/stores/storeNotifications'; import { TopBar } from './TopBar'; @@ -74,7 +74,7 @@ export function TopBarWithNotifications() { notifications: [ { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'banana', route: '', }, @@ -83,7 +83,7 @@ export function TopBarWithNotifications() { }, { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'apple', route: '', }, @@ -92,7 +92,7 @@ export function TopBarWithNotifications() { }, { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'carrot', route: '', }, diff --git a/web/packages/teleport/src/TopBar/TopBar.test.tsx b/web/packages/teleport/src/TopBar/TopBar.test.tsx index 8e2a5380b8d70..91e65e413b00a 100644 --- a/web/packages/teleport/src/TopBar/TopBar.test.tsx +++ b/web/packages/teleport/src/TopBar/TopBar.test.tsx @@ -17,9 +17,12 @@ */ import React from 'react'; +import { act } from '@testing-library/react'; +import { subSeconds, subMinutes } from 'date-fns'; import { render, screen, userEvent } from 'design/utils/testing'; import { Router } from 'react-router'; import { createMemoryHistory } from 'history'; +import { mockIntersectionObserver } from 'jsdom-testing-mocks'; import { LayoutContextProvider } from 'teleport/Main/LayoutContext'; import TeleportContextProvider from 'teleport/TeleportContextProvider'; @@ -31,14 +34,19 @@ import TeleportContext, { import { makeUserContext } from 'teleport/services/user'; import { mockUserContextProviderWith } from 'teleport/User/testHelpers/mockUserContextWith'; import { makeTestUserContext } from 'teleport/User/testHelpers/makeTestUserContext'; -import { NotificationKind } from 'teleport/stores/storeNotifications'; import { clusters } from 'teleport/Clusters/fixtures'; +import { NotificationSubKind } from 'teleport/services/notifications'; + import { TopBar } from './TopBar'; let ctx: TeleportContext; +const mio = mockIntersectionObserver(); + +beforeEach(() => jest.resetAllMocks()); + function setup(): void { ctx = new TeleportContext(); jest @@ -54,48 +62,70 @@ function setup(): void { lastConnected: Date.now(), }, }); + mockUserContextProviderWith(makeTestUserContext()); } -// TODO(rudream): adapt access list notifications to new notifications system -test.skip('notification bell without notification', async () => { +test('notification bell without notification', async () => { setup(); + jest.spyOn(ctx.notificationService, 'fetchNotifications').mockResolvedValue({ + nextKey: '', + userLastSeenNotification: subMinutes(Date.now(), 12), // 12 minutes ago + notifications: [], + }); + render(getTopBar()); - await screen.findByTestId('tb-note'); + await screen.findByTestId('tb-notifications'); - expect(screen.getByTestId('tb-note')).toBeInTheDocument(); - expect(screen.queryByTestId('tb-note-attention')).not.toBeInTheDocument(); + expect(screen.getByTestId('tb-notifications')).toBeInTheDocument(); + expect( + screen.queryByTestId('tb-notifications-badge') + ).not.toBeInTheDocument(); }); -// TODO(rudream): adapt access list notifications to new notifications system -test.skip('notification bell with notification', async () => { +test('notification bell with notification', async () => { setup(); - ctx.storeNotifications.state = { + + jest.spyOn(ctx.notificationService, 'fetchNotifications').mockResolvedValue({ + nextKey: '', + userLastSeenNotification: subMinutes(Date.now(), 12), // 12 minutes ago notifications: [ { - item: { - kind: NotificationKind.AccessList, - resourceName: 'banana', - route: '', - }, - id: 'abc', - date: new Date(), + id: '1', + title: 'Example notification 1', + subKind: NotificationSubKind.UserCreatedInformational, + createdDate: subSeconds(Date.now(), 15), // 15 seconds ago + clicked: false, + labels: [ + { + name: 'text-content', + value: 'This is the text content of the notification.', + }, + ], }, ], - }; + }); + + jest + .spyOn(ctx.notificationService, 'upsertLastSeenNotificationTime') + .mockResolvedValue({ + time: new Date(), + }); render(getTopBar()); - await screen.findByTestId('tb-note'); + await screen.findByTestId('tb-notifications-badge'); - expect(screen.getByTestId('tb-note')).toBeInTheDocument(); - expect(screen.getByTestId('tb-note-attention')).toBeInTheDocument(); + expect(screen.getByTestId('tb-notifications')).toBeInTheDocument(); + expect(screen.getByTestId('tb-notifications-badge')).toHaveTextContent('1'); // Test clicking and rendering of dropdown. - expect(screen.getByTestId('tb-note-dropdown')).not.toBeVisible(); + expect(screen.getByTestId('tb-notifications-dropdown')).not.toBeVisible(); + + act(mio.enterAll); - await userEvent.click(screen.getByTestId('tb-note-button')); - expect(screen.getByTestId('tb-note-dropdown')).toBeVisible(); + await userEvent.click(screen.getByTestId('tb-notifications-button')); + expect(screen.getByTestId('tb-notifications-dropdown')).toBeVisible(); }); const getTopBar = () => { diff --git a/web/packages/teleport/src/services/notifications/types.ts b/web/packages/teleport/src/services/notifications/types.ts index 29d2838d76f5d..d0c3bc6bb5a65 100644 --- a/web/packages/teleport/src/services/notifications/types.ts +++ b/web/packages/teleport/src/services/notifications/types.ts @@ -64,8 +64,11 @@ export type UpsertNotificationStateRequest = { export type Notification = { /** id is the uuid of this notification */ id: string; - /* subKind is a string which represents which type of notification this is, ie. "access-request-approved" */ - subKind: NotificationSubKind; + /* subKind is a string which represents which type of notification this is, ie. "access-request-approved"*/ + subKind: + | NotificationSubKind + | LocalNotificationKind + | LocalNotificationGroupedKind; /** createdDate is when the notification was created. */ createdDate: Date; /** clicked is whether this notification has been clicked on by this user. */ @@ -74,6 +77,11 @@ export type Notification = { labels: Label[]; /** title is the title of this notification. This can be overwritten in notificationContentFactory if needed. */ title: string; + /** localNotification is whether this is a notification stored in a frontend store as opposed to a "real" notification + * from the notifications system. The reason for this is that some notification types (such as access lists) are not supported + * by the backend notifications system, and are instead generated entirely on the frontend. + */ + localNotification?: boolean; }; /** NotificationSubKind is the subkind of notifications, these should be kept in sync with the values in api/types/constants.go */ @@ -87,6 +95,18 @@ export enum NotificationSubKind { AccessRequestDenied = 'access-request-denied', } +/** LocalNotificationKind is the kind of local notifications which are generated on the frontend and not stored in the backend. These do not need to be kept in sync with the backend. */ +export enum LocalNotificationKind { + /** AccessList is a notification for an access list reminder. */ + AccessList = 'access-list', +} + +/** LocalNotificationGroupedKind is the kind of groupings of local notifications. These do not need to be kept in sync with the backend. */ +export enum LocalNotificationGroupedKind { + /** AccessListGrouping is a notification which combines the notifications for multiple access list reminders into one to reduce clutter. */ + AccessListGrouping = 'access-list-grouping', +} + /** * NotificationState the state of a notification for a user. This can represent either "clicked" or "dismissed". * diff --git a/web/packages/teleport/src/services/storageService/types.ts b/web/packages/teleport/src/services/storageService/types.ts index a7b66321dc947..c335ca2f49df8 100644 --- a/web/packages/teleport/src/services/storageService/types.ts +++ b/web/packages/teleport/src/services/storageService/types.ts @@ -35,6 +35,8 @@ export const KeysEnum = { EXTERNAL_AUDIT_STORAGE_CTA_DISABLED: 'grv_teleport_external_audit_storage_disabled', LICENSE_ACKNOWLEDGED: 'grv_teleport_license_acknowledged', + + LOCAL_NOTIFICATION_STATES: 'grv_teleport_notification_states', }; // SurveyRequest is the request for sending data to the back end diff --git a/web/packages/teleport/src/stores/storeNotifications.test.ts b/web/packages/teleport/src/stores/storeNotifications.test.ts index c7c5e03d39766..c68b10dda8e80 100644 --- a/web/packages/teleport/src/stores/storeNotifications.test.ts +++ b/web/packages/teleport/src/stores/storeNotifications.test.ts @@ -16,22 +16,22 @@ * along with this program. If not, see . */ -import { - Notification, - NotificationKind, - StoreNotifications, -} from './storeNotifications'; +import { LocalNotificationKind } from 'teleport/services/notifications'; + +import { Notification, StoreNotifications } from './storeNotifications'; test('get/set/update notifications', async () => { const store = new StoreNotifications(); expect(store.getNotifications()).toStrictEqual([]); - expect(store.hasNotificationsByKind(NotificationKind.AccessList)).toBeFalsy(); + expect( + store.hasNotificationsByKind(LocalNotificationKind.AccessList) + ).toBeFalsy(); // set some notifications, sorted by earliest date. const newerNote: Notification = { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'apple', route: '', }, @@ -40,7 +40,7 @@ test('get/set/update notifications', async () => { }; const olderNote: Notification = { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'banana', route: '', }, @@ -54,7 +54,7 @@ test('get/set/update notifications', async () => { // Update notes, sorted by earliest date. const newestNote: Notification = { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'carrot', route: '', }, @@ -63,7 +63,7 @@ test('get/set/update notifications', async () => { }; const newestOlderNote: Notification = { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'carrot', route: '', }, @@ -72,7 +72,7 @@ test('get/set/update notifications', async () => { }; const newestOldestNote: Notification = { item: { - kind: NotificationKind.AccessList, + kind: LocalNotificationKind.AccessList, resourceName: 'carrot', route: '', }, @@ -88,6 +88,6 @@ test('get/set/update notifications', async () => { // Test has notifications expect( - store.hasNotificationsByKind(NotificationKind.AccessList) + store.hasNotificationsByKind(LocalNotificationKind.AccessList) ).toBeTruthy(); }); diff --git a/web/packages/teleport/src/stores/storeNotifications.ts b/web/packages/teleport/src/stores/storeNotifications.ts index 3068a75095d5e..ac81d3ac5c2d8 100644 --- a/web/packages/teleport/src/stores/storeNotifications.ts +++ b/web/packages/teleport/src/stores/storeNotifications.ts @@ -19,12 +19,11 @@ import { Store } from 'shared/libs/stores'; import { assertUnreachable } from 'shared/utils/assertUnreachable'; -export enum NotificationKind { - AccessList = 'access-list', -} +import { LocalNotificationKind } from 'teleport/services/notifications'; +import { KeysEnum } from 'teleport/services/storageService'; type AccessListNotification = { - kind: NotificationKind.AccessList; + kind: LocalNotificationKind.AccessList; resourceName: string; route: string; }; @@ -33,25 +32,57 @@ export type Notification = { item: AccessListNotification; id: string; date: Date; + clicked?: boolean; }; // TODO?: based on a feedback, consider representing // notifications as a collection of maps indexed by id // which is then converted to a sorted list as needed // (may be easier to work with) -export type NotificationState = { +export type NotificationStoreState = { notifications: Notification[]; }; -const defaultNotificationState: NotificationState = { +const defaultNotificationStoreState: NotificationStoreState = { notifications: [], }; -export class StoreNotifications extends Store { - state: NotificationState = defaultNotificationState; +export type LocalNotificationStates = { + clicked: string[]; + seen: string[]; +}; + +const defaultLocalNotificationStates: LocalNotificationStates = { + /** clicked contains the IDs of notifications which have been clicked on. */ + clicked: [], + /** seen contains the IDs of the notifications which have been seen in the notifications list, even if they were never clicked on. + * Opening the notifications list marks all notifications within it as seen. + */ + seen: [], +}; + +export class StoreNotifications extends Store { + state: NotificationStoreState = defaultNotificationStoreState; - getNotifications() { - return this.state.notifications; + getNotifications(): Notification[] { + const allNotifs = this.state.notifications; + const notifStates = this.getNotificationStates(); + + if (allNotifs.length === 0) { + localStorage.removeItem(KeysEnum.LOCAL_NOTIFICATION_STATES); + return []; + } + + return allNotifs.map(notification => { + // Mark clicked notifications as clicked. + if (notifStates.clicked.indexOf(notification.id) !== -1) { + return { + ...notification, + clicked: true, + }; + } + return notification; + }); } setNotifications(notices: Notification[]) { @@ -62,11 +93,14 @@ export class StoreNotifications extends Store { this.setState({ notifications: [...sortedNotices] }); } - updateNotificationsByKind(notices: Notification[], kind: NotificationKind) { + updateNotificationsByKind( + notices: Notification[], + kind: LocalNotificationKind + ) { switch (kind) { - case NotificationKind.AccessList: + case LocalNotificationKind.AccessList: const filtered = this.state.notifications.filter( - n => n.item.kind !== NotificationKind.AccessList + n => n.item.kind !== LocalNotificationKind.AccessList ); this.setNotifications([...filtered, ...notices]); return; @@ -75,14 +109,94 @@ export class StoreNotifications extends Store { } } - hasNotificationsByKind(kind: NotificationKind) { + hasNotificationsByKind(kind: LocalNotificationKind) { switch (kind) { - case NotificationKind.AccessList: + case LocalNotificationKind.AccessList: return this.getNotifications().some( - n => n.item.kind === NotificationKind.AccessList + n => n.item.kind === LocalNotificationKind.AccessList ); default: assertUnreachable(kind); } } + + getNotificationStates(): LocalNotificationStates { + const value = window.localStorage.getItem( + KeysEnum.LOCAL_NOTIFICATION_STATES + ); + + if (!value) { + return defaultLocalNotificationStates; + } + + try { + return JSON.parse(value) as LocalNotificationStates; + } catch (err) { + return defaultLocalNotificationStates; + } + } + + markNotificationAsClicked(id: string) { + const currentStates = this.getNotificationStates(); + + // If the notification is already marked as clicked, do nothing. + if (currentStates.clicked.includes(id)) { + return; + } + + const updatedStates: LocalNotificationStates = { + clicked: [...currentStates.clicked, id], + seen: currentStates.seen, + }; + + localStorage.setItem( + KeysEnum.LOCAL_NOTIFICATION_STATES, + JSON.stringify(updatedStates) + ); + } + + markNotificationsAsSeen(notificationIds: string[]) { + const currentStates = this.getNotificationStates(); + + // Only add new seen states that aren't already in the state, to prevent duplicates. + const newSeenStates = notificationIds.filter( + id => !currentStates.seen.includes(id) + ); + + const updatedStates: LocalNotificationStates = { + clicked: currentStates.clicked, + seen: [...currentStates.seen, ...newSeenStates], + }; + + localStorage.setItem( + KeysEnum.LOCAL_NOTIFICATION_STATES, + JSON.stringify(updatedStates) + ); + } + + resetStatesForNotification(notificationId: string) { + const currentStates = this.getNotificationStates(); + + const updatedStates = { ...currentStates }; + + // If there is a clicked state for this notification, remove it. + if (currentStates.clicked.includes(notificationId)) { + updatedStates.clicked.splice( + currentStates.clicked.indexOf(notificationId), + 1 + ); + } + + // If there is a seen state for this notification, remove it. + if (currentStates.seen.includes(notificationId)) { + updatedStates.seen.splice(currentStates.seen.indexOf(notificationId), 1); + } + + console.log(updatedStates); + + localStorage.setItem( + KeysEnum.LOCAL_NOTIFICATION_STATES, + JSON.stringify(updatedStates) + ); + } }