Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Walkthrough이 변경 사항은 대학 검색 기능에서 국가 코드 검증 로직을 강화하는 업데이트입니다. 여러 파일에 걸쳐 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
변경 사항 상세 내용
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/university/SearchResultsContent.tsx (1)
36-56:⚠️ Potential issue | 🟡 Minor
hasFilterParams분기 로직은 올바르게 수정되었습니다. 단,lang값의 유효성 검증이 누락되어 있습니다.
승인 사항
Object.hasOwn(COUNTRY_CODE_MAP, country)타입 가드를 통해 프로토타입 체인 없이 안전하게 유효한 국가 코드만 필터링합니다.hasFilterParams의||조건 추가로,countryCode만 있는 경우에도 필터 검색이 정상 동작하는 핵심 버그가 수정되었습니다. ✅사소한 우려 사항 —
lang미검증 캐스트
lang as LanguageTestType은 URL에 임의로 잘못된 값을 넣으면(?languageTestType=INVALID) 검증 없이 API로 그대로 전달됩니다.countryCode는COUNTRY_CODE_MAP으로 검증하면서languageTestType은 동일한 방어 로직이 없습니다.🛡️ languageTestType 유효성 검증 제안
+import { COUNTRY_CODE_MAP, LANGUAGE_TEST_TYPE_MAP } from "@/constants/university"; - const hasFilterParams = Boolean(lang) || filteredCountries.length > 0; + const validLang = lang && Object.hasOwn(LANGUAGE_TEST_TYPE_MAP, lang) ? (lang as LanguageTestType) : undefined; + const hasFilterParams = Boolean(validLang) || filteredCountries.length > 0; if (!hasFilterParams) { ... } else { return { isTextSearch: false, searchText: "", filterParams: { - languageTestType: (lang as LanguageTestType) || undefined, + languageTestType: validLang, countryCode: filteredCountries.length > 0 ? filteredCountries : undefined, }, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/SearchResultsContent.tsx` around lines 36 - 56, The lang value is being unsafely cast to LanguageTestType when building filterParams (languageTestType: (lang as LanguageTestType) || undefined) so invalid URL values could be passed to the API; update the logic where filterParams is constructed (in the same block that references filteredCountries, hasFilterParams and UniversitySearchFilterParams) to validate lang against the canonical LanguageTestType values (e.g., check membership in the LanguageTestType enum or an explicit allowed-values set derived from LanguageTestType) and only assign languageTestType when the value is valid, otherwise set it to undefined.apps/web/src/app/university/list/[homeUniversityName]/SearchResultsContent.tsx (1)
40-59:⚠️ Potential issue | 🟡 Minor핵심 로직은
apps/web/src/app/university/SearchResultsContent.tsx와 동일하게 올바르게 적용되었습니다.
승인 사항
Object.hasOwn타입 가드와hasFilterParams분기 조건이 동일하게 적용되어countryCode만 있는 경우에도 필터 검색이 작동합니다. ✅동일한 사소한 우려 사항
- Line 56의
(lang as LanguageTestType) || undefined역시 URL 파라미터를 검증 없이 캐스트합니다. 상위 파일(apps/web/src/app/university/SearchResultsContent.tsx)에 남긴 코멘트와 동일한 방어 로직을 적용하면 일관성이 유지됩니다.두
SearchResultsContent.tsx의 중복 코드
apps/web/src/app/university/SearchResultsContent.tsx와 이 파일의useMemo내부 로직(filteredCountries 필터링, hasFilterParams 계산, filterParams 반환 구조)이 사실상 동일합니다. 향후 로직 변경 시 두 곳을 동시에 수정해야 하는 부담이 생길 수 있습니다.♻️ 공통 훅 추출 제안
두 파일의 공통 로직을 커스텀 훅으로 추출하면 중복을 없앨 수 있습니다.
// 예: apps/web/src/hooks/useUniversitySearchParams.ts export function useUniversitySearchParams() { const searchParams = useSearchParams(); return useMemo(() => { const text = searchParams.get("searchText"); const lang = searchParams.get("languageTestType"); const countries = searchParams.getAll("countryCode"); const filteredCountries = countries.filter( (country): country is CountryCode => Object.hasOwn(COUNTRY_CODE_MAP, country), ); const hasFilterParams = Boolean(lang) || filteredCountries.length > 0; if (!hasFilterParams) { return { isTextSearch: true, searchText: text, filterParams: {} as UniversitySearchFilterParams }; } return { isTextSearch: false, searchText: "", filterParams: { languageTestType: (lang as LanguageTestType) || undefined, countryCode: filteredCountries.length > 0 ? filteredCountries : undefined, }, }; }, [searchParams]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/list/`[homeUniversityName]/SearchResultsContent.tsx around lines 40 - 59, The code unsafely casts URL param `lang` using `(lang as LanguageTestType) || undefined` in SearchResultsContent.tsx inside the useMemo block and duplicates the same filtering/hasFilterParams logic across two files; replace the cast with a validated lookup (e.g., check `lang` against the allowed LanguageTestType values or a LANGUAGE_TEST_TYPE_MAP using Object.hasOwn/Set membership and only return the validated value or undefined) and extract the shared logic (filteredCountries, hasFilterParams, and resulting object) into a single custom hook (suggested name: useUniversitySearchParams) so both SearchResultsContent.tsx instances call that hook and remove duplicated code.
🧹 Nitpick comments (3)
apps/web/src/app/university/search/PageContent.tsx (1)
49-68:availableCountryCodeSet을onSubmit내부에서 매번 생성하는 대신 메모이제이션하는 것이 좋습니다.
현재 코드 동작
availableCountryCodeSet이onSubmit(Line 49) 내부에서 생성되지만,availableCountries는 Line 74의useMemo에서 선언됩니다.- JavaScript의 TDZ는 시간(실행 순서) 기반이기 때문에,
onSubmit이 폼 제출 시점(컴포넌트 렌더링 이후)에 호출될 때는availableCountries가 이미 초기화되어 있어 런타임 오류는 발생하지 않습니다.- 그러나 선언 순서가 역전되어 있어 가독성이 떨어지고, 폼을 제출할 때마다
Set을 새로 생성합니다.개선 제안
availableCountryCodeSet을 별도의useMemo로 분리하면,watchedRegions변경 시에만 재계산되고 선언 순서도 자연스러워집니다.♻️ useMemo로 분리하는 방법
const availableCountries = useMemo(() => { if (watchedRegions.length === 0) return Object.entries(COUNTRY_CODE_MAP); const countrySet = new Set<string>(); watchedRegions.forEach((region) => REGION_TO_COUNTRIES_MAP[region]?.forEach((country) => countrySet.add(country))); return Object.entries(COUNTRY_CODE_MAP).filter(([_, name]) => countrySet.has(name)); }, [watchedRegions]); + const availableCountryCodeSet = useMemo( + () => new Set(availableCountries.map(([code]) => code as CountryCode)), + [availableCountries], + ); const onSubmit: SubmitHandler<SearchFormData> = (data) => { const queryParams = new URLSearchParams(); - const availableCountryCodeSet = new Set(availableCountries.map(([code]) => code as CountryCode)); // ... 이하 동일 };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/search/PageContent.tsx` around lines 49 - 68, Move creation of availableCountryCodeSet out of the onSubmit handler and memoize it with useMemo so it’s only recomputed when availableCountries (or watchedRegions if relevant) changes: create a const availableCountryCodeSet = useMemo(() => new Set(availableCountries.map(([code]) => code as CountryCode)), [availableCountries]) near where availableCountries is declared; then update onSubmit to use that memoized availableCountryCodeSet instead of recreating it each submit. Ensure the identifier names (availableCountryCodeSet, onSubmit, availableCountries, useMemo, watchedRegions) are used to locate and update the code.apps/web/src/constants/university.ts (1)
10-12:REGIONS_SEARCH와REGIONS_KO의 내용이 완전히 동일해졌습니다.
변경 내용 확인
REGIONS_SEARCH에"중국권"을 추가한 것은 올바른 수정입니다.REGION_KO_MAP,regionCountryMap,REGION_TO_COUNTRIES_MAP등 기존 데이터 구조들이 이미중국권을 지원하고 있어 일관성이 맞춰졌습니다.- 다만, 이제 두 상수의 내용이 동일해졌습니다.
REGIONS_SEARCH는as const튜플이고REGIONS_KO는 일반 배열로 타입만 다릅니다.선택적 개선 제안
- 두 상수를 별도로 유지하는 것보다 하나를 다른 것에서 파생시키면 향후 지역 추가 시 한 곳만 수정하면 됩니다.
♻️ 중복 제거 제안
-export const REGIONS_SEARCH = ["유럽권", "미주권", "아시아권", "중국권"] as const; +export const REGIONS_SEARCH = ["유럽권", "미주권", "아시아권", "중국권"] as const; -export const REGIONS_KO = ["유럽권", "미주권", "아시아권", "중국권"]; +export const REGIONS_KO: string[] = [...REGIONS_SEARCH];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/constants/university.ts` around lines 10 - 12, REGIONS_SEARCH and REGIONS_KO are now identical; to avoid duplication, derive one from the other so future changes only require a single edit. Replace the duplicated literal array by exporting REGIONS_SEARCH as the canonical const tuple (export const REGIONS_SEARCH = [... ] as const) and then build REGIONS_KO from it (e.g., Array.from(REGIONS_SEARCH) or REGIONS_SEARCH.slice()) so REGIONS_KO stays a plain string[] while REGIONS_SEARCH remains the readonly tuple; update any usages expecting the specific types accordingly.apps/web/src/app/university/[homeUniversity]/search/_ui/SearchPageContent.tsx (1)
55-75:availableCountryCodeSet을onSubmit내부에서 생성하는 패턴이apps/web/src/app/university/search/PageContent.tsx와 동일하게 반복됩니다.
동일한 개선 포인트
availableCountries(Line 77)가onSubmit(Line 55) 이후에 선언되지만, 런타임 오류는 발생하지 않습니다. 단, 가독성과 성능 측면에서useMemo로 분리하는 것을 권장합니다.PageContent.tsx에 남긴 리팩터링 제안과 동일하게 적용하면 두 파일의 일관성도 유지됩니다.코드 중복
onSubmit과availableCountries로직이PageContent.tsx와 거의 동일합니다. 두 파일을 동시에 수정하는 부담을 줄이려면 공통 로직을 추출하는 방안도 고려해 볼 수 있습니다.♻️ useMemo 분리 제안 (PageContent.tsx와 동일)
const availableCountries = useMemo(() => { if (watchedRegions.length === 0) return Object.entries(COUNTRY_CODE_MAP); const countrySet = new Set<string>(); watchedRegions.forEach((region) => REGION_TO_COUNTRIES_MAP[region]?.forEach((country) => countrySet.add(country))); return Object.entries(COUNTRY_CODE_MAP).filter(([_, name]) => countrySet.has(name)); }, [watchedRegions]); + const availableCountryCodeSet = useMemo( + () => new Set(availableCountries.map(([code]) => code as CountryCode)), + [availableCountries], + ); const onSubmit: SubmitHandler<SearchFormData> = (data) => { const queryParams = new URLSearchParams(); - const availableCountryCodeSet = new Set(availableCountries.map(([code]) => code as CountryCode)); // ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/`[homeUniversity]/search/_ui/SearchPageContent.tsx around lines 55 - 75, The onSubmit handler builds availableCountryCodeSet from availableCountries inside the function which duplicates logic from PageContent.tsx and hurts readability/perf; move the set creation out of onSubmit by memoizing it with useMemo (e.g., const availableCountryCodeSet = useMemo(() => new Set(availableCountries.map(([code]) => code as CountryCode)), [availableCountries])) and then let onSubmit reference that memoized availableCountryCodeSet, and optionally extract this into a shared helper (e.g., getAvailableCountryCodeSet) used by both SearchPageContent.tsx and PageContent.tsx to remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/web/src/app/university/list/`[homeUniversityName]/SearchResultsContent.tsx:
- Around line 40-59: The code unsafely casts URL param `lang` using `(lang as
LanguageTestType) || undefined` in SearchResultsContent.tsx inside the useMemo
block and duplicates the same filtering/hasFilterParams logic across two files;
replace the cast with a validated lookup (e.g., check `lang` against the allowed
LanguageTestType values or a LANGUAGE_TEST_TYPE_MAP using Object.hasOwn/Set
membership and only return the validated value or undefined) and extract the
shared logic (filteredCountries, hasFilterParams, and resulting object) into a
single custom hook (suggested name: useUniversitySearchParams) so both
SearchResultsContent.tsx instances call that hook and remove duplicated code.
In `@apps/web/src/app/university/SearchResultsContent.tsx`:
- Around line 36-56: The lang value is being unsafely cast to LanguageTestType
when building filterParams (languageTestType: (lang as LanguageTestType) ||
undefined) so invalid URL values could be passed to the API; update the logic
where filterParams is constructed (in the same block that references
filteredCountries, hasFilterParams and UniversitySearchFilterParams) to validate
lang against the canonical LanguageTestType values (e.g., check membership in
the LanguageTestType enum or an explicit allowed-values set derived from
LanguageTestType) and only assign languageTestType when the value is valid,
otherwise set it to undefined.
---
Nitpick comments:
In
`@apps/web/src/app/university/`[homeUniversity]/search/_ui/SearchPageContent.tsx:
- Around line 55-75: The onSubmit handler builds availableCountryCodeSet from
availableCountries inside the function which duplicates logic from
PageContent.tsx and hurts readability/perf; move the set creation out of
onSubmit by memoizing it with useMemo (e.g., const availableCountryCodeSet =
useMemo(() => new Set(availableCountries.map(([code]) => code as CountryCode)),
[availableCountries])) and then let onSubmit reference that memoized
availableCountryCodeSet, and optionally extract this into a shared helper (e.g.,
getAvailableCountryCodeSet) used by both SearchPageContent.tsx and
PageContent.tsx to remove duplication.
In `@apps/web/src/app/university/search/PageContent.tsx`:
- Around line 49-68: Move creation of availableCountryCodeSet out of the
onSubmit handler and memoize it with useMemo so it’s only recomputed when
availableCountries (or watchedRegions if relevant) changes: create a const
availableCountryCodeSet = useMemo(() => new Set(availableCountries.map(([code])
=> code as CountryCode)), [availableCountries]) near where availableCountries is
declared; then update onSubmit to use that memoized availableCountryCodeSet
instead of recreating it each submit. Ensure the identifier names
(availableCountryCodeSet, onSubmit, availableCountries, useMemo, watchedRegions)
are used to locate and update the code.
In `@apps/web/src/constants/university.ts`:
- Around line 10-12: REGIONS_SEARCH and REGIONS_KO are now identical; to avoid
duplication, derive one from the other so future changes only require a single
edit. Replace the duplicated literal array by exporting REGIONS_SEARCH as the
canonical const tuple (export const REGIONS_SEARCH = [... ] as const) and then
build REGIONS_KO from it (e.g., Array.from(REGIONS_SEARCH) or
REGIONS_SEARCH.slice()) so REGIONS_KO stays a plain string[] while
REGIONS_SEARCH remains the readonly tuple; update any usages expecting the
specific types accordingly.
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
/university/search필터 폼에서 선택한 지역(region)이 결과 페이지로 전달되도록 쿼리 구성 로직을 보완했습니다.countryCode만 있는 경우에도 필터 검색이 정상 동작하도록 수정했습니다.Verification
pnpm --filter @solid-connect/web run typecheckpnpm --filter @solid-connect/web run buildci:check) 통과