Repository navigation
Conversation
There was a problem hiding this comment.
Pull request overview
This is a release PR dated December 23, 2025, implementing two major features: removing the "writer" role from the user management system and introducing a comprehensive analytics tracking system with cookie consent.
Key changes:
- Consolidates user roles from three (admin, writer, user) to two (admin, user), promoting all writers to admin
- Implements privacy-compliant analytics with cookie consent, visitor tracking, and GDPR-friendly data handling
- Adds scheduled aggregation jobs for analytics data processing and retention management
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
vercel.json |
Configures nightly and weekly cron jobs for analytics aggregation |
src/schemas/common.ts |
Removes 'writer' from UserRoleSchema enum |
src/lib/permissions.ts |
Removes writer role definition and permissions |
src/lib/env.ts |
Adds optional CRON_SECRET for securing aggregation endpoints |
src/lib/auth.ts |
Removes writer role from auth configuration |
src/lib/auth-client.ts |
Removes writer role from client auth |
src/lib/analytics.ts |
New client-side analytics utilities for cookie management and event tracking |
src/hooks/use-notice-analytics.ts |
Custom hooks for tracking page and notice views with duration |
src/features/user-management/** |
Updates all user management UI components to remove writer role references |
src/features/auth/** |
Removes writer-specific auth components and hooks |
src/features/artworks/** |
Removes writer permission checks, adds notice analytics tracking |
src/features/admin-dashboard/** |
Adds comprehensive analytics dashboard with charts and visualizations |
src/components/** |
Adds CookieBanner, AnalyticsProvider, and chart components |
prisma/schema.prisma |
Adds analytics event models and aggregation summary tables |
prisma/migrations/** |
Database migrations for role removal and analytics tables |
package.json |
Updates dependencies including React 19.2.3, Next.js 16.0.10, Prisma 7.1.0, and adds recharts and ua-parser-js |
app/layout.tsx |
Integrates AnalyticsProvider and CookieBanner into root layout |
app/api/analytics/route.ts |
API endpoint for receiving and storing analytics events |
app/api/cron/** |
Scheduled aggregation jobs for daily and weekly analytics processing |
| const allEvents = await prisma.analyticsEvent.findMany({ | ||
| where: { timestamp: { gte: yesterday, lt: today } }, | ||
| select: { | ||
| eventType: true, | ||
| visitorId: true, | ||
| sessionId: true, | ||
| pageCategory: true, | ||
| deviceType: true, | ||
| browser: true, | ||
| referrer: true, | ||
| durationMs: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The nightly aggregation fetches all events for the day into memory before processing. For high-traffic sites, this could consume significant memory and cause performance issues. Consider processing events in batches or using streaming/cursor-based pagination.
| export async function sendEvent(payload: AnalyticsEventPayload): Promise<void> { | ||
| try { | ||
| const eventPayload = { | ||
| ...payload, | ||
| timestamp: payload.timestamp ?? new Date().toISOString(), | ||
| }; | ||
|
|
||
| await fetch('/api/analytics', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(eventPayload), | ||
| }); | ||
| } catch { | ||
| // Silently fail - analytics should not break the app | ||
| } |
There was a problem hiding this comment.
The error handling in analytics functions silently fails, which makes debugging issues difficult. While silent failures are intentional to prevent analytics from breaking the app, consider logging errors to a monitoring service (like Sentry) so issues can be detected and fixed.
| return { | ||
| browser: result.browser.name ?? null, | ||
| browserVer: result.browser.version ?? null, | ||
| os: result.os.name ?? null, | ||
| deviceType: result.device.type ?? 'desktop', |
There was a problem hiding this comment.
The device type detection defaults to 'desktop' when device type is undefined. This could skew analytics data as mobile devices without a device type in the parsed result would be incorrectly classified as desktop. Consider using 'unknown' or implementing additional heuristics to better classify devices.
| return { | |
| browser: result.browser.name ?? null, | |
| browserVer: result.browser.version ?? null, | |
| os: result.os.name ?? null, | |
| deviceType: result.device.type ?? 'desktop', | |
| // Derive device type with a safe fallback to avoid misclassifying unknowns as desktop | |
| const parsedDeviceType = result.device.type ?? null; | |
| const osName = result.os.name ?? null; | |
| let deviceType: string; | |
| if (parsedDeviceType) { | |
| deviceType = parsedDeviceType; | |
| } else if (osName && /Android|iOS|iPadOS/.test(osName)) { | |
| // Basic heuristic: common mobile OS names imply a mobile device | |
| deviceType = 'mobile'; | |
| } else { | |
| deviceType = 'unknown'; | |
| } | |
| return { | |
| browser: result.browser.name ?? null, | |
| browserVer: result.browser.version ?? null, | |
| os: osName, | |
| deviceType, |
| "crons": [ | ||
| { | ||
| "path": "/api/cron/aggregate-nightly", | ||
| "schedule": "0 2 * * *" | ||
| }, | ||
| { | ||
| "path": "/api/cron/aggregate-weekly", | ||
| "schedule": "0 3 * * 0" | ||
| } | ||
| ] |
There was a problem hiding this comment.
The cron schedule '0 2 * * *' runs at 2 AM daily and '0 3 * * 0' runs at 3 AM on Sundays. These times should be documented in terms of which timezone they use (likely UTC based on Vercel Cron). Consider adding a comment indicating the timezone to avoid confusion.
| import { UserRoleSchema } from '@/schemas'; | ||
| import { ReactNode, useEffect, useState } from 'react'; |
There was a problem hiding this comment.
The import order is inconsistent with the project's style. The UserRoleSchema import should remain at the top with other imports, not be moved after the component imports. This change appears to be an unintentional reordering.
| import { UserRoleSchema } from '@/schemas'; | |
| import { ReactNode, useEffect, useState } from 'react'; | |
| import { ReactNode, useEffect, useState } from 'react'; | |
| import { UserRoleSchema } from '@/schemas'; |
| function isAuthorizedRequest(request: NextRequest): boolean { | ||
| // Check for cron secret header | ||
| if (request.headers.get('Authorization') !== `Bearer ${env.CRON_SECRET}`) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; |
There was a problem hiding this comment.
The authorization check will fail when CRON_SECRET is undefined, as it will compare the Authorization header against "Bearer undefined". This could either block legitimate cron jobs or create a security vulnerability. The CRON_SECRET should either be required or the authorization logic should explicitly handle the undefined case.
| function isAuthorizedRequest(request: NextRequest): boolean { | ||
| // Check for cron secret header | ||
| if (request.headers.get('Authorization') !== `Bearer ${env.CRON_SECRET}`) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
The authorization check will fail when CRON_SECRET is undefined, as it will compare the Authorization header against "Bearer undefined". This could either block legitimate cron jobs or create a security vulnerability. The CRON_SECRET should either be required or the authorization logic should explicitly handle the undefined case.
| import { PropsWithChildren } from 'react'; | ||
| import { Toaster } from 'sonner'; | ||
| import './globals.css'; |
There was a problem hiding this comment.
The import order change is inconsistent with the rest of the file's import structure. The PropsWithChildren type import should remain at the end of the import block rather than being moved before the local imports, to maintain consistent import ordering (external packages first, then local imports).
| import { PropsWithChildren } from 'react'; | |
| import { Toaster } from 'sonner'; | |
| import './globals.css'; | |
| import { Toaster } from 'sonner'; | |
| import './globals.css'; | |
| import { PropsWithChildren } from 'react'; |
| "react-hook-form": "^7.66.1", | ||
| "react-hook-form": "^7.68.0", | ||
| "react-zoom-pan-pinch": "^3.7.0", | ||
| "recharts": "2.15.4", |
There was a problem hiding this comment.
The recharts package is pinned to version 2.15.4 instead of using a caret (^) or tilde (~) range. This prevents automatic patch updates which could include important bug fixes and security patches. Unless there's a specific reason for pinning, consider using ^2.15.4 to allow patch updates.
| "recharts": "2.15.4", | |
| "recharts": "^2.15.4", |
| export function setVisitorCookie(visitorId: string): void { | ||
| document.cookie = `${VISITOR_COOKIE_NAME}=${visitorId}; Path=/; Max-Age=${COOKIE_MAX_AGE}; SameSite=Lax`; | ||
| } | ||
|
|
||
| /** | ||
| * Get the visitor ID from cookie | ||
| */ | ||
| export function getVisitorCookie(): string | null { | ||
| if (typeof document === 'undefined') return null; | ||
| const match = document.cookie.split('; ').find(c => c.startsWith(`${VISITOR_COOKIE_NAME}=`)); | ||
| return match?.split('=')[1] ?? null; | ||
| } | ||
|
|
||
| /** | ||
| * Set consent cookie to track user's cookie preference | ||
| */ | ||
| export function setConsentCookie(accepted: boolean): void { | ||
| const value = accepted ? 'accepted' : 'rejected'; | ||
| document.cookie = `${CONSENT_COOKIE_NAME}=${value}; Path=/; Max-Age=${COOKIE_MAX_AGE}; SameSite=Lax`; |
There was a problem hiding this comment.
The cookie banner sets cookies without the Secure flag, which means cookies could be transmitted over unencrypted HTTP connections. For production environments, cookies should include the Secure flag to ensure they're only sent over HTTPS.
No description provided.