feat(subscriptions): inbox page + nav link#128
Conversation
New `/subscriptions` route (RequireAuth-gated) shows a grid of new videos from channels the signed-in user follows. On mount it fetches `GET /api/users/me/inbox?limit=50` and fires `POST /api/users/me/inbox/seen` to clear the unseen counter for the next visit. Empty state nudges the viewer back to trending. The signed-in header nav gains a "Subscriptions" button. The subscriptions Worker (`src/workers/subscriptions.ts`) and all its endpoints already shipped with full integration coverage (15 tests via PR #124) but had no frontend surface; this is the missing piece that closes the ALO-124 "subscribed viewer is notified when a creator they follow uploads" acceptance. Bundle: new `Subscriptions-*.js` chunk at 2.97 kB raw / 1.40 kB gz; no regression to other route chunks. Refs ALO-124 https://claude.ai/code/session_01KYVQhoiq656KTnYLnphKpJ
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds a new auth-protected Subscriptions inbox page to the frontend. It includes route registration with lazy loading, a React component that fetches and displays user inbox items with relative timestamps, and a comprehensive test suite covering success, empty, and error states. ChangesSubscriptions Inbox Page Feature
Sequence DiagramsequenceDiagram
participant SubscriptionsComponent
participant FetchAPI
participant InboxAPI as /api/users/me/inbox
participant SeenAPI as /api/users/me/inbox/seen
participant Analytics
SubscriptionsComponent->>FetchAPI: Fetch inbox items on mount
FetchAPI->>InboxAPI: GET with credentials and query params
InboxAPI-->>FetchAPI: Return items array or empty
FetchAPI-->>SubscriptionsComponent: Store items in state
SubscriptionsComponent->>Analytics: Track event with item count
SubscriptionsComponent->>SeenAPI: POST to mark items seen (fire-and-forget)
SubscriptionsComponent->>SubscriptionsComponent: Render error/loading/empty/InboxCard grid
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Subscriptions feature, adding a dedicated page, routing, and a navigation link in the header. The Subscriptions page fetches a user's inbox items, displays them in a grid of cards, and includes logic to mark the inbox as seen. Comprehensive DOM tests are also included. Feedback focuses on improving the reliability of date parsing for cross-browser compatibility, ensuring consistent credential handling in API requests to match the rest of the application, and adding a CSS class for visual consistency during page transitions.
| const t = Date.parse(iso); | ||
| if (Number.isNaN(t)) return ''; |
There was a problem hiding this comment.
The Date.parse call may fail or return incorrect results in some browsers (like Safari) because SQLite's CURRENT_TIMESTAMP format (YYYY-MM-DD HH:MM:SS) is not a strictly valid ISO 8601 string. This can lead to the date being interpreted as local time or failing to parse entirely (returning NaN). Normalizing the string by adding the 'T' separator and 'Z' suffix ensures reliable cross-browser parsing as UTC.
| const t = Date.parse(iso); | |
| if (Number.isNaN(t)) return ''; | |
| const normalized = iso.includes('T') ? iso : iso.replace(' ', 'T') + 'Z'; | |
| const t = Date.parse(normalized); | |
| if (Number.isNaN(t)) return ''; |
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
| void fetch(`/api/users/me/inbox?limit=${PAGE_LIMIT}`, { credentials: 'include' }) |
There was a problem hiding this comment.
The use of credentials: 'include' is inconsistent with other API calls in the application (e.g., in App.tsx), which use credentials: 'same-origin'. Since the API is on the same origin, same-origin is preferred to avoid unnecessarily sending credentials on potential cross-origin requests.
| void fetch(`/api/users/me/inbox?limit=${PAGE_LIMIT}`, { credentials: 'include' }) | |
| void fetch(`/api/users/me/inbox?limit=${PAGE_LIMIT}`, { credentials: 'same-origin' }) |
| // this call is non-fatal — the inbox itself loaded fine. | ||
| void fetch('/api/users/me/inbox/seen', { | ||
| method: 'POST', | ||
| credentials: 'include', |
There was a problem hiding this comment.
Consistency: Use credentials: 'same-origin' to match the rest of the application's API requests.
| credentials: 'include', | |
| credentials: 'same-origin', |
| }, []); | ||
|
|
||
| return ( | ||
| <main className="app-main stack"> |
There was a problem hiding this comment.
To maintain visual consistency with the Home page and other main routes, consider adding the fade-in class to the main container.
| <main className="app-main stack"> | |
| <main className="app-main stack fade-in"> |
Three gemini-code-assist review findings on PR #128: * `timeSince` was calling `Date.parse` on `added_at` straight from D1. SQLite's `CURRENT_TIMESTAMP` is `YYYY-MM-DD HH:MM:SS` (no T, no Z), which Safari's parser can refuse or interpret as local time. Normalize to ISO-8601 UTC before parsing so the relative-time string is correct cross-browser. * `credentials: 'include'` → `'same-origin'` on both `/inbox` GET and `/inbox/seen` POST to match the rest of the app (Watch, Comments, ReportButton, auth-client, sessions all use same-origin; only the legacy AccountSettings page still uses include). * Add `fade-in` to the root `<main>` and switch `stack` → `stack-lg` so the page matches the visual rhythm of every other discovery surface (Channel, Watch, Search, Tag, Pricing). https://claude.ai/code/session_01KYVQhoiq656KTnYLnphKpJ
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27843bc646
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const PAGE_LIMIT = 50; | ||
|
|
||
| function timeSince(iso: string): string { | ||
| const t = Date.parse(iso); |
There was a problem hiding this comment.
subscription_inbox.added_at is stored as SQLite CURRENT_TIMESTAMP (UTC without an explicit offset) and returned by /api/users/me/inbox unchanged, so feeding it directly into Date.parse(iso) makes browsers interpret it as local time; for non-UTC users this can skew relative times by hours (including showing recent uploads as just now for too long). Please normalize to an explicit UTC/W3C timestamp before computing timeSince.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
/subscriptionsroute (RequireAuth-gated) renders a grid of new videos from channels the signed-in user follows.GET /api/users/me/inbox?limit=50, then firesPOST /api/users/me/inbox/seento clear the unseen counter for the next visit.track('subscriptions_inbox_viewed', { count })on successful load.The subscriptions Worker (
src/workers/subscriptions.ts) and all its endpoints already shipped with full integration coverage (15 tests via #124), but had no frontend surface. This is the missing piece that closes the ALO-124 "subscribed viewer is notified when a creator they follow uploads" acceptance.Bundle impact: new
Subscriptions-*.jschunk at 2.97 kB raw / 1.40 kB gz; no regression to other route chunks.Test plan
npm run lint— clean (no errors)npm run type-check— cleannpm test -- --run— 48 files, 509 tests pass (+3 vs main)npm run build— cleanSubscriptions.tsxcover happy path, empty state, fetch errorRefs ALO-124
https://claude.ai/code/session_01KYVQhoiq656KTnYLnphKpJ
Generated by Claude Code
Summary by CodeRabbit
New Features
Tests