-
Notifications
You must be signed in to change notification settings - Fork 3
[refactor] 동아리 소개 컴포넌트 리팩토링 및 유틸 함수 단위 테스트 추가 #1068
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
[refactor] 동아리 소개 컴포넌트 리팩토링 및 유틸 함수 단위 테스트 추가 #1068
Conversation
- formatSemesterLabel: Award의 year와 semester를 포맷팅하여 문자열 반환 - getAwardKey: Award 객체와 인덱스로 고유 키 생성 - ClubIntroContent에서 사용하던 함수를 재사용 가능하도록 분리
- formatSemesterLabel 테스트 8개 (정상 케이스, null/undefined 처리) - getAwardKey 테스트 5개 (고유성 검증, 엣지 케이스) - Given 데이터 공통화로 테스트 가독성 향상 - 모든 테스트 통과 (13/13)
- FAQ 토글 상태를 Array에서 Set으로 변경 (O(n) → O(1) 조회) - handleToggleFaq를 useCallback으로 메모이제이션 - validAwards를 useMemo로 계산하여 불필요한 재계산 방지 - 유효하지 않은 award(year/semester 누락) 렌더링 방지 - formatSemesterLabel, getAwardKey 함수를 utils로 분리 및 import
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. Warning
|
| Cohort / File(s) | Summary |
|---|---|
Award 유틸리티 추가 frontend/src/utils/awardHelpers.ts |
상의 학기 레이블 포맷팅(formatSemesterLabel)과 복합 키 생성(getAwardKey)을 담당하는 새 유틸 함수 추가 |
Award 유틸리티 테스트 frontend/src/utils/awardHelpers.test.ts |
formatSemesterLabel 및 getAwardKey에 대한 단위 테스트 추가(연도/학기 없음 처리, 다양한 연도/인덱스 케이스 포함) |
ClubIntroContent 리팩토링 frontend/src/pages/ClubDetailPage/components/ClubIntroContent/ClubIntroContent.tsx |
내부 구현 변경: 로컬 학기 포맷 제거하고 유틸 사용, FAQ open 상태를 Set<number>로 전환, validAwards를 useMemo로 필터링, handleToggleFaq를 useCallback으로 변경 및 이벤트 추적 조건화, 렌더링에서 유틸 기반 키/레이블 사용 |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
- MOA-535: 동아리 소개 컴포넌트 리팩토링 및 유틸 함수 단위 테스트 추가 — 이 PR의 목적(컴포넌트 리팩토링 및 유틸 테스트 추가)과 직접 일치함
- [refactor] MOA-535 동아리 소개 컴포넌트 리팩토링 및 유틸 함수 단위 테스트 추가 #1067: Award 헬퍼 추출 및 ClubIntroContent 리팩토링과 중복되는 변경 지점이 있어 연관될 가능성 있음
Possibly related PRs
- [refactor] 동아리 설명 award의 semesterTerm과 year를 분리한다 #1046 — 백엔드 학기 표기를 year + SemesterTerm으로 분리한 변경과 프론트의 헬퍼 사용/포맷 변경이 연계됨
- [feature] 상세페이지 소개 내용 및 FAQ 컴포넌트 구현 #959 — FAQ 토글 상태 변경 및 상 렌더링/키 생성 로직 리팩토링과 중복되는 구현 요소가 있음
- [refactor] year, semester 타입 분리에 따른 리팩토링 #1051 — award 관련 유틸(학기 레이블/키) 추가/수정과 직접적인 기능 중복이 있음
Suggested reviewers
- oesnuj
- lepitaaar
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 주요 변경사항을 명확하게 설명합니다: ClubIntroContent 컴포넌트 리팩토링과 유틸 함수 단위 테스트 추가는 변경 사항의 핵심을 정확히 반영합니다. |
| Linked Issues check | ✅ Passed | PR의 모든 코딩 요구사항이 충족되었습니다: ClubIntroContent 컴포넌트 리팩토링(FAQ 상태 Array→Set 변경, useMemo/useCallback 적용), Award 유틸 함수 분리(formatSemesterLabel, getAwardKey), 유틸 함수에 대한 13개의 통과한 단위 테스트 추가. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 MOA-535 이슈의 범위 내에 있습니다: ClubIntroContent 리팩토링, Award 유틸 함수 분리, 단위 테스트 추가만 포함되며 범위 외 변경은 없습니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
lepitaaar
left a 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.
리팩토링 수고하셨습니다
frontend/src/pages/ClubDetailPage/components/ClubIntroContent/ClubIntroContent.tsx
Show resolved
Hide resolved
frontend/src/pages/ClubDetailPage/components/ClubIntroContent/ClubIntroContent.tsx
Show resolved
Hide resolved
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: 1
🤖 Fix all issues with AI agents
In
`@frontend/src/pages/ClubDetailPage/components/ClubIntroContent/ClubIntroContent.tsx`:
- Around line 25-55: The setOpenFaqIndexes updater currently calls trackEvent
inside the updater (in handleToggleFaq), which is a side effect and must be
moved out so the updater remains pure; change handleToggleFaq to compute whether
the index is opening by reading current state (e.g., using
openFaqIndexes.has(index) or by computing isOpening from the previous Set before
calling setOpenFaqIndexes), then call setOpenFaqIndexes with a pure updater to
add/remove the index, and finally, after setOpenFaqIndexes returns, call
trackEvent using faqs[index].question and action ('open'|'close'); ensure
references to setOpenFaqIndexes, handleToggleFaq, openFaqIndexes, faqs, and
trackEvent are used to locate and apply this fix.
🧹 Nitpick comments (2)
frontend/src/utils/awardHelpers.ts (1)
3-10: formatSemesterLabel 타입을 실제 입력 범위로 확장
현재 구현/테스트는 null/undefined와 부분 객체까지 처리하지만 시그니처는Award만 받아 캐스팅이 필요합니다.Partial<Award> | null | undefined로 넓히고 nullish 체크를 명확히 하면 타입 안정성과 테스트 가독성이 좋아집니다.♻️ 제안 수정
-export const formatSemesterLabel = (award: Award): string | null => { - if (award?.year && award?.semester) { - const semesterLabel = - award.semester === SemesterTerm.FIRST ? '1학기' : '2학기'; - return `${award.year} ${semesterLabel}`; - } - return null; -}; +export const formatSemesterLabel = ( + award?: Partial<Award> | null, +): string | null => { + if (award?.year == null || award?.semester == null) return null; + const semesterLabel = + award.semester === SemesterTerm.FIRST ? '1학기' : '2학기'; + return `${award.year} ${semesterLabel}`; +};frontend/src/pages/ClubDetailPage/components/ClubIntroContent/ClubIntroContent.tsx (1)
27-30: semesterLabel 중복 계산 및!제거 제안
useMemo에서 label까지 계산해 두면 렌더에서formatSemesterLabel(award)!와 중복 호출을 제거할 수 있습니다.♻️ 제안 수정
- const validAwards = useMemo( - () => awards?.filter((award) => formatSemesterLabel(award) !== null) || [], - [awards], - ); + const validAwards = useMemo( + () => + (awards ?? []) + .map((award) => { + const semesterLabel = formatSemesterLabel(award); + return semesterLabel ? { award, semesterLabel } : null; + }) + .filter( + (item): item is { award: Award; semesterLabel: string } => + item !== null, + ), + [awards], + );- {validAwards.map((award, index) => { - const semesterLabel = formatSemesterLabel(award)!; - const awardKey = getAwardKey(award, index); + {validAwards.map(({ award, semesterLabel }, index) => { + const awardKey = getAwardKey(award, index); return ( <Styled.AwardGroup key={awardKey}> <Styled.SemesterBadge>{semesterLabel}</Styled.SemesterBadge>Also applies to: 72-78
frontend/src/pages/ClubDetailPage/components/ClubIntroContent/ClubIntroContent.tsx
Show resolved
Hide resolved
- trackEvent 호출을 setOpenFaqIndexes updater 함수 외부로 이동 - React 공식 문서의 updater 순수성 요구사항 준수 - StrictMode에서 trackEvent 중복 호출 방지 - openFaqIndexes를 useCallback 의존성 배열에 추가
#️⃣연관된 이슈
📝작업 내용
ClubIntroContent 컴포넌트의 성능을 개선하고, 재사용 가능한 유틸 함수를 분리 및 테스트를 추가했습니다.
그리고 year/semester가 없는 유효하지 않은 award 는 렌더링되지 않도록 변경했습니다.
1. Award 유틸 함수 분리
formatSemesterLabel: Award 객체의 year와 semester를 포맷팅 (예: "2024 1학기")getAwardKey: Award 객체와 인덱스로 고유 키 생성2. 단위 테스트 추가
3. ClubIntroContent 성능 최적화
메모이제이션:
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리스 노트
성능 개선
테스트
리팩토링
✏️ Tip: You can customize this high-level summary in your review settings.