Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Web] Remove local notifications #51624

Merged
merged 1 commit into from
Jan 31, 2025
Merged

[Web] Remove local notifications #51624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 22 additions & 33 deletions web/packages/teleport/src/Notifications/Notification.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -192,7 +183,7 @@ export function Notification({
<ContentBody>
<Text>{content.title}</Text>
{content.kind === 'redirect' && content.QuickAction && (
<content.QuickAction markAsClicked={onMarkAsClicked} />
<content.QuickAction markAsClicked={markAsClicked} />
)}
{hideNotificationAttempt.status === 'error' && (
<Text typography="body4" color="error.main">
Expand All @@ -211,31 +202,29 @@ export function Notification({
{!content?.hideDate && (
<Text typography="body4">{formattedDate}</Text>
)}
{!notification.localNotification && (
<MenuIcon
menuProps={{
anchorOrigin: { vertical: 'bottom', horizontal: 'right' },
transformOrigin: { vertical: 'top', horizontal: 'right' },
backdropProps: { className: IGNORE_CLICK_CLASSNAME },
}}
buttonIconProps={{ style: { borderRadius: '4px' } }}
>
{!isClicked && (
<MenuItem
onClick={onMarkAsClicked}
className={IGNORE_CLICK_CLASSNAME}
>
Mark as read
</MenuItem>
)}
<MenuIcon
menuProps={{
anchorOrigin: { vertical: 'bottom', horizontal: 'right' },
transformOrigin: { vertical: 'top', horizontal: 'right' },
backdropProps: { className: IGNORE_CLICK_CLASSNAME },
}}
buttonIconProps={{ style: { borderRadius: '4px' } }}
>
{!isClicked && (
<MenuItem
onClick={hideNotification}
onClick={markAsClicked}
className={IGNORE_CLICK_CLASSNAME}
>
Hide this notification
Mark as read
</MenuItem>
</MenuIcon>
)}
)}
<MenuItem
onClick={hideNotification}
className={IGNORE_CLICK_CLASSNAME}
>
Hide this notification
</MenuItem>
</MenuIcon>
</SideContent>
</ContentContainer>
</Container>
Expand Down
175 changes: 13 additions & 162 deletions web/packages/teleport/src/Notifications/Notifications.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

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';
Expand All @@ -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';

Expand All @@ -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<Date>();

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,
Expand Down Expand Up @@ -109,10 +82,6 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) {
fetch();
}, []);

useEffect(() => {
setCombinedNotifications([...localNotifications, ...notifications]);
}, [localNotifications, notifications]);

const { setTrigger } = useInfiniteScroll({
fetch,
});
Expand All @@ -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.
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -239,13 +196,13 @@ export function Notifications({ iconSize = 24 }: { iconSize?: number }) {
<Alert>Could not load notifications: {attempt.statusText}</Alert>
</Box>
)}
{attempt.status === 'success' && combinedNotifications.length === 0 && (
{attempt.status === 'success' && notifications.length === 0 && (
<EmptyState />
)}
<NotificationsList>
<>
{!!combinedNotifications.length &&
combinedNotifications.map(notif => (
{!!notifications.length &&
notifications.map(notif => (
<Notification
notification={notif}
key={notif.id}
Expand Down Expand Up @@ -346,112 +303,6 @@ function EmptyState() {
);
}

//TODO(rudream): Delete local notifications
/** 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 <first notification in the list id>-<last notification in the list id>-<length of list>.
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 <first notification in the list id>-<last notification in the list id>-<length of list>.
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;
Expand Down
24 changes: 1 addition & 23 deletions web/packages/teleport/src/services/notifications/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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 */
Expand All @@ -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".
*
Expand Down
Loading
Loading