Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to close issues #41–#46 by improving unauthenticated UX (Live TV and “Add Torrent”), clarifying auth-gated sidebar navigation, preventing /pricing from hanging while loading payment methods, and cleaning up robots.txt / footer content.
Changes:
- Add a 10s abort timeout + improved timeout messaging to
useSupportedCoins. - Add “login required” affordances (tooltip/lock) to sidebar protected items and new sidebar tests.
- Update Live TV unauthenticated behavior to show an in-page login prompt; add footer copyright; update robots.txt disallow rules; adjust “Add Torrent” quick action for logged-out users.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/use-supported-coins.ts | Adds AbortController timeout + timeout-specific error message for supported coins fetch. |
| src/hooks/use-supported-coins.test.ts | Updates fetch expectations to include an AbortSignal. |
| src/components/layout/sidebar.tsx | Adds lock/tooltip affordances and routes protected items to /login. |
| src/components/layout/sidebar.test.tsx | Adds tests asserting lock indicator presence/absence by auth state. |
| src/components/layout/main-layout.tsx | Adds dynamic copyright year and keeps footer links. |
| src/components/home/quick-actions.tsx | Redirects logged-out users to /login when clicking “Add Torrent” and updates copy. |
| src/app/live-tv/page.tsx | Replaces redirect with an in-page “Login Required” prompt and sign-in CTA. |
| public/robots.txt | Replaces redundant Allow with Disallow rules for sensitive routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), 10000); | ||
|
|
||
| const response = await fetch( | ||
| `/api/supported-coins?active_only=${activeOnly}` | ||
| `/api/supported-coins?active_only=${activeOnly}`, | ||
| { signal: controller.signal } | ||
| ); | ||
| clearTimeout(timeoutId); |
There was a problem hiding this comment.
timeoutId is only cleared after fetch resolves successfully. If fetch rejects (network error, AbortError, JSON parsing error), the timeout remains scheduled and can fire later, holding closures and aborting a controller after the request has already failed. Clear the timeout in finally (and/or immediately after creating it via a scoped helper) so it’s always cleaned up regardless of how the request completes.
| expect(result.current.coins).toEqual(mockCoins); | ||
| expect(result.current.error).toBeNull(); | ||
| expect(mockFetch).toHaveBeenCalledWith('/api/supported-coins?active_only=true'); | ||
| expect(mockFetch).toHaveBeenCalledWith('/api/supported-coins?active_only=true', expect.objectContaining({ signal: expect.any(AbortSignal) })); |
There was a problem hiding this comment.
The hook now has special handling for AbortError (10s timeout), but the tests only assert that fetch is called with a signal. Consider adding a test that advances fake timers to trigger the abort and asserts the user-facing timeout message ('Payment methods unavailable...') so the new behavior doesn’t regress.
| // Show lock icon for auth-required items when not logged in | ||
| const needsLogin = (item.requiresAuth && !isLoggedIn) || (item.requiresPaid && !isPremium); | ||
| const href = needsLogin ? '/login' : item.href; | ||
|
|
||
| return ( | ||
| <li key={item.href}> | ||
| <Link | ||
| href={href} | ||
| href={needsLogin ? '/login' : item.href} | ||
| onClick={onItemClick} | ||
| title={needsLogin ? 'Login required' : undefined} | ||
| className={cn( |
There was a problem hiding this comment.
needsLogin combines both "requiresAuth and not logged in" and "requiresPaid and not premium", but the UI always redirects to /login and sets title="Login required". For logged-in users who aren’t premium, this is incorrect/misleading and sends them to the wrong place. Split this into separate flags (e.g., needsAuth vs needsUpgrade) and use an appropriate destination/message (e.g., pricing/upgrade + "Premium required").
|
|
||
| {/* Footer */} | ||
| <footer className="border-t border-border-primary px-4 py-4 text-center text-sm text-text-secondary md:px-6"> | ||
| <span>© {new Date().getFullYear()} Media Streamer</span> |
There was a problem hiding this comment.
Because MainLayout is a client component, rendering new Date().getFullYear() can cause a rare hydration mismatch around New Year (server-rendered year vs client-rendered year). To avoid this, consider hardcoding the year, computing it on the server, or using suppressHydrationWarning on the year span.
| <span>© {new Date().getFullYear()} Media Streamer</span> | |
| <span suppressHydrationWarning>© {new Date().getFullYear()} Media Streamer</span> |
Fixes
Addresses all 6 open GitHub issues:
#44 — robots.txt redundant Allow rule
Replaced
Allow: /*with meaningfulDisallowrules for/api/,/login,/settings.#45 — Footer missing copyright
Added
© 2026 BitTorrentedto the footer alongside existing links.#46 — Sidebar nav items all point to /login
Added 🔒 lock icon and
title="Login required"tooltip to auth-protected sidebar items. Items still redirect to/loginwhen not authenticated, but now users can see why.#43 — /pricing 'Loading payment methods...' hangs indefinitely
Added 10-second
AbortControllertimeout to theuseSupportedCoinshook. Shows "Payment methods unavailable. Please try again later." on timeout.#42 — 'Add Torrent' button unclear for non-logged-in users
Non-authenticated users now see a 🔒 indicator and are redirected to
/logininstead of opening the modal.#41 — /live-tv silently redirects to /login
Replaced the silent HTTP 307 redirect with an in-page "Login Required" message and a Sign In button.
Testing