-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add anonymous view notifications for non-logged-in viewers #1643
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.
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
Changes from 8 commits
cad016b
fbaa359
d47a072
7d7bfc4
a11e2da
b1d7d19
8d8229f
2ca2f56
93a7d14
83031ad
4962452
9f35e61
530e728
04dd97d
c1f59ee
57a49e9
cfccb35
f4d6cff
e77a15b
9136bb5
ec6eef3
d10e157
2a1e0df
8eae09f
b804dc7
0e406c5
a75b9ec
4c9d904
0c87ef5
915e508
509b9f5
d141666
ced5ce2
aa2fb75
ed0ce98
ff31881
a4d1625
935d152
d5674e0
9da8943
ee9a373
883bbf8
cb10822
4a03a8a
4d3c1c7
a734be2
6825367
9e854c9
3c69bca
cb235a3
0db13cc
cf495ee
2481d2b
2bcf989
5dfd9f1
c6c1eb2
080c81f
4305c6a
5ba1611
f587ef1
f3530e7
c1ec572
0cb4ad4
b4eb2d6
c37ec53
0223f5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,7 @@ const descriptionMap: Record<NotificationType, string> = { | |||||||||||||||
| reply: `replied to your comment`, | ||||||||||||||||
| view: `viewed your video`, | ||||||||||||||||
| reaction: `reacted to your video`, | ||||||||||||||||
| // mention: `mentioned you in a comment`, | ||||||||||||||||
| anon_view: `viewed your video`, | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| export const NotificationItem = ({ | ||||||||||||||||
|
|
@@ -36,6 +36,11 @@ export const NotificationItem = ({ | |||||||||||||||
| } | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| const isAnonView = notification.type === "anon_view"; | ||||||||||||||||
| const displayName = isAnonView | ||||||||||||||||
|
||||||||||||||||
| const displayName = isAnonView | |
| const displayName = | |
| notification.type === "anon_view" ? notification.anonName : notification.author.name; |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider reusing SignedImageUrl here for consistent avatar styling (and to avoid baking a special-case glyph into the UI).
| <div className="relative flex-shrink-0 size-7 rounded-full bg-gray-3 flex items-center justify-center text-sm"> | |
| <SignedImageUrl | |
| image={null} | |
| name={notification.anonName} | |
| className="relative flex-shrink-0 size-7" | |
| letterClass="text-sm" | |
| /> |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,8 @@ import { Effect, Option } from "effect"; | |||||
| import type { NextRequest } from "next/server"; | ||||||
| import UAParser from "ua-parser-js"; | ||||||
|
|
||||||
| import { getAnonymousName } from "@/lib/anonymous-names"; | ||||||
| import { createAnonymousViewNotification } from "@/lib/Notification"; | ||||||
| import { runPromise } from "@/lib/server"; | ||||||
|
|
||||||
| interface TrackPayload { | ||||||
|
|
@@ -100,6 +102,22 @@ export async function POST(request: NextRequest) { | |||||
| user_id: userId, | ||||||
| }, | ||||||
| ]); | ||||||
|
|
||||||
| if (!userId) { | ||||||
|
||||||
| if (!userId) { | |
| if (!userId && body.sessionId) { |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ import { type Comment, Video } from "@cap/web-domain"; | |||||
| import { and, eq, sql } from "drizzle-orm"; | ||||||
| import { revalidatePath } from "next/cache"; | ||||||
| import type { UserPreferences } from "@/app/(org)/dashboard/dashboard-data"; | ||||||
| import { getSessionHash } from "@/lib/anonymous-names"; | ||||||
|
|
||||||
| export type NotificationType = Notification["type"]; | ||||||
|
|
||||||
|
|
@@ -18,16 +19,16 @@ type NotificationSpecificData = DistributiveOmit< | |||||
| keyof NotificationBase | ||||||
| >; | ||||||
|
|
||||||
| // Replaces author object with authorId since we query for that info. | ||||||
| // If we add more notifications this would probably be better done manually | ||||||
| // Type is weird since we need to operate on each member of the NotificationSpecificData union | ||||||
| type CreateNotificationInput<D = NotificationSpecificData> = | ||||||
| D extends NotificationSpecificData | ||||||
| ? D["author"] extends never | ||||||
| ? D | ||||||
| : Omit<D, "author"> & { authorId: string } & { | ||||||
| parentCommentId?: Comment.CommentId; | ||||||
| } | ||||||
| type AuthoredNotificationData = Exclude< | ||||||
| NotificationSpecificData, | ||||||
| { type: "anon_view" } | ||||||
| >; | ||||||
|
|
||||||
| type CreateNotificationInput<D = AuthoredNotificationData> = | ||||||
| D extends AuthoredNotificationData | ||||||
| ? Omit<D, "author"> & { authorId: string } & { | ||||||
| parentCommentId?: Comment.CommentId; | ||||||
| } | ||||||
| : never; | ||||||
|
|
||||||
| export async function createNotification( | ||||||
|
|
@@ -217,3 +218,69 @@ export async function createNotification( | |||||
| throw error; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export async function createAnonymousViewNotification({ | ||||||
| videoId, | ||||||
| sessionId, | ||||||
| anonName, | ||||||
| location, | ||||||
| }: { | ||||||
| videoId: string; | ||||||
| sessionId: string; | ||||||
| anonName: string; | ||||||
| location: string | null; | ||||||
| }) { | ||||||
| try { | ||||||
| const sessionHash = getSessionHash(sessionId); | ||||||
|
|
||||||
| const [videoExists] = await db() | ||||||
| .select({ id: videos.id, ownerId: videos.ownerId }) | ||||||
| .from(videos) | ||||||
| .where(eq(videos.id, Video.VideoId.make(videoId))) | ||||||
| .limit(1); | ||||||
|
|
||||||
| if (!videoExists) return; | ||||||
|
|
||||||
| const [ownerResult] = await db() | ||||||
| .select({ | ||||||
| id: users.id, | ||||||
| activeOrganizationId: users.activeOrganizationId, | ||||||
| preferences: users.preferences, | ||||||
| }) | ||||||
| .from(users) | ||||||
| .where(eq(users.id, videoExists.ownerId)) | ||||||
| .limit(1); | ||||||
|
|
||||||
| if (!ownerResult?.activeOrganizationId) return; | ||||||
|
|
||||||
| const preferences = ownerResult.preferences as UserPreferences; | ||||||
| if (preferences?.notifications?.pauseViews) return; | ||||||
|
|
||||||
| const [existing] = await db() | ||||||
| .select({ id: notifications.id }) | ||||||
| .from(notifications) | ||||||
| .where( | ||||||
| and( | ||||||
| eq(notifications.type, "anon_view"), | ||||||
| eq(notifications.recipientId, ownerResult.id), | ||||||
| sql`JSON_EXTRACT(${notifications.data}, '$.videoId') = ${videoId}`, | ||||||
|
||||||
| sql`JSON_EXTRACT(${notifications.data}, '$.videoId') = ${videoId}`, | |
| sql`JSON_UNQUOTE(JSON_EXTRACT(${notifications.data}, '$.videoId')) = ${videoId}`, |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| const ANIMALS = [ | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Walrus", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Capybara", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Narwhal", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Quokka", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Axolotl", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Pangolin", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Okapi", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Platypus", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Wombat", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Chinchilla", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Manatee", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Flamingo", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Hedgehog", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Otter", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Puffin", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Raccoon", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Sloth", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Chameleon", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Penguin", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Koala", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Red Panda", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Seahorse", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Toucan", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Lemur", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Armadillo", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Alpaca", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Meerkat", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Ibex", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Tapir", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Kiwi", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Gecko", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Bison", | ||||||||||||||||||||||||||||||||||||||||||||||||
| ] as const; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| function hashSessionId(sessionId: string): number { | ||||||||||||||||||||||||||||||||||||||||||||||||
| let hash = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||
| for (let i = 0; i < sessionId.length; i++) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const char = sessionId.charCodeAt(i); | ||||||||||||||||||||||||||||||||||||||||||||||||
| hash = (hash << 5) - hash + char; | ||||||||||||||||||||||||||||||||||||||||||||||||
| hash |= 0; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| return hash >>> 0; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export function getAnonymousName(sessionId: string): string { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const index = hashSessionId(sessionId) % ANIMALS.length; | ||||||||||||||||||||||||||||||||||||||||||||||||
| return `Anonymous ${ANIMALS[index]}`; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export function getSessionHash(sessionId: string): string { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return hashSessionId(sessionId).toString(36); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
| function hashSessionId(sessionId: string): number { | |
| let hash = 0; | |
| for (let i = 0; i < sessionId.length; i++) { | |
| const char = sessionId.charCodeAt(i); | |
| hash = (hash << 5) - hash + char; | |
| hash |= 0; | |
| } | |
| return hash >>> 0; | |
| } | |
| export function getAnonymousName(sessionId: string): string { | |
| const index = hashSessionId(sessionId) % ANIMALS.length; | |
| return `Anonymous ${ANIMALS[index]}`; | |
| } | |
| export function getSessionHash(sessionId: string): string { | |
| return hashSessionId(sessionId).toString(36); | |
| } | |
| import { createHash } from "crypto"; | |
| export function getSessionHash(sessionId: string): string { | |
| return createHash("sha256").update(sessionId).digest("hex"); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/lib/anonymous-names.ts
Line: 36-53
Comment:
32-bit hash used for deduplication has a significant collision space.
`hashSessionId` returns a 32-bit unsigned integer (~4 billion possible values). With the birthday paradox, the probability of at least one collision becomes non-negligible once tens-of-thousands of distinct sessions have viewed a popular video.
A collision between two different legitimate sessions produces the **same** `sessionHash`, so the second viewer's notification is permanently silenced — they appear to never have viewed the video.
Additionally, `getAnonymousName` and `getSessionHash` both call the same underlying `hashSessionId`, so sessions that produce the same hash will also receive the same animal name (they are indistinguishable to the owner too).
Consider using a proper cryptographic hash (e.g. Node's `crypto.createHash('sha256').update(sessionId).digest('hex')`) for `getSessionHash` and keeping the modulo only for the display name:
```suggestion
import { createHash } from "crypto";
export function getSessionHash(sessionId: string): string {
return createHash("sha256").update(sessionId).digest("hex");
}
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anon_viewlabel inFilterLabelsis unreachable UIanon_viewis not present in theFiltersarray (lines 5–11), so this entry inFilterLabelsis only there for TypeScript type completeness (becauseFilterTypeincludesanon_view). This is not a bug, but a comment explaining why the entry exists would help future readers understand it is intentional and not dead code that can be removed:Prompt To Fix With AI