Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes introduce YouTube URL-based music functionality across the application: MusicModal UI updated for YouTube URL input, StudentHome refactored to POST music requests with YouTube URLs to a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MusicModal
participant StudentHome
participant API
participant TimerHome
participant AudioPlayer
User->>MusicModal: Submit YouTube URL
MusicModal->>StudentHome: handleMusicSubmit(youtubeUrl)
StudentHome->>API: POST /std/music/request<br/>{url: youtubeUrl}<br/>(with Authorization)
rect rgb(200, 220, 255)
Note over API: Process music request
alt Success
API-->>StudentHome: 200 + Response
StudentHome->>StudentHome: Log response, store data
StudentHome-->>MusicModal: Success (handled by modal)
else Error
API-->>StudentHome: Error response<br/>(4xx/5xx or custom code)
StudentHome->>StudentHome: Map error to category<br/>& compose message
StudentHome-->>MusicModal: Alert user & rethrow
end
end
User->>TimerHome: View timer page
TimerHome->>API: GET /music/queue
API-->>TimerHome: musicQueue list
loop Every 10 seconds
TimerHome->>API: Refresh queue
API-->>TimerHome: Updated queue
end
User->>TimerHome: Click play on music item
TimerHome->>API: GET /music/stream?id={itemId}<br/>(with Authorization)
API-->>TimerHome: Audio stream + metadata headers
TimerHome->>TimerHome: Extract metadata,<br/>create blob URL
TimerHome->>AudioPlayer: Update nowPlaying & audio src
AudioPlayer->>User: ▶️ Play audio
AudioPlayer->>TimerHome: onEnded event
TimerHome->>TimerHome: Clean up previous URL<br/>clear nowPlaying
TimerHome->>API: Refresh queue
alt Next track available
TimerHome->>TimerHome: Auto-play next track
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (4)
src/pages/StudentHome.jsx (3)
250-251: Consider removing debug logs before production.These
console.logstatements are useful for development but should be removed or converted to conditional logging before deploying to production to avoid leaking potentially sensitive information (like user-submitted URLs).🔎 Suggested fix
const handleMusicSubmit = async (youtubeUrl) => { try { const token = localStorage.getItem('auth_token'); - console.log("===== 음악 신청 시작 ====="); - console.log("YouTube URL:", youtubeUrl); - // 1단계: 상점에서 음악 신청 쿠폰 찾기 (ID: 998)
290-304: Approve API integration; consider removing debug logs.The API call structure is correct with proper authorization headers. The success messaging is handled by MusicModal as noted in the comment, which aligns with the component's design.
The
console.logon lines 302-303 should also be removed for production consistency.
338-340: String-based error filtering is fragile.The condition relies on checking if
errorMsg.includes()specific Korean strings. This can break if error messages are changed or translated. Consider using a local flag or error code instead.🔎 Suggested approach
+ let alertShown = false; + const handleMusicSubmit = async (youtubeUrl) => { try { // ... existing code ... if (!musicItem) { alert("음악 신청 상품을 찾을 수 없습니다."); + alertShown = true; throw new Error("음악 신청 상품을 찾을 수 없습니다."); } // ... similarly for other early throws ... } catch (error) { // ... existing error handling ... - } else if (!errorMsg.includes("크레딧 부족") && !errorMsg.includes("찾을 수 없습니다")) { + } else if (!alertShown) { alert(`음악 신청에 실패했습니다: ${errorMsg}`); } + alertShown = false; throw error; } };src/pages/TimerHome.jsx (1)
207-226: Music queue list is not rendered.The styled components
QueueList,QueueItem, andPlayButton(lines 31-89) are defined but never used in the render output. Currently, only the "Now Playing" section is displayed. If the queue display is intentional for a future iteration, consider adding a TODO comment; otherwise, the unused components should be removed or the queue should be rendered.Is this intentional? Would you like me to generate the missing queue display implementation?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/MusicModal.jsxsrc/pages/StudentHome.jsxsrc/pages/TimerHome.jsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/MusicModal.jsx (1)
src/components/AnnouncementModal.jsx (5)
Title(64-73)Example(75-85)InputBox(88-111)inputValue(180-180)handleKeyDown(220-225)
src/pages/StudentHome.jsx (4)
src/lib/customAxios.js (1)
token(34-34)src/lib/auth.js (1)
response(5-5)src/pages/TeamSpace.jsx (1)
error(290-290)src/pages/MyTeam.jsx (1)
error(218-218)
src/pages/TimerHome.jsx (1)
src/components/Timer.jsx (1)
Timer(128-269)
🔇 Additional comments (3)
src/components/MusicModal.jsx (1)
243-249: LGTM!The UI text updates are clear and consistent. The title, example, and placeholder all correctly guide users to input a YouTube URL. Validation is appropriately delegated to the submission handler.
src/pages/TimerHome.jsx (2)
113-116: LGTM on state and ref initialization.State variables and audio ref are properly initialized for managing the music queue and playback.
129-134: LGTM!The interval is properly cleaned up on unmount, preventing memory leaks. The 10-second polling interval is reasonable for keeping the queue updated.
| const handlePlay = async (musicId) => { | ||
| try { | ||
| setLoading(true); | ||
| const token = localStorage.getItem('auth_token'); | ||
|
|
||
| const url = musicId | ||
| ? `${import.meta.env.VITE_API_URL}tch/music/stream/${musicId}` | ||
| : `${import.meta.env.VITE_API_URL}tch/music/stream`; | ||
|
|
||
| const response = await customaxios.get(url, { | ||
| headers: { | ||
| 'Authorization': `Bearer ${token}` | ||
| }, | ||
| responseType: 'blob' | ||
| }); | ||
|
|
||
| // 헤더에서 메타데이터 추출 | ||
| const title = decodeURIComponent(response.headers['x-music-title'] || 'Unknown'); | ||
| const requester = decodeURIComponent(response.headers['x-music-requester'] || 'Unknown'); | ||
| const team = decodeURIComponent(response.headers['x-music-team'] || 'Unknown'); | ||
|
|
||
| // Blob URL 생성 | ||
| const audioBlob = new Blob([response.data], { type: 'audio/webm' }); | ||
| const audioUrl = URL.createObjectURL(audioBlob); | ||
|
|
||
| setNowPlaying({ | ||
| id: musicId, | ||
| title, | ||
| requester, | ||
| team, | ||
| url: audioUrl | ||
| }); | ||
|
|
||
| // 오디오 재생 | ||
| if (audioRef.current) { | ||
| audioRef.current.src = audioUrl; | ||
| audioRef.current.play(); | ||
| } | ||
|
|
||
| // 큐 업데이트 | ||
| await fetchMusicQueue(); | ||
|
|
||
| } catch (error) { | ||
| console.error("음악 재생 실패:", error); | ||
| if (error.response?.data?.error === "NOT_FOUND") { | ||
| alert("재생할 음악이 없습니다."); | ||
| } else { | ||
| alert("음악 재생에 실패했습니다."); | ||
| } | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Potential blob URL memory leak.
When handlePlay is called while another track is playing (or on unmount), the previous blob URL is not revoked. This can lead to memory leaks.
🔎 Suggested fix
const handlePlay = async (musicId) => {
try {
setLoading(true);
+
+ // Revoke previous blob URL if exists
+ if (nowPlaying?.url) {
+ URL.revokeObjectURL(nowPlaying.url);
+ }
+
const token = localStorage.getItem('auth_token');
// ... rest of the codeAlso add cleanup on unmount:
useEffect(() => {
return () => {
if (nowPlaying?.url) {
URL.revokeObjectURL(nowPlaying.url);
}
};
}, [nowPlaying?.url]);📝 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.
| const handlePlay = async (musicId) => { | |
| try { | |
| setLoading(true); | |
| const token = localStorage.getItem('auth_token'); | |
| const url = musicId | |
| ? `${import.meta.env.VITE_API_URL}tch/music/stream/${musicId}` | |
| : `${import.meta.env.VITE_API_URL}tch/music/stream`; | |
| const response = await customaxios.get(url, { | |
| headers: { | |
| 'Authorization': `Bearer ${token}` | |
| }, | |
| responseType: 'blob' | |
| }); | |
| // 헤더에서 메타데이터 추출 | |
| const title = decodeURIComponent(response.headers['x-music-title'] || 'Unknown'); | |
| const requester = decodeURIComponent(response.headers['x-music-requester'] || 'Unknown'); | |
| const team = decodeURIComponent(response.headers['x-music-team'] || 'Unknown'); | |
| // Blob URL 생성 | |
| const audioBlob = new Blob([response.data], { type: 'audio/webm' }); | |
| const audioUrl = URL.createObjectURL(audioBlob); | |
| setNowPlaying({ | |
| id: musicId, | |
| title, | |
| requester, | |
| team, | |
| url: audioUrl | |
| }); | |
| // 오디오 재생 | |
| if (audioRef.current) { | |
| audioRef.current.src = audioUrl; | |
| audioRef.current.play(); | |
| } | |
| // 큐 업데이트 | |
| await fetchMusicQueue(); | |
| } catch (error) { | |
| console.error("음악 재생 실패:", error); | |
| if (error.response?.data?.error === "NOT_FOUND") { | |
| alert("재생할 음악이 없습니다."); | |
| } else { | |
| alert("음악 재생에 실패했습니다."); | |
| } | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| const handlePlay = async (musicId) => { | |
| try { | |
| setLoading(true); | |
| // Revoke previous blob URL if exists | |
| if (nowPlaying?.url) { | |
| URL.revokeObjectURL(nowPlaying.url); | |
| } | |
| const token = localStorage.getItem('auth_token'); | |
| const url = musicId | |
| ? `${import.meta.env.VITE_API_URL}tch/music/stream/${musicId}` | |
| : `${import.meta.env.VITE_API_URL}tch/music/stream`; | |
| const response = await customaxios.get(url, { | |
| headers: { | |
| 'Authorization': `Bearer ${token}` | |
| }, | |
| responseType: 'blob' | |
| }); | |
| // 헤더에서 메타데이터 추출 | |
| const title = decodeURIComponent(response.headers['x-music-title'] || 'Unknown'); | |
| const requester = decodeURIComponent(response.headers['x-music-requester'] || 'Unknown'); | |
| const team = decodeURIComponent(response.headers['x-music-team'] || 'Unknown'); | |
| // Blob URL 생성 | |
| const audioBlob = new Blob([response.data], { type: 'audio/webm' }); | |
| const audioUrl = URL.createObjectURL(audioBlob); | |
| setNowPlaying({ | |
| id: musicId, | |
| title, | |
| requester, | |
| team, | |
| url: audioUrl | |
| }); | |
| // 오디오 재생 | |
| if (audioRef.current) { | |
| audioRef.current.src = audioUrl; | |
| audioRef.current.play(); | |
| } | |
| // 큐 업데이트 | |
| await fetchMusicQueue(); | |
| } catch (error) { | |
| console.error("음악 재생 실패:", error); | |
| if (error.response?.data?.error === "NOT_FOUND") { | |
| alert("재생할 음악이 없습니다."); | |
| } else { | |
| alert("음악 재생에 실패했습니다."); | |
| } | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/pages/TimerHome.jsx around lines 137 to 189, the handler creates blob
URLs for streamed audio but never revokes the previous URL, causing a memory
leak; update handlePlay to revoke the existing nowPlaying.url (if any) before
creating and setting a new audioUrl, and add a useEffect cleanup that revokes
the current nowPlaying.url on unmount (or when nowPlaying.url changes) so any
leftover object URLs are released.
| // 음악 종료 시 다음 곡 자동 재생 | ||
| const handleEnded = () => { | ||
| if (nowPlaying?.url) { | ||
| URL.revokeObjectURL(nowPlaying.url); | ||
| } | ||
| setNowPlaying(null); | ||
|
|
||
| // 큐에 다음 곡이 있으면 자동 재생 | ||
| if (musicQueue.length > 0) { | ||
| handlePlay(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Stale closure bug: musicQueue may be outdated when handleEnded executes.
The handleEnded callback captures musicQueue from the render when it was attached to the <audio> element. By the time the audio ends, the queue may have been refreshed multiple times, but handleEnded still references the old array.
🔎 Suggested fix using a ref
+ const musicQueueRef = useRef([]);
+
+ // Keep ref in sync with state
+ useEffect(() => {
+ musicQueueRef.current = musicQueue;
+ }, [musicQueue]);
const handleEnded = () => {
if (nowPlaying?.url) {
URL.revokeObjectURL(nowPlaying.url);
}
setNowPlaying(null);
// 큐에 다음 곡이 있으면 자동 재생
- if (musicQueue.length > 0) {
+ if (musicQueueRef.current.length > 0) {
handlePlay();
}
};📝 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.
| // 음악 종료 시 다음 곡 자동 재생 | |
| const handleEnded = () => { | |
| if (nowPlaying?.url) { | |
| URL.revokeObjectURL(nowPlaying.url); | |
| } | |
| setNowPlaying(null); | |
| // 큐에 다음 곡이 있으면 자동 재생 | |
| if (musicQueue.length > 0) { | |
| handlePlay(); | |
| } | |
| }; | |
| const musicQueueRef = useRef([]); | |
| // Keep ref in sync with state | |
| useEffect(() => { | |
| musicQueueRef.current = musicQueue; | |
| }, [musicQueue]); | |
| // 음악 종료 시 다음 곡 자동 재생 | |
| const handleEnded = () => { | |
| if (nowPlaying?.url) { | |
| URL.revokeObjectURL(nowPlaying.url); | |
| } | |
| setNowPlaying(null); | |
| // 큐에 다음 곡이 있으면 자동 재생 | |
| if (musicQueueRef.current.length > 0) { | |
| handlePlay(); | |
| } | |
| }; |
1 participant
#86
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.