-
Notifications
You must be signed in to change notification settings - Fork 1
[FE] 교수 사용자 관련 API 연동 #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- 교수 사용자 데이터 관련 API 연동
- 변경된 API URL 반영 - 예외 처리 추가
- AlertModal X 버튼을 해당 버튼 핸들러 입력에 따라 렌더링되게 변경 - 프로필 이미지 삭제 버튼 추가 - ProfessorHomeLayout을 사용하는 페이지는 레이아웃에서 모달을 관리하게 변경 - 에러에 맞게 모달이 나오도록 기능 추가
|
Warning Rate limit exceeded@DrCloy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 51 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (7)
WalkthroughThis pull request updates various professor-related components and repository methods to enhance modal error handling, profile image management, and component property definitions. The Changes
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (10)
front-end/src/pages/professor/home/profile/ProfessorProfile.module.css (1)
59-65: Confirm Fixed Width Usage for.profileImageContainer
The update sets the container width to 464px, which ensures consistency in layout. However, please verify that this fixed width integrates well with responsive design considerations. Depending on the overall design system, you might consider using relative units (e.g., percentages ormax-width) for better adaptability on smaller screens.front-end/src/repository/professorRepository.ts (3)
10-49: Consider centralizing your repetitive error handling blocks.Most methods follow a similar pattern for
fetchcalls and conditional checks (e.g., identifying status 502, distinguishing between client vs. server errors, etc.). This logic is duplicated across multiple methods, making the code lengthy to maintain.A possible approach is to extract shared fetch-and-error-check code into a helper function, for instance:
+async function fetchWithErrors( + url: string, + options: RequestInit +): Promise<Response> { + const response = await fetch(url, options); + // handle response checks, 502, etc. + // throw ClientError or ServerError as needed + return response; +} async createProfessor(name: string, profile: File | null): Promise<void> { const form = new FormData(); // ... - const response = await fetch('/api/professors/signup', {...}); + const response = await fetchWithErrors('/api/professors/signup', { + method: 'POST', + body: form + }); if (response.redirected) { window.location.href = response.url; } }This will keep the logic DRY and easier to maintain.
58-91: Ensure robust handling of JSON parse errors.After confirming
response.ok, the code callsresponse.json()but doesn't handle potential non-JSON or malformed JSON responses. Consider adding try-catch blocks or verifyingContent-Typeto mitigate runtime exceptions if the server returns an unexpected payload.
256-284: Validate any dependent logic after professor deletion.When deleting the professor, consider whether any in-memory or front-end state should be invalidated before redirecting the user. This ensures consistency and clear UX flow, particularly in large applications using client-side state management.
front-end/src/pages/professor/home/profile/ProfessorProfile.tsx (4)
44-49: Consider avoiding storing direct HTMLImageElement objects in React state.
Storing entire DOM elements (likeHTMLImageElement) in React state can sometimes lead to higher memory usage and potential serialization issues. Typically, storing a string URL or the file object itself is more common:- profileImage: HTMLImageElement | null; + profileImageUrl: string | null; // Then, to display images: <img src={profile.profileImageUrl ?? ''} alt="profile" />
67-86: Consider handling potential image load failures.
When instantiating a newImage()and assigningimg.srcdirectly, you may want to handleimg.onloadorimg.onerrorevents for robust error handling if the image fails to load. This is a minor improvement and not strictly required.
96-157: Check for null files before attempting profile update.
IfselectedProfileFileisnull, a user might trigger the update without selecting a new image. Consider adding a prompt or disabling the edit action until a file is chosen. Overall, the error handling with modals looks consistent.
159-176: Unify alert vs. modal error handling for name updates.
Currently,handleEditNamerelies onalertcalls for errors, whereas other functions display modals. Consider aligning this approach with the rest of the error handling to maintain UX consistency.front-end/src/pages/professor/home/layout/ProfessorHomeLayout.tsx (2)
50-64: Consider adding aria-label for keyboard accessibility.The search functionality is well implemented with proper keyboard event handling. Consider adding an
aria-labelto the search input for better accessibility.<input className={S.input} type="text" placeholder="수업 이름이나 코드로 수업을 검색해보세요" value={search} onChange={handleSearchInput} onKeyDown={handleSearchKeyDown} + aria-label="수업 검색" />
70-128: Refactor error handling to reduce code duplication.The error handling logic is comprehensive but contains duplicated code for showing the popup modal. Consider extracting the common popup modal logic into a reusable function.
+ const showTemporaryErrorModal = () => { + setModal( + <PopupModal + type="caution" + title="오류가 발생했습니다." + description="다시 시도해주세요." + /> + ); + openModal(); + setTimeout(() => { + closeModal(); + setModal(null); + }, 2000); + }; useEffect(() => { async function getProfile() { try { const res = await professorRepository.getProfessorProfile(); if (res) { const img = new Image(); img.src = res; img.onload = () => { setProfile(img); }; } } catch (error) { if (error instanceof ClientError || error instanceof ServerError) { if (error.errorCode === 'USER_NOT_FOUND') { setModal( <AlertModal type="caution" message={error.message} description="로그인 페이지로 이동합니다." buttonText="확인" onClickModalButton={() => navigate('/professor/login')} /> ); openModal(); } else { - setModal( - <PopupModal - type="caution" - title="오류가 발생했습니다." - description="다시 시도해주세요." - /> - ); - openModal(); - setTimeout(() => { - closeModal(); - setModal(null); - }, 2000); + showTemporaryErrorModal(); } } else { - setModal( - <PopupModal - type="caution" - title="오류가 발생했습니다." - description="다시 시도해주세요." - /> - ); - openModal(); - setTimeout(() => { - closeModal(); - setModal(null); - }, 2000); + showTemporaryErrorModal(); } } } getProfile(); }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
front-end/src/components/modal/AlertModal.tsx(2 hunks)front-end/src/pages/professor/home/layout/ProfessorHomeLayout.tsx(5 hunks)front-end/src/pages/professor/home/profile/ProfessorProfile.module.css(2 hunks)front-end/src/pages/professor/home/profile/ProfessorProfile.tsx(4 hunks)front-end/src/pages/professor/register/ProfessorRegister.tsx(2 hunks)front-end/src/repository/professorRepository.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
front-end/src/pages/professor/home/profile/ProfessorProfile.tsx
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (25)
front-end/src/pages/professor/home/profile/ProfessorProfile.module.css (1)
73-75: Enforcing Consistent Display withflex-shrink: 0on.profileImage
The addition offlex-shrink: 0to the.profileImagestyle prevents the profile image from shrinking when the container is resized, ensuring that the image maintains its intended dimensions. This change aligns with the new image handling requirements.front-end/src/repository/professorRepository.ts (6)
1-2: Imports look good.Bringing in
ClientErrorandServerErrorfrom your core error module is a neat way to centralize error handling logic.
46-48: Redirect logic might need consistent handling.When
response.redirectedis true, you directly navigate the user. Ensure you handle potential partial errors or data constraints before redirecting. Also verify that user interactions are completed or undone gracefully.Would you like a script to scan other parts of the repo for any additional direct usage of
window.location.hreffor consistency checks?
95-125: Profile image retrieval is clear and concise.Fetching a standalone image URL for the professor profile keeps this method focused and ensures clarity. The error-handling pattern is consistent with the rest of the repository. Good job.
132-163: Consider a fallback for network or connectivity issues.The
logoutmethod, like the others, checks HTTP status codes. For network errors,fetchmay throw before returning a response, bypassing the current error-check logic. Adding a try-catch around the fetch could improve resilience.
169-209: Returning updated name helps keep the UI consistent.Updating the professor name server-side and returning it in one go is a good approach. User-facing components won't have to fetch the data again for immediate feedback.
215-251: Good practice returning the new profile image URL.The pattern mirrors
updateProfessorNameby returning the freshly generated resource URL. This promotes a smooth user experience in the UI without a second fetch.front-end/src/pages/professor/home/profile/ProfessorProfile.tsx (7)
2-4: Imports look consistent and aligned with project structure.
These import statements appear to be correctly pointing to the necessary modules and types, with no duplicates or unused references.Also applies to: 9-11
51-56: State for editing flags and selected file is straightforward.
No issues found here; leveraging separate state forprofileImageediting and the selected file is clear and maintainable.
58-63: Context consumption is properly typed with OutletContext.
DestructuringopenModal,closeModal, etc., fromuseOutletContext<OutletContext>()is a neat approach for managing modals and navigation.
88-94: Cleanup logic for blob URLs is properly handled.
The optional chaining here (profile.profileImage?.src.startsWith('blob:')) is valid, and the cleanup withURL.revokeObjectURLproperly prevents memory leaks.
178-190: Edit cancellation logic is clear.
Reverting state to the previous name or profile image reference is properly handled here, ensuring a smooth user experience if edits are canceled.
192-205: Blob URL revocation logic is correctly reused.
Checkingprofile.profileImage?.src.startsWith('blob:')and revoking the URL insidehandleUpdateProfileImageis consistent with the cleanup approach in theuseEffect.🧰 Tools
🪛 Biome (1.9.4)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
255-409: UI and modal integration appear consistent and user-friendly.
The layout changes properly render the updated profile image, provide an intuitive editing flow, and integrate well with the existing modal approach.front-end/src/components/modal/AlertModal.tsx (3)
14-14: Making the close button callback optional adds flexibility.
This change ensures that theAlertModalcan be reused in scenarios where a dedicated close button is not required.
38-39: Defaulting onClickCloseButton to null is a neat way to simplify checks.
No issues found with setting a default value for the optional prop.
42-50: Conditional rendering of close button prevents undefined function calls.
This pattern avoids callingonClickCloseButtonwhen it's not provided, eliminating potential errors and offering a cleaner user experience.front-end/src/pages/professor/register/ProfessorRegister.tsx (6)
7-10: Imports look appropriate for error handling and modals.
IncludingClientError,ServerError, and the new modal components is consistent with the project's error-handling approach.
15-15: Storing modal content in state is straightforward and flexible.
Managing the actual modal<ReactNode>in state is a clear pattern for dynamic modal content.
18-18: Using custom modal hook is consistent.
DestructuringopenModal,closeModal, andModalfromuseModal()centralizes modal logic nicely.
40-72: Replacing alerts with modals clarifies user feedback.
Showing caution messages for empty or invalid names in a modal is a cohesive UI approach. The statefulsetModalusage andopenModal()call look correct.
79-108: Customized error handling for repository calls is well structured.
Client/Server error separation with tailored modals is consistent with the rest of the application’s patterns.
114-129: Form layout and final modal rendering are coherent.
The reworkedprofileimage display, name field, and final submission logic align with the new modal-based flow, promoting a uniform user experience.Also applies to: 131-131, 133-133, 137-178
front-end/src/pages/professor/home/layout/ProfessorHomeLayout.tsx (2)
1-24: LGTM! Well-structured imports and type definitions.The imports are well organized, and the
OutletContexttype is clearly defined with proper TypeScript practices.
163-176: LGTM! Clean and well-structured render logic.The conditional profile button rendering and modal management are well implemented. The Outlet context properly provides necessary functions to child components.
| width="1.1875rem" | ||
| height="1.1875rem" | ||
| color="var(--gray-600)" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class로 따로 빼서 rem 고치면 될거같습니당
wwweric12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다
#️⃣ 연관된 이슈
📝 작업 내용
Summary by CodeRabbit
New Features
Bug Fixes
Style