feat: 어드민 대학 이미지 업로드 연동 - #568
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Walkthrough이번 PR은 관리자 페이지의
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2fb02da67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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 (5)
apps/admin/src/lib/api/admin.ts (1)
351-372: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win두 업로드 메서드의 중복 코드를 헬퍼 함수로 추출하시는 것을 권장드립니다.
현재
uploadAdminUniversityLogo와uploadAdminUniversityBackground메서드는 엔드포인트만 다르고 나머지 로직이 동일합니다. 공통 헬퍼를 추출하면 유지보수가 더 쉬워집니다.♻️ 제안하는 리팩토링
+ uploadUniversityImage: (endpoint: string, file: File, englishName: string) => { + const formData = new FormData(); + formData.append("file", file); + formData.append("englishName", englishName); + return axiosInstance + .post<AdminUniversityImageUploadResponse>(endpoint, formData, { + headers: { "Content-Type": "multipart/form-data" }, + }) + .then((res) => res.data); + }, + uploadAdminUniversityLogo: (file: File, englishName: string) => { - const formData = new FormData(); - formData.append("file", file); - formData.append("englishName", englishName); - return axiosInstance - .post<AdminUniversityImageUploadResponse>("/file/admin/university/logo", formData, { - headers: { "Content-Type": "multipart/form-data" }, - }) - .then((res) => res.data); + return adminApi.uploadUniversityImage("/file/admin/university/logo", file, englishName); }, uploadAdminUniversityBackground: (file: File, englishName: string) => { - const formData = new FormData(); - formData.append("file", file); - formData.append("englishName", englishName); - return axiosInstance - .post<AdminUniversityImageUploadResponse>("/file/admin/university/background", formData, { - headers: { "Content-Type": "multipart/form-data" }, - }) - .then((res) => res.data); + return adminApi.uploadUniversityImage("/file/admin/university/background", file, englishName); },🤖 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.ts` around lines 351 - 372, The uploadAdminUniversityLogo and uploadAdminUniversityBackground methods contain duplicate code that differs only in the endpoint URL. Create a private helper function that accepts the endpoint URL as a parameter and handles the common logic of creating FormData, appending the file and englishName, and making the POST request with the multipart header. Then refactor both uploadAdminUniversityLogo and uploadAdminUniversityBackground to call this helper function with their respective endpoint paths.apps/admin/src/lib/api/admin.test.ts (1)
17-42: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win성공 시나리오 외에 실패 케이스 테스트도 추가하시는 것을 권장드립니다.
현재 테스트는 업로드 성공 시나리오만 검증합니다. 아래 케이스들을 추가하면 더 견고한 테스트 커버리지를 확보하실 수 있습니다:
- 네트워크 오류 시 에러 전파 확인
- 서버 응답 오류(4xx/5xx) 처리 확인
- 잘못된 응답 형식 처리
💚 제안하는 테스트 추가 예시
it("rejects when the server returns an error", async () => { post.mockRejectedValue(new Error("Upload failed")); const file = new File(["image"], "logo.png", { type: "image/png" }); await expect( adminApi.uploadAdminUniversityLogo(file, "university_of_test") ).rejects.toThrow("Upload failed"); });🤖 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 17 - 42, The current test suite for "admin university image uploads" only covers the success scenario. Add additional test cases to validate error handling by creating tests that cover network errors (using post.mockRejectedValue to simulate failures), server error responses (4xx/5xx status codes), and invalid response formats. Use the same parametrization pattern with it.each for the uploadAdminUniversityLogo and uploadAdminUniversityBackground functions to ensure both endpoints properly handle and propagate errors when the post mock is configured to reject or return malformed data.apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.tsx (2)
213-227: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win파일 크기 검증 추가를 고려하세요.
현재 업로드 전 파일 크기 검증이 없어 사용자가 매우 큰 이미지를 업로드할 수 있습니다. 클라이언트 측에서 파일 크기를 미리 검증하면 불필요한 서버 요청과 사용자 대기 시간을 줄일 수 있습니다.
📏 파일 크기 검증 예시
const uploadImage = (kind: "logo" | "background", file: File | undefined) => { if (!file) return; if (!form.formatName.trim()) { toast.error("표시명을 먼저 입력해 주세요."); return; } + const maxSize = 5 * 1024 * 1024; // 5MB + if (file.size > maxSize) { + toast.error("이미지 크기는 5MB 이하여야 합니다."); + return; + } const variables = { file, englishName: form.formatName };🤖 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.tsx` around lines 213 - 227, The uploadImage function lacks file size validation, allowing users to upload potentially large image files unnecessarily consuming server resources and user bandwidth. Add a file size check in the uploadImage function after verifying the file exists (the !file check) but before the formatName validation, and display a toast error message if the file exceeds an acceptable size limit. This prevents oversized uploads from reaching the mutation methods logoUploadMutation.mutate and backgroundUploadMutation.mutate.
442-533: 🧹 Nitpick | 🔵 Trivial | 💤 Low value미리보기 이미지 오류 처리 추가를 고려하세요.
Lines 446, 492의
<img>요소에onError핸들러가 없어,normalizeImageUrlToUploadCdn결과가 유효하지 않은 URL일 경우 깨진 이미지 아이콘이 표시됩니다. 오류 시 폴백 UI로 전환하면 사용자 경험을 개선할 수 있습니다.🖼️ 이미지 오류 처리 예시
+const [logoError, setLogoError] = useState(false); +const [backgroundError, setBackgroundError] = useState(false); {field === "logoImageUrl" && ( <div className="flex items-center gap-3 rounded-lg border border-k-100 bg-k-50 p-3"> - {form.logoImageUrl ? ( + {form.logoImageUrl && !logoError ? ( <img src={normalizeImageUrlToUploadCdn(form.logoImageUrl)} alt="로고 미리보기" className="h-14 w-14 shrink-0 rounded-md border border-k-100 bg-white object-contain p-1" + onError={() => setLogoError(true)} /> ) : (🤖 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.tsx` around lines 442 - 533, Add onError event handlers to both the logo and background image preview elements to handle invalid URLs gracefully. For the img element when field equals "logoImageUrl" and the one when field equals "backgroundImageUrl", add an onError handler that switches the display to show the empty state UI (the div with ImageIcon) instead of showing a broken image icon. This can be achieved by managing state for failed image loads or using a conditional check based on the image load success.apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.test.tsx (1)
52-68: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winCDN URL 하드코딩을 상수/설정으로 대체하세요.
Line 66에서 CDN 도메인
"https://cdn.upload.solid-connection.com"을 하드코딩하고 있습니다. 이 값은normalizeImageUrlToUploadCdn이 사용하는 설정과 일치해야 하는데, 별도로 관리하면 CDN URL 변경 시 테스트가 깨질 수 있습니다.🔧 CDN URL 상수화 예시
cdnUrl.ts에서 업로드 origin 상수를 export하고 테스트에서 import:+import { UPLOAD_ORIGIN } from "`@/lib/utils/cdnUrl`"; expect(screen.getByRole("img", { name: "로고 미리보기" }).getAttribute("src")).toBe( - "https://cdn.upload.solid-connection.com/admin/logo/test.webp", + `${UPLOAD_ORIGIN}/admin/logo/test.webp`, );또는
normalizeImageUrlToUploadCdn결과를 직접 사용:+import { normalizeImageUrlToUploadCdn } from "`@/lib/utils/cdnUrl`"; expect(screen.getByRole("img", { name: "로고 미리보기" }).getAttribute("src")).toBe( - "https://cdn.upload.solid-connection.com/admin/logo/test.webp", + normalizeImageUrlToUploadCdn("admin/logo/test.webp"), );🤖 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 52 - 68, The test hardcodes the CDN domain URL "https://cdn.upload.solid-connection.com" in the image source assertion, which will break if the CDN configuration changes. Instead of hardcoding the full URL, either extract the CDN domain as a constant and import it from a centralized configuration file, or use the normalizeImageUrlToUploadCdn function directly to construct the expected image source URL using the fileUrl returned from the uploadAdminUniversityLogo mock. This ensures the test remains synchronized with actual CDN configuration.
🤖 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.test.tsx`:
- Around line 52-68: The test for the "uploads a selected logo with formatName
and writes the returned URL" case is missing validation for the success toast
notification that is called on successful upload. Add a mock or spy on the
toast.success function and include an expectation to verify that toast.success
is called with the appropriate message after the upload completes. This ensures
the complete user feedback flow, including both the field updates and the toast
notification, is properly tested.
---
Nitpick comments:
In
`@apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.test.tsx`:
- Around line 52-68: The test hardcodes the CDN domain URL
"https://cdn.upload.solid-connection.com" in the image source assertion, which
will break if the CDN configuration changes. Instead of hardcoding the full URL,
either extract the CDN domain as a constant and import it from a centralized
configuration file, or use the normalizeImageUrlToUploadCdn function directly to
construct the expected image source URL using the fileUrl returned from the
uploadAdminUniversityLogo mock. This ensures the test remains synchronized with
actual CDN configuration.
In
`@apps/admin/src/components/features/univ-apply-infos/tabs/HostUniversityTab.tsx`:
- Around line 213-227: The uploadImage function lacks file size validation,
allowing users to upload potentially large image files unnecessarily consuming
server resources and user bandwidth. Add a file size check in the uploadImage
function after verifying the file exists (the !file check) but before the
formatName validation, and display a toast error message if the file exceeds an
acceptable size limit. This prevents oversized uploads from reaching the
mutation methods logoUploadMutation.mutate and backgroundUploadMutation.mutate.
- Around line 442-533: Add onError event handlers to both the logo and
background image preview elements to handle invalid URLs gracefully. For the img
element when field equals "logoImageUrl" and the one when field equals
"backgroundImageUrl", add an onError handler that switches the display to show
the empty state UI (the div with ImageIcon) instead of showing a broken image
icon. This can be achieved by managing state for failed image loads or using a
conditional check based on the image load success.
In `@apps/admin/src/lib/api/admin.test.ts`:
- Around line 17-42: The current test suite for "admin university image uploads"
only covers the success scenario. Add additional test cases to validate error
handling by creating tests that cover network errors (using
post.mockRejectedValue to simulate failures), server error responses (4xx/5xx
status codes), and invalid response formats. Use the same parametrization
pattern with it.each for the uploadAdminUniversityLogo and
uploadAdminUniversityBackground functions to ensure both endpoints properly
handle and propagate errors when the post mock is configured to reject or return
malformed data.
In `@apps/admin/src/lib/api/admin.ts`:
- Around line 351-372: The uploadAdminUniversityLogo and
uploadAdminUniversityBackground methods contain duplicate code that differs only
in the endpoint URL. Create a private helper function that accepts the endpoint
URL as a parameter and handles the common logic of creating FormData, appending
the file and englishName, and making the POST request with the multipart header.
Then refactor both uploadAdminUniversityLogo and uploadAdminUniversityBackground
to call this helper function with their respective endpoint paths.
🪄 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: c943c1ad-c746-4dd0-a1fb-62be85c660a8
📒 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.
작업 내용
englishName키에 호스트 대학의formatName값을 전달합니다.배경 및 원인
기존 어드민 화면은 대학 이미지 URL을 직접 입력해야 했습니다. 업로드 기능 추가 후 상대 경로를
img src에 그대로 사용하면 Vite RSC 개발 서버가 이미지 경로를 애플리케이션 요청으로 처리해 런타임 오류가 발생하므로, 기존 CDN 정규화 유틸을 미리보기에 적용했습니다.검증
pnpm --filter admin test— 18개 테스트 통과pnpm --filter admin lint:checkpnpm --filter admin typecheckpnpm --filter admin build