Conversation
…utton in closed modal v1.1.55
…redundant label v1.1.59
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Warning Rate limit exceeded@pavanpaik has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (6)
WalkthroughAdds an InquiryProvider and useInquiry hook for ticket state and unread badges; refactors AskPage to consume the context and updated ticket shapes; registers a service worker and offline page; integrates SW registration in the root layout; updates modal styling, global CSS, PWA manifest, Next config body size limit, and bumps web version/build flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant RootLayout
participant InquiryProvider
participant Auth
participant API
participant Nav
participant AskPage
User->>Browser: Open app
Browser->>RootLayout: Render RootLayout
RootLayout->>InquiryProvider: Mount provider
InquiryProvider->>Auth: Wait for session readiness
alt Signed in
Auth-->>InquiryProvider: user info
InquiryProvider->>API: fetch tickets (getTickets)
API-->>InquiryProvider: tickets
InquiryProvider->>InquiryProvider: hydrate read/ack from localStorage
InquiryProvider->>InquiryProvider: compute unreadCount & set badge
InquiryProvider->>Nav: provide unreadCount
InquiryProvider->>AskPage: provide tickets & actions
else Not signed in
Auth-->>InquiryProvider: no user (empty state)
end
Note over InquiryProvider: Starts 60s polling & visibility triggers
User->>AskPage: open/mark/send ticket
AskPage->>InquiryProvider: call markAsRead / refreshTickets / acknowledgeTicket
InquiryProvider->>InquiryProvider: update state & localStorage
InquiryProvider-->>Nav: updated unreadCount → re-render badges
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
web/src/context/InquiryContext.tsx (2)
51-62: Consider exposing error state to context consumers.The current implementation logs errors to the console but doesn't expose them to components consuming the context. This prevents the UI from showing user-friendly error messages or retry mechanisms when ticket fetching fails.
Consider adding an
errorfield toInquiryContextTypeand updating the state:const [error, setError] = useState<string | null>(null); const fetchTickets = useCallback(async () => { if (!isSignedIn) return; setError(null); try { const data = await getTickets(); setTickets(data as Ticket[]); } catch (error) { console.error('Failed to fetch tickets in context:', error); setError('Failed to load inquiries. Please try again.'); } finally { setIsLoading(false); } }, [isSignedIn]);
64-73: Consider optimizing the polling strategy.The current implementation polls every 60 seconds regardless of errors or page visibility. While this works, it could be more efficient and user-friendly.
Optional improvements:
- Pause polling when page is hidden using the Page Visibility API
- Implement exponential backoff after fetch errors
- Debounce rapid tab switches to avoid unnecessary fetches
Example with visibility API:
useEffect(() => { if (isLoaded && isSignedIn) { fetchTickets(); const interval = setInterval(() => { if (!document.hidden) { fetchTickets(); } }, 60000); return () => clearInterval(interval); } else if (isLoaded) { setIsLoading(false); } }, [isLoaded, isSignedIn, fetchTickets]);web/src/app/(main)/ask/page.tsx (1)
26-67: Consider splitting this large component.The component manages 11+ state variables and contains complex business logic. While functional, this level of complexity can make the component harder to test and maintain.
Optional refactoring strategies:
- Extract modal logic into separate components (
TicketModal,ArchiveModal)- Create custom hooks for ticket interactions (
useTicketActions,useTicketFiltering)- Split views into separate components (
GuestView,NewUserView,TicketListView)This would improve readability and make unit testing easier, but can be deferred if the current structure is working well for your team.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
web/next.config.ts(1 hunks)web/package.json(1 hunks)web/public/version.json(1 hunks)web/src/app/(main)/ask/page.tsx(7 hunks)web/src/components/common/Modal.tsx(1 hunks)web/src/components/layout/BottomNav.tsx(2 hunks)web/src/components/layout/Layout.tsx(1 hunks)web/src/components/layout/Sidebar.tsx(4 hunks)web/src/components/layout/UtilityMenu.tsx(6 hunks)web/src/context/InquiryContext.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
web/src/components/layout/Sidebar.tsx (1)
web/src/context/InquiryContext.tsx (1)
useInquiry(124-130)
web/src/context/InquiryContext.tsx (1)
web/src/actions/tickets.ts (1)
getTickets(8-51)
web/src/components/layout/BottomNav.tsx (1)
web/src/context/InquiryContext.tsx (1)
useInquiry(124-130)
web/src/components/layout/Layout.tsx (5)
web/src/context/InquiryContext.tsx (1)
InquiryProvider(35-122)web/src/components/layout/Sidebar.tsx (1)
Sidebar(22-105)web/src/components/layout/Header.tsx (1)
Header(4-21)web/src/components/audio/MiniPlayer.tsx (1)
MiniPlayer(7-79)web/src/components/layout/BottomNav.tsx (1)
BottomNav(16-50)
web/src/components/layout/UtilityMenu.tsx (1)
web/src/context/InquiryContext.tsx (1)
useInquiry(124-130)
🔇 Additional comments (19)
web/public/version.json (1)
2-3: LGTM!Version metadata correctly updated to reflect the 1.1.59 release, consistent with changes in package.json and UI footer displays.
web/next.config.ts (1)
4-8: LGTM!The experimental server actions body size limit configuration is valid for Next.js 15+. The 2MB limit appropriately supports larger form submissions for the inquiry/ticket system.
web/src/components/layout/Layout.tsx (1)
12-28: LGTM!The provider composition correctly wraps the layout with InquiryProvider at the outer level, enabling all descendant components to access inquiry state. The nesting order (InquiryProvider → AudioProvider → UI) is appropriate.
web/src/components/layout/BottomNav.tsx (1)
6-6: LGTM!The unread badge implementation is well-structured:
- Correctly consumes the inquiry context
- Uses proper relative/absolute positioning pattern
- Conditionally renders only for the '/ask' route when count > 0
- Includes smooth animations for visual polish
Also applies to: 18-18, 26-26, 35-42
web/src/components/common/Modal.tsx (1)
19-24: LGTM!Modal UI refinements improve consistency:
- Always centered positioning simplifies responsive behavior
- Uniform
rounded-2xlborder radius across breakpoints- Zoom-in animation provides smoother visual transition
These changes align well with the modal usage patterns in the inquiry/ticket system.
web/src/components/layout/Sidebar.tsx (2)
7-7: LGTM!The sidebar navigation correctly implements unread badges:
- Uses
justify-betweenlayout for proper label/badge spacing- Badge styling is consistent with mobile navigation
- Appropriate sizing for desktop viewport
Also applies to: 25-25, 50-50, 56-69
100-100: LGTM!Version string correctly updated to match the release version.
web/src/components/layout/UtilityMenu.tsx (3)
8-8: LGTM!The avatar badge implementation provides good visual feedback:
- Pulse animation draws attention without being intrusive
- Proper relative positioning on button container
- Compact design suitable for the avatar context
Also applies to: 20-20, 51-67
152-159: LGTM!The Guidance History badge effectively communicates unread count with appropriate styling and positioning for the drawer navigation context.
245-245: LGTM!Version string correctly updated to match the release version.
web/src/context/InquiryContext.tsx (4)
1-31: LGTM! Clean type definitions and imports.The interface definitions are well-structured and provide clear contracts for the Message, Ticket, and context types. The imports are appropriate for a client-side context.
75-87: LGTM! State management functions are well-implemented.Both
markAsReadandacknowledgeTicketproperly update React state and persist to localStorage. The duplicate check inacknowledgeTicket(line 82) prevents unnecessary updates.
108-130: LGTM! Standard React context pattern correctly implemented.The provider and hook follow React best practices with proper error boundaries for usage outside the provider.
89-106: Unread count logic is working as intended.After examining the status flow, OPEN tickets correctly never contribute to the unread count. The ticket status transitions prevent this scenario: when an admin replies to a ticket, the status automatically changes to ANSWERED (not OPEN), and when a user replies to an ANSWERED ticket, the status changes back to OPEN with the user's message as the last message. Therefore, an OPEN ticket cannot have an unread admin message—such messages always exist in the ANSWERED state. The current logic correctly handles unread ADMIN messages in ANSWERED and CLOSED states only.
web/src/app/(main)/ask/page.tsx (5)
69-80: LGTM! Ticket opening logic is well-implemented.The sticky session logic (lines 73-75) is a nice UX touch—keeping CLOSED tickets visible in the "Ongoing" tab temporarily after reading prevents jarring layout shifts. Automatically marking messages as read on open (lines 77-79) provides good user experience.
82-99: LGTM! Archive logic properly implemented.The handler correctly applies the sticky logic, acknowledges the ticket, refreshes the data, and provides user feedback through both success state and error notifications.
123-147: LGTM! Follow-up logic handles edge cases well.The implementation correctly marks the user's own reply as read (lines 132-137), which makes sense since users have inherently "seen" their own messages. The visual feedback via
lastSentMessageIdprovides good confirmation.
160-176: LGTM! Filtering logic correctly implements sticky tab behavior.The filtering logic is complex but well-reasoned. The sticky session logic (lines 167-171) prevents tickets from immediately disappearing after being read or archived, which significantly improves UX by avoiding jarring layout shifts during user actions.
178-576: Render logic is comprehensive and handles multiple states well.The component correctly handles:
- Guest vs authenticated views
- Loading states with appropriate feedback
- Empty states for new users
- Tab-based filtering with sticky session logic
- Modal-based interactions for detailed views
The implementation provides good user experience across all states.
| "scripts": { | ||
| "dev": "next dev", | ||
| "build": "prisma generate && next build", | ||
| "build": "prisma generate && prisma db push --accept-data-loss && next build", |
There was a problem hiding this comment.
Critical: Remove --accept-data-loss flag from production build script.
The --accept-data-loss flag in the build script will drop data during destructive schema changes without confirmation. This is extremely dangerous for production deployments and CI/CD pipelines.
Why this is critical:
- Production builds may silently drop columns, tables, or entire databases
- No warnings or confirmations are shown
- Data loss is irreversible
Recommendations:
- Remove
--accept-data-lossfrom the build script - Use
prisma migrate deployfor production builds (applies migration history) - Keep
prisma db push --accept-data-lossonly in local dev scripts
🔎 Proposed fix
- "build": "prisma generate && prisma db push --accept-data-loss && next build",
+ "build": "prisma generate && prisma migrate deploy && next build",For local development, add a separate script:
+ "dev:push": "prisma db push --accept-data-loss",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "build": "prisma generate && prisma db push --accept-data-loss && next build", | |
| "build": "prisma generate && prisma migrate deploy && next build", |
🤖 Prompt for AI Agents
In web/package.json around line 7, the production "build" script includes the
dangerous "--accept-data-loss" flag for "prisma db push"; remove that flag from
the production build, replace the Prisma step with "prisma migrate deploy" (so
migrations are applied safely in CI/production), and move "prisma db push
--accept-data-loss" into a new local-only script (e.g., "dev:db:reset") for
developer use; also ensure CI/build pipelines are updated to run "prisma migrate
deploy" instead of "prisma db push".
| type TicketMessage = { | ||
| id: string; | ||
| sender: 'USER' | 'ADMIN'; | ||
| text: string; | ||
| createdAt: Date; | ||
| createdAt: string; | ||
| }; | ||
|
|
||
| type Ticket = { | ||
| id: string; | ||
| subject: string; | ||
| status: 'OPEN' | 'ANSWERED' | 'CLOSED'; | ||
| createdAt: Date; | ||
| createdAt: string; | ||
| messages: TicketMessage[]; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Eliminate duplicate type definitions - import from context instead.
These type definitions are identical to those in InquiryContext.tsx (lines 7-20). Duplicating types violates DRY principles and creates a maintenance burden—if the context types evolve, these must be manually kept in sync.
🔎 Recommended refactor
Option 1: Export types from the context file and import them:
In web/src/context/InquiryContext.tsx:
export interface Message {
id: string;
text: string;
sender: 'USER' | 'ADMIN';
createdAt: string;
}
export interface Ticket {
id: string;
subject: string;
status: 'OPEN' | 'ANSWERED' | 'CLOSED';
createdAt: string;
messages: Message[];
}In this file:
-type TicketMessage = {
- id: string;
- sender: 'USER' | 'ADMIN';
- text: string;
- createdAt: string;
-};
-
-type Ticket = {
- id: string;
- subject: string;
- status: 'OPEN' | 'ANSWERED' | 'CLOSED';
- createdAt: string;
- messages: TicketMessage[];
-};
+import type { Message as TicketMessage, Ticket } from '@/context/InquiryContext';Option 2: Create a shared types file (@/types/inquiry.ts) and import in both locations.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/app/(main)/ask/page.tsx around lines 11 to 24 you have duplicate
Ticket and TicketMessage type definitions that are already defined in
web/src/context/InquiryContext.tsx; remove these local type declarations and
instead import the types from the canonical source—either export the types from
InquiryContext.tsx and import them here, or move the interfaces to a shared file
(e.g. web/src/types/inquiry.ts) and import from there; update the import
statement accordingly and ensure any references in this file use the imported
types.
| useEffect(() => { | ||
| const savedRead = localStorage.getItem('krishnasagar_read_messages'); | ||
| if (savedRead) setReadMessages(JSON.parse(savedRead)); | ||
|
|
||
| const savedAck = localStorage.getItem('krishnasagar_acknowledged_tickets'); | ||
| if (savedAck) setAcknowledgedTickets(JSON.parse(savedAck)); | ||
| }, []); |
There was a problem hiding this comment.
Add error handling for localStorage access.
While accessing localStorage in useEffect after hydration is generally safe, it can still fail in edge cases (private browsing modes, quota exceeded, or browser restrictions). Consider wrapping localStorage operations in try-catch blocks to prevent runtime errors.
🔎 Suggested improvement
useEffect(() => {
- const savedRead = localStorage.getItem('krishnasagar_read_messages');
- if (savedRead) setReadMessages(JSON.parse(savedRead));
-
- const savedAck = localStorage.getItem('krishnasagar_acknowledged_tickets');
- if (savedAck) setAcknowledgedTickets(JSON.parse(savedAck));
+ try {
+ const savedRead = localStorage.getItem('krishnasagar_read_messages');
+ if (savedRead) setReadMessages(JSON.parse(savedRead));
+
+ const savedAck = localStorage.getItem('krishnasagar_acknowledged_tickets');
+ if (savedAck) setAcknowledgedTickets(JSON.parse(savedAck));
+ } catch (error) {
+ console.warn('Failed to load saved state from localStorage:', error);
+ }
}, []);Apply the same pattern to localStorage.setItem calls on lines 78 and 85.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| const savedRead = localStorage.getItem('krishnasagar_read_messages'); | |
| if (savedRead) setReadMessages(JSON.parse(savedRead)); | |
| const savedAck = localStorage.getItem('krishnasagar_acknowledged_tickets'); | |
| if (savedAck) setAcknowledgedTickets(JSON.parse(savedAck)); | |
| }, []); | |
| useEffect(() => { | |
| try { | |
| const savedRead = localStorage.getItem('krishnasagar_read_messages'); | |
| if (savedRead) setReadMessages(JSON.parse(savedRead)); | |
| const savedAck = localStorage.getItem('krishnasagar_acknowledged_tickets'); | |
| if (savedAck) setAcknowledgedTickets(JSON.parse(savedAck)); | |
| } catch (error) { | |
| console.warn('Failed to load saved state from localStorage:', error); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In web/src/context/InquiryContext.tsx around lines 43-49 (and also update the
localStorage.setItem calls around lines 78 and 85), wrap all
localStorage.getItem and localStorage.setItem operations in try-catch blocks;
when reading, guard JSON.parse with try-catch and default to an empty array or
safe fallback if parsing fails or getItem throws, and when writing, catch errors
and silently fail or log a non-fatal warning without throwing so the component
continues to work in private/limited storage environments; ensure you replace
the direct calls with these guarded reads/writes and keep state updates only
when the read succeeded.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/src/components/common/ServiceWorkerRegistration.tsx (1)
12-12: Consider removing console.log statements in production.While helpful during development, these console statements will execute in production. Consider removing them or wrapping them in a development-only check.
🔎 Optional improvement
- console.log('SW registered: ', registration); + if (process.env.NODE_ENV === 'development') { + console.log('SW registered: ', registration); + }- console.log('SW registration failed: ', registrationError); + console.error('SW registration failed: ', registrationError);Note: Changed to
console.errorfor the failure case to ensure visibility of errors even in production.Also applies to: 15-15
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/public/sw.js(1 hunks)web/src/app/layout.tsx(2 hunks)web/src/components/common/ServiceWorkerRegistration.tsx(1 hunks)web/src/context/InquiryContext.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/context/InquiryContext.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/app/layout.tsx (1)
web/src/components/common/ServiceWorkerRegistration.tsx (1)
ServiceWorkerRegistration(5-22)
🔇 Additional comments (3)
web/src/app/layout.tsx (1)
7-7: LGTM! Clean integration of service worker registration.The ServiceWorkerRegistration component is properly imported and placed within the LanguageProvider alongside other global utilities, ensuring it runs once during app initialization.
Also applies to: 126-126
web/public/sw.js (2)
1-18: LGTM! Precache setup is well-structured.The install event handler correctly caches the essential assets and calls
skipWaiting()to activate the new service worker immediately.
20-33: LGTM! Cache cleanup and client claiming properly implemented.The activate handler correctly removes outdated caches and claims all clients immediately.
| useEffect(() => { | ||
| if ('serviceWorker' in navigator && window.location.hostname !== 'localhost') { | ||
| window.addEventListener('load', () => { | ||
| navigator.serviceWorker | ||
| .register('/sw.js') | ||
| .then((registration) => { | ||
| console.log('SW registered: ', registration); | ||
| }) | ||
| .catch((registrationError) => { | ||
| console.log('SW registration failed: ', registrationError); | ||
| }); | ||
| }); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Critical: Service worker registration will likely fail due to load event timing.
The load event listener is added inside useEffect, but by the time React components mount and useEffect runs, the window's load event has already fired. This means the service worker registration callback will never execute in most cases.
Additionally, the event listener is never cleaned up, creating a memory leak if the component were to unmount and remount.
🔎 Proposed fix
export default function ServiceWorkerRegistration() {
useEffect(() => {
- if ('serviceWorker' in navigator && window.location.hostname !== 'localhost') {
- window.addEventListener('load', () => {
- navigator.serviceWorker
- .register('/sw.js')
- .then((registration) => {
- console.log('SW registered: ', registration);
- })
- .catch((registrationError) => {
- console.log('SW registration failed: ', registrationError);
- });
- });
+ if (
+ 'serviceWorker' in navigator &&
+ window.location.hostname !== 'localhost' &&
+ window.location.hostname !== '127.0.0.1'
+ ) {
+ const registerSW = () => {
+ navigator.serviceWorker
+ .register('/sw.js')
+ .then((registration) => {
+ console.log('SW registered: ', registration);
+ })
+ .catch((registrationError) => {
+ console.log('SW registration failed: ', registrationError);
+ });
+ };
+
+ // Register immediately if already loaded, otherwise wait for load
+ if (document.readyState === 'complete') {
+ registerSW();
+ } else {
+ window.addEventListener('load', registerSW, { once: true });
+ }
}
}, []);
return null;
}This fix:
- Checks
document.readyStateand registers immediately if the page is already loaded - Uses
{ once: true }option to automatically clean up the listener after it fires - Adds
127.0.0.1to the hostname exclusion list
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if ('serviceWorker' in navigator && window.location.hostname !== 'localhost') { | |
| window.addEventListener('load', () => { | |
| navigator.serviceWorker | |
| .register('/sw.js') | |
| .then((registration) => { | |
| console.log('SW registered: ', registration); | |
| }) | |
| .catch((registrationError) => { | |
| console.log('SW registration failed: ', registrationError); | |
| }); | |
| }); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| if ( | |
| 'serviceWorker' in navigator && | |
| window.location.hostname !== 'localhost' && | |
| window.location.hostname !== '127.0.0.1' | |
| ) { | |
| const registerSW = () => { | |
| navigator.serviceWorker | |
| .register('/sw.js') | |
| .then((registration) => { | |
| console.log('SW registered: ', registration); | |
| }) | |
| .catch((registrationError) => { | |
| console.log('SW registration failed: ', registrationError); | |
| }); | |
| }; | |
| // Register immediately if already loaded, otherwise wait for load | |
| if (document.readyState === 'complete') { | |
| registerSW(); | |
| } else { | |
| window.addEventListener('load', registerSW, { once: true }); | |
| } | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In web/src/components/common/ServiceWorkerRegistration.tsx around lines 6 to 19,
the load event listener is added too late (useEffect runs after load) and never
cleaned up; update logic to (1) check navigator.serviceWorker and that
window.location.hostname is not 'localhost' or '127.0.0.1', (2) if
document.readyState is 'complete' (or not 'loading') register the service worker
immediately, otherwise add a window 'load' listener that calls the registration
and is created with the option { once: true } so it auto-removes (or
alternatively remove the listener in cleanup); ensure registration errors are
still handled in the promise chain.
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
…llback, and rich SEO schemas
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
web/src/context/InquiryContext.tsx (3)
43-49: Add error handling for localStorage access.This was flagged in a previous review and remains unaddressed. localStorage operations can fail in private browsing modes, when quota is exceeded, or due to browser restrictions. Additionally,
JSON.parse()can throw on malformed data.Based on the past review comment, wrap these operations in try-catch blocks to prevent runtime errors.
92-96: Add error handling for localStorage.setItem.Similar to the localStorage reads (lines 43-49), the
localStorage.setItemcall here lacks error handling. This operation can fail in private browsing modes or when storage quota is exceeded, causing the application to crash.Wrap this operation in a try-catch block as suggested in the earlier review comment.
98-104: Add error handling for localStorage.setItem.The
localStorage.setItemcall on line 102 also lacks error handling. Apply the same try-catch pattern suggested in the previous review to ensure resilience in restricted storage environments.web/public/sw.js (1)
50-68: Past review comment partially unaddressed: use networkResponse.ok instead of status === 200.While the error handling from the previous review was addressed (lines 60-62 now properly fall back to cache on network failure), line 56 still checks
networkResponse.status === 200instead of usingnetworkResponse.ok.Using
networkResponse.okis preferred because it covers all successful 2xx status codes (200-299), not just 200. For example, 201 (Created), 204 (No Content), and other successful responses won't be cached with the current implementation.🔎 Recommended fix
const fetchPromise = fetch(event.request).then((networkResponse) => { // Cache the new response if it's a valid GET request - if (event.request.method === 'GET' && networkResponse.status === 200) { + if (event.request.method === 'GET' && networkResponse.ok) { cache.put(event.request, networkResponse.clone()); } return networkResponse;
🧹 Nitpick comments (3)
web/src/app/(main)/bodhakatha/[articleId]/page.tsx (1)
59-103: Consider using environment variables for domain URLs.The JSON-LD structured data implementation follows schema.org standards correctly. However, the domain URL is hardcoded throughout (
https://saileelarahasya-web.vercel.app). Consider using an environment variable or themetadataBase(already defined in layout.tsx) to make this more maintainable across different environments.🔎 Suggested improvement using environment variable
+ const baseUrl = process.env.NEXT_PUBLIC_BASE_URL || 'https://saileelarahasya-web.vercel.app'; + const jsonLd = { "@context": "https://schema.org", "@type": "Article", "headline": article.title_english, "alternativeHeadline": article.title_hindi, "image": `https://img.youtube.com/vi/${article.youtube_id}/maxresdefault.jpg`, "author": { "@type": "Person", "name": "Krishnaji" }, "publisher": { "@type": "Organization", "name": "Sai Leela Rahasya", "logo": { "@type": "ImageObject", - "url": "https://saileelarahasya-web.vercel.app/icon-512.png" + "url": `${baseUrl}/icon-512.png` } }, "description": article.description.substring(0, 160) }; const breadcrumbLd = { "@context": "https://schema.org", "@type": "BreadcrumbList", "itemListElement": [ { "@type": "ListItem", "position": 1, "name": "Home", - "item": "https://saileelarahasya-web.vercel.app" + "item": baseUrl }, { "@type": "ListItem", "position": 2, "name": "Bodhakatha", - "item": "https://saileelarahasya-web.vercel.app/bodhakatha" + "item": `${baseUrl}/bodhakatha` }, { "@type": "ListItem", "position": 3, "name": article.title_english, - "item": `https://saileelarahasya-web.vercel.app/bodhakatha/${articleId}` + "item": `${baseUrl}/bodhakatha/${articleId}` } ] };web/src/app/(main)/leela/[articleId]/page.tsx (1)
59-103: Consider using environment variables for domain URLs.The JSON-LD structured data implementation follows schema.org standards correctly. However, the domain URL is hardcoded throughout (
https://saileelarahasya-web.vercel.app). Consider using an environment variable to make this more maintainable across different environments, consistent with the same pattern in the bodhakatha page.🔎 Suggested improvement using environment variable
+ const baseUrl = process.env.NEXT_PUBLIC_BASE_URL || 'https://saileelarahasya-web.vercel.app'; + const jsonLd = { "@context": "https://schema.org", "@type": "Article", "headline": article.title_english, "alternativeHeadline": article.title_hindi, "image": `https://img.youtube.com/vi/${article.youtube_id}/maxresdefault.jpg`, "author": { "@type": "Person", "name": "Krishnaji" }, "publisher": { "@type": "Organization", "name": "Sai Leela Rahasya", "logo": { "@type": "ImageObject", - "url": "https://saileelarahasya-web.vercel.app/icon-512.png" + "url": `${baseUrl}/icon-512.png` } }, "description": article.description.substring(0, 160) }; const breadcrumbLd = { "@context": "https://schema.org", "@type": "BreadcrumbList", "itemListElement": [ { "@type": "ListItem", "position": 1, "name": "Home", - "item": "https://saileelarahasya-web.vercel.app" + "item": baseUrl }, { "@type": "ListItem", "position": 2, "name": "Leela", - "item": "https://saileelarahasya-web.vercel.app/leela" + "item": `${baseUrl}/leela` }, { "@type": "ListItem", "position": 3, "name": article.title_english, - "item": `https://saileelarahasya-web.vercel.app/leela/${articleId}` + "item": `${baseUrl}/leela/${articleId}` } ] };web/src/app/layout.tsx (1)
47-47: Verify UI layout with black-translucent status bar.The
statusBarStylechange to"black-translucent"makes the iOS status bar transparent, allowing content to display behind it. Ensure that your app's layout accounts for the status bar height (especially on notched devices) to prevent content from being obscured.Consider using
safe-area-inset-topin your CSS to add proper padding:/* In your global CSS or component styles */ padding-top: env(safe-area-inset-top);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
web/public/favicon.pngis excluded by!**/*.pngweb/public/icon-192.pngis excluded by!**/*.pngweb/public/icon-512.pngis excluded by!**/*.pngweb/public/icon.pngis excluded by!**/*.pngweb/public/minimalist-premium-app-icon--a-single-centered-gol.pngis excluded by!**/*.pngweb/public/minimalist-premium-app-icon--a-single-centered-gol.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
web/public/offline.html(1 hunks)web/public/sw.js(1 hunks)web/src/app/(main)/bodhakatha/[articleId]/page.tsx(1 hunks)web/src/app/(main)/leela/[articleId]/page.tsx(1 hunks)web/src/app/globals.css(2 hunks)web/src/app/layout.tsx(3 hunks)web/src/context/InquiryContext.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/public/offline.html
🧰 Additional context used
🧬 Code graph analysis (2)
web/src/context/InquiryContext.tsx (1)
web/src/actions/tickets.ts (1)
getTickets(8-51)
web/src/app/layout.tsx (1)
web/src/components/common/ServiceWorkerRegistration.tsx (1)
ServiceWorkerRegistration(5-22)
🪛 ast-grep (0.40.0)
web/src/app/(main)/bodhakatha/[articleId]/page.tsx
[warning] 108-108: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
[warning] 112-112: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
web/src/app/(main)/leela/[articleId]/page.tsx
[warning] 108-108: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
[warning] 112-112: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
web/src/app/(main)/bodhakatha/[articleId]/page.tsx
[error] 109-109: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 113-113: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
web/src/app/(main)/leela/[articleId]/page.tsx
[error] 109-109: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 113-113: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (13)
web/src/app/globals.css (2)
22-27: LGTM! Good mobile scroll optimizations.The changes improve mobile UX:
overflow-y: autoonly shows scrollbars when neededoverscroll-behavior-y: noneprevents the bounce effect- The ochre background provides visual continuity during overscroll
29-42: Approve touch optimizations; verify accessibility requirements.The touch and scroll enhancements are well-suited for a PWA experience:
- Momentum scrolling and overscroll prevention improve mobile feel
min-height: 100vhensures proper layoutHowever, verify that the following accessibility trade-offs align with your requirements:
-webkit-touch-callout: nonedisables text selection/copy on iOStouch-action: pan-x pan-yprevents pinch-to-zoom-webkit-tap-highlight-color: transparentremoves visual tap feedbackThese are common PWA patterns but may impact users who rely on text selection or zoom for accessibility.
web/src/app/(main)/bodhakatha/[articleId]/page.tsx (1)
107-114: Static analysis warnings are false positives for JSON-LD injection.The static analysis tools flag
dangerouslySetInnerHTMLusage, but this is the standard and safe method for injecting JSON-LD structured data.JSON.stringify()automatically escapes any problematic characters, preventing XSS attacks. This pattern is recommended by Google and schema.org for SEO enhancement.web/src/app/(main)/leela/[articleId]/page.tsx (1)
107-114: Static analysis warnings are false positives for JSON-LD injection.The static analysis tools flag
dangerouslySetInnerHTMLusage, but this is the standard and safe method for injecting JSON-LD structured data.JSON.stringify()automatically escapes any problematic characters, preventing XSS attacks. This pattern is recommended by Google and schema.org for SEO enhancement.web/src/context/InquiryContext.tsx (4)
51-62: LGTM - Proper error handling in fetchTickets.The
fetchTicketsfunction correctly handles errors with try-catch and ensures the loading state is updated in the finally block. The early return for unauthenticated users is appropriate.
64-90: LGTM - Well-implemented visibility-aware polling.The polling implementation correctly:
- Only polls when the user is authenticated
- Respects document visibility to avoid unnecessary background requests
- Properly cleans up event listeners and intervals
- Handles the unauthenticated state by setting loading to false
107-123: LGTM - Unread count logic is well-defined.The unread count calculation correctly handles different ticket statuses:
- OPEN tickets are not counted (awaiting initial response)
- ANSWERED tickets are unread if the last message hasn't been read
- CLOSED tickets require both being unread and not acknowledged
The logic is sound and handles edge cases like empty message arrays.
126-138: LGTM - Proper PWA badge integration.The App Badge API integration correctly:
- Feature-detects support with
'setAppBadge' in navigator- Sets the badge count when there are unread items
- Clears the badge when count is zero
- Handles errors gracefully
The type cast to
anyis acceptable given TypeScript's incomplete typing for this newer API.web/src/app/layout.tsx (1)
7-7: LGTM - ServiceWorkerRegistration properly integrated.The ServiceWorkerRegistration component is correctly imported and placed within the LanguageProvider alongside other global components (UpdateDetector, Analytics, SpeedInsights). This enables the service worker registration for PWA functionality.
Also applies to: 126-126
web/public/sw.js (4)
1-10: LGTM - Precache assets are well-chosen.The precache list includes essential resources for offline functionality: the root page, manifest, icons, and offline page. The versioned cache name (
slr-cache-v1) enables proper cache invalidation when updates are needed.
12-19: LGTM - Standard install event implementation.The install handler correctly caches precache assets and calls
skipWaiting()to activate the new service worker immediately, providing users with updates without requiring them to close all tabs.
21-34: LGTM - Proper cache cleanup in activate event.The activate handler correctly removes outdated caches and uses
clients.claim()to take control of existing clients immediately. This ensures users get the latest cached resources.
36-48: LGTM - Navigation requests handled correctly.The fetch handler properly:
- Filters out cross-origin requests
- Provides a dedicated offline page for failed navigation requests
- Ensures users see a meaningful offline experience rather than a browser error
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/sage-vyasa?upgradeToPro=build-rate-limit |
1 participant
Summary by CodeRabbit
New Features
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.