[FE] 학생 SSE 연결 및 기능 구현 #209
Conversation
- 처음 학생이 강의 페이지 왔을때 SSE연결 요청 - SSE 메시지에 따라 처리 구현
WalkthroughThis pull request enhances the student course functionality by introducing centralized question management and improving error handling. The Changes
Sequence Diagram(s)sequenceDiagram
participant SC as StudentCourse
participant SSE as SSE Endpoint
participant util as studentPopupUtils
participant SQ as StudentQuestion
SC->>SSE: Establish connection (connectSSE)
SSE-->>SC: Send message (type: question / closedCourse)
alt Message type is "question"
SC->>SC: Update questions state
SC->>SQ: Pass updated questions and setQuestions props
else Message type is "closedCourse"
SC->>util: Trigger 'closedCourse' modal update
end
SSE-->>SC: On error, attempt reconnection
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
front-end/src/pages/student/home/StudentHome.tsx (1)
57-91:⚠️ Potential issueAdd handling for 'closedCourse' modal type.
Based on the PR objectives and AI summary, this component should handle SSE events for course closure. However, the
renderModalfunction doesn't include a case for the 'closedCourse' modal type.Add the missing case:
switch (modalType) { case 'classModal': { // ... existing code ... } case 'notFound': return getStudentPopup('notFound', closeModal, closeModal); case 'notStart': return getStudentPopup('notStart', closeModal, closeModal); case 'server': return getStudentPopup('server', closeModal, closeModal); + case 'closedCourse': + return getStudentPopup('closedCourse', closeModal, closeModal); default: return getStudentPopup('unknown', closeModal, closeModal); }front-end/src/pages/student/course/question/StudentQuestion.tsx (1)
47-53: 🛠️ Refactor suggestionReplace console.log with proper error handling in handleMessageClick.
The error handling in
handleMessageClickshould use the same error handling pattern ashandleInputSubmitfor consistency and better user feedback.const handleMessageClick = async (questionId: Question['id']) => { try { await classroomRepository.checkQuestionByStudent(questionId); } catch (error) { - console.log(error); + handleStudentError({ error, setModalType, openModal }); } };
🧹 Nitpick comments (2)
front-end/src/pages/student/course/question/StudentQuestion.tsx (2)
23-26: Consider extracting success popup logic to a custom hook.The success popup state and handling could be extracted to a reusable custom hook for better code organization and reusability.
// useSuccessPopup.ts const useSuccessPopup = (duration = 3000) => { const [isVisible, setIsVisible] = useState(false); const show = useCallback(() => { setIsVisible(true); }, []); const hide = useCallback(() => { setIsVisible(false); }, []); return { isVisible, show, hide }; };Also applies to: 98-103
72-77: Extract duplicate placeholder text to a constant.The placeholder text is duplicated in both QuestionForm instances. Consider extracting it to a constant for better maintainability.
+const QUESTION_PLACEHOLDER = "완전 익명으로 안심하고 질문해보세요"; const StudentQuestion = ({ // ... }) => { // ... return ( <> {questions.length === 0 ? ( // ... <QuestionForm inputValue={inputValue} setInputValue={setInputValue} onInputSubmit={handleInputSubmit} - placeholder="완전 익명으로 안심하고 질문해보세요" + placeholder={QUESTION_PLACEHOLDER} /> // ... ) : ( // ... <QuestionForm inputValue={inputValue} setInputValue={setInputValue} onInputSubmit={handleInputSubmit} - placeholder="완전 익명으로 안심하고 질문해보세요" + placeholder={QUESTION_PLACEHOLDER} /> // ... )} </> ); };Also applies to: 92-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
front-end/src/pages/student/course/StudentCourse.tsx(5 hunks)front-end/src/pages/student/course/question/StudentQuestion.tsx(2 hunks)front-end/src/pages/student/home/StudentHome.tsx(1 hunks)front-end/src/utils/studentPopupUtils.tsx(1 hunks)
🔇 Additional comments (13)
front-end/src/pages/student/home/StudentHome.tsx (1)
79-81: LGTM! Consistent casing in modal type.The casing change from 'notfound' to 'notFound' improves consistency with the PopupType enum.
front-end/src/pages/student/course/question/StudentQuestion.tsx (2)
10-15: LGTM! Props type definition is well-structured.The addition of
setQuestionsandquestionsprops with proper TypeScript types aligns well with lifting state management to the parent component.
17-22: LGTM! Component signature properly reflects the updated props.The destructuring of props is clean and follows React best practices.
front-end/src/pages/student/course/StudentCourse.tsx (9)
9-13: Imports look good.
The new imports fromstudentPopupUtilsalign well with the usage ofhandleStudentErrorandgetStudentPopup.
15-15: Model import verified.
ImportingQuestionfrom@/core/modelis consistent with the new questions-related state.
24-24: Ensure environment variable is set.
Usingimport.meta.env.VITE_API_URLis fine, but verify that this environment variable is configured properly in all deployment environments to avoid potential runtime errors.Do you want a script to check references to this variable throughout your codebase to ensure consistent usage?
30-30: Questions state usage is straightforward.
Storing the fetched questions in a React state here is appropriate. Consider adding loading indicators or error states if needed, but otherwise this is good.
34-34: Ref for EventSource is well-structured.
Maintaining theEventSourcein a ref avoids unnecessary re-renders. This approach is clear and maintainable.
38-48: Fetching questions on mount looks good.
The asyncfetchQuestionsand error handling viahandleStudentErroris straightforward. You might want to consider using anAbortControllerif there's a need to cancel this fetch on unmount, but it may not be necessary.
124-124: No further change needed.
This line appears to be a minor edit or reformat. The'notFound'case functionality remains consistent with existing logic.
142-147: New 'closedCourse' modal case is fitting.
Displaying a dedicated popup for the closed course scenario helps with clarity. Nicely done.
200-201: Props forwarding clarifies data flow.
PassingquestionsandsetQuestionsintoStudentQuestioncentralizes logic in the parent. This is coherent and keeps component responsibilities clear.front-end/src/utils/studentPopupUtils.tsx (1)
18-21: New popup config for 'closedCourse' is coherent.
Adding a dedicated configuration for the course-closed scenario aligns well with your SSE flow. This clearly communicates the intended user action.
| useEffect(() => { | ||
| const connectSSE = () => { | ||
| if (eventSourceRef.current) { | ||
| eventSourceRef.current.close(); | ||
| } | ||
|
|
||
| // eventSource.onopen = () => { | ||
| // console.log('🔗 SSE 연결됨'); | ||
| // }; | ||
| const eventSource = new EventSource(`${SSE_URL}/sse/connection/student`, { | ||
| withCredentials: true, | ||
| }); | ||
|
|
||
| // eventSource.onmessage = (event) => { | ||
| // console.log('📩 받은 데이터:', event.data); | ||
| // }; | ||
| eventSource.onmessage = (event) => { | ||
| const parsedData = JSON.parse(event.data); // JSON 형식으로 변환 | ||
|
|
||
| // messageType에 따라 분기 처리 | ||
| switch (parsedData.messageType) { | ||
| case 'QUESTION_CHECK': | ||
| setQuestions((prevQuestions) => | ||
| prevQuestions.filter((q) => q.id !== parsedData.data.id) | ||
| ); | ||
| break; | ||
|
|
||
| case 'COURSE_CLOSED': | ||
| setModalType('closedCourse'); | ||
| openModal(); | ||
| break; | ||
|
|
||
| default: | ||
| break; | ||
| } | ||
| }; | ||
|
|
||
| eventSource.onerror = () => { | ||
| eventSource.close(); | ||
| connectSSE(); | ||
| }; | ||
| eventSourceRef.current = eventSource; | ||
| }; | ||
|
|
||
| // eventSource.onerror = (error) => { | ||
| // console.error('❌ SSE 오류:', error); | ||
| // eventSource.close(); | ||
| // }; | ||
| connectSSE(); // 최초 연결 | ||
|
|
||
| // return () => { | ||
| // console.log('🔌 SSE 연결 종료'); | ||
| // eventSource.close(); | ||
| // }; | ||
| // }, []); | ||
| return () => { | ||
| eventSourceRef.current?.close(); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
SSE reconnection might need backoff logic.
The dynamic reconnection in onerror is useful, but re-calling connectSSE() immediately on every error can cause high retry frequency if the server is down. Consider adding a retry limit or exponential backoff to prevent potential infinite loops and reduce server load.
Below is an example diff introducing a simple exponential backoff mechanism:
useEffect(() => {
let retryDelay = 1000;
const connectSSE = () => {
if (eventSourceRef.current) {
eventSourceRef.current.close();
}
const eventSource = new EventSource(`${SSE_URL}/sse/connection/student`, {
withCredentials: true,
});
eventSource.onmessage = (event) => {
const parsedData = JSON.parse(event.data);
switch (parsedData.messageType) {
case 'QUESTION_CHECK':
setQuestions((prevQuestions) =>
prevQuestions.filter((q) => q.id !== parsedData.data.id)
);
break;
case 'COURSE_CLOSED':
setModalType('closedCourse');
openModal();
break;
default:
break;
}
// Reset delay upon successful message
+ retryDelay = 1000;
};
eventSource.onerror = () => {
eventSource.close();
+ setTimeout(() => {
+ connectSSE();
+ // Increase the delay for the next retry
+ retryDelay = Math.min(retryDelay * 2, 30000);
+ }, retryDelay);
};
eventSourceRef.current = eventSource;
};
connectSSE();
return () => {
eventSourceRef.current?.close();
};
}, []);📝 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 connectSSE = () => { | |
| if (eventSourceRef.current) { | |
| eventSourceRef.current.close(); | |
| } | |
| // eventSource.onopen = () => { | |
| // console.log('🔗 SSE 연결됨'); | |
| // }; | |
| const eventSource = new EventSource(`${SSE_URL}/sse/connection/student`, { | |
| withCredentials: true, | |
| }); | |
| // eventSource.onmessage = (event) => { | |
| // console.log('📩 받은 데이터:', event.data); | |
| // }; | |
| eventSource.onmessage = (event) => { | |
| const parsedData = JSON.parse(event.data); // JSON 형식으로 변환 | |
| // messageType에 따라 분기 처리 | |
| switch (parsedData.messageType) { | |
| case 'QUESTION_CHECK': | |
| setQuestions((prevQuestions) => | |
| prevQuestions.filter((q) => q.id !== parsedData.data.id) | |
| ); | |
| break; | |
| case 'COURSE_CLOSED': | |
| setModalType('closedCourse'); | |
| openModal(); | |
| break; | |
| default: | |
| break; | |
| } | |
| }; | |
| eventSource.onerror = () => { | |
| eventSource.close(); | |
| connectSSE(); | |
| }; | |
| eventSourceRef.current = eventSource; | |
| }; | |
| // eventSource.onerror = (error) => { | |
| // console.error('❌ SSE 오류:', error); | |
| // eventSource.close(); | |
| // }; | |
| connectSSE(); // 최초 연결 | |
| // return () => { | |
| // console.log('🔌 SSE 연결 종료'); | |
| // eventSource.close(); | |
| // }; | |
| // }, []); | |
| return () => { | |
| eventSourceRef.current?.close(); | |
| }; | |
| }, []); | |
| useEffect(() => { | |
| let retryDelay = 1000; | |
| const connectSSE = () => { | |
| if (eventSourceRef.current) { | |
| eventSourceRef.current.close(); | |
| } | |
| const eventSource = new EventSource(`${SSE_URL}/sse/connection/student`, { | |
| withCredentials: true, | |
| }); | |
| eventSource.onmessage = (event) => { | |
| const parsedData = JSON.parse(event.data); // JSON 형식으로 변환 | |
| // messageType에 따라 분기 처리 | |
| switch (parsedData.messageType) { | |
| case 'QUESTION_CHECK': | |
| setQuestions((prevQuestions) => | |
| prevQuestions.filter((q) => q.id !== parsedData.data.id) | |
| ); | |
| break; | |
| case 'COURSE_CLOSED': | |
| setModalType('closedCourse'); | |
| openModal(); | |
| break; | |
| default: | |
| break; | |
| } | |
| // Reset delay upon successful message | |
| retryDelay = 1000; | |
| }; | |
| eventSource.onerror = () => { | |
| eventSource.close(); | |
| setTimeout(() => { | |
| connectSSE(); | |
| // Increase the delay for the next retry | |
| retryDelay = Math.min(retryDelay * 2, 30000); | |
| }, retryDelay); | |
| }; | |
| eventSourceRef.current = eventSource; | |
| }; | |
| connectSSE(); // 최초 연결 | |
| return () => { | |
| eventSourceRef.current?.close(); | |
| }; | |
| }, []); |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
front-end/src/pages/student/course/question/StudentQuestion.tsx (1)
47-53: 🛠️ Refactor suggestionImprove error handling in
handleMessageClick.The error handling in
handleMessageClickonly logs errors to console, whilehandleInputSubmituses thehandleStudentErrorutility. Consider using consistent error handling patterns across all API calls.Apply this diff to improve error handling:
const handleMessageClick = async (questionId: Question['id']) => { try { await classroomRepository.checkQuestionByStudent(questionId); } catch (error) { - console.log(error); + handleStudentError({ error, setModalType, openModal }); } };
🧹 Nitpick comments (2)
front-end/src/pages/student/course/StudentCourse.tsx (2)
38-48: Asynchronous question fetching is well-structured.Consider encapsulating the fetch logic in a custom hook if you plan to reuse it or expand its functionality later.
58-67: Initial SSE setup is sound.You might add additional error-handling logic (e.g., timeouts or heartbeat checks) to further stabilize the connection if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
front-end/src/pages/student/course/StudentCourse.tsx(5 hunks)front-end/src/pages/student/course/question/StudentQuestion.tsx(2 hunks)front-end/src/pages/student/home/StudentHome.tsx(1 hunks)front-end/src/utils/studentPopupUtils.tsx(1 hunks)
🔇 Additional comments (12)
front-end/src/pages/student/course/question/StudentQuestion.tsx (3)
10-15: LGTM! Props type definition is well-structured.The addition of
setQuestionsandquestionsprops aligns with the component's new design where question state management is handled by the parent component.
17-22: LGTM! Component signature is consistent with type definition.The destructuring of props is well-organized and matches the
StudentQuestionPropstype definition.
23-25: LGTM! State management is well-organized.The component maintains only the necessary local state (
successPopupandinputValue) while delegating question state management to the parent component. This separation of concerns improves maintainability and reduces state duplication.front-end/src/pages/student/course/StudentCourse.tsx (7)
9-13: No issues with the new import statements.
15-16: No issues with these new imports.
24-24: Ensure SSE_URL is properly set in the environment.Consider verifying that
import.meta.env.VITE_API_URLis defined and valid across all deployment environments to prevent possible runtime errors.
30-30: Question state initialization is straightforward and correct.
34-34: Using a ref to store the EventSource instance looks good.
142-147: New 'closedCourse' modal case is consistent with existing error handling.
200-201: Passing 'questions' and 'setQuestions' props centralizes question state management effectively.front-end/src/utils/studentPopupUtils.tsx (1)
18-21: New 'closedCourse' popup config aligns well with existing patterns.front-end/src/pages/student/home/StudentHome.tsx (1)
80-80: Use of 'notFound' string is now consistent with the popup configuration keys.
| eventSource.onmessage = (event) => { | ||
| const parsedData = JSON.parse(event.data); // JSON 형식으로 변환 | ||
|
|
||
| // messageType에 따라 분기 처리 | ||
| switch (parsedData.messageType) { | ||
| case 'QUESTION_CHECK': | ||
| setQuestions((prevQuestions) => | ||
| prevQuestions.filter((q) => q.id !== parsedData.data.id) | ||
| ); | ||
| break; | ||
|
|
||
| case 'COURSE_CLOSED': | ||
| setModalType('closedCourse'); | ||
| openModal(); | ||
| break; | ||
|
|
||
| default: | ||
| break; | ||
| } | ||
| }; | ||
|
|
||
| eventSource.onerror = () => { | ||
| eventSource.close(); | ||
| connectSSE(); | ||
| }; | ||
| eventSourceRef.current = eventSource; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
SSE message handling is correct, but watch out for potential infinite reconnect loops.
Implementing a maximum reconnection limit or exponential backoff can prevent continual reconnection attempts when the server is permanently unreachable.
[FE/BE] PR 제목을 꼭 써주세요#️⃣ 연관된 이슈
📝 작업 내용
Summary by CodeRabbit
New Features
Bug Fixes