[FE] 교수 실시간 페이지 SSE 연동 #211
Conversation
WalkthroughThis pull request implements several functional enhancements to the professor’s classroom components. In the main component, a new SSE connection is set up to listen for real-time updates such as reactions, requests, and questions. The changes also include navigation improvements, refined error handling, and updates to type definitions and property structures in related components and repositories. Additionally, adjustments are made to the emoji mapping logic and HTTP methods used in API calls. Changes
Sequence Diagram(s)sequenceDiagram
participant PC as ProfessorClassroom Component
participant SSE as EventSource
participant Backend as Server
PC->>SSE: Call connectSSE()
SSE-->>PC: Send message (reaction, request, question, check)
PC->>PC: Process message via appropriate handler
alt Error occurs
SSE-->>PC: Error event
PC->>SSE: Attempt reconnection
end
sequenceDiagram
participant User as Professor
participant UI as Course Dashboard
participant Repo as classroomRepository
User->>UI: Initiates “close class” action
UI->>Repo: Calls closeCourse(course.id) using PATCH
Repo-->>UI: Returns response or error status
UI->>User: Navigates back with error handling if needed
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 (1)
front-end/src/utils/courseAction.tsx (1)
117-133:⚠️ Potential issueAdd error handling for the async startCourse operation.
The async operation could fail, but errors are not caught. This could lead to unhandled promise rejections and navigation to the classroom before the course is properly started.
Apply this diff to add proper error handling:
const handleStartCourse = (course: CourseMeta) => { if (!course) { return; } setModal( <ClassStartModal course={course} handleClickBackButton={offModal} - handleClickStartButton={() => { - classroomRepository.startCourse(course.id); + handleClickStartButton={async () => { + try { + await classroomRepository.startCourse(course.id); + offModal(); + navigate(`/professor/course/${course.id}/classroom`); + } catch (error) { + popupError(error); + } + }} - offModal(); - navigate(`/professor/course/${course.id}/classroom`); - }} /> ); openModal(); };
🧹 Nitpick comments (6)
front-end/src/pages/professor/course/classroom/components/RequestBox.tsx (3)
17-19: Consider improving type definitions for better type safety.The type definition could be enhanced in two ways:
- Use
Requests[]instead ofRequests | []to be more explicit about the array type- Rename the prop from
requesttorequeststo match its plural naturetype RequestBoxProps = { - request: Requests | []; + requests: Requests[]; };
29-82: Consider refactoring to reduce code duplication and improve error handling.The component could benefit from the following improvements:
- Extract the default request configurations to reduce duplication
- Add error handling for invalid request types
- Consider using a loading state for SSE integration
Here's a suggested refactor:
const DEFAULT_REQUESTS = [ RequestHard, RequestFast, RequestQuestion, RequestSize, RequestSound, ] as const; const RequestBox = ({ requests }: RequestBoxProps) => { const requestsToRender = requests.length === 0 ? DEFAULT_REQUESTS.map(type => ({ type, count: 0 })) : requests; return ( <div className={S.container}> {requestsToRender.map((request, index) => ( <RequestCard key={`${index}-${request.type.kind}`} Emoji={EmojiType[request.type.kind] ?? WishEmoji} // Fallback emoji title={request.type.title} description={request.type.description} count={request.count} /> ))} <div className={S.popup}> <span>오늘 누적 반응 학생 수</span> </div> </div> ); };
1-8: Add documentation about SSE integration.Since this component is part of the SSE integration for real-time updates, consider adding JSDoc comments to document:
- How the component integrates with SSE
- The expected format of real-time updates
- Any assumptions about update frequency
/** * Displays request cards with real-time updates via SSE. * @param props.requests - Array of requests updated in real-time through SSE * @example * // Parent component with SSE * const [requests, setRequests] = useState<Requests[]>([]); * useEffect(() => { * const eventSource = new EventSource('/api/requests'); * eventSource.onmessage = (event) => { * setRequests(JSON.parse(event.data)); * }; * return () => eventSource.close(); * }, []); */front-end/src/repository/courseRepository.ts (1)
282-308: Optimize request count access pattern.The current implementation performs multiple
findoperations on the same array. Consider transforming the array into a map for O(1) access.Apply this diff to optimize the code:
questions: data.questions.map( (question) => ({ id: question.id, createdAt: question.time, content: question.content, }) as Question ), + // Transform requests array into a map for O(1) access + const requestsMap = Object.fromEntries( + data.requests?.map(req => [req.type, req.count]) ?? [] + ); requests: [ { type: RequestHard, - count: data.requests?.find((req) => req.type === RequestHard.kind) - ?.count, + count: requestsMap[RequestHard.kind] ?? 0, }, { type: RequestFast, - count: data.requests?.find((req) => req.type === RequestFast.kind) - ?.count, + count: requestsMap[RequestFast.kind] ?? 0, }, { type: RequestQuestion, - count: data.requests?.find((req) => req.type === RequestQuestion.kind) - ?.count, + count: requestsMap[RequestQuestion.kind] ?? 0, }, { type: RequestSize, - count: data.requests?.find((req) => req.type === RequestSize.kind) - ?.count, + count: requestsMap[RequestSize.kind] ?? 0, }, { type: RequestSound, - count: data.requests?.find((req) => req.type === RequestSound.kind) - ?.count, + count: requestsMap[RequestSound.kind] ?? 0, }, ],front-end/src/pages/professor/course/classroom/ProfessorClassroom.tsx (2)
119-156: Potential infinite reconnect loop
Reconnecting immediately ineventSource.onerrorcan overwhelm the server if it remains unreachable. Consider adding exponential backoff or a retry delay to prevent rapid reconnect attempts.
172-186: Asynchronous call withoutawait
classroomRepository.closeCourse(courseId)may be asynchronous. Currently, the code navigates away immediately, potentially unfinished. Consider addingawaitfor a cleaner flow:- classroomRepository.closeCourse(courseId); + await classroomRepository.closeCourse(courseId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
front-end/src/pages/professor/course/classroom/ProfessorClassroom.tsx(4 hunks)front-end/src/pages/professor/course/classroom/components/PDFMainComponent.tsx(4 hunks)front-end/src/pages/professor/course/classroom/components/RequestBox.tsx(1 hunks)front-end/src/pages/professor/course/classroom/components/reaction/EmojiCounter.tsx(1 hunks)front-end/src/repository/classroomRepository.ts(2 hunks)front-end/src/repository/courseRepository.ts(3 hunks)front-end/src/utils/courseAction.tsx(2 hunks)
🔇 Additional comments (23)
front-end/src/pages/professor/course/classroom/components/RequestBox.tsx (1)
21-27: Great improvement to the EmojiType mapping!The use of computed property names (
[RequestType.kind]) creates a type-safe mapping between request types and their corresponding emoji components. This change:
- Prevents typos in string literals
- Makes refactoring easier as changes to request kinds will be caught by TypeScript
- Maintains a clear and maintainable relationship between request types and their visual representations
front-end/src/repository/classroomRepository.ts (3)
33-45: LGTM! Correct removal of unnecessary return statement.The method is properly declared to return void, and the removal of the unnecessary return statement aligns with the method's purpose as an action method.
24-31:Details
❓ Verification inconclusive
LGTM! Verify API endpoint compatibility with PATCH method.
The change to PATCH is semantically correct for this operation. Error handling is properly implemented.
Run the following script to verify the API endpoint compatibility:
🏁 Script executed:
#!/bin/bash # Description: Verify that the API endpoint accepts PATCH method. curl -X OPTIONS "/api/professors/courses/test/close" -iLength of output: 109
LGTM! However, please manually verify that the API endpoint supports the PATCH method.
The automated curl test failed with a URL format error ("curl: (3) URL using bad/illegal format or missing URL"), which suggests that the test URL might be missing a required protocol or domain. Verify that the correct, fully qualified URL is being used in your testing environment to confirm endpoint compatibility.
5-12:Details
❓ Verification inconclusive
LGTM! Verify API endpoint compatibility with PATCH method.
The change to PATCH is semantically correct for this operation. Error handling is properly implemented.
Run the following script to verify the API endpoint compatibility:
🏁 Script executed:
#!/bin/bash # Description: Verify that the API endpoint accepts PATCH method. curl -X OPTIONS "/api/professors/courses/test/start" -iLength of output: 109
ACTION REQUIRED: Endpoint Verification Inconclusive
The code changes infront-end/src/repository/classroomRepository.ts(lines 5–12) correctly use PATCH and implement error handling viathrowError. However, the automated verification script produced a URL formatting error (curl: (3) URL using bad/illegal format or missing URL), likely due to a missing protocol or base URL. Please manually verify that the PATCH endpoint (/api/professors/courses/{courseId}/start) is compatible with your backend API, or update the script to include a full, valid URL.
- File:
front-end/src/repository/classroomRepository.ts(lines 5–12)- Concern: PATCH endpoint verification script failed due to URL formatting issues.
front-end/src/utils/courseAction.tsx (1)
7-7: LGTM! Clean import statement.The import statement correctly combines both required repositories.
front-end/src/repository/courseRepository.ts (2)
371-371: LGTM!The update to omitted fields correctly reflects the renamed property.
60-60:Details
✅ Verification successful
Verify the impact of renaming
requesttorequests.The property rename from
requesttorequestsbetter reflects its array nature. However, this is a breaking change that requires verification.Run the following script to find any remaining references to the old property name:
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining references to the old property name 'request' # that might have been missed during the rename. rg -i 'request:' --type tsLength of output: 349
Ensure consistent integration of the renamed property
requestsacross the codebaseThe change in
front-end/src/repository/courseRepository.tsrenaming the property fromrequesttorequestscorrectly reflects its array nature. Based on our repository-wide search:
- ProfessorClassroom.tsx: The occurrence of the word “request” is used as a function parameter (e.g.,
const handleRequest = (request: RequestType) => { … }), which is unrelated to the property in question.- RequestBox.tsx: The usage
request: Requests | []confirms that the new naming convention is already in place.- classroomRepository.ts: The use of “request” as a parameter in
sendRequest(request: string)also represents a separate case and isn’t affected by the renaming.No unintended references to the old property were found in relation to the BackendCourse type. Please review the integration points to ensure that any component directly interacting with course data is correctly accessing the
requestsproperty.front-end/src/pages/professor/course/classroom/ProfessorClassroom.tsx (11)
12-12: Minor changed imports and constants
These lines add or modify imports (useNavigate,useParams,RequestType,ProfessorError) and defineSSE_URL. The changes appear straightforward and improve clarity.Also applies to: 21-21, 26-26, 32-32
38-45: Consistent uppercase keys for reactions
Switching reaction keys to uppercase aligns with the rest of the codebase and ensures consistency.
71-71: Unified state and error handling approach
The newly introduced state references (sseRef,navigate) and thepopupErrorinitialization streamline the architecture by consolidating error handling. No issues found.Also applies to: 74-74, 79-79, 81-86
88-98: Concurrent updates in handleReaction
While updating both reducer state and localStorage works well, consider potential concurrency issues when multiple reactions arrive in quick succession. The concurrency approach withstartTransitionshould help, but verify it doesn’t cause any race conditions under heavy load.
100-107: Ensure request types exist in the current array
If a new request type is reported that doesn’t exist inprev, it won’t be updated. Confirm that all possible request types are pre-initialized or can be dynamically added if necessary.
109-111: Straightforward question handling
The approach to append new questions is clear and consistent.
113-117: Removing question after check
Filtering out the question by ID is correct and efficient.
158-170: Question resolution
The logic is correct and neatly updates the questions list after a successful server call.
191-203: Course fetching with fallback logic
The code checks for a validcourseIdand gracefully navigates away if missing. Error handling withpopupErroris consistent.
226-235: SSE lifecycle management
Mounting/unmounting SSE properly closes connections to avoid memory leaks. This is good practice.
240-250: Passing error handling and reaction data
All relevant props are provided to the PDF component, ensuring consistent error handling and reaction state across the layout.front-end/src/pages/professor/course/classroom/components/reaction/EmojiCounter.tsx (1)
22-27: Updated emoji types to uppercase
Aligning emoji types with uppercase naming (e.g.,OKAY,CLAP) ensures consistency. Implementation looks correct and matches the reaction changes in the parent component.front-end/src/pages/professor/course/classroom/components/PDFMainComponent.tsx (4)
10-10: Injecting repository and error callback
IntroducingcourseRepository, markingcourseInfoas nullable, and passingpopupErrorenable flexible and centralized error handling.Also applies to: 14-14, 22-22, 34-34
52-67: Robust PDF fetching
The updated logic checks forcourseInfoandpdfUrlbefore proceeding, returning early if invalid. Graceful error handling withpopupErroris a nice addition.
95-96: Proper error handling on file upload
Stopping the upload spinner and notifying users viapopupErrorhelps keep them informed when something fails.
105-105: Safe optional chaining
UsingcourseInfo?.accessCodeavoids runtime errors ifcourseInfois null. A clean approach to uncertain data.
| count: data.requests?.find((req) => req.type === RequestHard.kind) | ||
| ?.count, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add default values for undefined request counts.
The current implementation might return undefined for request counts when the corresponding request type is not found. This could lead to UI issues.
Apply this diff to provide default values:
- count: data.requests?.find((req) => req.type === RequestHard.kind)
- ?.count,
+ count: data.requests?.find((req) => req.type === RequestHard.kind)
+ ?.count ?? 0,
- count: data.requests?.find((req) => req.type === RequestFast.kind)
- ?.count,
+ count: data.requests?.find((req) => req.type === RequestFast.kind)
+ ?.count ?? 0,
- count: data.requests?.find((req) => req.type === RequestQuestion.kind)
- ?.count,
+ count: data.requests?.find((req) => req.type === RequestQuestion.kind)
+ ?.count ?? 0,
- count: data.requests?.find((req) => req.type === RequestSize.kind)
- ?.count,
+ count: data.requests?.find((req) => req.type === RequestSize.kind)
+ ?.count ?? 0,
- count: data.requests?.find((req) => req.type === RequestSound.kind)
- ?.count,
+ count: data.requests?.find((req) => req.type === RequestSound.kind)
+ ?.count ?? 0,Also applies to: 290-291, 295-296, 300-301, 305-306
#️⃣ 연관된 이슈
📝 작업 내용
Summary by CodeRabbit