diff --git a/app/components/MobileNav.tsx b/app/components/MobileNav.tsx index 3193b2891..b3a8b4123 100644 --- a/app/components/MobileNav.tsx +++ b/app/components/MobileNav.tsx @@ -56,11 +56,10 @@ export function MobileNav({ sidebarData }: { sidebarData: SidebarData }) { const [activePanel, setActivePanel] = React.useState("closed"); const previousPanelRef = React.useRef("closed"); const user = useUser(); - const { unseenIds } = useNotifications(); + const { showUnseenDot } = useNotifications(); const chatContext = useChatContext(); const layoutSize = useLayoutSize(); - const hasUnseenNotifications = unseenIds.length > 0; const hasFriendInSendouQ = sidebarData?.friends.some((f) => f.subtitle === SENDOUQ_ACTIVITY_LABEL) ?? false; @@ -157,7 +156,7 @@ export function MobileNav({ sidebarData }: { sidebarData: SidebarData }) { activePanel={activePanel} onTabPress={handleTabPress} isLoggedIn={Boolean(user)} - hasUnseenNotifications={hasUnseenNotifications} + hasUnseenNotifications={showUnseenDot} hasFriendInSendouQ={hasFriendInSendouQ} unseenFriendRequests={unseenFriendRequests} /> diff --git a/app/components/NotificationDot.tsx b/app/components/NotificationDot.tsx index 26f85d0c7..fce3476df 100644 --- a/app/components/NotificationDot.tsx +++ b/app/components/NotificationDot.tsx @@ -1,9 +1,15 @@ import clsx from "clsx"; import styles from "./NotificationDot.module.css"; -export function NotificationDot({ className }: { className?: string }) { +export function NotificationDot({ + className, + testId, +}: { + className?: string; + testId?: string; +}) { return ( - + diff --git a/app/components/layout/NotificationPopover.module.css b/app/components/layout/NotificationPopover.module.css index c39ca16b5..f13773c2a 100644 --- a/app/components/layout/NotificationPopover.module.css +++ b/app/components/layout/NotificationPopover.module.css @@ -12,10 +12,11 @@ min-height: 200px; } -.topContainer { +.header { display: flex; - justify-content: space-between; align-items: center; + font-size: var(--font-sm); + gap: var(--s-2); padding: var(--s-1) var(--s-2); & svg { @@ -26,13 +27,6 @@ } } -.header { - display: flex; - align-items: center; - font-size: var(--font-sm); - gap: var(--s-2); -} - .noNotifications { display: grid; place-items: center; diff --git a/app/components/layout/NotificationPopover.tsx b/app/components/layout/NotificationPopover.tsx index cd331dd4e..a15838ab4 100644 --- a/app/components/layout/NotificationPopover.tsx +++ b/app/components/layout/NotificationPopover.tsx @@ -1,27 +1,30 @@ -import { Bell, ChevronRight, RefreshCcw } from "lucide-react"; +import { Bell, ChevronRight } from "lucide-react"; import * as React from "react"; import { useTranslation } from "react-i18next"; import { Link } from "react-router"; -import { useLayoutData } from "~/features/layout/LayoutDataProvider"; import { NotificationItem, NotificationItemDivider, NotificationsList, } from "~/features/notifications/components/NotificationList"; +import { + type NotificationsData, + useNotificationsData, +} from "~/features/notifications/NotificationsProvider"; import { NOTIFICATIONS } from "~/features/notifications/notifications-contants"; -import type { RootLoaderData } from "~/root"; import { NOTIFICATIONS_URL } from "~/utils/urls"; -import { useMarkNotificationsAsSeen } from "../../features/notifications/notifications-hooks"; -import { SendouButton } from "../elements/Button"; +import { + useMarkNotificationsAsSeen, + useShowUnseenDot, + useStickyUnseenIds, +} from "../../features/notifications/notifications-hooks"; import styles from "./NotificationPopover.module.css"; -export type LoaderNotification = NonNullable< - RootLoaderData["notifications"] ->[number]; +export type LoaderNotification = NonNullable[number]; export function useNotifications() { - const { notifications } = useLayoutData(); + const { notifications } = useNotificationsData(); const unseenIds = React.useMemo( () => @@ -31,7 +34,9 @@ export function useNotifications() { [notifications], ); - return { notifications, unseenIds }; + const showUnseenDot = useShowUnseenDot(notifications); + + return { notifications, unseenIds, showUnseenDot }; } export function NotificationContent({ @@ -44,24 +49,15 @@ export function NotificationContent({ onClose?: () => void; }) { const { t } = useTranslation(["common"]); - const { refresh, isRefreshing } = useLayoutData(); + const stickyUnseenIds = useStickyUnseenIds(notifications); useMarkNotificationsAsSeen(unseenIds); return ( <> -
-

- {t("common:notifications.title")} -

- } - shape="circle" - variant="minimal" - onPress={refresh} - isDisabled={isRefreshing} - /> -
+

+ {t("common:notifications.title")} +

{notifications.length === 0 ? (
@@ -73,7 +69,10 @@ export function NotificationContent({ {i !== notifications.length - 1 && } diff --git a/app/components/layout/index.tsx b/app/components/layout/index.tsx index 07d4dc926..e20923dc1 100644 --- a/app/components/layout/index.tsx +++ b/app/components/layout/index.tsx @@ -275,7 +275,7 @@ export function Layout({ } const user = useUser(); - const { unseenIds } = useNotifications(); + const { showUnseenDot } = useNotifications(); const { sidebar: sidebarData } = useLayoutData(); const events = sidebarData?.events ?? []; const friends = sidebarData?.friends ?? []; @@ -421,7 +421,7 @@ export function Layout({ > 0} + showNotificationDot={!sideNavModalOpen && showUnseenDot} badgeCount={!sideNavModalOpen ? unseenFriendRequests : 0} testId="sidenav-modal-trigger" /> @@ -458,7 +458,7 @@ export function Layout({ setSideNavCollapsed(!sideNavCollapsed)} className={styles.sideNavCollapseButton} - showNotificationDot={sideNavCollapsed && unseenIds.length > 0} + showNotificationDot={sideNavCollapsed && showUnseenDot} badgeCount={sideNavCollapsed ? unseenFriendRequests : 0} testId="sidenav-collapse-button" /> @@ -671,7 +671,7 @@ function SideNavUserPanel() { const { t } = useTranslation(); const location = useLocation(); const user = useUser(); - const { notifications, unseenIds } = useNotifications(); + const { notifications, unseenIds, showUnseenDot } = useNotifications(); if (user) { return ( @@ -688,9 +688,10 @@ function SideNavUserPanel() { className={sideNavStyles.sideNavFooterNotification} key={location.pathname} > - {unseenIds.length > 0 ? ( + {showUnseenDot ? ( ) : null} setChatLabels({}), []); + const [notificationsVersion, setNotificationsVersion] = React.useState(0); const ws = React.useRef(undefined); @@ -191,6 +192,13 @@ function ChatProviderInner({ return; } + // Notifications changed server-side; handled before the fallthrough below + // so a contentless ping is never treated as a chat message + if (parsed.event === "NOTIFICATIONS_CHANGED") { + setNotificationsVersion((version) => version + 1); + return; + } + // CHAT_HISTORY response (also returned by SUBSCRIBE with metadata) if (parsed.event === "CHAT_HISTORY" && Array.isArray(parsed.messages)) { logger.debug( @@ -487,6 +495,7 @@ function ChatProviderInner({ unreadCounts, totalUnreadCount, readyState, + notificationsVersion, chatUsers, chatOpen, setChatOpen, @@ -510,6 +519,7 @@ function ChatProviderInner({ unreadCounts, totalUnreadCount, readyState, + notificationsVersion, chatUsers, chatOpen, activeRooms, diff --git a/app/features/chat/ChatSystemMessage.server.ts b/app/features/chat/ChatSystemMessage.server.ts index 6646c3c7a..165eb5aaa 100644 --- a/app/features/chat/ChatSystemMessage.server.ts +++ b/app/features/chat/ChatSystemMessage.server.ts @@ -118,6 +118,28 @@ function postMessages(fullMessages: ChatMessage[]) { }).catch(logSkalpError("sendMessage")); } +/** + * Tells skalop to send a contentless "your notifications changed" ping to the + * users' websocket connections, prompting their clients to refetch. Fire and + * forget like the other system messages; a lost ping only delays the refetch. + */ +export function notifyNotificationsChanged(userIds: number[]) { + if (systemMessagesDisabled) return; + if (userIds.length === 0) return; + + return void fetch(ServerConfig.skalop.systemMessageUrl!, { + method: "POST", + body: JSON.stringify({ + action: "notifyUsers", + userIds, + }), + headers: [ + [SKALOP_TOKEN_HEADER_NAME, ServerConfig.skalop.token!], + ["Content-Type", "application/json"], + ], + }).catch(logSkalpError("notifyUsers")); +} + export function removeRoom(chatCode: string) { if (systemMessagesDisabled) return; diff --git a/app/features/chat/chat-provider-types.ts b/app/features/chat/chat-provider-types.ts index 2118f5801..cf0adf2c4 100644 --- a/app/features/chat/chat-provider-types.ts +++ b/app/features/chat/chat-provider-types.ts @@ -49,6 +49,11 @@ export interface ChatContextValue { unreadCounts: Record; totalUnreadCount: number; readyState: "CONNECTING" | "CONNECTED" | "CLOSED"; + /** + * Bumps every time skalop pings that the user's notifications changed + * server-side. Carries no data on purpose; watchers react by refetching. + */ + notificationsVersion: number; chatUsers: Record; chatOpen: boolean; setChatOpen: (open: boolean) => void; diff --git a/app/features/friends/FriendRepository.server.ts b/app/features/friends/FriendRepository.server.ts index c814ea2f0..792258ecd 100644 --- a/app/features/friends/FriendRepository.server.ts +++ b/app/features/friends/FriendRepository.server.ts @@ -158,7 +158,8 @@ export async function deleteFriendRequest({ .deleteFrom("FriendRequest") .where("FriendRequest.id", "=", id) .where("FriendRequest.senderId", "=", senderId) - .execute(); + .returning("FriendRequest.receiverId") + .executeTakeFirst(); } export function deleteOldPendingRequests() { @@ -276,7 +277,8 @@ export async function findFriendRequestByIdAndReceiver({ }) { return db .selectFrom("FriendRequest") - .select("FriendRequest.senderId") + .innerJoin("User", "User.id", "FriendRequest.senderId") + .select(["FriendRequest.senderId", "User.username as senderUsername"]) .where("FriendRequest.id", "=", id) .where("FriendRequest.receiverId", "=", receiverId) .executeTakeFirst(); diff --git a/app/features/friends/actions/friends.server.ts b/app/features/friends/actions/friends.server.ts index fdb012acc..a2fe72a16 100644 --- a/app/features/friends/actions/friends.server.ts +++ b/app/features/friends/actions/friends.server.ts @@ -1,6 +1,7 @@ import type { ActionFunction } from "react-router"; import { requireUser } from "~/features/auth/core/user.server"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { parseFormData } from "~/form/parse.server"; import { errorToastIfFalsy } from "~/utils/remix.server"; import * as FriendRepository from "../FriendRepository.server"; @@ -38,18 +39,27 @@ export const action: ActionFunction = async ({ request }) => { userIds: [result.data.userId], notification: { type: "FRIEND_REQUEST_RECEIVED", - meta: { senderUsername: user.username }, + meta: { senderId: user.id, senderUsername: user.username }, }, }); break; } case "CANCEL_REQUEST": { - await FriendRepository.deleteFriendRequest({ + const deleted = await FriendRepository.deleteFriendRequest({ id: result.data.friendRequestId, senderId: user.id, }); + if (deleted) { + // the receiver no longer has a request to respond to + await resolveNotifications({ + userIds: [deleted.receiverId], + type: "FRIEND_REQUEST_RECEIVED", + meta: { senderId: user.id }, + }); + } + break; } case "DELETE_FRIEND": { @@ -71,14 +81,33 @@ export const action: ActionFunction = async ({ request }) => { friendRequestId: result.data.friendRequestId, }); + await resolveNotifications({ + userIds: [user.id], + type: "FRIEND_REQUEST_RECEIVED", + meta: { senderId: friendRequest.senderId }, + }); + break; } case "DECLINE_REQUEST": { + const friendRequest = + await FriendRepository.findFriendRequestByIdAndReceiver({ + id: result.data.friendRequestId, + receiverId: user.id, + }); + if (!friendRequest) break; + await FriendRepository.deleteFriendRequestByReceiver({ id: result.data.friendRequestId, receiverId: user.id, }); + await resolveNotifications({ + userIds: [user.id], + type: "FRIEND_REQUEST_RECEIVED", + meta: { senderId: friendRequest.senderId }, + }); + break; } } diff --git a/app/features/layout/LayoutDataProvider.tsx b/app/features/layout/LayoutDataProvider.tsx index 9cfb606d1..43a640362 100644 --- a/app/features/layout/LayoutDataProvider.tsx +++ b/app/features/layout/LayoutDataProvider.tsx @@ -11,7 +11,6 @@ interface LayoutData { /** `null` when the server has no session, `undefined` when it has not said. */ loggedInUserId?: number | null; sidebar?: RootLoaderData["sidebar"]; - notifications?: RootLoaderData["notifications"]; buildCommit?: string; } @@ -69,6 +68,9 @@ export function LayoutDataProvider({ }; }, [load]); + // stable so effects that refresh after a mutation don't re-run every render + const refresh = React.useCallback(() => load(LAYOUT_DATA_ROUTE), [load]); + const newest = useNewestOf(data, fetcher.data); useReloadOnNewDeploy(newest.buildCommit ?? ""); @@ -79,7 +81,7 @@ export function LayoutDataProvider({ const value: LayoutDataContextValue = { ...newest, - refresh: () => load(LAYOUT_DATA_ROUTE), + refresh, isRefreshing: state !== "idle", }; @@ -91,8 +93,8 @@ export function LayoutDataProvider({ } /** - * App shell data (sidebar, notification peek, build commit), fresher than the - * root loader whenever a poll has landed since the last root revalidation. + * App shell data (sidebar, build commit), fresher than the root loader + * whenever a poll has landed since the last root revalidation. */ export function useLayoutData() { return React.useContext(LayoutDataContext); diff --git a/app/features/layout/core/layout.server.ts b/app/features/layout/core/layout.server.ts index eecb3426d..84a7b169a 100644 --- a/app/features/layout/core/layout.server.ts +++ b/app/features/layout/core/layout.server.ts @@ -1,23 +1,18 @@ import type { AuthenticatedUser } from "~/features/auth/core/user.server"; -import * as NotificationRepository from "~/features/notifications/NotificationRepository.server"; -import { NOTIFICATIONS } from "~/features/notifications/notifications-contants"; import { resolveSidebarData } from "~/features/sidebar/core/sidebar.server"; import { GIT_COMMIT } from "~/utils/git-commit"; /** * The parts of the app shell that go stale on their own while a page sits open. * Served both by the root loader and by the resource route `LayoutDataProvider` - * polls, so the two payloads can't drift apart. + * polls, so the two payloads can't drift apart. Notifications are not part of + * this: they live in their own resource route refetched when skalop pings that + * they changed (see `NotificationsProvider`). */ export async function resolveLayoutData(user: AuthenticatedUser | undefined) { return { loggedInUserId: user?.id ?? null, sidebar: await resolveSidebarData(user?.id ?? null), - notifications: user - ? await NotificationRepository.findByUserId(user.id, { - limit: NOTIFICATIONS.PEEK_COUNT, - }) - : undefined, buildCommit: GIT_COMMIT, }; } diff --git a/app/features/leaderboards/LeaderboardRepository.server.test.ts b/app/features/leaderboards/LeaderboardRepository.server.test.ts index edad6a091..2c19f2aaf 100644 --- a/app/features/leaderboards/LeaderboardRepository.server.test.ts +++ b/app/features/leaderboards/LeaderboardRepository.server.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/features/notifications/NotificationRepository.server.test.ts b/app/features/notifications/NotificationRepository.server.test.ts new file mode 100644 index 000000000..30901b2bc --- /dev/null +++ b/app/features/notifications/NotificationRepository.server.test.ts @@ -0,0 +1,216 @@ +import { beforeEach, describe, expect, test } from "vitest"; +import * as NotificationFactory from "~/db/seed/factories/NotificationFactory"; +import * as UserFactory from "~/db/seed/factories/UserFactory"; +import { withUserId } from "~/utils/Test"; +import * as NotificationRepository from "./NotificationRepository.server"; + +const users = UserFactory.pool(); + +const seenStatusOf = async (userId: number) => { + const notifications = await NotificationRepository.findByUserId(userId); + return notifications.map(({ type, seen }) => ({ type, seen })); +}; + +describe("markAsSeenByType", () => { + beforeEach(async () => { + await users.create(3); + }); + + test("marks unseen notification of the type as seen", async () => { + await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1) }], + }); + + await NotificationRepository.markAsSeenByType({ + userIds: [users.id(1)], + type: "SQ_READY_CHECK", + }); + + expect(await seenStatusOf(users.id(1))).toEqual([ + { type: "SQ_READY_CHECK", seen: 1 }, + ]); + }); + + test("leaves other users' notifications unseen", async () => { + await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1) }, { userId: users.id(2) }], + }); + + await NotificationRepository.markAsSeenByType({ + userIds: [users.id(1)], + type: "SQ_READY_CHECK", + }); + + expect(await seenStatusOf(users.id(2))).toEqual([ + { type: "SQ_READY_CHECK", seen: 0 }, + ]); + }); + + test("leaves notifications of other types unseen", async () => { + await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1) }], + }); + await NotificationFactory.create({ + notification: { type: "SQ_NEW_MATCH", meta: { matchId: 1 } }, + users: [{ userId: users.id(1) }], + }); + + await NotificationRepository.markAsSeenByType({ + userIds: [users.id(1)], + type: "SQ_READY_CHECK", + }); + + expect(await seenStatusOf(users.id(1))).toEqual([ + { type: "SQ_NEW_MATCH", seen: 0 }, + { type: "SQ_READY_CHECK", seen: 1 }, + ]); + }); + + test("meta filter only matches notifications with the given values", async () => { + await NotificationFactory.create({ + notification: { type: "SQ_NEW_MATCH", meta: { matchId: 1 } }, + users: [{ userId: users.id(1) }], + }); + await NotificationFactory.create({ + notification: { type: "SQ_NEW_MATCH", meta: { matchId: 2 } }, + users: [{ userId: users.id(1) }], + }); + + await NotificationRepository.markAsSeenByType({ + userIds: [users.id(1)], + type: "SQ_NEW_MATCH", + meta: { matchId: 2 }, + }); + + expect(await seenStatusOf(users.id(1))).toEqual([ + { type: "SQ_NEW_MATCH", seen: 1 }, + { type: "SQ_NEW_MATCH", seen: 0 }, + ]); + }); + + test("matches on a subset of meta keys, both string and number valued", async () => { + await NotificationFactory.create({ + notification: { + type: "SCRIM_NEW_REQUEST", + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 7 }, + }, + users: [{ userId: users.id(1) }], + }); + await NotificationFactory.create({ + notification: { + type: "SCRIM_NEW_REQUEST", + meta: { fromUserId: 2, fromUsername: "bob", scrimPostId: 7 }, + }, + users: [{ userId: users.id(1) }], + }); + + await NotificationRepository.markAsSeenByType({ + userIds: [users.id(1)], + type: "SCRIM_NEW_REQUEST", + meta: { fromUsername: "alice" }, + }); + + expect(await seenStatusOf(users.id(1))).toEqual([ + { type: "SCRIM_NEW_REQUEST", seen: 0 }, + { type: "SCRIM_NEW_REQUEST", seen: 1 }, + ]); + }); + + test("marks the notification as seen for every given user", async () => { + await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [ + { userId: users.id(1) }, + { userId: users.id(2) }, + { userId: users.id(3) }, + ], + }); + + await NotificationRepository.markAsSeenByType({ + userIds: [users.id(1), users.id(3)], + type: "SQ_READY_CHECK", + }); + + expect(await seenStatusOf(users.id(1))).toEqual([ + { type: "SQ_READY_CHECK", seen: 1 }, + ]); + expect(await seenStatusOf(users.id(2))).toEqual([ + { type: "SQ_READY_CHECK", seen: 0 }, + ]); + expect(await seenStatusOf(users.id(3))).toEqual([ + { type: "SQ_READY_CHECK", seen: 1 }, + ]); + }); +}); + +describe("markOwnAsSeen", () => { + beforeEach(async () => { + await users.create(2); + }); + + test("returns the actor's id when a notification flips to seen", async () => { + const notification = await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1) }], + }); + + const changedUserIds = await withUserId(users.id(1), () => + NotificationRepository.markOwnAsSeen([notification.id]), + ); + + expect(changedUserIds).toEqual([users.id(1)]); + expect(await seenStatusOf(users.id(1))).toEqual([ + { type: "SQ_READY_CHECK", seen: 1 }, + ]); + }); + + test("returns no user ids when the notifications were already seen", async () => { + const notification = await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1), seen: 1 }], + }); + + const changedUserIds = await withUserId(users.id(1), () => + NotificationRepository.markOwnAsSeen([notification.id]), + ); + + expect(changedUserIds).toEqual([]); + }); + + test("returns the actor's id once even if many notifications flip", async () => { + const notifications = await Promise.all([ + NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1) }], + }), + NotificationFactory.create({ + notification: { type: "SQ_NEW_MATCH", meta: { matchId: 1 } }, + users: [{ userId: users.id(1) }], + }), + ]); + + const changedUserIds = await withUserId(users.id(1), () => + NotificationRepository.markOwnAsSeen(notifications.map(({ id }) => id)), + ); + + expect(changedUserIds).toEqual([users.id(1)]); + }); + + test("leaves another user's copy of the notification unseen", async () => { + const notification = await NotificationFactory.create({ + notification: { type: "SQ_READY_CHECK" }, + users: [{ userId: users.id(1) }, { userId: users.id(2) }], + }); + + await withUserId(users.id(1), () => + NotificationRepository.markOwnAsSeen([notification.id]), + ); + + expect(await seenStatusOf(users.id(2))).toEqual([ + { type: "SQ_READY_CHECK", seen: 0 }, + ]); + }); +}); diff --git a/app/features/notifications/NotificationRepository.server.ts b/app/features/notifications/NotificationRepository.server.ts index 410175c57..06b4d1f06 100644 --- a/app/features/notifications/NotificationRepository.server.ts +++ b/app/features/notifications/NotificationRepository.server.ts @@ -1,4 +1,6 @@ import { sub } from "date-fns"; +import { sql } from "kysely"; +import * as R from "remeda"; import { db } from "~/db/sql"; import type { TablesInsertable } from "~/db/tables"; import type { NotificationSubscription } from "~/db/tables-json"; @@ -75,13 +77,75 @@ export function findAllByType(type: T) { .execute() as Promise>>; } -export function markOwnAsSeen(notificationIds: number[]) { - return db +/** + * Marks the users' unseen notifications of the given type as seen, optionally + * only those whose meta matches every given key/value pair. Used to clear the + * unseen dot when the user addresses what the notification is about. Returns + * the user ids whose rows actually changed. + * + * The correlated `exists` keeps this proportional to the users' own + * notifications. A `notificationId in (select ...)` reads the same but makes + * SQLite materialize every notification of the type (json_extract'ing each one) + * before touching the user's rows, which is ~80x slower on a hot path. + */ +export async function markAsSeenByType({ + userIds, + type, + meta, +}: { + userIds: number[]; + type: Notification["type"]; + meta?: Record; +}): Promise { + if (userIds.length === 0) return []; + + const updated = await db + .updateTable("NotificationUser") + .set("seen", 1) + .where("NotificationUser.seen", "=", 0) + .where("NotificationUser.userId", "in", userIds) + .where(({ exists, selectFrom, ref }) => { + let matchingNotification = selectFrom("Notification") + .select("Notification.id") + .whereRef( + "Notification.id", + "=", + ref("NotificationUser.notificationId"), + ) + .where("Notification.type", "=", type); + + for (const [key, value] of Object.entries(meta ?? {})) { + matchingNotification = matchingNotification.where( + sql`json_extract("Notification"."meta", ${`$.${key}`})`, + "=", + value, + ); + } + + return exists(matchingNotification); + }) + .returning("NotificationUser.userId") + .execute(); + + return R.unique(updated.map((row) => row.userId)); +} + +/** + * Marks the actor's notifications as seen. Returns the actor's user id in an + * array if any row actually changed (empty array otherwise), shaped for + * passing straight to `ChatSystemMessage.notifyNotificationsChanged`. + */ +export async function markOwnAsSeen(notificationIds: number[]) { + const updated = await db .updateTable("NotificationUser") .set("seen", 1) .where("NotificationUser.notificationId", "in", notificationIds) .where("NotificationUser.userId", "=", actorId()) + .where("NotificationUser.seen", "=", 0) + .returning("NotificationUser.userId") .execute(); + + return updated.length > 0 ? [updated[0].userId] : []; } export function deleteOld() { diff --git a/app/features/notifications/NotificationsProvider.tsx b/app/features/notifications/NotificationsProvider.tsx new file mode 100644 index 000000000..a103bd198 --- /dev/null +++ b/app/features/notifications/NotificationsProvider.tsx @@ -0,0 +1,227 @@ +import * as React from "react"; +import { + useFetcher, + useFetchers, + useLocation, + useNavigation, +} from "react-router"; +import { useChatContext } from "~/features/chat/useChatContext"; +import type { SerializeFrom } from "~/utils/remix"; +import { NOTIFICATIONS_DATA_ROUTE } from "~/utils/urls"; +import type { loader } from "./routes/api.notifications"; + +/** Spreads out the refetches when a notification fans out to many users at once. */ +const PING_REFRESH_JITTER_MS = 3_000; + +const WS_DOWN_POLL_MS = 2 * 60 * 1000; + +export type NotificationsData = SerializeFrom["notifications"]; + +interface NotificationsContextValue { + notifications?: NotificationsData; + /** Refetches the notification peek, without touching the page's own loaders. */ + refresh: () => void; +} + +const NotificationsContext = React.createContext({ + refresh: () => {}, +}); + +/** + * Serves the notification peek (bell popover + unseen dot) and keeps it fresh + * push-first: an initial fetch after mount (deliberately not part of any + * loader, so notifications never delay a page), then a refetch whenever skalop + * pings over the chat websocket that the user's notifications changed + * server-side (new notification, marked seen, resolved by an action + * elsewhere). Polling and refetch-on-activity heuristics only kick in as a + * fallback while the websocket is down. + */ +export function NotificationsProvider({ + user, + children, +}: { + user?: { id: number } | null; + children: React.ReactNode; +}) { + const fetcher = useFetcher(); + const chat = useChatContext(); + const { load } = fetcher; + + const loggedIn = Boolean(user); + const readyState = chat?.readyState ?? "CLOSED"; + const wsDown = loggedIn && readyState !== "CONNECTED"; + + // stable so effects that refresh after a mutation don't re-run every render + const refresh = React.useCallback( + () => load(NOTIFICATIONS_DATA_ROUTE), + [load], + ); + + React.useEffect(() => { + if (!loggedIn) return; + + refresh(); + }, [loggedIn, refresh]); + + useRefreshOnPing({ version: chat?.notificationsVersion ?? 0, refresh }); + useRefreshOnReconnect({ readyState, refresh }); + useRefreshOnVisible({ enabled: loggedIn, refresh }); + useFallbackPoll({ enabled: wsDown, refresh }); + + const notifications = fetcher.data?.notifications; + + useFallbackRefreshOnPotentialResolution({ + enabled: wsDown, + notifications, + refresh, + }); + + const value: NotificationsContextValue = { + notifications, + refresh, + }; + + return ( + + {children} + + ); +} + +/** The user's notification peek; `notifications` is `undefined` until the first fetch lands. */ +export function useNotificationsData() { + return React.useContext(NotificationsContext); +} + +function useRefreshOnPing({ + version, + refresh, +}: { + version: number; + refresh: () => void; +}) { + React.useEffect(() => { + if (version === 0) return; + + // jittered so a notification sent to a whole tournament's worth of users + // does not make every connected client refetch in the same instant; a + // follow-up ping inside the window replaces the pending refetch + const timeout = setTimeout(refresh, Math.random() * PING_REFRESH_JITTER_MS); + return () => clearTimeout(timeout); + }, [version, refresh]); +} + +/** Refetches after the websocket comes back up, covering pings missed while down. */ +function useRefreshOnReconnect({ + readyState, + refresh, +}: { + readyState: "CONNECTING" | "CONNECTED" | "CLOSED"; + refresh: () => void; +}) { + const hadConnectedRef = React.useRef(false); + + React.useEffect(() => { + if (readyState !== "CONNECTED") return; + + if (!hadConnectedRef.current) { + hadConnectedRef.current = true; + return; + } + + refresh(); + }, [readyState, refresh]); +} + +/** Refetches when the tab becomes visible again (e.g. waking from sleep). */ +function useRefreshOnVisible({ + enabled, + refresh, +}: { + enabled: boolean; + refresh: () => void; +}) { + React.useEffect(() => { + if (!enabled) return; + + const handleVisibilityChange = () => { + if (document.visibilityState === "visible") { + refresh(); + } + }; + + document.addEventListener("visibilitychange", handleVisibilityChange); + return () => + document.removeEventListener("visibilitychange", handleVisibilityChange); + }, [enabled, refresh]); +} + +function useFallbackPoll({ + enabled, + refresh, +}: { + enabled: boolean; + refresh: () => void; +}) { + React.useEffect(() => { + if (!enabled) return; + + const interval = setInterval(refresh, WS_DOWN_POLL_MS); + return () => clearInterval(interval); + }, [enabled, refresh]); +} + +/** + * Without the websocket there is no ping when something the user did resolves + * an unseen notification, so fall back to refetching after anything that may + * have: a navigation (loaders mark notifications seen when the user views the + * page a notification points at) or a settled action submission (actions mark + * them seen when the user addresses the thing itself). Only fires while an + * unseen notification exists, so it usually adds no server load even then. + */ +function useFallbackRefreshOnPotentialResolution({ + enabled, + notifications, + refresh, +}: { + enabled: boolean; + notifications: NotificationsData; + refresh: () => void; +}) { + const location = useLocation(); + const navigation = useNavigation(); + const fetchers = useFetchers(); + + const hasUnseen = + (enabled && notifications?.some((notification) => !notification.seen)) ?? + false; + + const submitting = + navigation.state === "submitting" || + fetchers.some((fetcher) => fetcher.state === "submitting"); + const allIdle = + navigation.state === "idle" && + fetchers.every((fetcher) => fetcher.state === "idle"); + + const refreshOnIdleRef = React.useRef(false); + if (submitting && hasUnseen) { + refreshOnIdleRef.current = true; + } + + const prevLocationKeyRef = React.useRef(location.key); + + React.useEffect(() => { + if (prevLocationKeyRef.current !== location.key) { + prevLocationKeyRef.current = location.key; + if (hasUnseen) { + refresh(); + } + return; + } + + if (allIdle && refreshOnIdleRef.current) { + refreshOnIdleRef.current = false; + refresh(); + } + }, [location.key, allIdle, hasUnseen, refresh]); +} diff --git a/app/features/notifications/components/NotificationList.tsx b/app/features/notifications/components/NotificationList.tsx index 2d13f7d0b..3ef05c437 100644 --- a/app/features/notifications/components/NotificationList.tsx +++ b/app/features/notifications/components/NotificationList.tsx @@ -33,7 +33,12 @@ export function NotificationItem({ onClick={onClose} > - {!notification.seen ?
: null} + {!notification.seen ? ( +
+ ) : null}
{t( diff --git a/app/features/notifications/core/notify.server.test.ts b/app/features/notifications/core/notify.server.test.ts index ecea0b87a..a08bb75f6 100644 --- a/app/features/notifications/core/notify.server.test.ts +++ b/app/features/notifications/core/notify.server.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import * as UserFactory from "~/db/seed/factories/UserFactory"; +import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { APP_ICON_URL } from "~/utils/urls"; import * as NotificationRepository from "../NotificationRepository.server"; import { notificationMeta } from "../notifications-utils"; @@ -21,10 +22,15 @@ vi.mock("./webPush.server", () => ({ }, })); +vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ + notifyNotificationsChanged: vi.fn(), +})); + describe("notify()", () => { beforeEach(async () => { await users.create(20); clearSentNotificationsForTesting(); + vi.mocked(ChatSystemMessage.notifyNotificationsChanged).mockClear(); }); test("different recipients receive same notification", async () => { @@ -32,7 +38,7 @@ describe("notify()", () => { userIds: [users.id(1), users.id(2)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "alice" }, + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 1 }, }, }); @@ -40,7 +46,7 @@ describe("notify()", () => { userIds: [users.id(3), users.id(4)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "alice" }, + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 1 }, }, }); @@ -64,7 +70,9 @@ describe("notify()", () => { expect(user1Notifications[0].type).toBe("SCRIM_NEW_REQUEST"); expect(notificationMeta(user1Notifications[0])).toEqual({ + fromUserId: 1, fromUsername: "alice", + scrimPostId: 1, }); }); @@ -96,6 +104,34 @@ describe("notify()", () => { expect(user6Notifications).toHaveLength(1); }); + test("pings recipients' websockets once per delivered notification", async () => { + await notify({ + userIds: [users.id(1), users.id(2)], + notification: { + type: "BADGE_ADDED", + meta: { badgeName: "Test", badgeId: 1 }, + }, + }); + + expect(ChatSystemMessage.notifyNotificationsChanged).toHaveBeenCalledWith([ + users.id(1), + users.id(2), + ]); + + // deduplicated resend delivers nothing, so it should not ping either + await notify({ + userIds: [users.id(1), users.id(2)], + notification: { + type: "BADGE_ADDED", + meta: { badgeName: "Test", badgeId: 1 }, + }, + }); + + expect(ChatSystemMessage.notifyNotificationsChanged).toHaveBeenCalledTimes( + 1, + ); + }); + test("identical notification is delivered again when repeated a day later", async () => { vi.useFakeTimers(); try { @@ -103,7 +139,7 @@ describe("notify()", () => { userIds: [users.id(5)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "alice" }, + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 1 }, }, }); @@ -113,7 +149,7 @@ describe("notify()", () => { userIds: [users.id(5)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "alice" }, + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 1 }, }, }); } finally { @@ -225,7 +261,7 @@ describe("notify()", () => { userIds: [users.id(12), users.id(13)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "bob" }, + meta: { fromUserId: 2, fromUsername: "bob", scrimPostId: 1 }, }, }); @@ -233,7 +269,7 @@ describe("notify()", () => { userIds: [users.id(12), users.id(13)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "charlie" }, + meta: { fromUserId: 3, fromUsername: "charlie", scrimPostId: 1 }, }, }); @@ -248,8 +284,16 @@ describe("notify()", () => { expect(user13Notifications).toHaveLength(2); const metas = user12Notifications.map(notificationMeta); - expect(metas).toContainEqual({ fromUsername: "bob" }); - expect(metas).toContainEqual({ fromUsername: "charlie" }); + expect(metas).toContainEqual({ + fromUserId: 2, + fromUsername: "bob", + scrimPostId: 1, + }); + expect(metas).toContainEqual({ + fromUserId: 3, + fromUsername: "charlie", + scrimPostId: 1, + }); }); test("duplicate user IDs in input array are deduplicated", async () => { @@ -312,7 +356,7 @@ describe("notify() - web push notifications", () => { userIds: [users.id(1)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "alice" }, + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 1 }, }, }); @@ -408,7 +452,7 @@ describe("notify() - web push notifications", () => { userIds: [users.id(1)], notification: { type: "SCRIM_NEW_REQUEST", - meta: { fromUsername: "alice" }, + meta: { fromUserId: 1, fromUsername: "alice", scrimPostId: 1 }, }, }); diff --git a/app/features/notifications/core/notify.server.ts b/app/features/notifications/core/notify.server.ts index dfbcff340..b53949208 100644 --- a/app/features/notifications/core/notify.server.ts +++ b/app/features/notifications/core/notify.server.ts @@ -2,6 +2,7 @@ import type { TFunction } from "i18next"; import pLimit from "p-limit"; import { type Urgency, WebPushError } from "web-push"; import type { NotificationSubscription } from "~/db/tables-json"; +import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { IS_E2E_TEST_RUN } from "~/utils/e2e"; import { APP_ICON_URL } from "~/utils/urls"; import { getFixedTForLanguage } from "../../../modules/i18n/i18next.server"; @@ -72,6 +73,7 @@ export async function notify({ seen: defaultSeenUserIds?.includes(userId) ? 1 : 0, })), ); + ChatSystemMessage.notifyNotificationsChanged(dededuplicatedUserIds); } catch (e) { logger.error("Failed to notify users", e); } diff --git a/app/features/notifications/core/resolve.server.ts b/app/features/notifications/core/resolve.server.ts new file mode 100644 index 000000000..9cbb5e8f1 --- /dev/null +++ b/app/features/notifications/core/resolve.server.ts @@ -0,0 +1,92 @@ +import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; +import { logger } from "~/utils/logger"; +import * as NotificationRepository from "../NotificationRepository.server"; +import type { Notification } from "../notifications-types"; + +type NotificationOfType = Extract< + Notification, + { type: T } +>; + +type MetaFilter = + NotificationOfType extends { meta: infer M } ? Partial : undefined; + +/** + * What resolves each notification type beyond opening the notification list. + * `null` means nothing does: the notification is informational, or resolving it + * would cost a query on a hot path for little gain (TO_ADDED_TO_TEAM, + * TO_TEST_CREATED). Exhaustive on purpose, so a new notification type has to + * pick a side, and only the types with a trigger can be resolved. + */ +const RESOLUTION_TRIGGERS = { + SQ_ADDED_TO_GROUP: "visits a SendouQ group page (preparing/looking)", + SQ_READY_CHECK: + "responds to the ready check, or it ends (match created or the check expired)", + SQ_NEW_MATCH: "visits the match page", + TO_ADDED_TO_TEAM: null, + TO_BRACKET_STARTED: "visits the tournament's brackets page", + TO_CHECK_IN_OPENED: "their team checks in (by a member or the organizer)", + TO_TEST_CREATED: null, + TO_LIKE_RECEIVED: + "visits the tournament's LFG page, or their group accepts a like", + TO_LIKE_ACCEPTED: "visits the tournament's LFG page", + BADGE_ADDED: null, + BADGE_MANAGER_ADDED: null, + TROPHY_SUBMITTED: + "a reviewer approves/declines the submission or it gets deleted (a lone approval that is not yet enough resolves the approver's own)", + TROPHY_SUBMISSION_ACCEPTED: null, + TROPHY_SUBMISSION_DECLINED: null, + PLUS_VOTING_STARTED: "casts their votes", + PLUS_SUGGESTION_ADDED: "visits the suggestions page of the tier", + TAGGED_TO_ART: null, + SEASON_STARTED: null, + SCRIM_NEW_REQUEST: + "a request for the post is accepted (settling the post), the request is canceled by its sender, or the post is deleted", + SCRIM_SCHEDULED: "visits the scrim's page, or the scrim gets canceled", + SCRIM_CANCELED: null, + SCRIM_STARTING_SOON: "visits the scrim's page, or the scrim gets canceled", + SCRIM_AUTO_DELETED: null, + COMMISSIONS_CLOSED: null, + FRIEND_REQUEST_RECEIVED: + "accepts or declines the request, or the sender cancels it", +} as const satisfies Record; + +type ResolvableNotificationType = { + [T in Notification["type"]]: (typeof RESOLUTION_TRIGGERS)[T] extends null + ? never + : T; +}[Notification["type"]]; + +/** + * Marks the users' unseen notifications of the given type as seen because they + * addressed the thing the notification is about, so the unseen dot only shows + * for notifications that still need the user's attention. Never throws; a + * failed resolution only logs since the caller's action/loader matters more. + * + * See `RESOLUTION_TRIGGERS` for what resolves each type. + */ +export async function resolveNotifications< + T extends ResolvableNotificationType, +>({ + userIds, + type, + meta, +}: { + /** Users whose notifications got addressed */ + userIds: Array; + /** Notification type to resolve */ + type: T; + /** Only notifications whose meta matches every given key/value pair are resolved (e.g. `{ tournamentId }`) */ + meta?: MetaFilter; +}) { + try { + const changedUserIds = await NotificationRepository.markAsSeenByType({ + userIds, + type, + meta, + }); + ChatSystemMessage.notifyNotificationsChanged(changedUserIds); + } catch (err) { + logger.error("Failed to resolve notifications", err); + } +} diff --git a/app/features/notifications/notifications-hooks.browser.test.tsx b/app/features/notifications/notifications-hooks.browser.test.tsx new file mode 100644 index 000000000..aca4523b7 --- /dev/null +++ b/app/features/notifications/notifications-hooks.browser.test.tsx @@ -0,0 +1,105 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { render } from "vitest-browser-react"; +import { useShowUnseenDot } from "./notifications-hooks"; + +const GRACE_MS = 10_000; + +function UnseenDot({ + notifications, +}: { + notifications: Array<{ createdAt: number; seen: number }>; +}) { + const showDot = useShowUnseenDot(notifications); + + return
{showDot ? "shown" : "hidden"}
; +} + +const dotStatus = (screen: Awaited>) => + screen.getByTestId("dot").element().textContent; + +/** Database timestamp (seconds) for a moment relative to the fake clock. */ +const createdAt = (offsetMs: number) => + Math.floor((Date.now() + offsetMs) / 1000); + +/** + * Runs the fake clock forward and lets React paint what the fired timers + * changed. React schedules its render through a MessageChannel, which fake + * timers do not control, so a message of our own posted afterwards is what + * tells us the render already happened. + */ +const advanceTimers = async (ms: number) => { + await vi.advanceTimersByTimeAsync(ms); + + return new Promise((resolve) => { + const channel = new MessageChannel(); + channel.port1.onmessage = () => resolve(); + channel.port2.postMessage(null); + }); +}; + +describe("useShowUnseenDot", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + test("shows the dot right away for a notification predating the session", async () => { + const screen = await render( + , + ); + + expect(dotStatus(screen)).toBe("shown"); + }); + + test("never shows the dot when every notification is seen", async () => { + const screen = await render( + , + ); + + expect(dotStatus(screen)).toBe("hidden"); + + await advanceTimers(GRACE_MS * 2); + + expect(dotStatus(screen)).toBe("hidden"); + }); + + test("holds the dot back until the grace period passes for one born mid-session", async () => { + const screen = await render( + , + ); + + expect(dotStatus(screen)).toBe("hidden"); + + await advanceTimers(GRACE_MS); + + expect(dotStatus(screen)).toBe("hidden"); + + await advanceTimers(2_000); + + expect(dotStatus(screen)).toBe("shown"); + }); + + test("shows the dot as soon as the earliest of many notifications is past the grace period", async () => { + const screen = await render( + , + ); + + expect(dotStatus(screen)).toBe("hidden"); + + await advanceTimers(GRACE_MS + 2_000); + + expect(dotStatus(screen)).toBe("shown"); + }); +}); diff --git a/app/features/notifications/notifications-hooks.ts b/app/features/notifications/notifications-hooks.ts index de61c63ae..f46b277d4 100644 --- a/app/features/notifications/notifications-hooks.ts +++ b/app/features/notifications/notifications-hooks.ts @@ -1,10 +1,13 @@ import * as React from "react"; import { useFetcher } from "react-router"; import { NOTIFICATIONS_MARK_AS_SEEN_ROUTE } from "~/utils/urls"; +import { useNotificationsData } from "./NotificationsProvider"; export function useMarkNotificationsAsSeen(unseenIds: number[]) { const fetcher = useFetcher(); + const { refresh } = useNotificationsData(); const submittedIdsRef = React.useRef(new Set()); + const refreshPendingRef = React.useRef(false); const { submit } = fetcher; React.useEffect(() => { @@ -12,6 +15,14 @@ export function useMarkNotificationsAsSeen(unseenIds: number[]) { // get submitted when the fetcher returns to idle if (fetcher.state !== "idle") return; + // the action's skalop ping also triggers a refetch, but only for clients + // with a live websocket; refetching here keeps the dot clearing promptly + // for the tab that did the marking either way + if (refreshPendingRef.current) { + refreshPendingRef.current = false; + refresh(); + } + const idsToSubmit = unseenIds.filter( (id) => !submittedIdsRef.current.has(id), ); @@ -20,6 +31,7 @@ export function useMarkNotificationsAsSeen(unseenIds: number[]) { for (const id of idsToSubmit) { submittedIdsRef.current.add(id); } + refreshPendingRef.current = true; submit( { notificationIds: idsToSubmit }, @@ -29,5 +41,93 @@ export function useMarkNotificationsAsSeen(unseenIds: number[]) { action: NOTIFICATIONS_MARK_AS_SEEN_ROUTE, }, ); - }, [submit, unseenIds, fetcher.state]); + }, [submit, unseenIds, fetcher.state, refresh]); +} + +const UNSEEN_DOT_GRACE_MS = 10_000; + +/** + * Whether the bell should show its unseen dot. An unseen notification born + * while the session is already open only counts once it has stayed unseen past + * a short grace period: one about something the user is already on their way + * to (e.g. a SendouQ match that just started, with the redirect to the match + * page a second away) resolves itself right after, and the dot flashing for + * it would be false signal. Notifications predating the session show the dot + * right away — anything that was going to resolve them (a loader of the page + * being landed on) already ran before the first notifications fetch. + */ +export function useShowUnseenDot( + notifications: Array<{ createdAt: number; seen: number }> | undefined, +) { + // time lives in state (only advanced by the timer below) because reading + // Date.now() during render would be frozen by the React Compiler's memoization + const [mountedAt] = React.useState(() => Date.now()); + const [now, setNow] = React.useState(mountedAt); + + const dotShowTimes = + notifications + ?.filter((notification) => !notification.seen) + .map((notification) => { + const createdAtMs = notification.createdAt * 1000; + + return createdAtMs <= mountedAt + ? mountedAt + : createdAtMs + UNSEEN_DOT_GRACE_MS; + }) ?? []; + + const showDot = dotShowTimes.some((showTime) => showTime <= now); + const nextShowTime = + !showDot && dotShowTimes.length > 0 ? Math.min(...dotShowTimes) : null; + + React.useEffect(() => { + if (nextShowTime === null) return; + + const timeout = setTimeout( + () => setNow(Date.now()), + Math.max(0, nextShowTime - Date.now()) + 100, + ); + return () => clearTimeout(timeout); + }, [nextShowTime]); + + return showDot; +} + +/** + * Ids of the notifications to show an unseen dot for, keeping the dot for as + * long as the list stays open. Opening the list marks its notifications as + * seen right away so the bell stops claiming there is something new, and this + * keeps the reader from losing track of which ones those were. + */ +export function useStickyUnseenIds( + notifications: Array<{ id: number; seen: number }>, +) { + const [unseenIds, setUnseenIds] = React.useState( + () => new Set(unseenIdsOf(notifications)), + ); + const [prevNotifications, setPrevNotifications] = + React.useState(notifications); + + if (prevNotifications !== notifications) { + setPrevNotifications(notifications); + setUnseenIds((prevUnseenIds) => { + const newUnseenIds = new Set(prevUnseenIds); + + for (const id of unseenIdsOf(notifications)) { + newUnseenIds.add(id); + } + + // optimize render by not updating state if nothing changed + if (newUnseenIds.size === prevUnseenIds.size) return prevUnseenIds; + + return newUnseenIds; + }); + } + + return unseenIds; +} + +function unseenIdsOf(notifications: Array<{ id: number; seen: number }>) { + return notifications + .filter((notification) => !notification.seen) + .map((notification) => notification.id); } diff --git a/app/features/notifications/notifications-types.ts b/app/features/notifications/notifications-types.ts index 43e0320ff..79f2ed06c 100644 --- a/app/features/notifications/notifications-types.ts +++ b/app/features/notifications/notifications-types.ts @@ -71,7 +71,10 @@ export type Notification = { adderUsername: string; adderDiscordId: string; artId: number } > | NotificationItem<"SEASON_STARTED", { seasonNth: number }> - | NotificationItem<"SCRIM_NEW_REQUEST", { fromUsername: string }> + | NotificationItem< + "SCRIM_NEW_REQUEST", + { fromUserId: number; fromUsername: string; scrimPostId: number } + > | NotificationItem< "SCRIM_SCHEDULED", { id: number; opponentTeamName: string } @@ -83,7 +86,10 @@ export type Notification = > | NotificationItem<"SCRIM_AUTO_DELETED", { at: number }> | NotificationItem<"COMMISSIONS_CLOSED", { discordId: string }> - | NotificationItem<"FRIEND_REQUEST_RECEIVED", { senderUsername: string }> + | NotificationItem< + "FRIEND_REQUEST_RECEIVED", + { senderId: number; senderUsername: string } + > | NotificationItem< "TO_LIKE_RECEIVED", { diff --git a/app/features/notifications/routes/api.notifications.ts b/app/features/notifications/routes/api.notifications.ts new file mode 100644 index 000000000..37fa09565 --- /dev/null +++ b/app/features/notifications/routes/api.notifications.ts @@ -0,0 +1,20 @@ +import { getUser } from "~/features/auth/core/user.server"; +import * as NotificationRepository from "../NotificationRepository.server"; +import { NOTIFICATIONS } from "../notifications-contants"; + +/** + * The notification peek shown in the bell popover. Fetched by + * `NotificationsProvider` whenever skalop pings that the user's notifications + * changed, instead of being polled with the rest of the app shell data. + */ +export const loader = async () => { + const user = getUser(); + + return { + notifications: user + ? await NotificationRepository.findByUserId(user.id, { + limit: NOTIFICATIONS.PEEK_COUNT, + }) + : undefined, + }; +}; diff --git a/app/features/notifications/routes/notifications.seen.ts b/app/features/notifications/routes/notifications.seen.ts index ec369e047..ef5544b61 100644 --- a/app/features/notifications/routes/notifications.seen.ts +++ b/app/features/notifications/routes/notifications.seen.ts @@ -1,4 +1,5 @@ import type { ActionFunctionArgs } from "react-router"; +import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { parseRequestPayload } from "~/utils/remix.server"; import * as NotificationRepository from "../NotificationRepository.server"; import { markAsSeenActionSchema } from "../notifications-schemas"; @@ -9,7 +10,11 @@ export const action = async ({ request }: ActionFunctionArgs) => { schema: markAsSeenActionSchema, }); - await NotificationRepository.markOwnAsSeen(data.notificationIds); + const changedUserIds = await NotificationRepository.markOwnAsSeen( + data.notificationIds, + ); + // so the unseen dot clears on the user's other open tabs and devices too + ChatSystemMessage.notifyNotificationsChanged(changedUserIds); return null; }; diff --git a/app/features/notifications/routes/notifications.tsx b/app/features/notifications/routes/notifications.tsx index 83f2f64e7..b4c937055 100644 --- a/app/features/notifications/routes/notifications.tsx +++ b/app/features/notifications/routes/notifications.tsx @@ -11,7 +11,10 @@ import { NotificationsList, } from "../components/NotificationList"; import { loader } from "../loaders/notifications.server"; -import { useMarkNotificationsAsSeen } from "../notifications-hooks"; +import { + useMarkNotificationsAsSeen, + useStickyUnseenIds, +} from "../notifications-hooks"; export { loader }; @@ -27,36 +30,7 @@ export const meta: MetaFunction = (args) => { export default function NotificationsPage() { const { t } = useTranslation(["common"]); const data = useLoaderData(); - const [unseenIds, setUnseenIds] = React.useState( - () => - new Set( - data.notifications - .filter((notification) => !notification.seen) - .map((notification) => notification.id), - ), - ); - const [prevNotifications, setPrevNotifications] = React.useState( - data.notifications, - ); - - // persist unseen dots for the duration of the page being viewed - if (prevNotifications !== data.notifications) { - setPrevNotifications(data.notifications); - setUnseenIds((prevUnseenIds) => { - const newUnseenIds = new Set(prevUnseenIds); - - for (const notification of data.notifications) { - if (!notification.seen) { - newUnseenIds.add(notification.id); - } - } - - // optimize render by not updating state if nothing changed - if (newUnseenIds.size === prevUnseenIds.size) return prevUnseenIds; - - return newUnseenIds; - }); - } + const unseenIds = useStickyUnseenIds(data.notifications); const unSeenIdsArr = React.useMemo(() => Array.from(unseenIds), [unseenIds]); diff --git a/app/features/plus-suggestions/loaders/plus.suggestions.server.ts b/app/features/plus-suggestions/loaders/plus.suggestions.server.ts index 30c99def7..ff252d0e8 100644 --- a/app/features/plus-suggestions/loaders/plus.suggestions.server.ts +++ b/app/features/plus-suggestions/loaders/plus.suggestions.server.ts @@ -1,5 +1,6 @@ import type { LoaderFunctionArgs } from "react-router"; import { getUser } from "~/features/auth/core/user.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as PlusSuggestionRepository from "~/features/plus-suggestions/PlusSuggestionRepository.server"; import { nextNonCompletedVoting, @@ -29,6 +30,14 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { const user = getUser(); const monthYear = rangeToMonthYear(nextVotingRange); + if (user) { + await resolveNotifications({ + userIds: [user.id], + type: "PLUS_SUGGESTION_ADDED", + meta: { tier: shownTier }, + }); + } + const [suggestions, summary] = await Promise.all([ PlusSuggestionRepository.findAllByMonth({ ...monthYear, tier: shownTier }), PlusSuggestionRepository.findMonthSummary({ diff --git a/app/features/plus-voting/actions/plus.voting.server.ts b/app/features/plus-voting/actions/plus.voting.server.ts index 5117d3db0..2ca6e74eb 100644 --- a/app/features/plus-voting/actions/plus.voting.server.ts +++ b/app/features/plus-voting/actions/plus.voting.server.ts @@ -1,5 +1,6 @@ import type { ActionFunction } from "react-router"; import { requireUser } from "~/features/auth/core/user.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import type { PlusVoteFromFE } from "~/features/plus-voting/core"; import { nextNonCompletedVoting, @@ -63,6 +64,11 @@ export const action: ActionFunction = async ({ request }) => { })), ); + await resolveNotifications({ + userIds: [user.id], + type: "PLUS_VOTING_STARTED", + }); + return null; }; diff --git a/app/features/scrims/actions/scrims.$id.server.ts b/app/features/scrims/actions/scrims.$id.server.ts index c6b121011..a1430b781 100644 --- a/app/features/scrims/actions/scrims.$id.server.ts +++ b/app/features/scrims/actions/scrims.$id.server.ts @@ -1,6 +1,7 @@ import type { ActionFunctionArgs } from "react-router"; import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { parseFormData } from "~/form/parse.server"; import { requirePermission } from "~/modules/permissions/guards.server"; import { @@ -74,6 +75,22 @@ export const action = async ({ request, params }: ActionFunctionArgs) => { meta: { id: post.id, opponentTeamName: postTeamName }, }, }); + + // the canceled scrim is no longer happening + const participantIds = [ + ...post.users.map((m) => m.id), + ...acceptedRequest.users.map((m) => m.id), + ]; + await resolveNotifications({ + userIds: participantIds, + type: "SCRIM_SCHEDULED", + meta: { id: post.id }, + }); + await resolveNotifications({ + userIds: participantIds, + type: "SCRIM_STARTING_SOON", + meta: { id: post.id }, + }); } break; diff --git a/app/features/scrims/actions/scrims.server.ts b/app/features/scrims/actions/scrims.server.ts index 97669a768..7deeaf2b7 100644 --- a/app/features/scrims/actions/scrims.server.ts +++ b/app/features/scrims/actions/scrims.server.ts @@ -7,6 +7,7 @@ import { requireUser } from "~/features/auth/core/user.server"; import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { datePlaceholder } from "~/features/chat/chat-utils"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as UserRepository from "~/features/user-page/UserRepository.server"; import { parseFormData } from "~/form/parse.server"; import { requirePermission } from "~/modules/permissions/guards.server"; @@ -58,6 +59,13 @@ export const action = async ({ request }: ActionFunctionArgs) => { await ScrimPostRepository.deleteById(post.id); + // requests to the deleted post can no longer be accepted + await resolveNotifications({ + userIds: post.users.filter((u) => u.isOwner).map((u) => u.id), + type: "SCRIM_NEW_REQUEST", + meta: { scrimPostId: post.id }, + }); + break; } case "NEW_REQUEST": { @@ -129,7 +137,9 @@ export const action = async ({ request }: ActionFunctionArgs) => { notification: { type: "SCRIM_NEW_REQUEST", meta: { + fromUserId: user.id, fromUsername: user.username, + scrimPostId: post.id, }, }, }); @@ -155,6 +165,13 @@ export const action = async ({ request }: ActionFunctionArgs) => { throw error; } + // accepting one request settles the post, the rest can no longer be accepted + await resolveNotifications({ + userIds: post.users.filter((u) => u.isOwner).map((u) => u.id), + type: "SCRIM_NEW_REQUEST", + meta: { scrimPostId: post.id }, + }); + const fullPost = await ScrimPostRepository.findById(post.id); if (fullPost?.chatCode) { ChatSystemMessage.setMetadata({ @@ -230,6 +247,11 @@ export const action = async ({ request }: ActionFunctionArgs) => { meta: { at: removed.startsAt }, }, }); + await resolveNotifications({ + userIds: removed.memberIds, + type: "SCRIM_NEW_REQUEST", + meta: { scrimPostId: removed.id }, + }); } } catch (error) { logger.error("Failed to auto-cancel overlapping scrims", error); @@ -239,7 +261,7 @@ export const action = async ({ request }: ActionFunctionArgs) => { break; } case "CANCEL_REQUEST": { - const { request } = await findRequest({ + const { post, request } = await findRequest({ requestId: data.scrimPostRequestId, }); requirePermission(request, "CANCEL"); @@ -251,6 +273,18 @@ export const action = async ({ request }: ActionFunctionArgs) => { await ScrimPostRepository.deleteRequest(data.scrimPostRequestId); + const requestOwner = request.users.find((u) => u.isOwner); + if (requestOwner) { + await resolveNotifications({ + userIds: post.users.filter((u) => u.isOwner).map((u) => u.id), + type: "SCRIM_NEW_REQUEST", + meta: { + scrimPostId: post.id, + fromUserId: requestOwner.id, + }, + }); + } + break; } case "PERSIST_SCRIM_FILTERS": { diff --git a/app/features/scrims/loaders/scrims.$id.server.ts b/app/features/scrims/loaders/scrims.$id.server.ts index a2a06409e..89773e90b 100644 --- a/app/features/scrims/loaders/scrims.$id.server.ts +++ b/app/features/scrims/loaders/scrims.$id.server.ts @@ -1,5 +1,6 @@ import type { LoaderFunctionArgs } from "react-router"; import { chatAccessible } from "~/features/chat/chat-utils"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as UserCardRepository from "~/features/user-card/UserCardRepository.server"; import * as UserRepository from "~/features/user-page/UserRepository.server"; import { databaseTimestampToDate } from "~/utils/dates"; @@ -29,6 +30,17 @@ export const loader = async ({ params }: LoaderFunctionArgs) => { throw new Response(null, { status: 403 }); } + await resolveNotifications({ + userIds: [user.id], + type: "SCRIM_SCHEDULED", + meta: { id: post.id }, + }); + await resolveNotifications({ + userIds: [user.id], + type: "SCRIM_STARTING_SOON", + meta: { id: post.id }, + }); + const participantIds = Scrim.participantIdsListFromAccepted(post); const anyUserPrefersNoScreen = diff --git a/app/features/sendouq-match/loaders/q.match.$id.server.ts b/app/features/sendouq-match/loaders/q.match.$id.server.ts index 60610833f..b33e38a56 100644 --- a/app/features/sendouq-match/loaders/q.match.$id.server.ts +++ b/app/features/sendouq-match/loaders/q.match.$id.server.ts @@ -2,6 +2,7 @@ import type { LoaderFunctionArgs } from "react-router"; import { getUser } from "~/features/auth/core/user.server"; import { chatAccessible } from "~/features/chat/chat-utils"; import * as Seasons from "~/features/mmr/core/Seasons"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { SendouQ } from "~/features/sendouq/core/SendouQ.server"; import * as ReportedWeaponRepository from "~/features/sendouq-match/ReportedWeaponRepository.server"; import * as SQMatchRepository from "~/features/sendouq-match/SQMatchRepository.server"; @@ -30,6 +31,14 @@ export const loader = async ({ params }: LoaderFunctionArgs) => { const isStaff = user?.roles.includes("STAFF") ?? false; const isParticipant = Boolean(user && matchUsers.includes(user.id)); + if (user && isParticipant) { + await resolveNotifications({ + userIds: [user.id], + type: "SQ_NEW_MATCH", + meta: { matchId }, + }); + } + const reportedWeapons = await ReportedWeaponRepository.findByMatchId(matchId); const match = SendouQ.mapMatch(matchUnmapped, user); diff --git a/app/features/sendouq/SQGroupRepository.server.ts b/app/features/sendouq/SQGroupRepository.server.ts index 358eb5c9a..c3d144c72 100644 --- a/app/features/sendouq/SQGroupRepository.server.ts +++ b/app/features/sendouq/SQGroupRepository.server.ts @@ -873,10 +873,29 @@ export async function findReadyCheckByGroupId(groupId: number) { export function findAllReadyChecksStartedBefore(date: Date) { return db .selectFrom("GroupReadyCheck") - .select([ + .select((eb) => [ "GroupReadyCheck.id", "GroupReadyCheck.alphaGroupId", "GroupReadyCheck.bravoGroupId", + jsonArrayFrom( + eb + .selectFrom("GroupMember") + .select("GroupMember.userId") + .where((innerEb) => + innerEb.or([ + innerEb( + "GroupMember.groupId", + "=", + innerEb.ref("GroupReadyCheck.alphaGroupId"), + ), + innerEb( + "GroupMember.groupId", + "=", + innerEb.ref("GroupReadyCheck.bravoGroupId"), + ), + ]), + ), + ).as("members"), ]) .where("GroupReadyCheck.createdAt", "<", dateToDatabaseTimestamp(date)) .execute(); diff --git a/app/features/sendouq/core/ready-check.server.test.ts b/app/features/sendouq/core/ready-check.server.test.ts index 2186282aa..f73fd2c2e 100644 --- a/app/features/sendouq/core/ready-check.server.test.ts +++ b/app/features/sendouq/core/ready-check.server.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/features/sendouq/core/ready-check.server.ts b/app/features/sendouq/core/ready-check.server.ts index 7b700d52a..e7501be6a 100644 --- a/app/features/sendouq/core/ready-check.server.ts +++ b/app/features/sendouq/core/ready-check.server.ts @@ -1,6 +1,7 @@ import { addMinutes } from "date-fns"; import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as SQGroupRepository from "~/features/sendouq/SQGroupRepository.server"; import { createMatchMemento, @@ -125,6 +126,8 @@ export async function confirm({ // the ready check ended (e.g. ran out of time) while this request was in flight if (!confirmation) return null; + await resolveNotifications({ userIds: [userId], type: "SQ_READY_CHECK" }); + if (!confirmation.everyoneIsReady) { revalidateGroups(readyCheck); @@ -143,12 +146,19 @@ export async function expire(readyCheck: { id: number; alphaGroupId: number; bravoGroupId: number; + members: Array<{ userId: number }>; }) { await SQGroupRepository.deleteReadyCheck({ id: readyCheck.id, markMissedMembers: true, }); + // the ready check no longer exists so there is nothing to respond to + await resolveNotifications({ + userIds: readyCheck.members.map((member) => member.userId), + type: "SQ_READY_CHECK", + }); + await refreshSendouQInstance(); revalidateGroups(readyCheck); @@ -240,6 +250,12 @@ async function createMatch({ }, }); + // the match superseded the ready check everyone was notified about + await resolveNotifications({ + userIds: readyCheck.members.map((member) => member.userId), + type: "SQ_READY_CHECK", + }); + return createdMatch.id; } diff --git a/app/features/sendouq/loaders/q.looking.server.ts b/app/features/sendouq/loaders/q.looking.server.ts index a22620454..3f82f210b 100644 --- a/app/features/sendouq/loaders/q.looking.server.ts +++ b/app/features/sendouq/loaders/q.looking.server.ts @@ -1,6 +1,7 @@ import type { LoaderFunctionArgs } from "react-router"; import * as R from "remeda"; import { requireUser } from "~/features/auth/core/user.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as SQGroupRepository from "~/features/sendouq/SQGroupRepository.server"; import { cachedStreams } from "~/features/sendouq-streams/core/streams.server"; import * as UserCardRepository from "~/features/user-card/UserCardRepository.server"; @@ -28,6 +29,13 @@ export const loader = async ({ url }: LoaderFunctionArgs) => { }); } + if (ownGroup) { + await resolveNotifications({ + userIds: [user.id], + type: "SQ_ADDED_TO_GROUP", + }); + } + const groupsToShow = ownGroup && groupExpiryStatus(ownGroup.latestActionAt) === "EXPIRED" ? [] diff --git a/app/features/sendouq/loaders/q.preparing.server.ts b/app/features/sendouq/loaders/q.preparing.server.ts index bdd70f1a9..ee5f7f35a 100644 --- a/app/features/sendouq/loaders/q.preparing.server.ts +++ b/app/features/sendouq/loaders/q.preparing.server.ts @@ -1,4 +1,5 @@ import { requireUser } from "~/features/auth/core/user.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { SendouQ } from "../core/SendouQ.server"; import { sqRedirectIfNeeded } from "../q-utils.server"; @@ -12,6 +13,11 @@ export const loader = async () => { currentLocation: "preparing", }); + await resolveNotifications({ + userIds: [user.id], + type: "SQ_ADDED_TO_GROUP", + }); + return { group: ownGroup!, }; diff --git a/app/features/sendouq/routes/q.looking.test.ts b/app/features/sendouq/routes/q.looking.test.ts index e69b33d9c..bf3c9024b 100644 --- a/app/features/sendouq/routes/q.looking.test.ts +++ b/app/features/sendouq/routes/q.looking.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/features/sendouq/routes/q.ready.test.ts b/app/features/sendouq/routes/q.ready.test.ts index 877dd8a12..a2f623d12 100644 --- a/app/features/sendouq/routes/q.ready.test.ts +++ b/app/features/sendouq/routes/q.ready.test.ts @@ -3,6 +3,7 @@ import { describe, expect, test, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/features/tournament-admin/actions/to.$id.admin.index.server.ts b/app/features/tournament-admin/actions/to.$id.admin.index.server.ts index 7b790ab3d..b5aa1f204 100644 --- a/app/features/tournament-admin/actions/to.$id.admin.index.server.ts +++ b/app/features/tournament-admin/actions/to.$id.admin.index.server.ts @@ -3,6 +3,7 @@ import * as R from "remeda"; import { db } from "~/db/sql"; import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import * as ShowcaseTournaments from "~/features/front-page/core/ShowcaseTournaments.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as TournamentTeamRepository from "~/features/tournament/TournamentTeamRepository.server"; import { endDroppedTeamMatches } from "~/features/tournament/tournament-utils.server"; import * as BracketRepository from "~/features/tournament-bracket/BracketRepository.server"; @@ -56,6 +57,14 @@ export const action: ActionFunction = async ({ request, params }) => { bracketIdx: bracket.sources ? data.bracketIdx : undefined, }); + if (!bracket.sources) { + await resolveNotifications({ + userIds: team.memberUserIds, + type: "TO_CHECK_IN_OPENED", + meta: { tournamentId }, + }); + } + break; } case "CHECK_OUT": { diff --git a/app/features/tournament-bracket/loaders/to.$id.brackets.server.ts b/app/features/tournament-bracket/loaders/to.$id.brackets.server.ts index 82f0ef980..65adc1111 100644 --- a/app/features/tournament-bracket/loaders/to.$id.brackets.server.ts +++ b/app/features/tournament-bracket/loaders/to.$id.brackets.server.ts @@ -1,4 +1,5 @@ import type { LoaderFunctionArgs } from "react-router"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import type { SerializeFrom } from "~/utils/remix"; import type { Tournament } from "../core/Tournament"; import { @@ -24,6 +25,14 @@ export const loader = async ({ params, request }: LoaderFunctionArgs) => { ); const bracket = tournament.bracketByIdx(bracketIdx); + if (user) { + await resolveNotifications({ + userIds: [user.id], + type: "TO_BRACKET_STARTED", + meta: { tournamentId: tournament.ctx.id, bracketIdx }, + }); + } + return { bracketIdx, bracket: bracket ? serializeBracket(bracket) : null, diff --git a/app/features/tournament-lfg/actions/to.$id.looking.server.ts b/app/features/tournament-lfg/actions/to.$id.looking.server.ts index 87b9d1091..07e46b708 100644 --- a/app/features/tournament-lfg/actions/to.$id.looking.server.ts +++ b/app/features/tournament-lfg/actions/to.$id.looking.server.ts @@ -1,6 +1,7 @@ import type { ActionFunctionArgs } from "react-router"; import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { requireNotBannedByOrganization } from "~/features/tournament/tournament-utils.server"; import { clearTournamentDataCache, @@ -223,6 +224,12 @@ export const action = async ({ request, params }: ActionFunctionArgs) => { }, }); + await resolveNotifications({ + userIds: ownGroup.members.map((m) => m.id), + type: "TO_LIKE_RECEIVED", + meta: { tournamentId }, + }); + break; } case "GIVE_MANAGER": { diff --git a/app/features/tournament-lfg/loaders/to.$id.looking.server.ts b/app/features/tournament-lfg/loaders/to.$id.looking.server.ts index 4dc428394..b7f51a54f 100644 --- a/app/features/tournament-lfg/loaders/to.$id.looking.server.ts +++ b/app/features/tournament-lfg/loaders/to.$id.looking.server.ts @@ -1,6 +1,7 @@ import type { LoaderFunctionArgs } from "react-router"; import * as R from "remeda"; import type { getUser } from "~/features/auth/core/user.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { tournamentFromDBCached, tournamentFromParams, @@ -38,6 +39,19 @@ export const loader = async ({ params }: LoaderFunctionArgs) => { throw new Response(null, { status: 404 }); } + if (user) { + await resolveNotifications({ + userIds: [user.id], + type: "TO_LIKE_RECEIVED", + meta: { tournamentId }, + }); + await resolveNotifications({ + userIds: [user.id], + type: "TO_LIKE_ACCEPTED", + meta: { tournamentId }, + }); + } + if (tournament.registrationOpen) { return lookingMode({ tournamentId, user }); } diff --git a/app/features/tournament-match/routes/to.$id.matches.$mid.test.ts b/app/features/tournament-match/routes/to.$id.matches.$mid.test.ts index e1505ca1c..7a42684b2 100644 --- a/app/features/tournament-match/routes/to.$id.matches.$mid.test.ts +++ b/app/features/tournament-match/routes/to.$id.matches.$mid.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/features/tournament/actions/to.$id.register.server.ts b/app/features/tournament/actions/to.$id.register.server.ts index 116a6e77a..5eade0d7f 100644 --- a/app/features/tournament/actions/to.$id.register.server.ts +++ b/app/features/tournament/actions/to.$id.register.server.ts @@ -3,6 +3,7 @@ import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import * as ShowcaseTournaments from "~/features/front-page/core/ShowcaseTournaments.server"; import { MapPool } from "~/features/map-list-generator/core/map-pool"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as SQGroupRepository from "~/features/sendouq/SQGroupRepository.server"; import * as TeamRepository from "~/features/team/TeamRepository.server"; import * as SavedCalendarEventRepository from "~/features/tournament/SavedCalendarEventRepository.server"; @@ -260,6 +261,12 @@ export const action: ActionFunction = async ({ request, params }) => { logger.info( `Checking in (success): tournament team id: ${teamMemberOf.id} - user id: ${user.id} - tournament id: ${tournamentId}`, ); + + await resolveNotifications({ + userIds: teamMemberOf.memberUserIds, + type: "TO_CHECK_IN_OPENED", + meta: { tournamentId }, + }); break; } case "ADD_PLAYER": { diff --git a/app/features/trophies/actions/trophies.new.server.ts b/app/features/trophies/actions/trophies.new.server.ts index 93820f207..cd62702dd 100644 --- a/app/features/trophies/actions/trophies.new.server.ts +++ b/app/features/trophies/actions/trophies.new.server.ts @@ -5,6 +5,7 @@ import { requireUser, } from "~/features/auth/core/user.server"; import { notify } from "~/features/notifications/core/notify.server"; +import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import { clearTrophiesCache } from "~/features/trophies/loaders/trophies.server"; import * as UserRepository from "~/features/user-page/UserRepository.server"; import { parseFormData } from "~/form/parse.server"; @@ -121,6 +122,8 @@ export const action: ActionFunction = async ({ request }) => { errorToastIfFalsy(isOwner || canReview, "Not allowed"); await TrophyRepository.deletePending(data.pendingTrophyId); + + await resolveSubmittedNotification(pending.name); return null; } case "DECLINE": { @@ -153,6 +156,8 @@ export const action: ActionFunction = async ({ request }) => { }); } + await resolveSubmittedNotification(pending.name); + return null; } case "APPROVE": { @@ -190,6 +195,15 @@ export const action: ActionFunction = async ({ request }) => { }, }); } + + await resolveSubmittedNotification(pending.name); + } else { + // still needs approvals from the other reviewers + await resolveNotifications({ + userIds: [user.id], + type: "TROPHY_SUBMITTED", + meta: { trophyName: pending.name }, + }); } return null; @@ -200,6 +214,14 @@ export const action: ActionFunction = async ({ request }) => { } }; +function resolveSubmittedNotification(trophyName: string) { + return resolveNotifications({ + userIds: [ADMIN_ID, ...QA_IDS], + type: "TROPHY_SUBMITTED", + meta: { trophyName }, + }); +} + async function notifyReviewersOfSubmission({ trophyName, submitter, diff --git a/app/root.tsx b/app/root.tsx index f3afac9b5..feae410e7 100644 --- a/app/root.tsx +++ b/app/root.tsx @@ -45,6 +45,7 @@ import { ChatProvider } from "./features/chat/ChatProvider"; import { isMatchResultsScopedRevalidation } from "./features/chat/revalidation-scope"; import { getSidenavSession } from "./features/layout/core/sidenav-session.server"; import { LayoutDataProvider } from "./features/layout/LayoutDataProvider"; +import { NotificationsProvider } from "./features/notifications/NotificationsProvider"; import { sessionIdMiddleware } from "./features/session-id/session-id-middleware.server"; import { isTheme, @@ -241,9 +242,11 @@ function Document({ - - {children} - + + + {children} + + diff --git a/app/routes.ts b/app/routes.ts index 508891207..e51345d88 100644 --- a/app/routes.ts +++ b/app/routes.ts @@ -320,6 +320,10 @@ export default [ route("/admin/streams", "features/admin/routes/admin.streams.tsx"), route("/api/chat-users", "features/chat/routes/api.chat-users.ts"), route("/api/layout", "features/layout/routes/api.layout.ts"), + route( + "/api/notifications", + "features/notifications/routes/api.notifications.ts", + ), route("/api", "features/api/routes/api.tsx"), ...prefix("/a", [ diff --git a/app/routines/closeExpiredContinueVotes.test.ts b/app/routines/closeExpiredContinueVotes.test.ts index f9a6c940d..96a416099 100644 --- a/app/routines/closeExpiredContinueVotes.test.ts +++ b/app/routines/closeExpiredContinueVotes.test.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/routines/resolveStaleSQMatches.test.ts b/app/routines/resolveStaleSQMatches.test.ts index d129541f5..d2e06bf8c 100644 --- a/app/routines/resolveStaleSQMatches.test.ts +++ b/app/routines/resolveStaleSQMatches.test.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("~/features/chat/ChatSystemMessage.server", () => ({ send: vi.fn(), + notifyNotificationsChanged: vi.fn(), removeRoom: vi.fn(), setMetadata: vi.fn(), })); diff --git a/app/utils/urls.ts b/app/utils/urls.ts index be5052bcb..a55de9386 100644 --- a/app/utils/urls.ts +++ b/app/utils/urls.ts @@ -214,6 +214,7 @@ export const PATRONS_LIST_ROUTE = "/patrons-list"; export const LAYOUT_DATA_ROUTE = "/api/layout"; export const NOTIFICATIONS_URL = "/notifications"; export const NOTIFICATIONS_MARK_AS_SEEN_ROUTE = "/notifications/seen"; +export const NOTIFICATIONS_DATA_ROUTE = "/api/notifications"; export const userCardFriendshipPage = (userId: number) => `/user-card/${userId}/friendship`; diff --git a/e2e/friends.spec.ts b/e2e/friends.spec.ts index 42e1dff0a..b18c7e99d 100644 --- a/e2e/friends.spec.ts +++ b/e2e/friends.spec.ts @@ -1,6 +1,7 @@ import { NZAP_TEST_ID } from "~/db/seed/constants"; import { expect, impersonate, test } from "./helpers/playwright"; import { FriendsPage } from "./pages/friends/friends-page"; +import { NotificationPopover } from "./pages/layout/notification-popover"; test.describe("Friends", () => { test("send friend request, accept it, then delete friend", async ({ @@ -17,9 +18,16 @@ test.describe("Friends", () => { await impersonate(page, NZAP_TEST_ID); await friends.goto(); + const notifications = new NotificationPopover(page); + await expect(notifications.locators.bellDot).toBeVisible(); + await expect(friends.locators.acceptButton).toBeVisible(); await friends.acceptRequest(); + // accepting resolved the friend request notification without the bell + // having been opened + await expect(notifications.locators.bellDot).toBeHidden(); + await friends.friend("Sendou").deleteFriend(); await expect(friends.locators.noFriendsText).toBeVisible(); diff --git a/e2e/notifications.spec.ts b/e2e/notifications.spec.ts new file mode 100644 index 000000000..b53fda90c --- /dev/null +++ b/e2e/notifications.spec.ts @@ -0,0 +1,38 @@ +import { ADMIN_ID } from "~/features/admin/admin-constants"; +import { expect, impersonate, navigate, test } from "./helpers/playwright"; +import { NotificationPopover } from "./pages/layout/notification-popover"; + +const UNSEEN_COUNT = 2; + +test.describe("Notifications", () => { + test("opening the popover clears the bell dot but keeps the unseen dots listed", async ({ + page, + factories, + }) => { + for (let seasonNth = 1; seasonNth <= UNSEEN_COUNT; seasonNth++) { + await factories.NotificationFactory.create({ + notification: { type: "SEASON_STARTED", meta: { seasonNth } }, + users: [{ userId: ADMIN_ID, seen: 0 }], + }); + } + + await impersonate(page); + await navigate({ page, url: "/" }); + + const notifications = new NotificationPopover(page); + await expect(notifications.locators.bellDot).toBeVisible(); + + await notifications.open(); + + await expect(notifications.locators.bellDot).toBeHidden(); + await expect(notifications.locators.unseenDots).toHaveCount(UNSEEN_COUNT); + + await notifications.close(); + await expect(notifications.locators.items).toHaveCount(0); + + await notifications.open(); + + await expect(notifications.locators.items).toHaveCount(UNSEEN_COUNT); + await expect(notifications.locators.unseenDots).toHaveCount(0); + }); +}); diff --git a/e2e/pages/friends/friends-page.ts b/e2e/pages/friends/friends-page.ts index 50d2a3269..08fcdaa7b 100644 --- a/e2e/pages/friends/friends-page.ts +++ b/e2e/pages/friends/friends-page.ts @@ -60,7 +60,9 @@ class FriendMenu { constructor(page: Page, name: string) { this.page = page; - this.trigger = page.getByRole("button", { name }); + // scoped to the page content because the sidebar's friends section shows + // a button with the same name once the friendship data refreshes + this.trigger = page.getByRole("main").getByRole("button", { name }); } async deleteFriend() { diff --git a/e2e/pages/layout/notification-popover.ts b/e2e/pages/layout/notification-popover.ts index 11386eae7..fd746767b 100644 --- a/e2e/pages/layout/notification-popover.ts +++ b/e2e/pages/layout/notification-popover.ts @@ -12,6 +12,10 @@ export class NotificationPopover { openButton: this.page.getByTestId("notifications-button"), items: this.page.getByTestId("notification-item"), seeAllLink: this.page.getByTestId("notifications-see-all-button"), + /** Shown on the bell while unseen notifications exist. */ + bellDot: this.page.getByTestId("notifications-bell-dot"), + /** Per notification, marking it as one the user has not read yet. */ + unseenDots: this.page.getByTestId("notification-unseen-dot"), }; } @@ -19,6 +23,10 @@ export class NotificationPopover { await this.locators.openButton.click(); } + async close() { + await this.page.keyboard.press("Escape"); + } + notification(text: string) { return this.locators.items.filter({ hasText: text }); } diff --git a/e2e/scrims.spec.ts b/e2e/scrims.spec.ts index 062d46aab..3196712b7 100644 --- a/e2e/scrims.spec.ts +++ b/e2e/scrims.spec.ts @@ -14,6 +14,7 @@ import { test, } from "./helpers/playwright"; import { AnythingAdder } from "./pages/layout/anything-adder"; +import { NotificationPopover } from "./pages/layout/notification-popover"; import { NewScrimPostPage } from "./pages/scrims/new-scrim-post-page"; import { ScrimPage } from "./pages/scrims/scrim-page"; import { ScrimsPage } from "./pages/scrims/scrims-page"; @@ -128,14 +129,35 @@ test.describe("Scrims", () => { }); test("accepts a request", async ({ page, factories }) => { - await createPostWithRequest(factories, { ownerUserId: ADMIN_ID }); + const post = await createPostWithRequest(factories, { + ownerUserId: ADMIN_ID, + }); + await factories.NotificationFactory.create({ + notification: { + type: "SCRIM_NEW_REQUEST", + meta: { + fromUserId: NZAP_TEST_ID, + fromUsername: "N-ZAP", + scrimPostId: post.id, + }, + }, + users: [{ userId: ADMIN_ID }], + }); await impersonate(page, ADMIN_ID); const scrims = new ScrimsPage(page); await scrims.goto(); + + const notifications = new NotificationPopover(page); + await expect(notifications.locators.bellDot).toBeVisible(); + await scrims.acceptFirstRequest(); + // accepting settled the post, resolving the request notification without + // the bell having been opened + await expect(notifications.locators.bellDot).toBeHidden(); + await scrims.openTab("booked"); await expect(scrims.locators.contactLinks).toHaveCount(1); diff --git a/e2e/sendouq.spec.ts b/e2e/sendouq.spec.ts index 4be5126e8..f8e8b6ab9 100644 --- a/e2e/sendouq.spec.ts +++ b/e2e/sendouq.spec.ts @@ -13,6 +13,7 @@ import { runRoutine, test, } from "./helpers/playwright"; +import { NotificationPopover } from "./pages/layout/notification-popover"; import { SendouQLookingPage } from "./pages/sendouq/sendouq-looking-page"; import { SendouQPage } from "./pages/sendouq/sendouq-page"; import { SendouQReadyPage } from "./pages/sendouq/sendouq-ready-page"; @@ -263,22 +264,36 @@ test.describe("SendouQ", () => { await expect(ready.locators.membersReady).toHaveCount(1); await expect(ready.locators.confirmedText).toBeVisible(); + const notifications = new NotificationPopover(page); + const restOfTheQueue = [...accepters.slice(1), ...challengers]; for (const member of restOfTheQueue.slice(0, -1)) { await impersonate(page, member.id); await ready.goto(); + + // the ready check notification stays unseen until they respond to it + await expect(notifications.locators.bellDot).toBeVisible(); + await ready.confirmReady(); await expect(page).toHaveURL(SENDOUQ_READY_PAGE); await expect(ready.locators.confirmedText).toBeVisible(); + await expect(notifications.locators.bellDot).toBeHidden(); } // the last one to confirm gets everyone into the match await impersonate(page, restOfTheQueue.at(-1)!.id); await ready.goto(); + + await expect(notifications.locators.bellDot).toBeVisible(); + await ready.confirmReady(); await expect(page).toHaveURL(/\/q\/match\/\d+/); + + // confirming resolved the ready check notification and the new match + // notification arrives already seen for the one who created the match + await expect(notifications.locators.bellDot).toBeHidden(); }); test("Ready check expiring sends the groups back to looking and lets them kick who missed it", async ({ diff --git a/e2e/tournament.spec.ts b/e2e/tournament.spec.ts index e8146df4b..f0e1ef094 100644 --- a/e2e/tournament.spec.ts +++ b/e2e/tournament.spec.ts @@ -8,6 +8,7 @@ import { navigate, test, } from "./helpers/playwright"; +import { NotificationPopover } from "./pages/layout/notification-popover"; import { TournamentBracketsPage } from "./pages/tournament/tournament-brackets-page"; import { TournamentPage } from "./pages/tournament/tournament-page"; import { TournamentRegisterPage } from "./pages/tournament/tournament-register-page"; @@ -75,6 +76,13 @@ test.describe("Tournament", () => { team: pickUpTeam(TEAM_NAME), memberUserIds: roster.map((user) => user.id), }); + await factories.NotificationFactory.create({ + notification: { + type: "TO_CHECK_IN_OPENED", + meta: { tournamentId: tournament.id, tournamentName: "In The Zone" }, + }, + users: roster.map((user) => ({ userId: user.id })), + }); const opponents = await factories.UserFactory.createMany(2); for (const [i, opponent] of opponents.entries()) { @@ -97,8 +105,16 @@ test.describe("Tournament", () => { const register = new TournamentRegisterPage(page); await register.goto(tournament.id); + + const notifications = new NotificationPopover(page); + await expect(notifications.locators.bellDot).toBeVisible(); + await register.checkIn(); + // checking in resolved the check-in notification without the bell + // having been opened + await expect(notifications.locators.bellDot).toBeHidden(); + const bracketsAfterCheckIn = await register.openBrackets(); await expect(bracketsAfterCheckIn.locators.bracketsViewer).toBeVisible(); diff --git a/e2e/trophies.spec.ts b/e2e/trophies.spec.ts index 98c76028a..07312ecd5 100644 --- a/e2e/trophies.spec.ts +++ b/e2e/trophies.spec.ts @@ -7,6 +7,7 @@ import { dateToDatabaseTimestamp } from "~/utils/dates"; import { TROPHIES_PAGE } from "~/utils/urls"; import type { Factories } from "./helpers/factories"; import { expect, impersonate, isNotVisible, test } from "./helpers/playwright"; +import { NotificationPopover } from "./pages/layout/notification-popover"; import { NewTrophyPage } from "./pages/trophies/new-trophy-page"; import { TrophiesPage } from "./pages/trophies/trophies-page"; import { UserPage } from "./pages/user/user-page"; @@ -165,6 +166,13 @@ test.describe("Trophies", () => { organizationId: organization.id, submitterUserId: NZAP_TEST_ID, }); + await factories.NotificationFactory.create({ + notification: { + type: "TROPHY_SUBMITTED", + meta: { trophyName: name, submitterUsername: "N-ZAP" }, + }, + users: [{ userId: ADMIN_ID }], + }); } await impersonate(page); @@ -174,6 +182,9 @@ test.describe("Trophies", () => { const pending = await newTrophy.openPending(); + const notifications = new NotificationPopover(page); + await expect(notifications.locators.bellDot).toBeVisible(); + await pending.approve(approvedName); await expect( pending @@ -181,9 +192,16 @@ test.describe("Trophies", () => { .getByText(`1/${TROPHY_APPROVALS_REQUIRED} approvals`), ).toBeVisible(); + // approving resolved that submission's notification, but the other + // submission still waits for a review + await expect(notifications.locators.bellDot).toBeVisible(); + await pending.decline(declinedName, "Does not meet the requirements"); await isNotVisible(pending.row(declinedName)); + // with the last pending submission reviewed nothing needs attention anymore + await expect(notifications.locators.bellDot).toBeHidden(); + const reviewed = await newTrophy.openReviewed(); await expect(reviewed.row(declinedName)).toBeVisible(); await expect(