refactor: 호스트 대학교 이미지 업로드 multipart/form-data 방식으로 전환#571
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Warning Review limit reached
More reviews will be available in 42 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🧹 Nitpick comments (3)
apps/admin/src/lib/api/admin.test.ts (2)
23-43: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win2)
request파트는 MIME 타입뿐 아니라 JSON 본문도 검증해 주세요.지금은
application/json타입만 확인해서, 직렬화 필드 누락/오타 회귀를 잡기 어렵습니다.Blob.text()로 파싱 후 핵심 필드를 함께 검증하는 편이 안전합니다.diff 제안
it("createHostUniversity sends multipart/form-data with request JSON blob and files", async () => { post.mockResolvedValue({ data: { id: 1, koreanName: "테스트 대학교" } }); @@ const requestBlob = formData.get("request") as Blob; expect(requestBlob.type).toBe("application/json"); + await expect(requestBlob.text()).resolves.toBe(JSON.stringify(request)); }); it("updateHostUniversity sends multipart/form-data with optional files", async () => { @@ const formData = put.mock.calls[0]?.[1] as FormData; expect(formData.get("logoFile")).toBe(logoFile); expect(formData.get("backgroundFile")).toBeNull(); + const requestBlob = formData.get("request") as Blob; + expect(requestBlob.type).toBe("application/json"); + await expect(requestBlob.text()).resolves.toBe(JSON.stringify(request)); });Also applies to: 45-62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/src/lib/api/admin.test.ts` around lines 23 - 43, The test for createHostUniversity currently only validates the MIME type of the request blob but does not validate the actual JSON content, which could miss serialization errors like missing or misspelled fields. After retrieving the requestBlob from FormData in the test, use the Blob.text() method to parse the blob content, then parse it as JSON and validate that the critical fields (koreanName, englishName, formatName, countryCode, regionCode) from the original request object are correctly present in the JSON with their expected values.
64-79: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win1) 세 번째 update 테스트에도 호출 경로 계약 검증을 고정해 주세요.
현재 케이스는
FormData내부 키만 확인해서, 잘못된 URL로put이 호출되는 회귀를 놓칠 수 있습니다. 호출 경로/인자 검증을 추가해 주세요.diff 제안
it("updateHostUniversity omits both file parts when no files are provided", async () => { put.mockResolvedValue({ data: { id: 1 } }); const request = { koreanName: "대학교", englishName: "University", formatName: "U", countryCode: "JP", regionCode: "ASIA", }; await adminApi.updateHostUniversity(1, request); + expect(put).toHaveBeenCalledWith("/admin/host-universities/1", expect.any(FormData)); const formData = put.mock.calls[0]?.[1] as FormData; expect(formData.get("logoFile")).toBeNull(); expect(formData.get("backgroundFile")).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/src/lib/api/admin.test.ts` around lines 64 - 79, The test "updateHostUniversity omits both file parts when no files are provided" only validates the FormData contents but does not verify that the put method was called with the correct URL path and arguments. Add an assertion to validate the first argument (the URL/endpoint) passed to the put mock call, similar to how other update tests validate the call path, to prevent regressions where the request goes to the wrong endpoint. Use put.mock.calls[0]?.[0] to access the URL argument and add an expect assertion to verify it matches the expected endpoint for updating a host university with ID 1.apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.test.tsx (1)
57-64: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
waitFor+getByRole+toBeTruthy()대신findByRole사용을 권장합니다.
getByRole은 요소를 찾지 못하면 예외를 던지므로toBeTruthy()단언은 불필요합니다.findByRole은waitFor+getByRole을 결합한 것으로, 더 간결하고 관용적인 패턴입니다.♻️ 권장 수정
it("shows logo preview immediately after file selection without calling the API", async () => { await openCreateModal(); const file = new File(["logo"], "logo.png", { type: "image/png" }); fireEvent.change(screen.getByLabelText("로고 이미지 파일"), { target: { files: [file] } }); - await waitFor(() => expect(screen.getByRole("img", { name: "로고 미리보기" })).toBeTruthy()); + await screen.findByRole("img", { name: "로고 미리보기" }); expect(toastError).not.toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.test.tsx` around lines 57 - 64, The test is using an inefficient pattern with waitFor combined with getByRole and toBeTruthy assertion. Replace the waitFor with getByRole pattern (line with "await waitFor(() => expect(screen.getByRole("img", { name: "로고 미리보기" })).toBeTruthy())") with a direct call to screen.findByRole instead, which is the idiomatic approach in React Testing Library that combines waiting and querying in one step. The findByRole method will automatically wait for the element to appear and throw if it does not, making the explicit toBeTruthy assertion unnecessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.tsx`:
- Around line 205-215: The handleLogoChange and handleBackgroundChange functions
create blob URLs using URL.createObjectURL without ever revoking them, causing
memory leaks. In each handler, check if the form already has an existing blob
URL (logoImageUrl or backgroundImageUrl respectively) and revoke it using
URL.revokeObjectURL before creating a new one. Additionally, ensure that when
the modal closes in the closeModal function or when the component unmounts, any
remaining blob URLs for both logo and background images are properly revoked to
clean up memory. Consider adding a useEffect cleanup function to handle
unmounting scenarios as well.
---
Nitpick comments:
In
`@apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.test.tsx`:
- Around line 57-64: The test is using an inefficient pattern with waitFor
combined with getByRole and toBeTruthy assertion. Replace the waitFor with
getByRole pattern (line with "await waitFor(() => expect(screen.getByRole("img",
{ name: "로고 미리보기" })).toBeTruthy())") with a direct call to screen.findByRole
instead, which is the idiomatic approach in React Testing Library that combines
waiting and querying in one step. The findByRole method will automatically wait
for the element to appear and throw if it does not, making the explicit
toBeTruthy assertion unnecessary.
In `@apps/admin/src/lib/api/admin.test.ts`:
- Around line 23-43: The test for createHostUniversity currently only validates
the MIME type of the request blob but does not validate the actual JSON content,
which could miss serialization errors like missing or misspelled fields. After
retrieving the requestBlob from FormData in the test, use the Blob.text() method
to parse the blob content, then parse it as JSON and validate that the critical
fields (koreanName, englishName, formatName, countryCode, regionCode) from the
original request object are correctly present in the JSON with their expected
values.
- Around line 64-79: The test "updateHostUniversity omits both file parts when
no files are provided" only validates the FormData contents but does not verify
that the put method was called with the correct URL path and arguments. Add an
assertion to validate the first argument (the URL/endpoint) passed to the put
mock call, similar to how other update tests validate the call path, to prevent
regressions where the request goes to the wrong endpoint. Use
put.mock.calls[0]?.[0] to access the URL argument and add an expect assertion to
verify it matches the expected endpoint for updating a host university with ID
1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7774a1cb-c028-4860-8ab1-40fe272eedd7
📒 Files selected for processing (4)
apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.test.tsxapps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.tsxapps/admin/src/lib/api/admin.test.tsapps/admin/src/lib/api/admin.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/file/admin/university/logo,/file/admin/university/background엔드포인트로 즉시 업로드 후 URL을 form state에 저장multipart/form-data로 request JSON + 파일을 함께 전송주요 변경사항
admin.ts:HostUniversityPayload에서logoImageUrl,backgroundImageUrl제거;createHostUniversity,updateHostUniversity함수가multipart/form-data방식으로 변경; 구 업로드 전용 함수(uploadAdminUniversityLogo,uploadAdminUniversityBackground) 제거HostUniversityTab.tsx: 이미지 파일을 로컬 state로 보관 후 submit 시 전달; 파일 선택 시URL.createObjectURL()로 즉시 미리보기 표시; create 모드에서 파일 미선택 시 에러 toast 표시Test plan
🤖 Generated with Claude Code