Conversation
…to feat/#161/company-detail-bookmark-api
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthrough북마크 기능(추가/삭제/조회) 훅과 persisted Zustand 스토어를 추가하고, 북마크 페이지를 mock 데이터에서 API 기반으로 전환했습니다. 회사 상세에 북마크 UI/스타일/상태 관리를 통합했고, HTTP 클라이언트 DTO·엔드포인트(토큰 재발급, 온보딩, AI 매칭 등)를 광범위하게 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as CompanyDetail<br/>Component
participant Hook as useCompanyBookmark<br/>Hook
participant Store as BookmarkStore<br/>(Zustand)
participant MutationPost as usePostBookmark<br/>Mutation
participant MutationDel as useDeleteBookmark<br/>Mutation
participant API as API Client
participant Cache as QueryClient<br/>Cache
User->>UI: 북마크 버튼 클릭
UI->>Hook: handleBookmarkClick()
Hook->>Store: override 조회/설정
alt 현재 북마크 상태 = false
Hook->>MutationPost: postBookmark(companyId)
MutationPost->>API: POST /api/me/bookmarks
alt 성공
API-->>MutationPost: bookmarkId
MutationPost->>Cache: setQueryData(isLiked=true)
MutationPost->>Cache: invalidateQueries(meQueryKey.bookmarkCompanyLists())
MutationPost-->>Hook: 성공 콜백
else 400 중복
API-->>MutationPost: AxiosError 400
MutationPost->>Store: setBookmarkOverride(companyId,true)
MutationPost->>Cache: setQueryData(isLiked=true)
else 실패
API-->>MutationPost: Error
MutationPost->>Store: clearBookmarkOverride(companyId)
MutationPost-->>Hook: 에러 플래그 열기
end
else 현재 북마크 상태 = true
Hook->>MutationDel: deleteBookmark(companyId)
MutationDel->>API: DELETE /api/me/bookmarks/{companyId}
alt 성공
API-->>MutationDel: void
MutationDel->>Cache: setQueryData(isLiked=false)
MutationDel->>Cache: invalidateQueries(companyQueryKey.detail(companyId))
else 실패
API-->>MutationDel: Error
MutationDel->>Store: setBookmarkOverride(companyId,true)
MutationDel-->>Hook: 에러 플래그 열기
end
end
Hook-->>UI: 상태 업데이트 / 에러 표시
sequenceDiagram
actor User
participant Page as BookmarkPage<br/>Component
participant Query as useGetBookmarkCompaniesQuery
participant Mutation as useDeleteBookmark
participant API as API Client
participant Cache as QueryClient<br/>Cache
Page->>Query: useGetBookmarkCompaniesQuery(page, sort)
Query->>API: GET /api/me/bookmark-companies?page={page}&sort={sort}
API-->>Query: { content, totalPage, ... }
Query->>Cache: 캐시 저장
Query-->>Page: 데이터 바인딩
User->>Page: 항목 선택 후 삭제 요청
Page->>Mutation: Promise.allSettled([deleteBookmark(cId)...])
loop 각 삭제 요청
Mutation->>API: DELETE /api/me/bookmarks/{companyId}
end
alt 모든 삭제 성공
Mutation->>Cache: invalidateQueries(meQueryKey.bookmarkCompanyLists())
Mutation-->>Page: 성공 목록
Page->>Page: 성공한 행만 제거, 선택 초기화
else 일부 실패
Mutation-->>Page: 결과(성공/실패)
Page->>Page: 성공한 행만 제거
Page->>User: isDeleteErrorOpen = true (Alert)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared/api/generate/http-client.ts (1)
902-941:⚠️ Potential issue | 🔴 Critical
matchExperience호출 엔드포인트 변경으로 인한 실제 Breaking Change 발생 - 즉시 수정 필요엔드포인트 매핑이 변경되어 기존 기능이 손상될 수 있습니다:
matchExperienceVirtualThread→POST /api/v1/ai-reports(AI 리포트 생성 API)matchExperience→POST /api/v1/ai-reports/match/sync(동기 매칭)현재
src/features/experience-matching/api/use-create-report.mutation.ts의api.aiReports.matchExperience(data)호출이 리포트 생성이 아닌 다른 동기 매칭 엔드포인트로 요청을 보내고 있습니다. AI 리포트 생성 기능이 제대로 작동하려면matchExperienceVirtualThread를 사용해야 하므로 다음과 같이 수정해주세요:api.aiReports.matchExperienceVirtualThread(data)참고로 새로운
matchExperienceVirtualThread메서드는 현재 코드베이스에서 사용되지 않고 있으며, 더 엄격한 에러 타입(CustomErrorResponse)을 제공합니다. 변경 의도를 확인하신 후 필요한 모든 호출 지점을 함께 업데이트해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/api/generate/http-client.ts` around lines 902 - 941, The API client method mapping swapped endpoints so calls to api.aiReports.matchExperience are hitting the sync matching endpoint; update any callers (e.g., the call in src/features/experience-matching/api/use-create-report.mutation.ts) to use api.aiReports.matchExperienceVirtualThread(data) instead of api.aiReports.matchExperience(data), and handle the stricter error type returned (CustomErrorResponse) where applicable; search for other usages of matchExperience and replace them with matchExperienceVirtualThread or adjust intent accordingly.src/pages/company-detail/ui/company-detail-section.tsx (1)
30-42: 🛠️ Refactor suggestion | 🟠 Major객체 shape 선언은
type대신interface로 통일해 주세요.Line [30]-[42]는 union/literal이 아닌 객체 구조라
interface가 팀 컨벤션과 맞습니다.💡 제안 수정
-type CompanyDetailSummary = { +interface CompanyDetailSummary { companyId: number; name: string; isBookmarked: boolean; isRecruiting?: boolean; logo: string; industry?: IndustryCode; scale?: ScaleCode; companyURL: string; summary: string; talentProfile: string; issueList: IssueItem[]; -}; +}As per coding guidelines:
Prefer interface for defining object shapes; use type only for union types and literal types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/company-detail/ui/company-detail-section.tsx` around lines 30 - 42, The declaration CompanyDetailSummary is currently a type alias but should be an interface per team convention; replace the "type CompanyDetailSummary = { ... }" with "interface CompanyDetailSummary { ... }" preserving all properties (companyId, name, isBookmarked, isRecruiting, logo, industry, scale, companyURL, summary, talentProfile, issueList) and their optional markers so the object shape uses an interface instead of a type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/bookmark/api/use-get-bookmark-companies.query.ts`:
- Around line 37-40: The code casts response.result to
BookmarkCompaniesApiResponse and accesses result.content directly, which can
throw if response.result is undefined; update the code that uses response.result
(the const result variable and any subsequent accesses like result.content /
result.pageable / result.totalElements) to safely guard by providing a default
empty BookmarkCompaniesApiResponse-shaped object when response.result is falsy
or by early-returning a safe default response; specifically wrap or replace the
cast of response.result with a null-coalescing fallback (or a guard check) so
the mapping over content and accesses to pagination fields never access
properties on undefined.
- Around line 40-45: The mapping over result.content currently fills missing
id/companyId with 0 which can cause incorrect navigation/deletion; instead,
filter out invalid rows before mapping by excluding entries where id or
companyId are null/undefined (or not positive) and then map the remaining items
to the shape (preserve companyName/name, scrapedAt/createdAt, isConnected)
without substituting 0 for id/companyId; update the code around the
(result.content ?? []).map(...) to use .filter(...) .map(...) so only valid
bookmarkCompany objects are transformed.
In `@src/features/bookmark/model/use-company-bookmark.ts`:
- Around line 64-84: The onSuccess handler for usePostBookmark calls
updateDetailQuery(...) and then immediately calls
queryClient.invalidateQueries(...) which forces a redundant refetch; change the
onSuccess in usePostBookmark to either remove the unconditional
invalidateQueries call and rely on the local cache update via updateDetailQuery
(and/or refetchOnMount: 'always' if needed), or make invalidateQueries
conditional (e.g., only if updateDetailQuery did not update the cache). Update
the onSuccess logic around addBookmark/usePostBookmark to avoid the immediate
double-refresh while keeping updateDetailQuery and queryClient.invalidateQueries
(used only when necessary).
- Around line 18-19: isDuplicateBookmarkError currently only checks for HTTP 400
and can misclassify other validation errors; update it to first narrow to Axios
responses that match our ApiErrorResponse shape and then check
response.data.prefix against our known duplicate-bookmark identifier (use the
ERROR_MESSAGE mapping or the specific server code like BOOKMARK_409_001) and
optionally assert the expected status (e.g., 409) so only the specific
duplicate-bookmark error returns true; reference the isDuplicateBookmarkError
function, ApiErrorResponse type, and ERROR_MESSAGE/BOOKMARK_409_001 symbol when
making the change.
In `@src/features/bookmark/store/bookmark.store.ts`:
- Around line 10-35: The bookmark override state is being persisted to
sessionStorage which can mask server-refetched bookmark state; remove
persistence by eliminating the persist wrapper and
createJSONStorage/sessionStorage usage around the create(...) call so
useBookmarkStore returns a non-persisted store (keep the internal state shape
and methods: bookmarkOverrides, setBookmarkOverride, clearBookmarkOverride,
BookmarkState) to ensure overrides are cleared on full reload and do not survive
across tab navigation.
In `@src/pages/bookmark/bookmark-page.tsx`:
- Around line 56-58: The local rows state (setRows/bookmarkCompanies) is being
used for optimistic updates but the delete handler (useDeleteBookmark) does not
update React Query's cache, causing deleted items to reappear; update
useDeleteBookmark's onSuccess to call the QueryClient (e.g., useQueryClient) and
either invalidate the bookmark list query key (the query that returns
bookmarkCompanies) or update it via setQueryData to remove the deleted item so
the server cache and local rows stay in sync after a successful delete.
- Around line 300-308: The JSX uses a ternary to conditionally render the error
Alert; simplify by using short-circuit rendering: replace the expression using
isDeleteErrorOpen ? (<Alert ... />) : null with isDeleteErrorOpen && (<Alert ...
/>), keeping the same Alert props (variant, title, description, onClose that
calls setIsDeleteErrorOpen(false)) so behavior is identical while the markup is
more concise; locate the conditional around the Alert component that references
isDeleteErrorOpen and setIsDeleteErrorOpen.
In `@src/pages/company-detail/ui/company-detail-section.tsx`:
- Around line 186-191: 현재 isBookmarkErrorOpen 상태에서 렌더되는 Alert 컴포넌트의 description이
"북마크 저장에 실패했습니다"로 고정되어 있어 북마크 삭제 실패 시에도 잘못된 메시지가 보입니다; Alert의 description을 더
중립적인 문구(예: "북마크 처리에 실패했습니다" 또는 "요청을 처리하는 중 오류가 발생했습니다")로 변경하고, Alert을 렌더링하는 조건부
렌더링 블록(관련 변수: isBookmarkErrorOpen, 콜백: closeBookmarkError, 컴포넌트: Alert)의
description 텍스트를 업데이트해 추가/삭제 모두에 적합한 메시지가 표시되도록 수정하세요.
---
Outside diff comments:
In `@src/pages/company-detail/ui/company-detail-section.tsx`:
- Around line 30-42: The declaration CompanyDetailSummary is currently a type
alias but should be an interface per team convention; replace the "type
CompanyDetailSummary = { ... }" with "interface CompanyDetailSummary { ... }"
preserving all properties (companyId, name, isBookmarked, isRecruiting, logo,
industry, scale, companyURL, summary, talentProfile, issueList) and their
optional markers so the object shape uses an interface instead of a type.
In `@src/shared/api/generate/http-client.ts`:
- Around line 902-941: The API client method mapping swapped endpoints so calls
to api.aiReports.matchExperience are hitting the sync matching endpoint; update
any callers (e.g., the call in
src/features/experience-matching/api/use-create-report.mutation.ts) to use
api.aiReports.matchExperienceVirtualThread(data) instead of
api.aiReports.matchExperience(data), and handle the stricter error type returned
(CustomErrorResponse) where applicable; search for other usages of
matchExperience and replace them with matchExperienceVirtualThread or adjust
intent accordingly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 798b99f3-d3d0-49c2-86d5-d51f44abbb7f
⛔ Files ignored due to path filters (1)
src/shared/assets/icons/icon_bookmark.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (19)
src/features/bookmark/api/use-delete-bookmark.mutation.tssrc/features/bookmark/api/use-get-bookmark-companies.query.tssrc/features/bookmark/api/use-post-bookmark.mutation.tssrc/features/bookmark/config/bookmark-page.constants.tssrc/features/bookmark/index.tssrc/features/bookmark/model/use-company-bookmark.tssrc/features/bookmark/store/bookmark.store.tssrc/features/bookmark/types/bookmark.type.tssrc/features/bookmark/ui/bookmark-table.tsxsrc/features/company-detail/api/use-get-company-detail.query.tssrc/features/home/api/use-get-companies.query.tssrc/pages/bookmark/bookmark-page.tsxsrc/pages/company-detail/company-detail-page.tsxsrc/pages/company-detail/ui/company-detail-section.css.tssrc/pages/company-detail/ui/company-detail-section.tsxsrc/pages/onboarding/onboarding-page.tsxsrc/shared/api/config/query-key.tssrc/shared/api/generate/http-client.tssrc/shared/assets/icons/index.ts
💤 Files with no reviewable changes (1)
- src/features/bookmark/config/bookmark-page.constants.ts
src/pages/bookmark/bookmark-page.tsx
Outdated
| useEffect(() => { | ||
| setRows(bookmarkCompanies?.content ?? []); | ||
| }, [bookmarkCompanies?.content]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
서버 상태와 로컬 상태 동기화 패턴에 대한 고려가 필요해요.
현재 구현은 삭제 시 optimistic update를 위해 로컬 rows 상태를 사용하고 있어요. 하지만 이 패턴은 React Query의 캐시와 로컬 상태 간 불일치가 발생할 수 있어요.
현재 구조에서는 삭제 성공 후 쿼리를 invalidate하지 않아서, 다음 페이지 이동 후 돌아오면 삭제된 항목이 다시 나타날 수 있어요. useDeleteBookmark의 onSuccess에서 북마크 목록 쿼리를 invalidate하는 것을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/bookmark/bookmark-page.tsx` around lines 56 - 58, The local rows
state (setRows/bookmarkCompanies) is being used for optimistic updates but the
delete handler (useDeleteBookmark) does not update React Query's cache, causing
deleted items to reappear; update useDeleteBookmark's onSuccess to call the
QueryClient (e.g., useQueryClient) and either invalidate the bookmark list query
key (the query that returns bookmarkCompanies) or update it via setQueryData to
remove the deleted item so the server cache and local rows stay in sync after a
successful delete.
| const handleDeleteSelected = async () => { | ||
| const rowsToDelete = rows.filter((row) => selectedIds.has(row.id)); | ||
| const deleteResults = await Promise.allSettled( | ||
| rowsToDelete.map((row) => deleteBookmark(row.companyId)) | ||
| ); | ||
|
|
||
| const succeededRows = rowsToDelete.filter( | ||
| (_, index) => deleteResults[index]?.status === "fulfilled" | ||
| ); | ||
| const hasFailedDelete = deleteResults.some( | ||
| (result) => result.status === "rejected" | ||
| ); | ||
|
|
||
| if (succeededRows.length > 0) { | ||
| const succeededRowIds = new Set(succeededRows.map((row) => row.id)); | ||
|
|
||
| succeededRows.forEach((row) => { | ||
| setBookmarkOverride(row.companyId, false); | ||
| }); | ||
|
|
||
| setRows((prev) => prev.filter((row) => !succeededRowIds.has(row.id))); | ||
| } | ||
|
|
||
| setSelectedIds(new Set()); | ||
| modalStore.close(BOOKMARK_DELETE_MODAL_ID); | ||
|
|
||
| if (hasFailedDelete) { | ||
| setIsDeleteErrorOpen(true); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Bulk delete 구현이 잘 되어 있어요! 몇 가지 개선 포인트를 제안드려요.
Promise.allSettled를 사용한 부분 실패 처리와 성공 항목만 제거하는 로직이 좋아요. 다만 두 가지 고려 사항이 있어요:
-
빈 페이지 처리: 현재 페이지의 모든 항목을 삭제하면 빈 페이지가 될 수 있어요. 삭제 후 남은 항목이 없고 이전 페이지가 존재하면 이전 페이지로 이동하는 것이 UX에 좋아요.
-
Async handler와 모달:
handleDeleteSelected가 async 함수인데,ModalBasic의onConfirm이 Promise를 await하지 않으면 모달이 즉시 닫히면서 삭제 중 상태 표시가 어려울 수 있어요.isDeletingBookmark상태를 활용해 삭제 버튼에 로딩 상태를 표시하는 것을 고려해 주세요.
📝 빈 페이지 처리 예시
if (succeededRows.length > 0) {
const succeededRowIds = new Set(succeededRows.map((row) => row.id));
succeededRows.forEach((row) => {
setBookmarkOverride(row.companyId, false);
});
- setRows((prev) => prev.filter((row) => !succeededRowIds.has(row.id)));
+ setRows((prev) => {
+ const nextRows = prev.filter((row) => !succeededRowIds.has(row.id));
+ // 현재 페이지가 비어있고 이전 페이지가 있으면 이동
+ if (nextRows.length === 0 && currentPage > 1) {
+ updateSearchParams(keyword, currentPage - 1);
+ }
+ return nextRows;
+ });
}|
|
||
| {isDeleteErrorOpen ? ( | ||
| <Alert | ||
| variant="error" | ||
| title="오류" | ||
| description="북마크 삭제에 실패했습니다" | ||
| onClose={() => setIsDeleteErrorOpen(false)} | ||
| /> | ||
| ) : null} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
조건부 렌더링 간소화를 제안드려요.
삼항 연산자 대신 && 연산자를 사용하면 더 간결해져요.
♻️ 간결한 조건부 렌더링
- {isDeleteErrorOpen ? (
- <Alert
- variant="error"
- title="오류"
- description="북마크 삭제에 실패했습니다"
- onClose={() => setIsDeleteErrorOpen(false)}
- />
- ) : null}
+ {isDeleteErrorOpen && (
+ <Alert
+ variant="error"
+ title="오류"
+ description="북마크 삭제에 실패했습니다"
+ onClose={() => setIsDeleteErrorOpen(false)}
+ />
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isDeleteErrorOpen ? ( | |
| <Alert | |
| variant="error" | |
| title="오류" | |
| description="북마크 삭제에 실패했습니다" | |
| onClose={() => setIsDeleteErrorOpen(false)} | |
| /> | |
| ) : null} | |
| {isDeleteErrorOpen && ( | |
| <Alert | |
| variant="error" | |
| title="오류" | |
| description="북마크 삭제에 실패했습니다" | |
| onClose={() => setIsDeleteErrorOpen(false)} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/bookmark/bookmark-page.tsx` around lines 300 - 308, The JSX uses a
ternary to conditionally render the error Alert; simplify by using short-circuit
rendering: replace the expression using isDeleteErrorOpen ? (<Alert ... />) :
null with isDeleteErrorOpen && (<Alert ... />), keeping the same Alert props
(variant, title, description, onClose that calls setIsDeleteErrorOpen(false)) so
behavior is identical while the markup is more concise; locate the conditional
around the Alert component that references isDeleteErrorOpen and
setIsDeleteErrorOpen.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/company-detail/ui/company-detail-section.tsx (1)
30-42: 🛠️ Refactor suggestion | 🟠 Major객체 형태 타입은
type대신interface로 통일해 주세요.
CompanyDetailSummary는 객체 shape이므로 팀 규칙에 맞춰interface로 선언하는 편이 좋습니다.제안 변경
-type CompanyDetailSummary = { +interface CompanyDetailSummary { companyId: number; name: string; isBookmarked: boolean; isRecruiting?: boolean; logo: string; industry?: IndustryCode; scale?: ScaleCode; companyURL: string; summary: string; talentProfile: string; issueList: IssueItem[]; -}; +}As per coding guidelines, "Prefer
interfacefor defining object shapes; usetypeonly for union types and literal types".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/company-detail/ui/company-detail-section.tsx` around lines 30 - 42, Replace the object-shape alias CompanyDetailSummary (declared with `type`) with an equivalent `interface CompanyDetailSummary` declaration; keep the same property names and optional modifiers (companyId, name, isBookmarked, isRecruiting?, logo, industry?, scale?, companyURL, summary, talentProfile, issueList) and ensure any imports like IndustryCode, ScaleCode, IssueItem remain referenced the same so usages of CompanyDetailSummary continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/bookmark/api/use-get-bookmark-companies.query.ts`:
- Line 8: The literal default sort "LATEST" is duplicated; define a single
UPPER_SNAKE_CASE constant (e.g., BOOKMARK_COMPANY_DEFAULT_SORT = "LATEST") and
replace all occurrences of the string literal in this file (references related
to the BookmarkCompanySort type, default parameter values, and any
comparisons/assignments currently using "LATEST") with that constant to remove
duplication and follow the constant naming guideline.
In `@src/features/bookmark/model/use-company-bookmark.ts`:
- Line 53: The issue is that bookmarkOverride (used to compute isBookmarked via
const isBookmarked = bookmarkOverride ?? initialIsBookmarked) is not cleared
after mutation success or duplicate handling, causing the UI to prefer the stale
override over fresh server refetches; update the hook (useCompanyBookmark) to
reset/clear bookmarkOverride (e.g., setBookmarkOverride(undefined)) inside the
mutation success handlers and inside the duplicate-handling branch so that after
toggleBookmark/submitBookmark completes successfully (and on duplicate
resolution) the override is removed and isBookmarked will reflect
initialIsBookmarked from the latest server state; apply the same change to the
other affected places referenced in the comment (the blocks around the existing
mutation success/duplicate handling at the other noted sections).
---
Outside diff comments:
In `@src/pages/company-detail/ui/company-detail-section.tsx`:
- Around line 30-42: Replace the object-shape alias CompanyDetailSummary
(declared with `type`) with an equivalent `interface CompanyDetailSummary`
declaration; keep the same property names and optional modifiers (companyId,
name, isBookmarked, isRecruiting?, logo, industry?, scale?, companyURL, summary,
talentProfile, issueList) and ensure any imports like IndustryCode, ScaleCode,
IssueItem remain referenced the same so usages of CompanyDetailSummary continue
to work.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 072595b4-afca-4c3c-9cd4-ca985a87a77b
📒 Files selected for processing (4)
src/features/bookmark/api/use-get-bookmark-companies.query.tssrc/features/bookmark/model/use-company-bookmark.tssrc/features/bookmark/store/bookmark.store.tssrc/pages/company-detail/ui/company-detail-section.tsx
|
|
||
| import type { BookmarkRow } from "../types/bookmark.type"; | ||
|
|
||
| export type BookmarkCompanySort = "NAME" | "LIKE" | "LATEST" | "OLDEST"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
기본 정렬값 "LATEST"는 상수로 통합해 중복을 제거하는 것을 권장합니다.
동일 리터럴이 여러 곳에 있어 이후 변경 시 누락 가능성이 있습니다. 상수화하면 유지보수가 쉬워집니다.
♻️ 제안 수정
export type BookmarkCompanySort = "NAME" | "LIKE" | "LATEST" | "OLDEST";
+const DEFAULT_BOOKMARK_COMPANY_SORT: BookmarkCompanySort = "LATEST";
@@
export const getBookmarkCompanies = async (
page: number,
- sort: BookmarkCompanySort = "LATEST"
+ sort: BookmarkCompanySort = DEFAULT_BOOKMARK_COMPANY_SORT
): Promise<BookmarkCompaniesResponse> => {
@@
export const useGetBookmarkCompaniesQuery = (
page: number,
- sort: BookmarkCompanySort = "LATEST"
+ sort: BookmarkCompanySort = DEFAULT_BOOKMARK_COMPANY_SORT
) => {As per coding guidelines, Use UPPER_SNAKE_CASE for constants (e.g., VITE_API_KEY, ROTATE_DELAY).
Also applies to: 52-53, 76-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/bookmark/api/use-get-bookmark-companies.query.ts` at line 8, The
literal default sort "LATEST" is duplicated; define a single UPPER_SNAKE_CASE
constant (e.g., BOOKMARK_COMPANY_DEFAULT_SORT = "LATEST") and replace all
occurrences of the string literal in this file (references related to the
BookmarkCompanySort type, default parameter values, and any
comparisons/assignments currently using "LATEST") with that constant to remove
duplication and follow the constant naming guideline.
|
기간이 촉박한 작업이었는데 빠르게 구현해주셔서 고생 많으셨습니다 🥹 특히 상세페이지에서 낙관적으로 상태를 반영한 부분과, 목록 페이지에서 다중 삭제를 몇 가지 짚어보면 좋을 것 같은 부분이 있어서 코멘트 남겼습니다. 시간 되실 때 확인 부탁드립니다 :) |
| const updateCompanyBookmarkCache = ( | ||
| previousData: GetCompanyResponseDto | undefined, | ||
| isLiked: boolean |
There was a problem hiding this comment.
동작에는 문제 없지만, 북마크 기능을 다루는 흐름에서 isLiked 네이밍이 일부 남아 있어서 처음 읽을 때 의미가 조금 헷갈릴 수 있을 것 같았습니다!
예를 들어 page 단에서는
isBookmarked: companyDetail.isLiked ?? false처럼 이미 북마크 의미로 매핑해서 사용하고 있는데, 캐시 갱신 쪽에서는
const updateCompanyBookmarkCache = (previousData, isLiked) => ({
...previousData,
isLiked,
});처럼 DTO 네이밍이 그대로 남아 있어서 프론트 내부에서 어떤 개념으로 다루는지 바로 들어오지는 않았던 것 같아요.
서버 응답 필드명은 isLiked를 유지하더라도, 내부 변수/파라미터명은 isBookmarked로 통일해두면 이후 유지보수할 때도 더 읽기 쉬울 것 같은데, 어떻게 생각하시나요? 🙂
| queryClient.invalidateQueries({ | ||
| queryKey: meQueryKey.all(), | ||
| }); |
There was a problem hiding this comment.
여기서는 북마크 변경 이후 meQueryKey.all() 전체를 invalidate하고 있는데, 영향이 미치는 범위에 비해 invalidate 범위가 조금 넓은 것 같아요 🥹
현재 직접적으로 갱신이 필요한 건 북마크 목록 쪽이라서, 가능하다면 bookmarkCompanyList 관련 query만 invalidate하는 쪽이 더 좋을 것 같은데, 어떻게 생각하시나요? :)
이미 query key가 세분화되어 있어서, invalidate도 그 단위에 맞춰주면 불필요한 refetch를 줄이고 의도도 더 잘 드러날 것 같아요!
There was a problem hiding this comment.
유진님 의견에 동의하는 입장이에요!
현재 query-key.ts 파일에는 북마크를 위한 별도의 키를 설정해주셨기 때문에, 해당 계층 키를 추가하여 적용하면 좋을 것 같습니다! (delete api에도!!)
src/pages/bookmark/bookmark-page.tsx
Outdated
| const [searchParams, setSearchParams] = useSearchParams(); | ||
|
|
||
| const [rows, setRows] = useState(BOOKMARK_MOCK_ROWS); | ||
| const [rows, setRows] = useState<BookmarkRow[]>([]); |
There was a problem hiding this comment.
여기서는 query data(bookmarkCompanies?.content)를 rows라는 로컬 state로 한 번 더 복사해서 관리하고 있어서, 현재 구조상 서버 상태와 로컬 상태를 이중으로 들고 가는 형태인것 같아요 ㅠ !!
예를 들어 지금은
const [rows, setRows] = useState<BookmarkRow[]>([]);useEffect(() => {
setRows(bookmarkCompanies?.content ?? []);
}, [bookmarkCompanies?.content]);처럼 query 결과를 rows로 옮겨 담고 있고, 삭제 시에도 query cache를 기준으로 처리하기보다
setRows((prev) => prev.filter((row) => !succeededRowIds.has(row.id)));처럼 로컬 state를 직접 수정하고 있어서, 이후 action이 겹칠수록 동기화가 될 수 있지도 않을까..
가능하면 목록 원본은 query data를 유일한걸로 두고, 화면에 필요한 값만 파생해서 쓰는 방향도 고려해보면 좋을 것 같습니다 🙂
| const BOOKMARK_QUERY_KEY = "keyword"; | ||
| const BOOKMARK_PAGE_QUERY_KEY = "page"; | ||
| const BOOKMARK_DELETE_MODAL_ID = "bookmark-delete-modal"; | ||
| const BOOKMARK_PAGE_SIZE = 4; |
There was a problem hiding this comment.
여기 페이지 사이즈가 프론트 상수로 고정되어 있는데, 현재 북마크 목록은 서버 페이징 응답(currentPage, totalPage)을 같이 쓰고 있는 것 같아서 검색 모드와 일반 모드의 페이지 계산 방식이 서로 달라지는 것 같아요!
일반 목록은 서버 totalPage을 사용하고, 검색 결과는 프론트에서 BOOKMARK_PAGE_SIZE로 다시 계산하는
구조인 것 같은데, 나중에 호오옥시 서버 페이지 사이즈가 바뀌면 검색 결과 쪽 페이징만 따로 수정해야 하니까 서버 페이지 크기랑 맞춘다는 주석을 추가하거나 상수명 보완 간단하게 해놓으면 좋을 것 같아요 :)
odukong
left a comment
There was a problem hiding this comment.
북마크 페이지에서 page, sort, keyword 세 가지 속성을 통해 페이지네이션이 진행되고 있는데,
사실상 피그마를 참고하면, page, keyword 만 사용이 되고, sort는 사용되고 있지 않은 상황 같습니다! (api 스펙이 ui랑 일치하지 않아.. 생긴는 문제겠죠..ㅜ)
그래서 keyword를 통한 검색 필터링은 서버에서 받아온 값이 아닌 클라이언트 측에서 진행하게 되는데 이렇게 유지될 경우, 속성 값에 대한 결과가 꼬이는 문제가 발생할 수 있을 것 같아요. 이 부분은 추후 서버와 논의 후에 keyword 필터링(검색 키워드 추가, sort 속성 삭제)에 대해서 API 수정을 요청하면 좋을 것 같습니다!
그리고 유사한 이슈인데 delete-api에서 단일 id만 받아 삭제를 진행하다보니, 북마크 삭제를 담당하는 handleDeleteSelected 함수 자체가 비대해진 것 같습니다. 이 부분도 삭제 id리스트를 받는 방향으로 수정 가능한지 함께 논의해주시면 좋을 것 같아요!
이 두가지 이슈가 해결되면 bookmark-page 페이지 자체가 간소화될 것 같다는 생각입니다!
우선 간단한 부분들에 대해서만 리뷰남겨놓았습니다! 확인부탁드려요! ^_____^
| queryClient.invalidateQueries({ | ||
| queryKey: meQueryKey.all(), | ||
| }); |
There was a problem hiding this comment.
유진님 의견에 동의하는 입장이에요!
현재 query-key.ts 파일에는 북마크를 위한 별도의 키를 설정해주셨기 때문에, 해당 계층 키를 추가하여 적용하면 좋을 것 같습니다! (delete api에도!!)
src/pages/bookmark/bookmark-page.tsx
Outdated
| @@ -49,22 +68,30 @@ const BookmarkPage = () => { | |||
| }, []); | |||
There was a problem hiding this comment.
modalStore에 대한 구독은 이미 전역적으로 구독 중인 상태이기 때문에, bookmark-page 내부에서 하나의 리스너를 더 만들어 줄 필요는 없습니다! 해당 코드와 modal관련 state는 삭제부탁드립니다!
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/features/bookmark/model/use-company-bookmark.ts (1)
18-21: 🧹 Nitpick | 🔵 Trivial중복 북마크 판별 기준은 전용 코드로 더 좁히는 편이 안전합니다.
현재 구현은
400 && prefix.startsWith("BOOKMARK_")만 보고 있어서, 북마크 카테고리의 다른 4xx 응답도 duplicate로 흡수할 여지가 있습니다. Swagger/백엔드 계약에 duplicate 전용prefix나 status가 정의돼 있다면 그 값으로 정확히 매칭해 두는 편이 더 견고합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/bookmark/model/use-company-bookmark.ts` around lines 18 - 21, The current isDuplicateBookmarkError helper is too broad (it treats any 400 with prefix starting with "BOOKMARK_" as duplicate); update it to match the exact duplicate indicator from the API contract instead: inside isDuplicateBookmarkError (and still guard with isValidCustomError), check for the exact status and exact prefix string defined by backend (e.g., error.response.status === <duplicateStatus> and error.response.data.prefix === '<EXACT_DUPLICATE_PREFIX>') rather than startsWith; adjust any callers/tests accordingly and run the verification tests to confirm duplicate detection is now precise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/bookmark/model/use-company-bookmark.ts`:
- Line 2: The local state isBookmarkErrorOpen is not tied to companyId so an
error alert can persist when switching companies; update use-company-bookmark.ts
to reset error state whenever companyId changes by adding an effect that listens
to companyId and calls setIsBookmarkErrorOpen(false) and setBookmarkError(null)
(or clears the error) — alternatively include companyId in the state key (e.g.,
store errors in a map keyed by companyId) so functions like toggleBookmark,
setIsBookmarkErrorOpen, and setBookmarkError are cleared when companyId updates.
- Around line 72-75: The duplicate-error branch is setting then immediately
clearing the override so the override never takes effect; in the
isDuplicateBookmarkError branch either drop the override calls and just call
updateDetailQuery(true), or if you need the override to persist for subsequent
logic keep setBookmarkOverride(companyId, true) but move
clearBookmarkOverride(companyId) out of this immediate branch to a later point
(or a finally/timeout) where the override is actually required to be removed;
update the code around isDuplicateBookmarkError, setBookmarkOverride,
clearBookmarkOverride, updateDetailQuery and companyId accordingly.
---
Duplicate comments:
In `@src/features/bookmark/model/use-company-bookmark.ts`:
- Around line 18-21: The current isDuplicateBookmarkError helper is too broad
(it treats any 400 with prefix starting with "BOOKMARK_" as duplicate); update
it to match the exact duplicate indicator from the API contract instead: inside
isDuplicateBookmarkError (and still guard with isValidCustomError), check for
the exact status and exact prefix string defined by backend (e.g.,
error.response.status === <duplicateStatus> and error.response.data.prefix ===
'<EXACT_DUPLICATE_PREFIX>') rather than startsWith; adjust any callers/tests
accordingly and run the verification tests to confirm duplicate detection is now
precise.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e7a4c6db-7d67-4b94-8e42-3abf71fef3e3
📒 Files selected for processing (1)
src/features/bookmark/model/use-company-bookmark.ts
| @@ -0,0 +1,134 @@ | |||
| import { useQueryClient } from "@tanstack/react-query"; | |||
| import { useCallback, useState } from "react"; | |||
There was a problem hiding this comment.
다른 기업으로 이동해도 이전 에러 Alert가 남을 수 있어요.
isBookmarkErrorOpen이 companyId와 분리된 로컬 state라서, 같은 컴포넌트 인스턴스에서 기업 상세만 바뀌면 이전 실패 상태가 새 기업 화면에 그대로 보일 수 있습니다. companyId 변경 시 초기화해 두는 편이 안전합니다.
🔧 예시
-import { useCallback, useState } from "react";
+import { useCallback, useEffect, useState } from "react";
@@
const [isBookmarkErrorOpen, setIsBookmarkErrorOpen] = useState(false);
+
+ useEffect(() => {
+ setIsBookmarkErrorOpen(false);
+ }, [companyId]);Also applies to: 42-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/bookmark/model/use-company-bookmark.ts` at line 2, The local
state isBookmarkErrorOpen is not tied to companyId so an error alert can persist
when switching companies; update use-company-bookmark.ts to reset error state
whenever companyId changes by adding an effect that listens to companyId and
calls setIsBookmarkErrorOpen(false) and setBookmarkError(null) (or clears the
error) — alternatively include companyId in the state key (e.g., store errors in
a map keyed by companyId) so functions like toggleBookmark,
setIsBookmarkErrorOpen, and setBookmarkError are cleared when companyId updates.
✏️ Summary
📑 Tasks
1. swagger 업데이트 반영
수빈님이 공유해주신 방식대로 swagger를 업데이트했습니다 !
pnpm gen:swagger-ts-api실행 후 생성된http-client.ts에서 기존에 수동으로 보정하던 타입도 같이 맞춰주었습니다.2. 기업 상세페이지 디자인 수정
피그마에 수정된 UI대로 기업 상세페이지의 섹션 구조와 스타일을 정리했습니다.
3. 북마크 조회 API 연동
북마크 목록을 실제 서버 데이터로 가져올 수 있도록 조회 훅을 추가했습니다.
useGetBookmarkCompaniesQuery추가4. 북마크 추가 / 삭제 API 연동
기업 상세페이지에서 북마크 추가 / 해제가 실제로 동작하도록 mutation 훅을 연결했습니다.
5. 상세페이지 북마크 상태 관리 및 목록 삭제 처리
상세페이지에서는
useCompanyBookmark훅으로 북마크 상태를 분리해 관리했고,북마크 페이지에서는 선택 삭제 시 여러 요청을 한 번에 처리할 수 있도록 정리했습니다.
👀 To Reviewer
📸 Screenshot
Summary by CodeRabbit
릴리스 노트
New Features
Refactor
Style
Bug Fixes
Chores