diff --git a/web/packages/teleport/src/Notifications/Notification.tsx b/web/packages/teleport/src/Notifications/Notification.tsx index 4f8717fd78720..5278bfcf22c84 100644 --- a/web/packages/teleport/src/Notifications/Notification.tsx +++ b/web/packages/teleport/src/Notifications/Notification.tsx @@ -90,15 +90,6 @@ export function Notification({ }); }); - 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); @@ -159,10 +150,10 @@ export function Notification({ function onClick() { if (content.kind === 'text') { setShowTextContentDialog(true); - onMarkAsClicked(); + markAsClicked(); return; } - onMarkAsClicked(); + markAsClicked(); closeNotificationsList(); history.push(content.redirectRoute); } @@ -192,7 +183,7 @@ export function Notification({ {content.title} {content.kind === 'redirect' && content.QuickAction && ( - + )} {hideNotificationAttempt.status === 'error' && ( @@ -211,31 +202,29 @@ export function Notification({ {!content?.hideDate && ( {formattedDate} )} - {!notification.localNotification && ( - - {!isClicked && ( - - Mark as read - - )} + + {!isClicked && ( - Hide this notification + Mark as read - - )} + )} + + Hide this notification + + diff --git a/web/packages/teleport/src/Notifications/Notifications.tsx b/web/packages/teleport/src/Notifications/Notifications.tsx index a82b338ba2365..974f1416f4cce 100644 --- a/web/packages/teleport/src/Notifications/Notifications.tsx +++ b/web/packages/teleport/src/Notifications/Notifications.tsx @@ -16,8 +16,8 @@ * along with this program. If not, see . */ -import { formatDistanceToNowStrict, isAfter, isBefore } from 'date-fns'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { isBefore } from 'date-fns'; +import { useCallback, useEffect, useState } from 'react'; import styled from 'styled-components'; import { Alert, Box, Flex, Indicator, Text } from 'design'; @@ -30,19 +30,9 @@ import { import { useRefClickOutside } from 'shared/hooks/useRefClickOutside'; import { IGNORE_CLICK_CLASSNAME } from 'shared/hooks/useRefClickOutside/useRefClickOutside'; import Logger from 'shared/libs/logger'; -import { useStore } from 'shared/libs/stores'; import { useTeleport } from 'teleport'; import { Dropdown } from 'teleport/components/Dropdown'; -import { - LocalNotificationGroupedKind, - LocalNotificationKind, - Notification as NotificationType, -} from 'teleport/services/notifications'; -import { - Notification as AccessListNotification, - LocalNotificationStates, -} from 'teleport/stores/storeNotifications'; import { ButtonIconContainer } from 'teleport/TopBar/Shared'; import useStickyClusterId from 'teleport/useStickyClusterId'; @@ -57,31 +47,14 @@ const NOTIFICATION_DROPDOWN_ID = 'tb-notifications-dropdown'; 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 = useMemo( - () => - accessListStoreNotificationsToNotifications( - store.getNotifications(), - store.getNotificationStates() - ), - [store.state] - ); - const { resources: notifications, fetch, attempt, + updateFetchedResources, } = useKeyBasedPagination({ fetchMoreSize: PAGE_SIZE, initialFetchSize: PAGE_SIZE, @@ -109,10 +82,6 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { fetch(); }, []); - useEffect(() => { - setCombinedNotifications([...localNotifications, ...notifications]); - }, [localNotifications, notifications]); - const { setTrigger } = useInfiniteScroll({ fetch, }); @@ -126,12 +95,6 @@ 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. @@ -159,34 +122,28 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { } } - const unseenNotifsCount = combinedNotifications.filter(notif => { - if (notif.localNotification) { - const seenNotifications = store.getNotificationStates().seen; - - return !seenNotifications.includes(notif.id); - } - - return isBefore(userLastSeenNotification, notif.createdDate); - }).length; + const unseenNotifsCount = notifications.filter(notif => + isBefore(userLastSeenNotification, notif.createdDate) + ).length; function removeNotification(notificationId: string) { - const notificationsCopy = [...combinedNotifications]; + const notificationsCopy = [...notifications]; const index = notificationsCopy.findIndex( notif => notif.id == notificationId ); notificationsCopy.splice(index, 1); - setCombinedNotifications(notificationsCopy); + updateFetchedResources(notificationsCopy); } function markNotificationAsClicked(notificationId: string) { - const newNotifications = combinedNotifications.map(notification => { + const newNotifications = notifications.map(notification => { return notification.id === notificationId ? { ...notification, clicked: true } : notification; }); - setCombinedNotifications(newNotifications); + updateFetchedResources(newNotifications); } return ( @@ -239,13 +196,13 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) { Could not load notifications: {attempt.statusText} )} - {attempt.status === 'success' && combinedNotifications.length === 0 && ( + {attempt.status === 'success' && notifications.length === 0 && ( )} <> - {!!combinedNotifications.length && - combinedNotifications.map(notif => ( + {!!notifications.length && + notifications.map(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/services/notifications/types.ts b/web/packages/teleport/src/services/notifications/types.ts index 01286e6b93bab..2a7df41a7f497 100644 --- a/web/packages/teleport/src/services/notifications/types.ts +++ b/web/packages/teleport/src/services/notifications/types.ts @@ -65,10 +65,7 @@ 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 - | LocalNotificationKind - | LocalNotificationGroupedKind; + subKind: NotificationSubKind; /** createdDate is when the notification was created. */ createdDate: Date; /** clicked is whether this notification has been clicked on by this user. */ @@ -81,11 +78,6 @@ export type Notification = { * This is the text that will be displayed in a dialog upon clicking the notification. */ textContent?: 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 */ @@ -109,20 +101,6 @@ export enum NotificationSubKind { NotificationAccessListReviewOverdue7d = 'access-list-review-overdue-7d', } -//TODO(rudream): Delete local notifications -/** 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', -} - -//TODO(rudream): Delete local notifications -/** 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/stores/index.ts b/web/packages/teleport/src/stores/index.ts index 9f26e00f28ed8..2746604c8895b 100644 --- a/web/packages/teleport/src/stores/index.ts +++ b/web/packages/teleport/src/stores/index.ts @@ -17,7 +17,6 @@ */ import StoreNav, { defaultNavState } from './storeNav'; -import { StoreNotifications } from './storeNotifications'; import StoreUserContext from './storeUserContext'; -export { StoreNav, StoreUserContext, StoreNotifications, defaultNavState }; +export { StoreNav, StoreUserContext, defaultNavState }; diff --git a/web/packages/teleport/src/stores/storeNotifications.test.ts b/web/packages/teleport/src/stores/storeNotifications.test.ts deleted file mode 100644 index c68b10dda8e80..0000000000000 --- a/web/packages/teleport/src/stores/storeNotifications.test.ts +++ /dev/null @@ -1,93 +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 { 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(LocalNotificationKind.AccessList) - ).toBeFalsy(); - - // set some notifications, sorted by earliest date. - const newerNote: Notification = { - item: { - kind: LocalNotificationKind.AccessList, - resourceName: 'apple', - route: '', - }, - id: '111', - date: new Date('2023-10-04T09:09:22-07:00'), - }; - const olderNote: Notification = { - item: { - kind: LocalNotificationKind.AccessList, - resourceName: 'banana', - route: '', - }, - id: '222', - date: new Date('2023-10-01T09:09:22-07:00'), - }; - - store.setNotifications([newerNote, olderNote]); - expect(store.getNotifications()).toStrictEqual([olderNote, newerNote]); - - // Update notes, sorted by earliest date. - const newestNote: Notification = { - item: { - kind: LocalNotificationKind.AccessList, - resourceName: 'carrot', - route: '', - }, - id: '333', - date: new Date('2023-11-23T09:09:22-07:00'), - }; - const newestOlderNote: Notification = { - item: { - kind: LocalNotificationKind.AccessList, - resourceName: 'carrot', - route: '', - }, - id: '444', - date: new Date('2023-10-03T09:09:22-07:00'), - }; - const newestOldestNote: Notification = { - item: { - kind: LocalNotificationKind.AccessList, - resourceName: 'carrot', - route: '', - }, - id: '444', - date: new Date('2023-10-01T09:09:22-07:00'), - }; - store.setNotifications([newestNote, newestOldestNote, newestOlderNote]); - expect(store.getNotifications()).toStrictEqual([ - newestOldestNote, - newestOlderNote, - newestNote, - ]); - - // Test has notifications - expect( - store.hasNotificationsByKind(LocalNotificationKind.AccessList) - ).toBeTruthy(); -}); diff --git a/web/packages/teleport/src/stores/storeNotifications.ts b/web/packages/teleport/src/stores/storeNotifications.ts deleted file mode 100644 index ac81d3ac5c2d8..0000000000000 --- a/web/packages/teleport/src/stores/storeNotifications.ts +++ /dev/null @@ -1,202 +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 { Store } from 'shared/libs/stores'; -import { assertUnreachable } from 'shared/utils/assertUnreachable'; - -import { LocalNotificationKind } from 'teleport/services/notifications'; -import { KeysEnum } from 'teleport/services/storageService'; - -type AccessListNotification = { - kind: LocalNotificationKind.AccessList; - resourceName: string; - route: string; -}; - -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 NotificationStoreState = { - notifications: Notification[]; -}; - -const defaultNotificationStoreState: NotificationStoreState = { - notifications: [], -}; - -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(): 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[]) { - // Sort by earliest dates. - const sortedNotices = notices.sort((a, b) => { - return a.date.getTime() - b.date.getTime(); - }); - this.setState({ notifications: [...sortedNotices] }); - } - - updateNotificationsByKind( - notices: Notification[], - kind: LocalNotificationKind - ) { - switch (kind) { - case LocalNotificationKind.AccessList: - const filtered = this.state.notifications.filter( - n => n.item.kind !== LocalNotificationKind.AccessList - ); - this.setNotifications([...filtered, ...notices]); - return; - default: - assertUnreachable(kind); - } - } - - hasNotificationsByKind(kind: LocalNotificationKind) { - switch (kind) { - case LocalNotificationKind.AccessList: - return this.getNotifications().some( - 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) - ); - } -} diff --git a/web/packages/teleport/src/teleportContext.tsx b/web/packages/teleport/src/teleportContext.tsx index a089ca2023895..e8da5e2d2b2c0 100644 --- a/web/packages/teleport/src/teleportContext.tsx +++ b/web/packages/teleport/src/teleportContext.tsx @@ -38,14 +38,13 @@ import sessionService from './services/session'; import { storageService } from './services/storageService'; import userService from './services/user'; import userGroupService from './services/userGroups'; -import { StoreNav, StoreNotifications, StoreUserContext } from './stores'; +import { StoreNav, StoreUserContext } from './stores'; import * as types from './types'; class TeleportContext implements types.Context { // stores storeNav = new StoreNav(); storeUser = new StoreUserContext(); - storeNotifications = new StoreNotifications(); // services auditService = new AuditService();