Skip to content

improvement: mark as read on click #867

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
ameer2468 merged 1 commit into from
Aug 8, 2025
Merged

improvement: mark as read on click #867

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions apps/web/actions/notifications/mark-all-as-read.ts
View file
Open in desktop

This file was deleted.

34 changes: 34 additions & 0 deletions apps/web/actions/notifications/mark-as-read.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
"use server";

import { getCurrentUser } from "@cap/database/auth/session";
import { notifications } from "@cap/database/schema";
import { db } from "@cap/database";
import { eq } from "drizzle-orm";
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 8, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Scope single-item update to the current user and update only unread notifications

Currently, the single-item branch updates by ID only, allowing any authenticated user to mark any notification as read if they know its ID. Also, both branches overwrite readAt for already read notifications. Tighten authorization and update only unread rows.

Apply:

-import { eq } from "drizzle-orm";
+import { and, eq, isNull } from "drizzle-orm";
-    if (notificationId) {
-      await db()
-        .update(notifications)
-        .set({ readAt: now })
-        .where(eq(notifications.id, notificationId));
-    } else {
-      await db()
-        .update(notifications)
-        .set({ readAt: now })
-        .where(eq(notifications.recipientId, currentUser.id));
-    }
+    if (notificationId) {
+      await db()
+        .update(notifications)
+        .set({ readAt: now })
+        .where(
+          and(
+            eq(notifications.id, notificationId),
+            eq(notifications.recipientId, currentUser.id),
+            isNull(notifications.readAt)
+          )
+        );
+    } else {
+      await db()
+        .update(notifications)
+        .set({ readAt: now })
+        .where(
+          and(
+            eq(notifications.recipientId, currentUser.id),
+            isNull(notifications.readAt)
+          )
+        );
+    }

Also applies to: 17-27

🤖 Prompt for AI Agents 
In apps/web/actions/notifications/mark-as-read.ts around lines 6 and 17 to 27,
the update operation currently allows any authenticated user to mark any
notification as read by ID alone and overwrites the readAt timestamp even for
already read notifications. To fix this, modify the update queries to include a
condition that the notification belongs to the current user and that readAt is
null (unread). This will restrict updates to only unread notifications owned by
the user, tightening authorization and preventing unnecessary overwrites.

import { revalidatePath } from "next/cache";

export const markAsRead = async (notificationId?: string) => {
const currentUser = await getCurrentUser();
if (!currentUser) {
throw new Error("User not found");
}

try {
const now = new Date();
if (notificationId) {
await db()
.update(notifications)
.set({ readAt: now })
.where(eq(notifications.id, notificationId));
} else {
await db()
.update(notifications)
.set({ readAt: now })
.where(eq(notifications.recipientId, currentUser.id));
}
} catch (error) {
console.log(error);
throw new Error("Error marking notification(s) as read");
}

revalidatePath("/dashboard");
};
8 changes: 4 additions & 4 deletions apps/web/app/(org)/dashboard/_components/Notifications/NotificationHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { faCheckDouble } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { toast } from "sonner";
import clsx from "clsx";
import { markAllAsRead } from "@/actions/notifications/mark-all-as-read";
import { markAsRead } from "@/actions/notifications/mark-as-read";
import { useQueryClient } from "@tanstack/react-query";
import { useState } from "react";

Expand All @@ -11,10 +11,10 @@ export const NotificationHeader = () => {
const [loading, setLoading] = useState(false);
const queryClient = useQueryClient();

const markAllAsReadHandler = async () => {
const markAsReadHandler = async () => {
try {
setLoading(true);
await markAllAsRead();
await markAsRead();
toast.success("Notifications marked as read");
queryClient.invalidateQueries({
queryKey: ["notifications"],
Expand All @@ -31,7 +31,7 @@ export const NotificationHeader = () => {
<div className="flex justify-between items-center px-6 py-3 rounded-t-xl border bg-gray-3 border-gray-4">
<p className="text-md text-gray-12">Notifications</p>
<div
onClick={markAllAsReadHandler}
onClick={markAsReadHandler}
className={clsx("flex gap-1 items-center transition-opacity duration-200 cursor-pointer hover:opacity-70", loading ? "opacity-50 cursor-not-allowed" : "")}
>
<FontAwesomeIcon
Expand Down
10 changes: 10 additions & 0 deletions apps/web/app/(org)/dashboard/_components/Notifications/NotificationItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import clsx from "clsx";
import Link from "next/link";
import moment from "moment";
import { NotificationType } from "@/lib/Notification";
import { markAsRead } from "@/actions/notifications/mark-as-read";
import { Notification as APINotification } from "@cap/web-api-contract";

type NotificationItemProps = {
Expand All @@ -25,9 +26,18 @@ export const NotificationItem = ({
}: NotificationItemProps) => {
const link = getLink(notification);

const markAsReadHandler = async () => {
try {
await markAsRead(notification.id);
} catch (error) {
console.error("Error marking notification as read:", error);
}
};

return (
<Link
href={link}
onClick={markAsReadHandler}
className={clsx(
"flex gap-3 p-4 transition-colors cursor-pointer min-h-fit border-gray-3 hover:bg-gray-2",
className
Expand Down
Loading