-
Notifications
You must be signed in to change notification settings - Fork 0
fix: prevent desktop schedule service row overlap #283
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| import { describe, it, expect, jest, beforeEach, afterEach } from "@jest/globals"; | ||
| import { buildFavoriteSessionNotifications, scheduleFavoriteSessionNotifications } from "@/lib/session-notifications"; | ||
| import type { DailySchedule } from "@/hooks/useSchedule"; | ||
|
|
||
| const createSchedule = (startsAt: string): DailySchedule[] => [ | ||
| { | ||
| date: "2026-06-17", | ||
| rooms: [ | ||
| { | ||
| id: 1, | ||
| name: "Auditorium", | ||
| hasOnlyPlenumSessions: false, | ||
| sessions: [ | ||
| { | ||
| id: "session-1", | ||
| title: "Practical React", | ||
| description: null, | ||
| startsAt, | ||
| endsAt: "2026-06-17T10:50:00.000Z", | ||
| isServiceSession: false, | ||
| isPlenumSession: false, | ||
| speakers: [], | ||
| roomId: 1, | ||
| room: "Auditorium", | ||
| status: "accepted", | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| timeSlots: [], | ||
| }, | ||
| ]; | ||
|
|
||
| const createGroupedSchedule = (): DailySchedule[] => [ | ||
| { | ||
| date: "2026-06-17", | ||
| rooms: [ | ||
| { | ||
| id: 1, | ||
| name: "Auditorium", | ||
| hasOnlyPlenumSessions: false, | ||
| sessions: [ | ||
| { | ||
| id: "session-1", | ||
| title: "Practical React", | ||
| description: null, | ||
| startsAt: "2026-06-17T10:00:00.000Z", | ||
| endsAt: "2026-06-17T10:50:00.000Z", | ||
| isServiceSession: false, | ||
| isPlenumSession: false, | ||
| speakers: [], | ||
| roomId: 1, | ||
| room: "Auditorium", | ||
| status: "accepted", | ||
| }, | ||
| { | ||
| id: "session-2", | ||
| title: "Advanced GraphQL", | ||
| description: null, | ||
| startsAt: "2026-06-17T10:00:00.000Z", | ||
| endsAt: "2026-06-17T10:50:00.000Z", | ||
| isServiceSession: false, | ||
| isPlenumSession: false, | ||
| speakers: [], | ||
| roomId: 1, | ||
| room: "Auditorium", | ||
| status: "accepted", | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| timeSlots: [], | ||
| }, | ||
| ]; | ||
|
|
||
| describe("session notifications", () => { | ||
| beforeEach(() => { | ||
| jest.useFakeTimers(); | ||
| jest.setSystemTime(new Date("2026-06-17T09:58:30.000Z")); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.useRealTimers(); | ||
| jest.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it("builds reminders one minute before favorite sessions start", () => { | ||
| const notifications = buildFavoriteSessionNotifications(createSchedule("2026-06-17T10:00:00.000Z"), ["session-1"], Date.now()); | ||
|
|
||
| expect(notifications).toEqual([ | ||
| { | ||
| id: "session-1", | ||
| title: "Practical React starts in 1 minute", | ||
| body: "Auditorium · 12:00", | ||
| notifyAt: new Date("2026-06-17T09:59:00.000Z").getTime(), | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("groups favorite sessions that start at the same time into one reminder", () => { | ||
| const notifications = buildFavoriteSessionNotifications(createGroupedSchedule(), ["session-1", "session-2"], Date.now()); | ||
|
|
||
| expect(notifications).toEqual([ | ||
| { | ||
| id: "session-1|session-2", | ||
| title: "2 favorite sessions start in 1 minute", | ||
| body: "Practical React, Advanced GraphQL · 12:00", | ||
| notifyAt: new Date("2026-06-17T09:59:00.000Z").getTime(), | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("ignores non-favorites, service sessions, and reminders that are no longer in the future", () => { | ||
| const schedule = createSchedule("2026-06-17T09:59:00.000Z"); | ||
| schedule[0].rooms[0].sessions.push({ ...schedule[0].rooms[0].sessions[0], id: "session-2", isServiceSession: true }); | ||
|
|
||
| expect(buildFavoriteSessionNotifications(schedule, ["session-1", "session-2"], Date.now())).toEqual([]); | ||
| }); | ||
|
|
||
| it("schedules browser notifications and returns a cleanup function", () => { | ||
| const notificationConstructor = jest.fn(); | ||
| class MockNotification { | ||
| static permission: NotificationPermission = "granted"; | ||
|
|
||
| constructor(title: string, options?: NotificationOptions) { | ||
| notificationConstructor(title, options); | ||
| } | ||
| } | ||
|
|
||
| const cleanup = scheduleFavoriteSessionNotifications(createSchedule("2026-06-17T10:00:00.000Z"), ["session-1"], MockNotification, Date.now()); | ||
|
|
||
| jest.advanceTimersByTime(29_999); | ||
| expect(notificationConstructor).not.toHaveBeenCalled(); | ||
|
|
||
| jest.advanceTimersByTime(1); | ||
| expect(notificationConstructor).toHaveBeenCalledWith("Practical React starts in 1 minute", { body: "Auditorium · 12:00", tag: "devbcn-session-session-1" }); | ||
|
|
||
| cleanup(); | ||
| }); | ||
|
|
||
| it("schedules a single browser notification for simultaneous favorite sessions", () => { | ||
| const notificationConstructor = jest.fn(); | ||
| class MockNotification { | ||
| static permission: NotificationPermission = "granted"; | ||
|
|
||
| constructor(title: string, options?: NotificationOptions) { | ||
| notificationConstructor(title, options); | ||
| } | ||
| } | ||
|
|
||
| scheduleFavoriteSessionNotifications(createGroupedSchedule(), ["session-1", "session-2"], MockNotification, Date.now()); | ||
|
|
||
| jest.advanceTimersByTime(30_000); | ||
|
|
||
| expect(notificationConstructor).toHaveBeenCalledTimes(1); | ||
| expect(notificationConstructor).toHaveBeenCalledWith("2 favorite sessions start in 1 minute", { | ||
| body: "Practical React, Advanced GraphQL · 12:00", | ||
| tag: "devbcn-session-session-1|session-2", | ||
| }); | ||
| }); | ||
|
|
||
| it("does not schedule notifications without browser permission", () => { | ||
| const notificationConstructor = jest.fn(); | ||
| class MockNotification { | ||
| static permission: NotificationPermission = "default"; | ||
|
|
||
| constructor(title: string, options?: NotificationOptions) { | ||
| notificationConstructor(title, options); | ||
| } | ||
| } | ||
|
|
||
| scheduleFavoriteSessionNotifications(createSchedule("2026-06-17T10:00:00.000Z"), ["session-1"], MockNotification, Date.now()); | ||
|
|
||
| jest.advanceTimersByTime(30_000); | ||
| expect(notificationConstructor).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| import { format, parseISO } from "date-fns"; | ||
| import type { DailySchedule, GridSession } from "@/hooks/useSchedule"; | ||
|
|
||
| const SESSION_REMINDER_LEAD_TIME_MS = 60_000; | ||
|
|
||
| export interface FavoriteSessionNotification { | ||
| id: string; | ||
| title: string; | ||
| body: string; | ||
| notifyAt: number; | ||
| } | ||
|
|
||
| type BrowserNotification = Pick<typeof Notification, "permission"> & { | ||
| new (title: string, options?: NotificationOptions): unknown; | ||
| }; | ||
|
|
||
| interface GroupedFavoriteSessionNotification { | ||
| ids: string[]; | ||
| sessionCount: number; | ||
| titles: string[]; | ||
| rooms: string[]; | ||
| timeLabel: string; | ||
| notifyAt: number; | ||
| } | ||
|
|
||
| const getUniqueSessions = (schedule: DailySchedule[]): GridSession[] => { | ||
| const sessionsById = new Map<string, GridSession>(); | ||
| const rooms = schedule.flatMap((day) => day.rooms); | ||
|
|
||
| for (const room of rooms) { | ||
| for (const session of room.sessions) { | ||
| if (!sessionsById.has(session.id)) { | ||
| sessionsById.set(session.id, session); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return Array.from(sessionsById.values()); | ||
| }; | ||
|
|
||
| const addSessionToNotificationGroup = (notificationsByNotifyAt: Map<number, GroupedFavoriteSessionNotification>, session: GridSession, now: number): void => { | ||
| const startsAt = parseISO(session.startsAt); | ||
| const notifyAt = startsAt.getTime() - SESSION_REMINDER_LEAD_TIME_MS; | ||
|
|
||
| if (notifyAt <= now) { | ||
| return; | ||
| } | ||
|
|
||
| const existingNotification = notificationsByNotifyAt.get(notifyAt); | ||
| if (existingNotification === undefined) { | ||
| notificationsByNotifyAt.set(notifyAt, { | ||
| ids: [session.id], | ||
| sessionCount: 1, | ||
| titles: [session.title], | ||
| rooms: [session.room], | ||
| timeLabel: format(startsAt, "HH:mm"), | ||
| notifyAt, | ||
|
Comment on lines
+42
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, locate and examine the session-notifications.ts file
find . -name "session-notifications.ts" -type fRepository: anyulled/devbcn-nextjs Length of output: 97 🏁 Script executed: # Check the content around line 56
cat -n lib/session-notifications.ts | sed -n '40,70p'Repository: anyulled/devbcn-nextjs Length of output: 1433 🏁 Script executed: # Search for test files related to session-notifications
fd -t f ".*test.*" | grep -i "session.*notif" | head -20Repository: anyulled/devbcn-nextjs Length of output: 110 🏁 Script executed: # Look for any timezone configuration in the project
rg -i "timezone|timeZone|europe.*madrid" --type ts --type tsx --type js --type json -C 2Repository: anyulled/devbcn-nextjs Length of output: 94 🏁 Script executed: # Search for timezone configuration without specifying tsx
rg -i "timezone|timeZone|europe.*madrid" -C 2 | head -50Repository: anyulled/devbcn-nextjs Length of output: 2792 🏁 Script executed: # Examine the test file to see what the expected vs actual values are
cat -n __tests__/lib/session-notifications.test.ts | head -100Repository: anyulled/devbcn-nextjs Length of output: 3532 🏁 Script executed: # Look for any environment setup or timezone mocking in tests
rg -i "mock|jest|setup|timezone" __tests__/lib/session-notifications.test.ts -C 2Repository: anyulled/devbcn-nextjs Length of output: 2174 🏁 Script executed: # Check the imports and format usage in session-notifications.ts
cat -n lib/session-notifications.ts | head -30Repository: anyulled/devbcn-nextjs Length of output: 1080 Make reminder time formatting timezone-explicit. Line 56 formats the start time using Use 🤖 Prompt for AI AgentsSource: Pipeline failures |
||
| }); | ||
| return; | ||
| } | ||
|
|
||
| existingNotification.ids.push(session.id); | ||
| existingNotification.sessionCount += 1; | ||
| existingNotification.titles.push(session.title); | ||
| existingNotification.rooms.push(session.room); | ||
| }; | ||
|
|
||
| export const buildFavoriteSessionNotifications = (schedule: DailySchedule[], savedSessionIds: string[], now: number): FavoriteSessionNotification[] => { | ||
| const savedIds = new Set(savedSessionIds); | ||
| const notificationsByNotifyAt = new Map<number, GroupedFavoriteSessionNotification>(); | ||
|
|
||
| for (const session of getUniqueSessions(schedule)) { | ||
| if (!savedIds.has(session.id) || session.isServiceSession) { | ||
| continue; | ||
| } | ||
|
|
||
| addSessionToNotificationGroup(notificationsByNotifyAt, session, now); | ||
| } | ||
|
|
||
| return Array.from(notificationsByNotifyAt.entries()) | ||
| .sort(([leftNotifyAt], [rightNotifyAt]) => leftNotifyAt - rightNotifyAt) | ||
| .map(([notifyAt, notification]) => { | ||
| if (notification.sessionCount === 1) { | ||
| return { | ||
| id: notification.ids[0], | ||
| title: `${notification.titles[0]} starts in 1 minute`, | ||
| body: `${notification.rooms[0]} · ${notification.timeLabel}`, | ||
| notifyAt, | ||
| }; | ||
| } | ||
|
|
||
| const sessionSummary = | ||
| notification.sessionCount <= 2 | ||
| ? notification.titles.join(", ") | ||
| : `${notification.titles.slice(0, 2).join(", ")} +${notification.sessionCount - 2} more`; | ||
|
|
||
| return { | ||
| id: notification.ids.slice().sort().join("|"), | ||
| title: `${notification.sessionCount} favorite sessions start in 1 minute`, | ||
| body: `${sessionSummary} · ${notification.timeLabel}`, | ||
| notifyAt, | ||
| }; | ||
| }); | ||
| }; | ||
|
|
||
| export const scheduleFavoriteSessionNotifications = ( | ||
| schedule: DailySchedule[], | ||
| savedSessionIds: string[], | ||
| notificationApi: BrowserNotification | undefined, | ||
| now: number = Date.now() | ||
| ): (() => void) => { | ||
| if (!notificationApi || notificationApi.permission !== "granted") { | ||
| return () => undefined; | ||
| } | ||
|
|
||
| const timeoutIds = buildFavoriteSessionNotifications(schedule, savedSessionIds, now).map((notification) => | ||
| window.setTimeout(() => { | ||
| new notificationApi(notification.title, { | ||
| body: notification.body, | ||
| tag: `devbcn-session-${notification.id}`, | ||
| }); | ||
| }, notification.notifyAt - now) | ||
| ); | ||
|
|
||
| return () => { | ||
| timeoutIds.forEach((timeoutId) => window.clearTimeout(timeoutId)); | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,3 +9,4 @@ | |
| @forward "venue-wtc"; | ||
| @forward "convince-your-boss"; | ||
| @forward "features"; | ||
| @forward "live-schedule"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix missing forwarded stylesheet before merge. Line 12 forwards 🧰 Tools🪛 GitHub Actions: E2E Tests / 0_cypress-run (home-pages).txt[error] 12-12: Turbopack build failed: Can't find stylesheet to import. The 🪛 GitHub Actions: E2E Tests / 1_cypress-run (speakers).txt[error] 12-12: Sass build failed: Can't find stylesheet to import for ' 🪛 GitHub Actions: E2E Tests / 2_cypress-run (talks).txt[error] 12-12: Sass/Turbopack build failed: Can't find stylesheet to import. Missing import in 🪛 GitHub Actions: E2E Tests / cypress-run (home-pages)[error] 12-12: Sass build failed: Can't find stylesheet to import for ' 🪛 GitHub Actions: E2E Tests / cypress-run (speakers)[error] 12-12: Sass build failed while processing ' 🪛 GitHub Actions: E2E Tests / cypress-run (talks)[error] 12-12: Sass build failed: Can't find stylesheet to import for 🤖 Prompt for AI AgentsSource: Pipeline failures |
||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: anyulled/devbcn-nextjs
Length of output: 96
🏁 Script executed:
cat -n context/ScheduleContext.tsx | head -70Repository: anyulled/devbcn-nextjs
Length of output: 2815
🏁 Script executed:
rg "scheduleFavoriteSessionNotifications" -A 10 -B 2 --type ts --type tsxRepository: anyulled/devbcn-nextjs
Length of output: 94
🏁 Script executed:
rg "scheduleFavoriteSessionNotifications" -A 10 -B 2Repository: anyulled/devbcn-nextjs
Length of output: 6300
🏁 Script executed:
cat -n lib/session-notifications.ts | head -100Repository: anyulled/devbcn-nextjs
Length of output: 4017
🏁 Script executed:
Repository: anyulled/devbcn-nextjs
Length of output: 503
🏁 Script executed:
Repository: anyulled/devbcn-nextjs
Length of output: 470
🏁 Script executed:
Repository: anyulled/devbcn-nextjs
Length of output: 543
🏁 Script executed:
Repository: anyulled/devbcn-nextjs
Length of output: 790
🏁 Script executed:
Repository: anyulled/devbcn-nextjs
Length of output: 968
Add
notificationPermissionstate and dependency to reschedule when permission is granted.When a user saves a session before granting notification permission, the effect at lines 42-44 runs with
Notification.permission === "default", causingscheduleFavoriteSessionNotificationsto return early without scheduling any notifications (line 112 check). The permission then transitions to"granted"asynchronously viarequestNotificationPermission(), but because the effect has no dependency on permission state, it never re-runs to schedule those saved sessions.Track permission changes in state and include it as an effect dependency so scheduling occurs after permission is granted.
💡 Proposed fix
export function ScheduleProvider({ children, schedule = [] }: ScheduleProviderProps) { const [savedSessionIds, setSavedSessionIds] = useState<string[]>([]); + const [notificationPermission, setNotificationPermission] = useState<NotificationPermission>( + typeof Notification === "undefined" ? "denied" : Notification.permission + ); const [isLoaded, setIsLoaded] = useState(false); useEffect(() => { - return scheduleFavoriteSessionNotifications(schedule, savedSessionIds, typeof Notification === "undefined" ? undefined : Notification); - }, [schedule, savedSessionIds]); + return scheduleFavoriteSessionNotifications( + schedule, + savedSessionIds, + typeof Notification === "undefined" || notificationPermission !== "granted" ? undefined : Notification + ); + }, [schedule, savedSessionIds, notificationPermission]); const requestNotificationPermission = useCallback(() => { if (typeof Notification === "undefined" || Notification.permission !== "default") { return; } - Notification.requestPermission().catch((error: unknown) => { - console.error("Failed to request notification permission", error); - }); + Notification.requestPermission() + .then((permission) => setNotificationPermission(permission)) + .catch((error: unknown) => { + console.error("Failed to request notification permission", error); + }); }, []);📝 Committable suggestion
🤖 Prompt for AI Agents