fix: 어학 시험 로고 매핑 보강#592
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough이번 PR은 언어 테스트 로고 관련 유틸리티를
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 1522121000
ℹ️ 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".
| [LanguageTestEnum.TCF]: "/images/language/tcf.png", | ||
| [LanguageTestEnum.TEF]: "/images/language/tef.png", | ||
| [LanguageTestEnum.DUOLINGO]: "/images/language/duolingo.svg", | ||
| } satisfies Record<LanguageTestEnum, string>; |
There was a problem hiding this comment.
When a university requirement contains DELF (it is a valid LanguageTestType in apps/university-web/src/types/university.ts and is exposed in the language filter constants), getLanguageTestLogo("DELF") falls through to the generic default because this new exhaustive check is against the score-submission LanguageTestEnum, which does not include DELF. That leaves DELF requirements without the intended exam logo; use the university language enum or add a DELF mapping here as well.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/language-test-logos.ts (1)
21-25: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
in연산자가 프로토타입 키까지 잡아내는 점을 짚어둘게요.
type in languageTestLogoMap는 상속된 프로퍼티(constructor,toString등)에도true를 반환합니다. 예를 들어getLanguageTestLogo("constructor")를 호출하면 문자열 경로 대신Object.prototype.constructor함수가 반환되어 타입 시그니처(: string)와 어긋나게 됩니다.현재는 입력이
LanguageTestEnum값으로 한정되어 실제 사고로 이어질 가능성은 낮지만, 자기 자신의 키만 확인하도록 바꿔두면 더 안전합니다.♻️ 제안하는 수정
export const getLanguageTestLogo = (type: string): string => { - return type in languageTestLogoMap + return Object.prototype.hasOwnProperty.call(languageTestLogoMap, type) ? languageTestLogoMap[type as LanguageTestLogoType] : DEFAULT_LANGUAGE_TEST_LOGO_SRC; };🤖 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 `@packages/ui/src/language-test-logos.ts` around lines 21 - 25, The `getLanguageTestLogo` lookup currently uses the `in` operator, which can match inherited properties like `constructor` or `toString` and return non-string values. Update the `getLanguageTestLogo` function to check only own keys on `languageTestLogoMap` before indexing, so the fallback to `DEFAULT_LANGUAGE_TEST_LOGO_SRC` is used for any non-explicit logo type.
🤖 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.
Nitpick comments:
In `@packages/ui/src/language-test-logos.ts`:
- Around line 21-25: The `getLanguageTestLogo` lookup currently uses the `in`
operator, which can match inherited properties like `constructor` or `toString`
and return non-string values. Update the `getLanguageTestLogo` function to check
only own keys on `languageTestLogoMap` before indexing, so the fallback to
`DEFAULT_LANGUAGE_TEST_LOGO_SRC` is used for any non-explicit logo type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f9e39d60-c7ac-4fc4-9293-7aacb086e402
⛔ Files ignored due to path filters (21)
apps/university-web/public/images/language/default.pngis excluded by!**/*.pngapps/web/public/images/language/default.pngis excluded by!**/*.pngapps/web/public/images/language/ielts.pngis excluded by!**/*.pngapps/web/public/images/language/toefl_ibt.pngis excluded by!**/*.pngapps/web/public/images/language/toefl_itp.pngis excluded by!**/*.pngapps/web/public/images/language/toeic.pngis excluded by!**/*.pngpackages/ui/public/images/language/cefr.pngis excluded by!**/*.pngpackages/ui/public/images/language/dalf.pngis excluded by!**/*.pngpackages/ui/public/images/language/default.pngis excluded by!**/*.pngpackages/ui/public/images/language/delf.jpgis excluded by!**/*.jpgpackages/ui/public/images/language/duolingo.svgis excluded by!**/*.svgpackages/ui/public/images/language/etc.pngis excluded by!**/*.pngpackages/ui/public/images/language/ielts.pngis excluded by!**/*.pngpackages/ui/public/images/language/jlpt.pngis excluded by!**/*.pngpackages/ui/public/images/language/new_hsk.pngis excluded by!**/*.pngpackages/ui/public/images/language/tcf.pngis excluded by!**/*.pngpackages/ui/public/images/language/tef.pngis excluded by!**/*.pngpackages/ui/public/images/language/toefl_ibt.pngis excluded by!**/*.pngpackages/ui/public/images/language/toefl_itp.pngis excluded by!**/*.pngpackages/ui/public/images/language/toeic.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.gitignoreapps/university-web/next.config.mjsapps/university-web/package.jsonapps/university-web/src/app/university/[homeUniversity]/[id]/_ui/UniversityDetail/_ui/LanguageSection.tsxapps/university-web/src/utils/languageUtils.tsapps/web/next.config.mjsapps/web/package.jsonapps/web/src/utils/languageUtils.tspackages/ui/package.jsonpackages/ui/src/language-test-logos.tsscripts/sync-ui-public-assets.mjs
1 participant
작업 내용
default.pngTOEIC 중복 이미지를 generic 언어 badge로 교체했습니다.DELF전용 로고 자산과 매핑을 추가했습니다.object-contain을 적용했습니다.university-web대학 상세뿐이라, 로고 자산과getLanguageTestLogo유틸을apps/university-web에 종속되도록 되돌렸습니다.apps/web의 미사용 어학 로고 유틸,@solid-connect/ui/language-test-logosexport, public asset sync script, web/university-web의 불필요한@solid-connect/ui의존성을 제거했습니다.검증
pnpm --filter @solid-connect/web lint:checkpnpm --filter @solid-connect/web typecheck:cipnpm --filter @solid-connect/web build(UNIVERSITY_WEB_DOMAIN=http://localhost:3001)pnpm --filter @solid-connect/university-web lint:checkpnpm --filter @solid-connect/university-web typecheck:cipnpm --filter @solid-connect/university-web buildpnpm --filter @solid-connect/ui lint:checkpnpm --filter @solid-connect/ui typecheck@solid-connect/weblint, typecheck, build@solid-connect/university-weblint, typecheck, build@solid-connect/adminlint, typecheck, build/university/kyunghee/3155에서 JLPT 로고가/images/language/jlpt.png로 로드되고 TOEIC 로고가 표시되지 않는 것 확인참고
apps/web에서는 어학 로고 이미지를 렌더링하지 않아 공통 패키지화하지 않았습니다.apps/web의 어학 성적 제출/조회 화면에서도 로고를 표시하게 되면, 그 시점에 어학 로고 맵과 자산 배포 방식을 다시 공통화하는 것이 좋습니다.