Merged
Conversation
Walkthrough이번 변경은 대학 상세 페이지로의 라우팅을 ID 기반에서 slug 기반으로 전환하는 작업입니다. 세 개 파일에 걸쳐 다음과 같은 업데이트가 이루어졌습니다.
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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/`(home)/_ui/PopularUniversitySection/_ui/PopularUniversityCard.tsx:
- Around line 22-25: The current logic building universityDetailHref uses
getHomeUniversitySlugByName and drops the university ID when homeUniversitySlug
is falsy, causing links to point to "/university" instead of the specific
detail; update the construction of universityDetailHref in PopularUniversityCard
so that when homeUniversitySlug is undefined it falls back to
`/university/${university.id}` (i.e., include university.id in the fallback),
ensuring links still navigate to the university detail page; modify the
expression that assigns universityDetailHref (references:
getHomeUniversitySlugByName, homeUniversitySlug, universityDetailHref,
university.id) to use the ID-based URL as the fallback.
In `@apps/web/src/components/ui/UniverSityCard/index.tsx`:
- Around line 21-27: The URL fallback and priority rules around
getHomeUniversitySlugByName, mappedHomeUniversitySlug, linkPrefix and
universityDetailHref are unclear; either make the current behavior explicit or
change it: if you want to keep the existing rule (mappedHomeUniversitySlug wins
and missing slug falls back to "/university"), add a concise inline comment
above the universityDetailHref computation documenting that
mappedHomeUniversitySlug takes precedence and that missing mapping intentionally
falls back to "/university"; if instead linkPrefix should be allowed to override
even when mappedHomeUniversitySlug exists, flip the condition so
hasExplicitPrefix is checked before mappedHomeUniversitySlug (i.e., prefer
linkPrefix when provided), and update the comment accordingly.
Comment on lines
+22
to
+25
| const homeUniversitySlug = getHomeUniversitySlugByName(university.homeUniversityName); | ||
| const universityDetailHref = homeUniversitySlug | ||
| ? `/university/${homeUniversitySlug}/${university.id}` | ||
| : "/university"; |
There was a problem hiding this comment.
slug 미확인 시 폴백 URL에서 ID가 제외되는 동작을 확인해 주세요.
UniverSityCard와 동일하게, homeUniversitySlug가 undefined로 반환될 경우 href가 /university/{id} 대신 /university(목록 페이지)가 됩니다.
- 이 컴포넌트는 메인 홈의 "인기 대학" 카드이므로
homeUniversityName이 항상 inha/incheon/sungshin 중 하나일 가능성이 높습니다. - 하지만 API 응답에서
homeUniversityName이 누락되거나 예상치 못한 값이 오면 카드가 목록 페이지로 이동해 사용자가 상세 정보에 접근하지 못할 수 있습니다. /university/[id]라우트가 제거됐다면/university가 최선의 폴백이 맞지만, 존재한다면"/university"대신`/university/${university.id}`를 폴백으로 사용하는 것이 더 견고합니다.
🛡️ 폴백에 ID를 포함하는 예시 (라우트가 여전히 존재하는 경우)
const universityDetailHref = homeUniversitySlug
? `/university/${homeUniversitySlug}/${university.id}`
- : "/university";
+ : `/university/${university.id}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/`(home)/_ui/PopularUniversitySection/_ui/PopularUniversityCard.tsx
around lines 22 - 25, The current logic building universityDetailHref uses
getHomeUniversitySlugByName and drops the university ID when homeUniversitySlug
is falsy, causing links to point to "/university" instead of the specific
detail; update the construction of universityDetailHref in PopularUniversityCard
so that when homeUniversitySlug is undefined it falls back to
`/university/${university.id}` (i.e., include university.id in the fallback),
ensuring links still navigate to the university detail page; modify the
expression that assigns universityDetailHref (references:
getHomeUniversitySlugByName, homeUniversitySlug, universityDetailHref,
university.id) to use the ID-based URL as the fallback.
Comment on lines
+21
to
+27
| const mappedHomeUniversitySlug = getHomeUniversitySlugByName(university.homeUniversityName); | ||
| const hasExplicitPrefix = linkPrefix !== "/university"; | ||
| const universityDetailHref = mappedHomeUniversitySlug | ||
| ? `/university/${mappedHomeUniversitySlug}/${university.id}` | ||
| : hasExplicitPrefix | ||
| ? `${linkPrefix}/${university.id}` | ||
| : "/university"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the structure of the university routes in the Next.js app directory
find apps/web/src/app -type d -name "*university*" 2>/dev/null | sortRepository: solid-connection/solid-connect-web
Length of output: 105
🏁 Script executed:
#!/bin/bash
# List the contents of the university-related directories to see route patterns
if [ -d "apps/web/src/app" ]; then
find apps/web/src/app -path "*university*" -type f -name "*.tsx" -o -path "*university*" -type f -name "*.ts" -o -path "*university*" -type f -name "page.tsx" | sort
fiRepository: solid-connection/solid-connect-web
Length of output: 3384
🏁 Script executed:
#!/bin/bash
# Check for the specific route patterns: [id] and [slug]/[id]
find apps/web/src/app -type f \( -name "page.tsx" -o -name "layout.tsx" \) | xargs grep -l "university" 2>/dev/null | head -20Repository: solid-connection/solid-connect-web
Length of output: 433
🏁 Script executed:
#!/bin/bash
# Read the UniversityCard component
cat -n apps/web/src/components/ui/UniverSityCard/index.tsxRepository: solid-connection/solid-connect-web
Length of output: 4036
🏁 Script executed:
#!/bin/bash
# Find and examine the getHomeUniversitySlugByName function
rg "getHomeUniversitySlugByName" --type ts --type tsx -B 2 -A 5 | head -50Repository: solid-connection/solid-connect-web
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Check how UniversityCard is used/imported
rg "UniverSityCard|UniversityCard" --type ts --type tsx | grep -E "import|from" | head -20Repository: solid-connection/solid-connect-web
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Find the getHomeUniversitySlugByName function
rg "getHomeUniversitySlugByName" --type ts -B 2 -A 10Repository: solid-connection/solid-connect-web
Length of output: 6579
🏁 Script executed:
#!/bin/bash
# Check where UniversityCard is used and with what props
rg "UniverSityCard" --type ts | head -30Repository: solid-connection/solid-connect-web
Length of output: 312
🏁 Script executed:
#!/bin/bash
# Check imports of UniversityCard to see usage contexts
rg "from.*UniverSityCard|import.*UniverSityCard" --type tsRepository: solid-connection/solid-connect-web
Length of output: 312
🏁 Script executed:
#!/bin/bash
# Check UniversityCards component to see how it uses UniversityCard
cat -n apps/web/src/components/university/UniversityCards/index.tsxRepository: solid-connection/solid-connect-web
Length of output: 1062
🏁 Script executed:
#!/bin/bash
# Check FavoriteContent component to see how it uses UniversityCard
cat -n apps/web/src/app/my/favorite/_ui/FavoriteContent/index.tsxRepository: solid-connection/solid-connect-web
Length of output: 4157
🏁 Script executed:
#!/bin/bash
# Search for any usage of UniversityCard with linkPrefix prop
rg "UniversityCard|UniverSityCard" --type ts -A 3 -B 1 | grep -E "linkPrefix|UniverSityCard|UniversityCard" | head -40Repository: solid-connection/solid-connect-web
Length of output: 4101
🏁 Script executed:
#!/bin/bash
# Search for all places where UniversityCards is used with linkPrefix
rg "UniversityCards" --type ts -B 2 -A 2 | grep -E "linkPrefix|UniversityCards"Repository: solid-connection/solid-connect-web
Length of output: 2306
🏁 Script executed:
#!/bin/bash
# Check UniversityListContent to see the full context of linkPrefix usage
cat -n apps/web/src/app/university/[homeUniversity]/_ui/UniversityListContent.tsx | head -50Repository: solid-connection/solid-connect-web
Length of output: 2113
🏁 Script executed:
#!/bin/bash
# Continue reading to see the full linkPrefix usage
cat -n apps/web/src/app/university/[homeUniversity]/_ui/UniversityListContent.tsx | tail -30Repository: solid-connection/solid-connect-web
Length of output: 1261
URL 폴백 로직 재검토 요청
현재 라우팅 구조를 확인한 결과, 다음을 안내합니다.
-
ID-only 라우트 폴백 관련
/university/[homeUniversity]/[id]가 유일한 대학 상세 페이지 라우트입니다/university/[id](slug 없는 경로)는 존재하지 않습니다homeUniversityName매핑 실패 시/university로 폴백하는 것은 의도된 동작입니다
-
linkPrefix우선순위 관련mappedHomeUniversitySlug가 존재하면linkPrefix가 무시되는 로직은 설계상 의도된 것으로 보입니다- 현재 코드베이스에서는
linkPrefix={/university/${homeUniversitySlug}}로 이미 slug가 포함되어 전달되므로 문제가 없습니다 - 다만 이 우선순위 규칙을 명확히 문서화하거나, 향후 비-퍼블릭 컨텍스트(예: 어드민 페이지)에서 재사용될 경우를 대비해 조건을 재검토할 것을 권장합니다
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ui/UniverSityCard/index.tsx` around lines 21 - 27,
The URL fallback and priority rules around getHomeUniversitySlugByName,
mappedHomeUniversitySlug, linkPrefix and universityDetailHref are unclear;
either make the current behavior explicit or change it: if you want to keep the
existing rule (mappedHomeUniversitySlug wins and missing slug falls back to
"/university"), add a concise inline comment above the universityDetailHref
computation documenting that mappedHomeUniversitySlug takes precedence and that
missing mapping intentionally falls back to "/university"; if instead linkPrefix
should be allowed to override even when mappedHomeUniversitySlug exists, flip
the condition so hasExplicitPrefix is checked before mappedHomeUniversitySlug
(i.e., prefer linkPrefix when provided), and update the comment 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/{slug}/{id}형태(예:/university/inha/1899)로 생성하도록 매핑 로직을 적용했습니다.homeUniversityName이 정식명/약칭으로 들어와도 slug를 안정적으로 계산하도록 유틸을 추가했습니다.Changed Files
apps/web/src/constants/university.tsapps/web/src/components/ui/UniverSityCard/index.tsxapps/web/src/app/(home)/_ui/PopularUniversitySection/_ui/PopularUniversityCard.tsxVerification
pnpm --filter @solid-connect/web run ci:checkpnpm --filter @solid-connect/web run build