-
Notifications
You must be signed in to change notification settings - Fork 4
feat: 대학교 선택 온보딩 페이지 및 대학별 파견학교 목록 추가 #392
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
- HomeUniversityName enum 추가 (인하대학교, 인천대학교, 성신여자대학교) - /university 온보딩 페이지 생성 (대학 선택 카드 UI) - /university/list/[homeUniversityName] 동적 라우트 페이지 생성 (ISR) - API 훅에 homeUniversityName 클라이언트 필터링 추가 - ListUniversity 타입에 homeUniversityName 속성 추가
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough이번 변경사항은 대학교 목록 페이지에 홈 대학(파견 대학)별 필터링 기능을 추가하는 종합적인 업데이트입니다. 구체적으로 다음과 같은 내용을 포함합니다:
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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: 2
🤖 Fix all issues with AI agents
In
`@apps/web/src/app/university/list/`[homeUniversityName]/SearchResultsContent.tsx:
- Around line 36-47: The check `if (!lang || !countries)` is incorrect because
`countries` from `searchParams.getAll()` is always an array (even empty), so
replace the falsy array check with an explicit length check: treat text-search
mode when `lang` is missing or `countries.length === 0`; update the conditional
in the block that references `countries`/`filteredCountries` accordingly and
keep the `filteredCountries` cast to `CountryCode[]` (from
`searchParams.getAll`) intact so types remain correct.
In `@apps/web/src/types/university.ts`:
- Around line 53-58: The entry for HomeUniversityName.SUNGSHIN has a mismatch
between the slug ("sungshin") and imageUrl ("/images/unvis/sungsin.jpg"); fix by
making naming consistent—either rename the image file to "sungshin.jpg" or
update the imageUrl to "/images/unvis/sungsin.jpg" to match the chosen slug;
preferred: set imageUrl to "/images/unvis/sungshin.jpg" so
HomeUniversityName.SUNGSHIN, the slug "sungshin", and the image name all match.
♻️ Duplicate comments (1)
apps/web/src/apis/universities/getSearchFilter.ts (1)
14-17: 1. 중복 타입 정의앞서
getSearchText.ts리뷰에서 언급한 것처럼,ListUniversityWithHome타입이 중복 정의되어 있습니다. 공통 타입으로 추출하는 것을 권장합니다.
🧹 Nitpick comments (3)
apps/web/src/app/university/list/[homeUniversityName]/SearchResultsContent.tsx (1)
26-31: 2. 초기 상태와 useEffect 동기화 개선 제안
selectedRegion을RegionEnumExtend.ALL로 초기화한 후,useEffect에서 URL 파라미터를 읽어 업데이트하는 방식은 초기 렌더링 시 깜빡임을 유발할 수 있습니다.
useMemo를 사용해 초기값을 URL에서 직접 계산하면 더 깔끔합니다.♻️ 리팩토링 제안
- // 지역 상태 관리 - const [selectedRegion, setSelectedRegion] = useState<RegionEnumExtend>(RegionEnumExtend.ALL); + // 지역 상태 관리 - URL에서 초기값 설정 + const initialRegion = useMemo(() => { + const region = searchParams.get("region"); + if (region && Object.values(RegionEnumExtend).includes(region as RegionEnumExtend)) { + return region as RegionEnumExtend; + } + return RegionEnumExtend.ALL; + }, [searchParams]); + const [selectedRegion, setSelectedRegion] = useState<RegionEnumExtend>(initialRegion);그리고 기존
useEffect(Line 78-83)는 제거하시면 됩니다.Also applies to: 77-83
apps/web/src/apis/universities/getSearchText.ts (1)
9-12: 1. 중복 타입 정의 - 공통 모듈로 추출 제안
ListUniversityWithHome타입이getSearchFilter.ts에도 동일하게 정의되어 있습니다. 두 파일에서 같은 타입을 사용하므로,@/types/university.ts에서 export하거나 별도의 공통 파일로 추출하면 유지보수가 더 쉬워집니다.참고:
ListUniversity인터페이스 자체에 이미homeUniversityName?: HomeUniversityName이 포함되어 있어서(apps/web/src/types/university.ts Line 138),ListUniversityWithHome이 사실상ListUniversity와 동일합니다. 이 별도 타입이 필요한지 재검토해 보세요.apps/web/src/app/university/list/[homeUniversityName]/page.tsx (1)
9-10: 주석 내용과 실제 동작이 약간 다릅니다.
revalidate = false는 ISR이 아닌 **완전한 정적 생성(SSG)**을 의미합니다.
- ISR은 보통
revalidate = 60같이 숫자를 사용하여 주기적으로 재생성합니다.false는 빌드 시점에 생성된 페이지를 절대 재생성하지 않습니다.동작 자체는 의도한 대로 정상 작동하며, 상위 페이지(
/university/page.tsx)와도 일관성이 있습니다.♻️ 주석 수정 제안 (선택 사항)
-// ISR: 정적 페이지 생성 +// SSG: 빌드 시 정적 페이지 생성 (재검증 비활성화) export const revalidate = false;
| const countries = searchParams.getAll("countryCode"); | ||
|
|
||
| // URL에서 전달된 국가 목록을 기본으로 사용 | ||
| const filteredCountries = countries as CountryCode[]; | ||
|
|
||
| if (!lang || !countries) { | ||
| return { | ||
| isTextSearch: true, | ||
| searchText: text, | ||
| filterParams: {} as UniversitySearchFilterParams, | ||
| }; | ||
| } |
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.
1. 배열 falsy 체크 버그
countries는 searchParams.getAll()의 반환값으로 항상 배열입니다. 빈 배열 []도 JavaScript에서 truthy이므로 !countries 조건은 항상 false입니다.
현재 로직에서는 lang만 없으면 텍스트 검색 모드로 전환되는데, 의도한 동작인지 확인이 필요합니다.
🐛 수정 제안
- if (!lang || !countries) {
+ if (!lang || countries.length === 0) {🤖 Prompt for AI Agents
In
`@apps/web/src/app/university/list/`[homeUniversityName]/SearchResultsContent.tsx
around lines 36 - 47, The check `if (!lang || !countries)` is incorrect because
`countries` from `searchParams.getAll()` is always an array (even empty), so
replace the falsy array check with an explicit length check: treat text-search
mode when `lang` is missing or `countries.length === 0`; update the conditional
in the block that references `countries`/`filteredCountries` accordingly and
keep the `filteredCountries` cast to `CountryCode[]` (from
`searchParams.getAll`) intact so types remain correct.
| { | ||
| name: HomeUniversityName.SUNGSHIN, | ||
| slug: "sungshin", | ||
| imageUrl: "/images/unvis/sungsin.jpg", | ||
| description: "성신여자대학교 파견 교환학생 정보", | ||
| }, |
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.
🧩 Analysis chain
🏁 Script executed:
fd -type f \( -name "*sung*" -o -name "*.jpg" -o -name "*.png" \) | head -20Repository: solid-connection/solid-connect-web
Length of output: 248
🏁 Script executed:
find . -path "*/images/unvis*" -type f 2>/dev/null | head -20Repository: solid-connection/solid-connect-web
Length of output: 203
🏁 Script executed:
find . -path "*public/images*" -type f 2>/dev/null | grep -i sungRepository: solid-connection/solid-connect-web
Length of output: 120
이미지 파일명과 대학교명의 로마자 표기 불일치 발견
성신여자대학교의 로마자 표기가 코드에서는 sungshin으로 되어 있지만, 실제 이미지 파일명은 sungsin.jpg로 저장되어 있습니다. 파일은 정상적으로 존재하고 코드가 올바르게 참조하고 있지만, 네이밍 규칙의 일관성을 위해 다음 중 하나로 정정하시는 것이 좋겠습니다.
- 이미지 파일명을
sungsin.jpg→sungshin.jpg로 변경하여 코드의 네이밍과 일치 - 또는 slug를
sungshin→sungsin으로 변경하여 실제 파일명과 일치
현재 상황:
- enum:
SUNGSHIN(정확한 로마자 표기) - slug:
sungshin(정확한 로마자 표기) - imageUrl:
sungsin.jpg← 한 글자 누락
🤖 Prompt for AI Agents
In `@apps/web/src/types/university.ts` around lines 53 - 58, The entry for
HomeUniversityName.SUNGSHIN has a mismatch between the slug ("sungshin") and
imageUrl ("/images/unvis/sungsin.jpg"); fix by making naming consistent—either
rename the image file to "sungshin.jpg" or update the imageUrl to
"/images/unvis/sungsin.jpg" to match the chosen slug; preferred: set imageUrl to
"/images/unvis/sungshin.jpg" so HomeUniversityName.SUNGSHIN, the slug
"sungshin", and the image name all match.
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)