Skip to content

feat: add notifications widget in the navbar #16983

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

Merged
merged 5 commits into from
Mar 18, 2025
Merged

feat: add notifications widget in the navbar #16983

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c182295
feat: add notifications widget in the navbar
BrunoQuaresma Mar 18, 2025
a696bd7
FMT
BrunoQuaresma Mar 18, 2025
328854d
Merge branch 'main' of https://github.com/coder/coder into bq/notific…
BrunoQuaresma Mar 18, 2025
281dadf
Apply review comments and fix storybook tests
BrunoQuaresma Mar 18, 2025
4d0f010
Fix lint
BrunoQuaresma Mar 18, 2025
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
107 changes: 81 additions & 26 deletions site/src/api/api.ts
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Just making a note for myself: this looks like a good candidate for swapping in the OneWayWebSocket class once that PR is merged in

Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,33 @@ export const watchWorkspace = (workspaceId: string): EventSource => {
);
};

type WatchInboxNotificationsParams = {
read_status?: "read" | "unread" | "all";
};

export const watchInboxNotifications = (
onNewNotification: (res: TypesGen.GetInboxNotificationResponse) => void,
params?: WatchInboxNotificationsParams,
) => {
const searchParams = new URLSearchParams(params);
const socket = createWebSocket(
"/api/v2/notifications/inbox/watch",
searchParams,
);

socket.addEventListener("message", (event) => {
const res = JSON.parse(event.data) as TypesGen.GetInboxNotificationResponse;
onNewNotification(res);
Copy link
Member

Choose a reason for hiding this comment

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

The OneWayWebSocket class handles this, but in the meantime, do we want to add error handling if JSON.parse ever throws an error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! I think it is always good to handle errors. In this case, I think we can just log a warn. What do you think?

});

socket.addEventListener("error", (event) => {
console.warn("Watch inbox notifications error: ", event);
socket.close();
});

return socket;
};

export const getURLWithSearchParams = (
basePath: string,
options?: SearchParamOptions,
Expand Down Expand Up @@ -184,15 +211,11 @@ export const watchBuildLogsByTemplateVersionId = (
searchParams.append("after", after.toString());
}

const proto = location.protocol === "https:" ? "wss:" : "ws:";
const socket = new WebSocket(
`${proto}//${
location.host
}/api/v2/templateversions/${versionId}/logs?${searchParams.toString()}`,
const socket = createWebSocket(
`/api/v2/templateversions/${versionId}/logs`,
searchParams,
);

socket.binaryType = "blob";

socket.addEventListener("message", (event) =>
onMessage(JSON.parse(event.data) as TypesGen.ProvisionerJobLog),
);
Expand All @@ -214,21 +237,21 @@ export const watchWorkspaceAgentLogs = (
agentId: string,
{ after, onMessage, onDone, onError }: WatchWorkspaceAgentLogsOptions,
) => {
// WebSocket compression in Safari (confirmed in 16.5) is broken when
// the server sends large messages. The following error is seen:
//
// WebSocket connection to 'wss://.../logs?follow&after=0' failed: The operation couldn’t be completed. Protocol error
//
const noCompression =
userAgentParser(navigator.userAgent).browser.name === "Safari"
? "&no_compression"
: "";
const searchParams = new URLSearchParams({ after: after.toString() });

const proto = location.protocol === "https:" ? "wss:" : "ws:";
const socket = new WebSocket(
`${proto}//${location.host}/api/v2/workspaceagents/${agentId}/logs?follow&after=${after}${noCompression}`,
/**
* WebSocket compression in Safari (confirmed in 16.5) is broken when
* the server sends large messages. The following error is seen:
* WebSocket connection to 'wss://...' failed: The operation couldn’t be completed.
*/
if (userAgentParser(navigator.userAgent).browser.name === "Safari") {
searchParams.set("no_compression", "");
}

const socket = createWebSocket(
`/api/v2/workspaceagents/${agentId}/logs`,
searchParams,
);
socket.binaryType = "blob";

socket.addEventListener("message", (event) => {
const logs = JSON.parse(event.data) as TypesGen.WorkspaceAgentLog[];
Expand Down Expand Up @@ -267,13 +290,11 @@ export const watchBuildLogsByBuildId = (
if (after !== undefined) {
searchParams.append("after", after.toString());
}
const proto = location.protocol === "https:" ? "wss:" : "ws:";
const socket = new WebSocket(
`${proto}//${
location.host
}/api/v2/workspacebuilds/${buildId}/logs?${searchParams.toString()}`,

const socket = createWebSocket(
`/api/v2/workspacebuilds/${buildId}/logs`,
searchParams,
);
socket.binaryType = "blob";

socket.addEventListener("message", (event) =>
onMessage(JSON.parse(event.data) as TypesGen.ProvisionerJobLog),
Expand Down Expand Up @@ -2406,6 +2427,25 @@ class ApiMethods {
);
return res.data;
};

getInboxNotifications = async () => {
const res = await this.axios.get<TypesGen.ListInboxNotificationsResponse>(
"/api/v2/notifications/inbox",
);
return res.data;
};

updateInboxNotificationReadStatus = async (
notificationId: string,
req: TypesGen.UpdateInboxNotificationReadStatusRequest,
) => {
const res =
await this.axios.put<TypesGen.UpdateInboxNotificationReadStatusResponse>(
`/api/v2/notifications/inbox/${notificationId}/read-status`,
req,
);
return res.data;
};
}

// This is a hard coded CSRF token/cookie pair for local development. In prod,
Expand Down Expand Up @@ -2457,6 +2497,21 @@ function getConfiguredAxiosInstance(): AxiosInstance {
return instance;
}

/**
* Utility function to help create a WebSocket connection with Coder's API.
*/
function createWebSocket(
path: string,
params: URLSearchParams = new URLSearchParams(),
) {
const protocol = location.protocol === "https:" ? "wss:" : "ws:";
const socket = new WebSocket(
`${protocol}//${location.host}${path}?${params.toString()}`,
);
socket.binaryType = "blob";
return socket;
}

// Other non-API methods defined here to make it a little easier to find them.
interface ClientApi extends ApiMethods {
getCsrfToken: () => string;
Expand Down
14 changes: 14 additions & 0 deletions site/src/modules/dashboard/Navbar/NavbarView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { API } from "api/api";
import type * as TypesGen from "api/typesGenerated";
import { ExternalImage } from "components/ExternalImage/ExternalImage";
import { CoderIcon } from "components/Icons/CoderIcon";
import type { ProxyContextValue } from "contexts/ProxyContext";
import { NotificationsInbox } from "modules/notifications/NotificationsInbox/NotificationsInbox";
import type { FC } from "react";
import { NavLink, useLocation } from "react-router-dom";
import { cn } from "utils/cn";
Expand Down Expand Up @@ -65,6 +67,18 @@ export const NavbarView: FC<NavbarViewProps> = ({
canViewHealth={canViewHealth}
/>

<NotificationsInbox
fetchNotifications={API.getInboxNotifications}
markAllAsRead={(): Promise<void> => {
throw new Error("Function not implemented.");
}}
Copy link
Member

Choose a reason for hiding this comment

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

Just to be on the extra safe side, the return type can be changed to never to indicate that it will always throw an error. Luckily all promise values are assignable to never, too, so you don't even need Promise<never>

markNotificationAsRead={(notificationId) =>
API.updateInboxNotificationReadStatus(notificationId, {
is_read: true,
})
}
/>

{user && (
<UserDropdown
user={user}
Expand Down
2 changes: 1 addition & 1 deletion site/src/modules/notifications/NotificationsInbox/InboxButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Button, type ButtonProps } from "components/Button/Button";
import { BellIcon } from "lucide-react";
import { type FC, forwardRef } from "react";
import { forwardRef } from "react";
import { UnreadBadge } from "./UnreadBadge";

type InboxButtonProps = {
Expand Down
9 changes: 5 additions & 4 deletions site/src/modules/notifications/NotificationsInbox/InboxItem.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Meta, StoryObj } from "@storybook/react";
import { expect, fn, userEvent, within } from "@storybook/test";
import { MockNotification } from "testHelpers/entities";
import { daysAgo } from "utils/time";
import { InboxItem } from "./InboxItem";

const meta: Meta<typeof InboxItem> = {
Expand All @@ -22,7 +23,7 @@ export const Read: Story = {
args: {
notification: {
...MockNotification,
read_status: "read",
read_at: daysAgo(1),
},
},
};
Expand All @@ -31,7 +32,7 @@ export const Unread: Story = {
args: {
notification: {
...MockNotification,
read_status: "unread",
read_at: null,
},
},
};
Expand All @@ -40,7 +41,7 @@ export const UnreadFocus: Story = {
args: {
notification: {
...MockNotification,
read_status: "unread",
read_at: null,
},
},
play: async ({ canvasElement }) => {
Expand All @@ -54,7 +55,7 @@ export const OnMarkNotificationAsRead: Story = {
args: {
notification: {
...MockNotification,
read_status: "unread",
read_at: null,
},
onMarkNotificationAsRead: fn(),
},
Expand Down
8 changes: 4 additions & 4 deletions site/src/modules/notifications/NotificationsInbox/InboxItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import type { InboxNotification } from "api/typesGenerated";
import { Avatar } from "components/Avatar/Avatar";
import { Button } from "components/Button/Button";
import { SquareCheckBig } from "lucide-react";
import type { FC } from "react";
import { Link as RouterLink } from "react-router-dom";
import { relativeTime } from "utils/time";
import type { Notification } from "./types";

type InboxItemProps = {
notification: Notification;
notification: InboxNotification;
onMarkNotificationAsRead: (notificationId: string) => void;
};

Expand All @@ -25,7 +25,7 @@ export const InboxItem: FC<InboxItemProps> = ({
<Avatar fallback="AR" />
</div>

<div className="flex flex-col gap-3">
<div className="flex flex-col gap-3 flex-1">
<span className="text-content-secondary text-sm font-medium">
{notification.content}
</span>
Expand All @@ -41,7 +41,7 @@ export const InboxItem: FC<InboxItemProps> = ({
</div>

<div className="w-12 flex flex-col items-end flex-shrink-0">
{notification.read_status === "unread" && (
{notification.read_at === null && (
<>
<div className="group-focus:hidden group-hover:hidden size-2.5 rounded-full bg-highlight-sky">
<span className="sr-only">Unread</span>
Expand Down
4 changes: 2 additions & 2 deletions site/src/modules/notifications/NotificationsInbox/InboxPopover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { InboxNotification } from "api/typesGenerated";
import { Button } from "components/Button/Button";
import {
Popover,
Expand All @@ -13,10 +14,9 @@ import { cn } from "utils/cn";
import { InboxButton } from "./InboxButton";
import { InboxItem } from "./InboxItem";
import { UnreadBadge } from "./UnreadBadge";
import type { Notification } from "./types";

type InboxPopoverProps = {
notifications: Notification[] | undefined;
notifications: readonly InboxNotification[] | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Appreciate the readonly update!

unreadCount: number;
error: unknown;
onRetry: () => void;
Expand Down
51 changes: 36 additions & 15 deletions site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import { API, watchInboxNotifications } from "api/api";
import { getErrorDetail, getErrorMessage } from "api/errors";
import type {
ListInboxNotificationsResponse,
UpdateInboxNotificationReadStatusResponse,
} from "api/typesGenerated";
import { displayError } from "components/GlobalSnackbar/utils";
import type { FC } from "react";
import { type FC, useEffect, useRef } from "react";
import { useMutation, useQuery, useQueryClient } from "react-query";
import { InboxPopover } from "./InboxPopover";
import type { Notification } from "./types";

const NOTIFICATIONS_QUERY_KEY = ["notifications"];

type NotificationsResponse = {
notifications: Notification[];
unread_count: number;
};

type NotificationsInboxProps = {
defaultOpen?: boolean;
fetchNotifications: () => Promise<NotificationsResponse>;
fetchNotifications: () => Promise<ListInboxNotificationsResponse>;
markAllAsRead: () => Promise<void>;
markNotificationAsRead: (notificationId: string) => Promise<void>;
markNotificationAsRead: (
notificationId: string,
) => Promise<UpdateInboxNotificationReadStatusResponse>;
};

export const NotificationsInbox: FC<NotificationsInboxProps> = ({
Expand All @@ -36,6 +37,24 @@ export const NotificationsInbox: FC<NotificationsInboxProps> = ({
queryFn: fetchNotifications,
});

useEffect(() => {
const socket = watchInboxNotifications(
(res) => {
safeUpdateNotificationsCache((prev) => {
Copy link
Member

Choose a reason for hiding this comment

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

JavaScript rules will ensure that this works, but just to help make sure the code stays readable, could safeUpdateNotificationsCache be moved above the effect?

Copy link
Member

Choose a reason for hiding this comment

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

Also, I'm really surprised that Biome didn't flag this as a missing effect dependency. I would expect that we would need to wrap the callback in useEffectEvent, too

return {
unread_count: res.unread_count,
notifications: [res.notification, ...prev.notifications],
};
});
},
{ read_status: "unread" },
);

return () => {
socket.close();
};
}, []);

const markAllAsReadMutation = useMutation({
mutationFn: markAllAsRead,
onSuccess: () => {
Expand All @@ -59,15 +78,15 @@ export const NotificationsInbox: FC<NotificationsInboxProps> = ({

const markNotificationAsReadMutation = useMutation({
mutationFn: markNotificationAsRead,
onSuccess: (_, notificationId) => {
onSuccess: (res) => {
safeUpdateNotificationsCache((prev) => {
Copy link
Member

Choose a reason for hiding this comment

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

Same note, here, too. When I read the code top to bottom, I find it slightly harder to follow the logic when we reference values that haven't been explicitly declared yet

return {
unread_count: prev.unread_count - 1,
unread_count: res.unread_count,
notifications: prev.notifications.map((n) => {
if (n.id !== notificationId) {
if (n.id !== res.notification.id) {
return n;
}
return { ...n, read_status: "read" };
return res.notification;
}),
};
});
Expand All @@ -81,10 +100,12 @@ export const NotificationsInbox: FC<NotificationsInboxProps> = ({
});

async function safeUpdateNotificationsCache(
callback: (res: NotificationsResponse) => NotificationsResponse,
callback: (
res: ListInboxNotificationsResponse,
) => ListInboxNotificationsResponse,
) {
await queryClient.cancelQueries(NOTIFICATIONS_QUERY_KEY);
queryClient.setQueryData<NotificationsResponse>(
queryClient.setQueryData<ListInboxNotificationsResponse>(
NOTIFICATIONS_QUERY_KEY,
(prev) => {
if (!prev) {
Expand Down
12 changes: 0 additions & 12 deletions site/src/modules/notifications/NotificationsInbox/types.ts
View file
Open in desktop

This file was deleted.

Loading
Loading