refactor: 임시 로그인 제거, 견적요청 컴포넌트 분리 및 리팩토링 - #31
Conversation
- EstimateRequestForm에서 TEST_CUSTOMER 임시 로그인 코드 제거, hasAuthSession 기반으로 전환 - auth.ts (미사용) 삭제 - 견적요청 전용 컴포넌트 6개를 estimate/request/ 폴더로 이동 - MOVE_TYPES 상수를 constants/moveType.ts로 분리 - Toast 반복 렌더링을 변수로 통합 - Header에서 isLogin prop 제거, 알림 영역을 NotificationTrigger로 분리 - NotificationPanel 알림 아이템 중복 렌더링을 NotificationContent로 추출 - AddressSelectModal inline style을 Tailwind 클래스로 전환 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough견적 요청 폼은 세션 기반 로그인 리다이렉트와 공용 이사 유형 카드 데이터를 사용합니다. 헤더는 인증 상태와 분리된 알림 트리거를 적용합니다. 주소 검색은 좌표·지번·도로명 룩업으로 우편번호를 단계적으로 보강합니다. Changes견적 요청 흐름
헤더 알림 UI
주소 검색 우편번호 보강
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EstimateRequestForm
participant AuthSession
participant Router
participant ActiveEstimateQuery
participant CreateMutation
EstimateRequestForm->>AuthSession: 세션 유무 확인
EstimateRequestForm->>Router: 비로그인 시 로그인 경로로 이동
EstimateRequestForm->>ActiveEstimateQuery: 로그인 상태에서 활성 견적 조회
EstimateRequestForm->>CreateMutation: 입력 검증 후 견적 요청 제출
sequenceDiagram
participant Header
participant NotificationTrigger
participant NotificationPanel
Header->>NotificationTrigger: 로그인 상태에서 렌더링
NotificationTrigger->>NotificationPanel: 버튼 클릭으로 패널 열기
NotificationTrigger->>NotificationPanel: 외부 클릭 또는 Escape로 닫기
NotificationPanel->>NotificationTrigger: 링크 클릭 시 닫기와 포커스 복귀
sequenceDiagram
participant AddressSearchRoute
participant KakaoAddressSearch
participant CoordinateLookup
participant AddressQueryLookup
AddressSearchRoute->>KakaoAddressSearch: 주소·키워드 문서 병렬 검색
AddressSearchRoute->>CoordinateLookup: 누락 좌표 우편번호 보강
AddressSearchRoute->>AddressQueryLookup: 누락 지번·도로명 재검색
AddressSearchRoute->>KakaoAddressSearch: 룩업 기반 우편번호 해석
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (3)
src/lib/constants/moveType.ts (1)
20-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win카드 배열의 리터럴 타입을 보존하세요.
MoveTypeCardInfo[]는 배열과 원소의 변경을 허용합니다.as const satisfies readonly MoveTypeCardInfo[]로 선언해 상수 계약과 리터럴 타입을 유지하세요.As per path instructions,
src/lib/constants/**/*.ts에는as const를 붙여 리터럴 타입을 유지합니다.🤖 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 `@src/lib/constants/moveType.ts` around lines 20 - 39, Update MOVE_TYPE_CARDS to use `as const satisfies readonly MoveTypeCardInfo[]`, preserving readonly array and element literal types while still validating the MoveTypeCardInfo shape.Source: Path instructions
src/components/estimate/request/ActiveEstimateBlocked.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win불필요한 Client Component 경계를 제거해 주세요.
이 컴포넌트는 순수하게
EmptyState에 props를 전달하므로"use client"가 필요하지 않습니다. 경계를 제거해 Server Component로 유지하세요.As per coding guidelines, 불필요한 상태·
useEffect·Client Component를 추가하지 않는다.🤖 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 `@src/components/estimate/request/ActiveEstimateBlocked.tsx` at line 1, Remove the unnecessary "use client" directive from ActiveEstimateBlocked so the component remains a Server Component. Keep its existing prop-passing behavior to EmptyState unchanged and do not add client state or effects.Source: Coding guidelines
src/components/estimate/request/AddressSelectModal.tsx (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win아이콘 import를 프로젝트 공통 절대 경로로 통일해 주세요.
상위 경로 상대 import는 폴더 이동 시 쉽게 깨지고 프로젝트 규칙에도 맞지 않습니다. 공통 아이콘 export인
@/icons를 사용하세요.
src/components/estimate/request/AddressSelectModal.tsx#L10-L10:../icons대신@/icons에서ClearCircleIcon,SearchIcon을 import하세요.src/components/estimate/request/Calendar.tsx#L9-L9:../icons대신@/icons에서 월 이동 아이콘을 import하세요.src/components/estimate/request/DatePickerField.tsx#L10-L10:../icons대신@/icons에서 날짜 선택 아이콘을 import하세요.src/components/estimate/request/MoveTypeCard.tsx#L8-L8:../icons대신@/icons에서CheckIcon을 import하세요.As per coding guidelines and path instructions, import는
@/*절대 경로를 사용하고 아이콘은@/icons에서 import합니다.🤖 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 `@src/components/estimate/request/AddressSelectModal.tsx` at line 10, Replace the relative icon imports with the project-standard `@/icons` absolute import in AddressSelectModal.tsx (line 10), Calendar.tsx (line 9), DatePickerField.tsx (line 10), and MoveTypeCard.tsx (line 8), preserving each file’s existing icon symbols, including ClearCircleIcon, SearchIcon, the month-navigation icons, the date-selection icon, and CheckIcon.Sources: Coding guidelines, Path instructions
🤖 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.
Inline comments:
In `@src/components/common/Header/NotificationTrigger.tsx`:
- Around line 38-44: Update the notification trigger button in
NotificationTrigger to provide at least a size-40 touch target around the icon
and add a token-based focus-visible ring style. Preserve its existing toggle
behavior, accessibility attributes, and relative positioning while extending the
className.
- Around line 17-21: Update the NotificationTrigger close handling so Escape and
the close-button paths restore focus to the trigger after closing, using the
trigger ref alongside useClickOutside. Keep outside-click dismissal unchanged so
it does not move focus, and ensure the focus restoration occurs after the panel
closes to avoid targeting an unmounted element.
---
Nitpick comments:
In `@src/components/estimate/request/ActiveEstimateBlocked.tsx`:
- Line 1: Remove the unnecessary "use client" directive from
ActiveEstimateBlocked so the component remains a Server Component. Keep its
existing prop-passing behavior to EmptyState unchanged and do not add client
state or effects.
In `@src/components/estimate/request/AddressSelectModal.tsx`:
- Line 10: Replace the relative icon imports with the project-standard `@/icons`
absolute import in AddressSelectModal.tsx (line 10), Calendar.tsx (line 9),
DatePickerField.tsx (line 10), and MoveTypeCard.tsx (line 8), preserving each
file’s existing icon symbols, including ClearCircleIcon, SearchIcon, the
month-navigation icons, the date-selection icon, and CheckIcon.
In `@src/lib/constants/moveType.ts`:
- Around line 20-39: Update MOVE_TYPE_CARDS to use `as const satisfies readonly
MoveTypeCardInfo[]`, preserving readonly array and element literal types while
still validating the MoveTypeCardInfo shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bccc4ba4-8c30-474b-9ef0-156792142d82
📒 Files selected for processing (12)
src/app/estimate-request/page.tsxsrc/components/common/Header/Header.tsxsrc/components/common/Header/NotificationPanel.tsxsrc/components/common/Header/NotificationTrigger.tsxsrc/components/estimate/request/ActiveEstimateBlocked.tsxsrc/components/estimate/request/AddressSelectModal.tsxsrc/components/estimate/request/Calendar.tsxsrc/components/estimate/request/DatePickerField.tsxsrc/components/estimate/request/EstimateRequestForm.tsxsrc/components/estimate/request/MoveTypeCard.tsxsrc/lib/api/auth.tssrc/lib/constants/moveType.ts
💤 Files with no reviewable changes (1)
- src/lib/api/auth.ts
- Esc 키, 닫기 버튼: 포커스를 트리거 버튼으로 복귀 - 외부 클릭: 포커스 이동 없이 패널만 닫음 Co-authored-by: Cursor <cursoragent@cursor.com>
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 테스트를 위해 임시로 넣었던 하드코딩 계정과 자동 로그인 로직을 제거하고, 프로젝트의 실제 세션 판별 방식으로 전환한 점이 좋았습니다.
- 미로그인 상태에서는 견적 관련 API를 실행하지 않도록 enabled: isLoggedIn을 적용해 불필요한 인증 오류 요청을 막았습니다.
- 제출 시점에도 로그인 상태를 한 번 더 검사해, 화면 진입 이후 세션이 사라진 경우를 방어했습니다.
- 견적 요청 관련 컴포넌트를 estimate/request 폴더로 이동해 받은 견적·견적 요청 등 하위 도메인을 구분하기 쉬워졌습니다.
- EstimateRequestForm 내부에 있던 이사 유형 카드 데이터를 공통 상수 파일로 이동해 화면 컴포넌트의 책임을 줄였습니다.
- 여러 분기마다 반복되던 Toast 렌더링을 toastElement로 통합해 중복을 줄였습니다.
- Header에서 알림 상태와 이벤트 로직을 NotificationTrigger로 분리해 Header 컴포넌트가 내비게이션과 프로필 영역에 집중할 수 있게 됐습니다.
- 알림 패널을 Esc 또는 닫기 버튼으로 닫을 때 트리거 버튼으로 포커스를 복귀시켜 접근성을 보완했습니다.
- 알림의 링크 유무에 따라 반복되던 메시지와 날짜 렌더링을 NotificationContent로 추출해 코드 중복을 크게 줄였습니다.
- 사용되지 않는 임시 인증 API 파일까지 함께 삭제해 기존 로그인 담당자의 구현과 역할이 겹치지 않도록 정리했습니다.
🔍 확인 및 제안
인라인 코멘트로 아래 내용을 남기면 좋겠습니다.
- 현재 hasAuthSession()은 토큰 존재 여부만 확인하므로 기사님이나 관리자 계정도 견적 요청 API를 호출할 수 있습니다. Customer 전용 페이지 접근 제어를 어디에서 처리할 예정인지 확인이 필요합니다.
- 이사 유형이 대문자 API 값과 소문자 카드 ID로 나뉘어 여러 상수에서 관리되고 있습니다. 추후 카드 ID도 MoveType으로 통일하면 변환 로직과 중복 기준을 줄일 수 있을 것 같습니다.
전체적으로 임시 인증 로직을 제거하면서 견적 요청과 알림 관련 코드의 책임도 함께 정리한 깔끔한 리팩토링이었습니다. 수고하셨습니다! 😊
9g-g9
left a comment
9g-g9
left a comment
There was a problem hiding this comment.
작업 고생많으셨습니다!
header 및 auth.ts 연결 부분이 제 부분하고 상당 수 차이가 나고 있어 충돌이 일어날 수 있을 것 같습니다. (isLogin 제거 및 hasSession -> isLogin 변경 부분, auth.ts 제거 등)
현재 인증인가 구현 완료로 머지 예정이어서, 아래 인라인 리뷰 해당 부분 확인하고 수정하시면 될 것 같습니다!
인증 store 기반 Header를 유지하고, 알림은 NotificationTrigger로 통합했다. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/components/common/Header/Header.tsx`:
- Around line 68-76: Update useAuthStore.logout so it calls clearSession only
when logoutGeneration equals curSessionGeneration, preserving a newer session
when the generation changes. Keep Header’s handleLogout routing behavior
unchanged while ensuring the current session’s stored token, nickname, and
authentication state are cleared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4400fc05-aabb-43fe-be02-0bf8f55b2876
📒 Files selected for processing (1)
src/components/common/Header/Header.tsx
coord2address와 지번/도로명 주소 재조회로 키워드 결과의 빈 zone_no를 채운다. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/api/address/search/route.ts (1)
154-164: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win좌표 보강 전에 지번/도로명으로 이미 해결 가능한지 확인하지 않아 불필요한 API 호출 발생.
missingCoordinateKeys는lookups.byCoordinate.has(coordinateKey)만 검사합니다. 반면 아래collectAddressQueriesForMissingZip은resolveKeywordZipCode(document, lookups)로 이미 지번/도로명 매칭이 가능한지부터 확인합니다. 즉, 특정 키워드 문서가 지번/도로명으로 이미 우편번호를 알 수 있는 상태여도, 좌표 룩업에 없다는 이유만으로coord2address호출이 추가로 발생합니다. 카카오 API 호출 수·레이트리밋 부담을 줄이려면 이 단계에서도resolveKeywordZipCode로 선체크하는 것을 권장합니다.♻️ 제안 수정안
const missingCoordinateKeys = [ ...new Set( keywordDocuments + .filter((document) => !resolveKeywordZipCode(document, lookups)) .map((document) => toCoordinateKey(document.x, document.y)) .filter((coordinateKey) => !lookups.byCoordinate.has(coordinateKey)), ), ];🤖 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 `@src/app/api/address/search/route.ts` around lines 154 - 164, Update the missingCoordinateKeys construction in the keyword-document enrichment flow to exclude documents for which resolveKeywordZipCode(document, lookups) already returns a postal code, while retaining the existing byCoordinate lookup check for unresolved documents. Keep enrichZipCodesByCoordinates limited to coordinates that require enrichment.Source: Path instructions
src/lib/kakao/addressSearch.ts (1)
106-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
setZipLookup의 "이미 있으면 skip" 정책이route.ts에서 지켜지지 않음.
setZipLookup은 이미 값이 있는 키는 덮어쓰지 않는(skip-if-present) 정책으로 구현돼 있는데, 이 함수가export되지 않아route.ts의enrichZipCodesByAddressQueries가 동일 로직을 직접 재구현하면서 반대로 무조건 덮어쓰는(.set()) 방식을 사용하게 됩니다. 이는 데이터 정합성 버그로 이어집니다(자세한 내용은route.ts리뷰 및 통합 코멘트 참고).
setZipLookup을 export해서route.ts에서 재사용하도록 하면 중복 로직과 정책 불일치를 동시에 해결할 수 있습니다.🤖 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 `@src/lib/kakao/addressSearch.ts` around lines 106 - 131, Export setZipLookup from addressSearch.ts and update route.ts’s enrichZipCodesByAddressQueries to reuse it instead of directly calling Map.set. Preserve the existing skip-if-present behavior, including key trimming and ignoring empty keys or zip codes, so lookup values are not overwritten.
🤖 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.
Inline comments:
In `@src/app/api/address/search/route.ts`:
- Around line 82-110: Update enrichZipCodesByAddressQueries to reuse the
exported setZipLookup policy from addressSearch.ts for every coordinate, jibun,
road, and query mapping, preventing existing confirmed ZIP codes from being
overwritten. Ensure each query’s own mapping is recorded only from the first
relevant document rather than repeatedly inside the document loop, while
preserving the existing lookup population for document-derived addresses.
---
Nitpick comments:
In `@src/app/api/address/search/route.ts`:
- Around line 154-164: Update the missingCoordinateKeys construction in the
keyword-document enrichment flow to exclude documents for which
resolveKeywordZipCode(document, lookups) already returns a postal code, while
retaining the existing byCoordinate lookup check for unresolved documents. Keep
enrichZipCodesByCoordinates limited to coordinates that require enrichment.
In `@src/lib/kakao/addressSearch.ts`:
- Around line 106-131: Export setZipLookup from addressSearch.ts and update
route.ts’s enrichZipCodesByAddressQueries to reuse it instead of directly
calling Map.set. Preserve the existing skip-if-present behavior, including key
trimming and ignoring empty keys or zip codes, so lookup values are not
overwritten.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a90a6538-5895-4d30-850e-79d9e913af2a
📒 Files selected for processing (2)
src/app/api/address/search/route.tssrc/lib/kakao/addressSearch.ts
지정 견적 요청 기능을 병합하고 EstimateRequestForm은 useActiveEstimateRequest를 사용하도록 충돌을 해결했다. Co-authored-by: Cursor <cursoragent@cursor.com>
보낸 견적 요청 목록·상세 경로를 병합하고 appRoutes 충돌을 해결했다. Co-authored-by: Cursor <cursoragent@cursor.com>
setZipLookup을 재사용해 기존 룩업을 유지하고, query 매핑은 첫 유효 문서만 사용한다. Co-authored-by: Cursor <cursoragent@cursor.com>
| setAuthReady(true); | ||
| } | ||
| } | ||
| if (!isLoggedIn) { |
There was a problem hiding this comment.
hasAuthSession()이 메모리에 있는 access token만 확인하는 것 같은데, 새로고침 직후에는 토큰 복구(checkAuth)가 끝나기 전이라 값이 비어있을 것 같습니다. 그러면 로그인 상태여도 이 페이지를 새로고침했을 때 로그인 페이지로 이동될 수 있을 것 같은데, 로그인 후 /estimate-request에서 새로고침해서 한번 확인 해보시면 좋을 것 같습니다!
만약 재현된다면, Header처럼 useAuthStore의 상태(hasHydrated, isCheckingAuth, isAuthenticated)를 구독해서 세션 확인이 끝난 뒤에만 리다이렉트하는 방식을 권장드립니다.
|
정리하시느라 고생 많으셨습니다! 테스트용 계정과 자동 로그인 로직을 걷어내면서 폼의 상태가 훨씬 단순해졌고, 알림 로직을 NotificationTrigger로 분리해서 Header가 내비게이션에 집중하게 된 구조도 좋은 것 같습니다. appRoutes.ts에 중복 선언되어 있던 키를 하나로 합친 것도 잠재적인 버그를 줄인 좋은 정리인 것 같습니다. 👍 |
4 participants
📋 작업 내용
🔥 변경 사항
src/app/estimate-request/page.tsx: EstimateRequestForm import 경로를@/components/estimate/EstimateRequestForm에서@/components/estimate/request/EstimateRequestForm으로 변경src/components/common/Header/Header.tsx: isLogin prop 및 HeaderProps 인터페이스 삭제, hasSession을 isLogin으로 직접 사용하도록 변경, 알림 버튼·뱃지·패널·Esc·outside click 로직을 모두 제거하고 NotificationTrigger 컴포넌트로 대체, AlarmIcon·NotificationPanel·MOCK_NOTIFICATIONS 등 불필요해진 import 정리src/components/common/Header/NotificationTrigger.tsx: Header에서 분리된 알림 영역 전용 컴포넌트 신규 생성, 알림 버튼·읽지 않은 개수 뱃지·패널 토글·Esc 닫기·outside click 닫기 로직 포함src/components/common/Header/NotificationPanel.tsx: linkUrl 유무 분기에서 동일하게 반복되던 메시지+날짜 렌더링 코드를 NotificationContent 컴포넌트로 추출하여 중복 약 40줄 제거src/components/estimate/request/EstimateRequestForm.tsx: estimate/ → estimate/request/로 이동, TEST_CUSTOMER 하드코딩 계정·ensureLogin useEffect·handleSubmit 내 임시 로그인 fallback·isLoggingIn·authReady state·TOAST_LOGIN_FAILURE_MESSAGE 삭제, hasAuthSession 기반 로그인 확인 및 미로그인 시 getLoginRedirectPath로 리다이렉트 방식으로 전환, 인라인 MOVE_TYPES 상수를 constants/moveType.ts의 MOVE_TYPE_CARDS로 교체, 4개 분기에서 반복되던 Toast 렌더링을 toastElement 변수로 통합src/components/estimate/request/Calendar.tsx: estimate/ → estimate/request/로 이동, icons import 경로를./icons에서../icons로 변경src/components/estimate/request/DatePickerField.tsx: estimate/ → estimate/request/로 이동, icons import 경로를./icons에서../icons로 변경src/components/estimate/request/MoveTypeCard.tsx: estimate/ → estimate/request/로 이동, icons import 경로를./icons에서../icons로 변경src/components/estimate/request/AddressSelectModal.tsx: estimate/ → estimate/request/로 이동, icons import 경로를./icons에서../icons로 변경,style={{ maxHeight: 280, height: 280 }}inline style을 Tailwind 클래스h-[280px] max-h-[280px]로 전환src/components/estimate/request/ActiveEstimateBlocked.tsx: estimate/ → estimate/request/로 이동 (내용 변경 없음)src/lib/constants/moveType.ts: 견적요청 폼에서 사용하는 이사 유형 카드 데이터 MOVE_TYPE_CARDS 배열과 MoveTypeCardInfo 인터페이스 추가src/lib/api/auth.ts: 어디에서도 import하지 않는 미사용 파일이므로 삭제 (login, refreshSession, logout 함수 및 관련 타입 포함)✅ 체크리스트
📷 스크린샷 (선택)
🔗 관련 이슈
Closes #
💬 To Reviewer
Summary by CodeRabbit