Conversation
Closed
Walkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/pre-opinion/components/PreOpinionDetail.tsx (1)
33-33:⚠️ Potential issue | 🟡 Minor
memberInfonull 가드가 타입 정의와 불일치합니다.
PreOpinionMember.memberInfo는PreOpinionMemberInfo타입으로 필수 필드입니다. Line 33의memberInfo?.userId옵셔널 체이닝과 Line 52의{memberInfo && ...}null 가드는 타입과 맞지 않아 유지보수 시 혼란을 줄 수 있습니다. 실제로 null이 될 수 있다면 타입에| null을 반영하고, 아니라면 방어 코드를 제거해야 합니다.🔧 타입에 null 가능성이 없는 경우 제안 수정
- const isMyOpinion = currentUser?.userId === memberInfo?.userId + const isMyOpinion = currentUser?.userId === memberInfo.userId- {memberInfo && ( + {( <div className="flex justify-between items-center"> ... </div> - )} + )}Also applies to: 52-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionDetail.tsx` at line 33, The code inconsistently treats PreOpinionMember.memberInfo as possibly null (using memberInfo? and {memberInfo && ...}) while the type declares it non-null; update the code to match the type by removing defensive null checks (e.g., change isMyOpinion to use currentUser.userId === memberInfo.userId and remove {memberInfo && ...} guards in PreOpinionDetail) OR, if memberInfo can actually be null, update the type to PreOpinionMemberInfo | null and adjust usages accordingly so memberInfo is consistently nullable (including type defs and any consumers like isMyOpinion and the JSX render guard).
🧹 Nitpick comments (10)
src/features/pre-opinion/preOpinion.types.ts (2)
68-77:GetPreOpinionAnswersParams와DeleteMyPreOpinionAnswerParams의 형태가 동일합니다.두 타입 모두
gatheringId: number,meetingId: number만 포함해 완전히 중복됩니다. 공통 베이스 타입으로 분리하거나 하나를 재사용하면 중복을 줄일 수 있습니다.♻️ 공통 타입 추출 제안
+/** 모임/약속 ID 공통 파라미터 */ +type MeetingParams = { + gatheringId: number + meetingId: number +} -/** 사전 의견 목록 조회 파라미터 */ -export type GetPreOpinionAnswersParams = { - gatheringId: number - meetingId: number -} - -/** 내 사전 의견 삭제 파라미터 */ -export type DeleteMyPreOpinionAnswerParams = { - gatheringId: number - meetingId: number -} +/** 사전 의견 목록 조회 파라미터 */ +export type GetPreOpinionAnswersParams = MeetingParams +/** 내 사전 의견 삭제 파라미터 */ +export type DeleteMyPreOpinionAnswerParams = MeetingParams🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.types.ts` around lines 68 - 77, GetPreOpinionAnswersParams and DeleteMyPreOpinionAnswerParams are identical (both have gatheringId:number and meetingId:number); refactor by extracting a shared type (e.g., PreOpinionBase or PreOpinionIds) or have one alias reuse the other (e.g., type DeleteMyPreOpinionAnswerParams = GetPreOpinionAnswersParams) and update any references to use the new/common type; ensure exported names remain consistent and update imports/usages of GetPreOpinionAnswersParams or DeleteMyPreOpinionAnswerParams accordingly.
44-51:PreOpinionAnswerTopic과PreOpinionTopic의 필드 네이밍 불일치유사한 역할의 두 타입이
titlevstopicTitle,descriptionvstopicDescription으로 다르게 명명되어 있습니다. API 응답이 실제로 다르다면 어쩔 수 없지만, 그렇지 않다면 일관성을 맞추는 것이 유지보수에 유리합니다. API 계약과 일치하는지 확인을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.types.ts` around lines 44 - 51, The PreOpinionAnswerTopic type uses title/topicDescription naming that doesn't match PreOpinionTopic (which uses topicTitle/topicDescription); verify the API contract and then make the types consistent: either rename PreOpinionAnswerTopic fields to topicTitle/topicDescription to match PreOpinionTopic, or rename PreOpinionTopic to use title/description; if the API actually returns different keys, keep both types but add a clear mapping function (e.g., mapApiTopicToPreOpinionTopic) and document the difference. Ensure you update any usages of PreOpinionAnswerTopic, tests, and serializers/deserializers to reflect the chosen naming.src/features/pre-opinion/preOpinion.endpoints.ts (1)
5-16:ANSWERS와CREATE의 생성 경로가 동일합니다.두 함수 모두
.../meetings/${meetingId}/answers를 반환해 경로 문자열이 완전히 중복됩니다. REST GET/POST 설계 자체는 맞지만, 나중에 URL이 바뀌면 두 곳을 동시에 수정해야 해 실수 가능성이 생깁니다.♻️ 공통 경로 분리 제안
+ // 사전 의견 목록/생성 공통 경로 + ANSWERS_BASE: (gatheringId: number, meetingId: number) => + `${API_PATHS.GATHERINGS}/${gatheringId}/meetings/${meetingId}/answers`, // 사전 의견 목록 조회 (GET /api/gatherings/{gatheringId}/meetings/{meetingId}/answers) - ANSWERS: (gatheringId: number, meetingId: number) => - `${API_PATHS.GATHERINGS}/${gatheringId}/meetings/${meetingId}/answers`, + ANSWERS: (gatheringId: number, meetingId: number) => + PRE_OPINION_ENDPOINTS.ANSWERS_BASE(gatheringId, meetingId), // 사전 의견 최초 저장 (POST /api/gatherings/{gatheringId}/meetings/{meetingId}/answers) - CREATE: (gatheringId: number, meetingId: number) => - `${API_PATHS.GATHERINGS}/${gatheringId}/meetings/${meetingId}/answers`, + CREATE: (gatheringId: number, meetingId: number) => + PRE_OPINION_ENDPOINTS.ANSWERS_BASE(gatheringId, meetingId),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.endpoints.ts` around lines 5 - 16, ANSWERS and CREATE return identical paths; extract the common path into a single reusable symbol (e.g., ANSWERS_BASE or answersPath function) and have both ANSWERS and CREATE reference that symbol instead of duplicating the template literal; update the definitions for ANSWERS and CREATE to return or call that shared symbol so future URL changes are made in one place (refer to the existing ANSWERS and CREATE identifiers to locate where to replace the duplicated template).src/features/pre-opinion/components/PreOpinionDetail.tsx (2)
61-61: 불필요한 화살표 함수 래퍼입니다.
handleDelete는 이미async () => {}형태이므로 래퍼 없이 직접 전달할 수 있습니다.- {isMyOpinion && <TextButton onClick={() => handleDelete()}>내 의견 삭제하기</TextButton>} + {isMyOpinion && <TextButton onClick={handleDelete}>내 의견 삭제하기</TextButton>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionDetail.tsx` at line 61, The TextButton currently uses an unnecessary arrow wrapper around handleDelete; change the JSX in PreOpinionDetail (where isMyOpinion and TextButton are used) to pass handleDelete directly as the onClick prop (i.e., onClick={handleDelete}) instead of onClick={() => handleDelete()}, leaving the existing async function handleDelete unchanged.
77-107:keywordInfo.filter()중복 호출 — 변수로 추출하는 것을 권장합니다.
filter를 조건 체크용(length > 0)과 렌더링용(.map())으로 두 번씩 호출하고 있습니다. 동일 배열을 두 번 순회하므로 변수로 분리하면 가독성과 성능이 모두 개선됩니다.♻️ 중복 filter 제거
+ const bookKeywords = bookReview.keywordInfo.filter((k) => k.type === 'BOOK') + const impressionKeywords = bookReview.keywordInfo.filter((k) => k.type === 'IMPRESSION') - {bookReview.keywordInfo.filter((k) => k.type === 'BOOK').length > 0 && ( + {bookKeywords.length > 0 && ( <div> ... - {bookReview.keywordInfo - .filter((k) => k.type === 'BOOK') - .map((keyword) => ( + {bookKeywords.map((keyword) => ( <Chip key={keyword.id} variant="success">{keyword.name}</Chip> ))} </div> )} - {bookReview.keywordInfo.filter((k) => k.type === 'IMPRESSION').length > 0 && ( + {impressionKeywords.length > 0 && ( <div> ... - {bookReview.keywordInfo - .filter((k) => k.type === 'IMPRESSION') - .map((keyword) => ( + {impressionKeywords.map((keyword) => ( <Chip key={keyword.id} ...>{keyword.name}</Chip> ))} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionDetail.tsx` around lines 77 - 107, The code repeatedly calls bookReview.keywordInfo.filter(...) for the same predicates, causing duplicate array traversals; inside PreOpinionDetail extract and reuse filtered arrays (e.g., const bookKeywords = bookReview.keywordInfo.filter(k => k.type === 'BOOK') and const impressionKeywords = bookReview.keywordInfo.filter(k => k.type === 'IMPRESSION')) then use bookKeywords.length > 0 and bookKeywords.map(...) (and likewise for impressionKeywords) for rendering so each list is computed once and reused for both the existence check and the Chip mapping.src/features/pre-opinion/hooks/usePreOpinionAnswers.ts (1)
30-35:staleTime누락 —gcTime만 설정 시 매 마운트마다 백그라운드 리패치가 발생합니다.
staleTime기본값은 0이므로 캐시된 데이터는 즉시 stale 상태가 되어 마운트·윈도우 포커스·재연결 시마다 백그라운드 리패치가 트리거됩니다.gcTime을 10분으로 늘린 의도(캐시 유지)와 상충됩니다.사전 의견 목록은 변동이 적으므로
staleTime을 함께 지정해 불필요한 API 호출을 줄이는 것을 권장합니다.♻️ staleTime 추가
return useQuery<PreOpinionAnswersData, ApiError>({ queryKey: preOpinionQueryKeys.answerList({ gatheringId, meetingId }), queryFn: () => getPreOpinionAnswers({ gatheringId, meetingId }), enabled: isValidParams, + staleTime: 5 * 60 * 1000, // 5분간 fresh 유지 gcTime: 10 * 60 * 1000, })As per coding guidelines, "invalidate/refetch 타이밍을 중점적으로 봐줘"에 해당하는 사항입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/hooks/usePreOpinionAnswers.ts` around lines 30 - 35, usePreOpinionAnswers sets gcTime but not staleTime, so cached data is immediately considered stale and triggers background refetches on mount/focus; update the useQuery call (in usePreOpinionAnswers) to provide a suitable staleTime (e.g., same 10 * 60 * 1000 or another value appropriate for "rarely changing" pre-opinion data) alongside gcTime for the queryKey from preOpinionQueryKeys.answerList and the queryFn getPreOpinionAnswers to prevent unnecessary refetching.src/features/pre-opinion/index.ts (1)
10-10: Mock 파일을 public barrel에서 제외하는 것을 권장합니다.
preOpinion.mock.ts가 배럴에 포함되면 소비자가 mock 함수를 직접 임포트할 수 있는 표면이 노출됩니다.USE_MOCK플래그로 호출 자체는 제어되지만, 테스트·개발 전용 함수가 production API와 같은 배럴에 노출되는 것은 의도를 흐립니다.♻️ mock 제거 제안
// API export * from './preOpinion.api' export * from './preOpinion.endpoints' -export * from './preOpinion.mock'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/index.ts` at line 10, The barrel export currently re-exports test utilities: remove the public export of preOpinion.mock by deleting or commenting out the "export * from './preOpinion.mock'" line in the module barrel so mock helpers are not exposed; instead keep mocks only for tests by importing them directly from their file in test fixtures or move the mock into a test-only directory (or add an index.test-only.ts barrel) and ensure no other symbols in src/features/pre-opinion/index.ts (the barrel) re-export testing functions.src/features/retrospectives/hooks/usePersonalRetrospective.ts (1)
38-43:staleTime미설정 — window focus마다 불필요한 refetch가 발생합니다.
gcTime만 설정되어 있고staleTime이 기본값0이면, 마운트/포커스 시마다 서버에 재요청합니다. 개인 회고는 자주 바뀌지 않는 데이터이므로staleTime을 명시적으로 지정하는 것이 좋습니다.♻️ staleTime 추가 제안
return useQuery<GetPersonalRetrospectiveResponse, ApiError>({ queryKey: personalRetrospectiveQueryKeys.detail(params), queryFn: () => getPersonalRetrospective(params), enabled: isValidParams, + staleTime: 5 * 60 * 1000, gcTime: 10 * 60 * 1000, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/retrospectives/hooks/usePersonalRetrospective.ts` around lines 38 - 43, The useQuery call in usePersonalRetrospective (queryKey personalRetrospectiveQueryKeys.detail(params), queryFn getPersonalRetrospective) doesn't set staleTime so it refetches on window focus; add a suitable staleTime (e.g., a few minutes like 5*60*1000 or another product-appropriate duration) to the useQuery options to prevent unnecessary refetches for this infrequently changing personal retrospective data.src/pages/Retrospectives/PersonalRetrospectivePage.tsx (1)
14-19:Number(undefined) = NaN→ hook 호출 후 조기 반환 순서가 맞는지 확인합니다.훅이 Line 17에서
NaN파라미터로 호출되지만enabled: isValidParams가 false가 되므로 실제 fetch는 발생하지 않습니다. React 훅 규칙(hooks before early returns)도 준수합니다. 다만return null이 아무 피드백 없이 화면을 비우므로, 라우트 설정이 잘못됐을 때 디버깅이 어려울 수 있습니다.♻️ fallback 개선 제안
-if (!gatheringIdParam || !meetingIdParam) return null +if (!gatheringIdParam || !meetingIdParam) { + return <p className="text-grey-400 typo-body3">잘못된 접근입니다.</p> +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Retrospectives/PersonalRetrospectivePage.tsx` around lines 14 - 19, The code converts gatheringIdParam/meetingIdParam to Numbers before checking their presence, so usePersonalRetrospective is invoked with NaN even though fetch is disabled; fix by validating presence first (check gatheringIdParam and meetingIdParam or compute isValidParams) then parse to numbers and call usePersonalRetrospective with those parsed values (or compute enabled from isValidParams), and replace the silent "return null" with a minimal fallback UI or a clear error/log message so routing/missing-param issues are visible; key symbols: gatheringIdParam, meetingIdParam, gatheringId, meetingId, usePersonalRetrospective.src/features/retrospectives/components/PersonalRetrospectiveContent.tsx (1)
67-77: 빈 상태 조건 렌더링 패턴 개선 제안
map()을 먼저 실행한 뒤length === 0을 별도로 체크하는 구조는 동작에는 문제없지만, 삼항 연산자로 합치면 의도가 더 명확하게 드러납니다.♻️ 제안
- {preOpinions.map((opinion) => ( - <Card key={opinion.topicId}> - <p className="text-grey-500 typo-body4">{opinion.topicName}</p> - <p className="mt-xsmall text-black typo-body2">{opinion.content}</p> - </Card> - ))} - {preOpinions.length === 0 && ( - <p className="text-grey-400 typo-body3">작성한 사전 의견이 없습니다.</p> - )} + {preOpinions.length === 0 ? ( + <p className="text-grey-400 typo-body3">작성한 사전 의견이 없습니다.</p> + ) : ( + preOpinions.map((opinion) => ( + <Card key={opinion.topicId}> + <p className="text-grey-500 typo-body4">{opinion.topicName}</p> + <p className="mt-xsmall text-black typo-body2">{opinion.content}</p> + </Card> + )) + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/retrospectives/components/PersonalRetrospectiveContent.tsx` around lines 67 - 77, The current render runs preOpinions.map(...) and then separately checks preOpinions.length === 0 for the empty state; change this to a single conditional expression so the intent is clearer: inside the JSX return of the PersonalRetrospectiveContent component, replace the separate map + length check with a ternary (or &&/|| expression) that renders preOpinions.map(opinion => <Card key={opinion.topicId}>...) when preOpinions has items, otherwise renders the empty-state paragraph ("작성한 사전 의견이 없습니다."); keep the same elements (Card, opinion.topicId, opinion.topicName, opinion.content) and classes so behavior and keys are preserved.
🤖 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/pre-opinion/preOpinion.api.ts`:
- Around line 117-122: The deleteMyPreOpinionAnswer function lacks a USE_MOCK
branch and will call the real API in mock mode; update deleteMyPreOpinionAnswer
to mirror getPreOpinionAnswers/getPreOpinion by checking the USE_MOCK flag (or
mock helper) and short-circuiting to the mock behavior (e.g., return a resolved
Promise or call the mock service) instead of invoking
api.delete(PRE_OPINION_ENDPOINTS.DELETE_MY_ANSWER(...)), or if the real call is
intentional, add a clarifying comment inside deleteMyPreOpinionAnswer explaining
why the mock branch is omitted.
In `@src/features/retrospectives/components/PersonalRetrospectiveContent.tsx`:
- Around line 12-15: The JSDoc `@description` in the PersonalRetrospectiveContent
component lists "내 사전 의견, 토론 주제" but the JSX renders "토론 주제" before "내 사전 의견";
either update the `@description` to match the actual render order or reorder the
JSX so the "내 사전 의견" section appears before the "토론 주제" section. Locate the
sections by the visible headings/strings "토론 주제" and "내 사전 의견" within the
PersonalRetrospectiveContent component and make the description and
implementation consistent.
In `@src/features/retrospectives/personalRetrospective.types.ts`:
- Around line 65-69: The properties preOpinions, topics, and meetingMembers on
PersonalRetrospective types are declared as non-nullable arrays but the API may
return null, causing runtime errors (e.g., preOpinions.length === 0). Fix by
either changing the types to nullable arrays (PersonalRetrospectivePreOpinion[]
| null, PersonalRetrospectiveTopic[] | null, PersonalRetrospectiveMember[] |
null) and adding null-guards where those arrays are used, or normalize responses
in the API client to coerce null -> [] before constructing the
PersonalRetrospective model (ensure callers of preOpinions, topics,
meetingMembers can safely use .map()/.length without additional checks).
- Around line 38-39: The PersonalRetrospectiveMember.profileImage field is
currently typed as string but other user/profile types (e.g., in user.types.ts,
auth.types.ts, gatherings.types.ts) allow null when no image is set; update the
type of PersonalRetrospectiveMember.profileImage to string | null to match those
types and runtime responses, ensuring any usages handle null; locate the
PersonalRetrospectiveMember interface/type in personalRetrospective.types.ts and
change profileImage's type accordingly.
In `@src/pages/Retrospectives/PersonalRetrospectivePage.tsx`:
- Line 3: The import fails because PersonalRetrospectiveContent and
usePersonalRetrospective are not re-exported from the retrospectives feature;
add an export for the hook (usePersonalRetrospective) in the feature root index
(re-export the hook from wherever it is implemented) and add an export for the
component (PersonalRetrospectiveContent) in the components index (re-export the
component from its module) so that both symbols are available for import as
shown in PersonalRetrospectivePage.tsx.
In `@src/shared/components/SubPageHeader.tsx`:
- Line 62: The div in SubPageHeader always applies the "flex items-end" classes
which forces bottom alignment for the TextButton regardless of context; change
the class application in the SubPageHeader component so that "flex items-end" is
applied conditionally (e.g., only when the disableShadow prop is true) or
introduce a new prop (e.g., alignBottom) and use that to toggle the classes;
update the JSX where the outer div (the one with className "mx-auto
max-w-layout-max px-layout-padding py-small h-[59px]") constructs its className
to include "flex items-end" only when the chosen condition (disableShadow or the
new alignBottom prop) is true so existing usages that rely on current top/center
alignment aren’t affected.
---
Outside diff comments:
In `@src/features/pre-opinion/components/PreOpinionDetail.tsx`:
- Line 33: The code inconsistently treats PreOpinionMember.memberInfo as
possibly null (using memberInfo? and {memberInfo && ...}) while the type
declares it non-null; update the code to match the type by removing defensive
null checks (e.g., change isMyOpinion to use currentUser.userId ===
memberInfo.userId and remove {memberInfo && ...} guards in PreOpinionDetail) OR,
if memberInfo can actually be null, update the type to PreOpinionMemberInfo |
null and adjust usages accordingly so memberInfo is consistently nullable
(including type defs and any consumers like isMyOpinion and the JSX render
guard).
---
Nitpick comments:
In `@src/features/pre-opinion/components/PreOpinionDetail.tsx`:
- Line 61: The TextButton currently uses an unnecessary arrow wrapper around
handleDelete; change the JSX in PreOpinionDetail (where isMyOpinion and
TextButton are used) to pass handleDelete directly as the onClick prop (i.e.,
onClick={handleDelete}) instead of onClick={() => handleDelete()}, leaving the
existing async function handleDelete unchanged.
- Around line 77-107: The code repeatedly calls
bookReview.keywordInfo.filter(...) for the same predicates, causing duplicate
array traversals; inside PreOpinionDetail extract and reuse filtered arrays
(e.g., const bookKeywords = bookReview.keywordInfo.filter(k => k.type ===
'BOOK') and const impressionKeywords = bookReview.keywordInfo.filter(k => k.type
=== 'IMPRESSION')) then use bookKeywords.length > 0 and bookKeywords.map(...)
(and likewise for impressionKeywords) for rendering so each list is computed
once and reused for both the existence check and the Chip mapping.
In `@src/features/pre-opinion/hooks/usePreOpinionAnswers.ts`:
- Around line 30-35: usePreOpinionAnswers sets gcTime but not staleTime, so
cached data is immediately considered stale and triggers background refetches on
mount/focus; update the useQuery call (in usePreOpinionAnswers) to provide a
suitable staleTime (e.g., same 10 * 60 * 1000 or another value appropriate for
"rarely changing" pre-opinion data) alongside gcTime for the queryKey from
preOpinionQueryKeys.answerList and the queryFn getPreOpinionAnswers to prevent
unnecessary refetching.
In `@src/features/pre-opinion/index.ts`:
- Line 10: The barrel export currently re-exports test utilities: remove the
public export of preOpinion.mock by deleting or commenting out the "export *
from './preOpinion.mock'" line in the module barrel so mock helpers are not
exposed; instead keep mocks only for tests by importing them directly from their
file in test fixtures or move the mock into a test-only directory (or add an
index.test-only.ts barrel) and ensure no other symbols in
src/features/pre-opinion/index.ts (the barrel) re-export testing functions.
In `@src/features/pre-opinion/preOpinion.endpoints.ts`:
- Around line 5-16: ANSWERS and CREATE return identical paths; extract the
common path into a single reusable symbol (e.g., ANSWERS_BASE or answersPath
function) and have both ANSWERS and CREATE reference that symbol instead of
duplicating the template literal; update the definitions for ANSWERS and CREATE
to return or call that shared symbol so future URL changes are made in one place
(refer to the existing ANSWERS and CREATE identifiers to locate where to replace
the duplicated template).
In `@src/features/pre-opinion/preOpinion.types.ts`:
- Around line 68-77: GetPreOpinionAnswersParams and
DeleteMyPreOpinionAnswerParams are identical (both have gatheringId:number and
meetingId:number); refactor by extracting a shared type (e.g., PreOpinionBase or
PreOpinionIds) or have one alias reuse the other (e.g., type
DeleteMyPreOpinionAnswerParams = GetPreOpinionAnswersParams) and update any
references to use the new/common type; ensure exported names remain consistent
and update imports/usages of GetPreOpinionAnswersParams or
DeleteMyPreOpinionAnswerParams accordingly.
- Around line 44-51: The PreOpinionAnswerTopic type uses title/topicDescription
naming that doesn't match PreOpinionTopic (which uses
topicTitle/topicDescription); verify the API contract and then make the types
consistent: either rename PreOpinionAnswerTopic fields to
topicTitle/topicDescription to match PreOpinionTopic, or rename PreOpinionTopic
to use title/description; if the API actually returns different keys, keep both
types but add a clear mapping function (e.g., mapApiTopicToPreOpinionTopic) and
document the difference. Ensure you update any usages of PreOpinionAnswerTopic,
tests, and serializers/deserializers to reflect the chosen naming.
In `@src/features/retrospectives/components/PersonalRetrospectiveContent.tsx`:
- Around line 67-77: The current render runs preOpinions.map(...) and then
separately checks preOpinions.length === 0 for the empty state; change this to a
single conditional expression so the intent is clearer: inside the JSX return of
the PersonalRetrospectiveContent component, replace the separate map + length
check with a ternary (or &&/|| expression) that renders preOpinions.map(opinion
=> <Card key={opinion.topicId}>...) when preOpinions has items, otherwise
renders the empty-state paragraph ("작성한 사전 의견이 없습니다."); keep the same elements
(Card, opinion.topicId, opinion.topicName, opinion.content) and classes so
behavior and keys are preserved.
In `@src/features/retrospectives/hooks/usePersonalRetrospective.ts`:
- Around line 38-43: The useQuery call in usePersonalRetrospective (queryKey
personalRetrospectiveQueryKeys.detail(params), queryFn getPersonalRetrospective)
doesn't set staleTime so it refetches on window focus; add a suitable staleTime
(e.g., a few minutes like 5*60*1000 or another product-appropriate duration) to
the useQuery options to prevent unnecessary refetches for this infrequently
changing personal retrospective data.
In `@src/pages/Retrospectives/PersonalRetrospectivePage.tsx`:
- Around line 14-19: The code converts gatheringIdParam/meetingIdParam to
Numbers before checking their presence, so usePersonalRetrospective is invoked
with NaN even though fetch is disabled; fix by validating presence first (check
gatheringIdParam and meetingIdParam or compute isValidParams) then parse to
numbers and call usePersonalRetrospective with those parsed values (or compute
enabled from isValidParams), and replace the silent "return null" with a minimal
fallback UI or a clear error/log message so routing/missing-param issues are
visible; key symbols: gatheringIdParam, meetingIdParam, gatheringId, meetingId,
usePersonalRetrospective.
src/features/retrospectives/components/PersonalRetrospectiveContent.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/pre-opinion/preOpinion.api.ts (1)
100-111:params구조 분해 위치가 mock 가드 이후인deleteMyPreOpinionAnswer와 불일치합니다.현재
getPreOpinionAnswers는 mock 가드(line 105) 이전 (line 103)에서params를 분해합니다. mock 모드에서gatheringId,meetingId는 사용되지 않으므로 불필요한 연산이 발생하고,deleteMyPreOpinionAnswer(가드 이후 분해)와 스타일이 불일치합니다.♻️ 구조 분해를 mock 가드 이후로 이동
export const getPreOpinionAnswers = async ( params: GetPreOpinionAnswersParams ): Promise<PreOpinionAnswersData> => { - const { gatheringId, meetingId } = params - if (USE_MOCK) { await new Promise((resolve) => setTimeout(resolve, 500)) return getMockPreOpinionAnswers() } + const { gatheringId, meetingId } = params return api.get<PreOpinionAnswersData>(PRE_OPINION_ENDPOINTS.ANSWERS(gatheringId, meetingId)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.api.ts` around lines 100 - 111, In getPreOpinionAnswers, move the destructuring of params (gatheringId, meetingId) to after the USE_MOCK guard so we don't extract unused values in mock mode and to match the style used in deleteMyPreOpinionAnswer; specifically, keep the early return to await getMockPreOpinionAnswers() when USE_MOCK is true, and only then destructure params before calling PRE_OPINION_ENDPOINTS.ANSWERS(gatheringId, meetingId) for the api.get call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/features/pre-opinion/preOpinion.api.ts`:
- Around line 121-128: The early mock branch in deleteMyPreOpinionAnswer
currently just does "if (USE_MOCK) return" which can be unclear; update
deleteMyPreOpinionAnswer to explicitly return a resolved Promise when USE_MOCK
is true (e.g., return Promise.resolve()) so the function's Promise<void>
contract is explicit and clear; locate the async function
deleteMyPreOpinionAnswer and change the USE_MOCK branch accordingly.
---
Nitpick comments:
In `@src/features/pre-opinion/preOpinion.api.ts`:
- Around line 100-111: In getPreOpinionAnswers, move the destructuring of params
(gatheringId, meetingId) to after the USE_MOCK guard so we don't extract unused
values in mock mode and to match the style used in deleteMyPreOpinionAnswer;
specifically, keep the early return to await getMockPreOpinionAnswers() when
USE_MOCK is true, and only then destructure params before calling
PRE_OPINION_ENDPOINTS.ANSWERS(gatheringId, meetingId) for the api.get call.
mgYang53
approved these changes
Feb 22, 2026
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
2 participants
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.
🚀 풀 리퀘스트 제안
📋 작업 내용
preOpinions 폴더의 파일들을 pre-opinion로 옮겼습니다.
SubPageHeader 그림자 관련 props를 추가했습니다.
🔧 변경 사항
주요 변경 사항을 요약해 주세요.
📸 스크린샷 (선택 사항)
수정된 화면 또는 기능을 시연할 수 있는 스크린샷을 첨부해 주세요.
📄 기타
추가적으로 전달하고 싶은 내용이나 특별한 요구 사항이 있으면 작성해 주세요.
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항