[Feature/#282] 타임라인 수정/삭제/AI 요약 API 연동#283
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough타임라인 수정·삭제·AI 요약 요청 API와 훅을 추가하고, 생성 모달을 수정 모드까지 확장했다. TimelinePerformancePanel은 외부 요청 상태를 받도록 바뀌었고, Timeline 페이지는 수정·삭제 모달과 요약 폴링 흐름을 실제 동작에 연결했다. Changes타임라인 CRUD/요약 연동
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/hooks/timeline/useUpdateTimeline.ts (1)
35-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win세 훅에서 중복되는 에러 메시지 추출 로직 헬퍼로 뽑아내면 좋겠어요
useUpdateTimeline,useDeleteTimeline,useRequestTimelineSummary모두(error as IApiErrorResponse).message ?? fallback패턴을 그대로 복붙해서 쓰고 있어요. 공용 함수로 추출하면 나중에 에러 응답 형태가 바뀌었을 때 한 곳만 수정하면 됩니다.♻️ 제안하는 헬퍼 추출
// src/utils/getErrorMessage.ts import type { IApiErrorResponse } from "`@/types/common/common`"; export function getErrorMessage(error: unknown, fallback: string): string { return (error as IApiErrorResponse)?.message ?? fallback; }- userOnError: (error) => { - const message = - (error as IApiErrorResponse).message ?? - "타임라인 수정에 실패했습니다. 다시 시도해주세요"; - toast.error(message); - }, + userOnError: (error) => { + toast.error(getErrorMessage(error, "타임라인 수정에 실패했습니다. 다시 시도해주세요")); + },🤖 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/hooks/timeline/useUpdateTimeline.ts` around lines 35 - 40, The repeated `(error as IApiErrorResponse).message ?? fallback` logic in useUpdateTimeline, useDeleteTimeline, and useRequestTimelineSummary should be extracted into a shared helper. Create a reusable function like getErrorMessage that accepts unknown error input and a fallback string, returns the API message when present, and update each hook’s userOnError handler to call it instead of duplicating the cast-and-fallback pattern.Source: Path instructions
src/hooks/timeline/useTimelineDetail.ts (1)
10-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
options.enabled로 null 가드가 무력화될 수 있어요지금 스프레드 순서상
options에enabled가 포함되면 우리가 강제한orgId != null && timelineId != null체크를 덮어써버립니다. 그러면getTimelineDetail(orgId!, timelineId!)의 non-null assertion 때문에 실제로 null인 상태에서 호출되어 런타임 에러로 이어질 수 있어요. 지금 당장 Timeline.tsx 호출부는enabled를 안 넘기니 문제 없지만, 향후 다른 곳에서 사용할 때 터질 수 있는 구조입니다.🛡️ 제안하는 수정
return useCoreQuery( QUERY_KEYS.timeline.detail(orgId, timelineId), () => getTimelineDetail(orgId!, timelineId!), - { enabled: orgId != null && timelineId != null, ...options }, + { + ...options, + enabled: + orgId != null && + timelineId != null && + (options?.enabled ?? true), + }, );🤖 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/hooks/timeline/useTimelineDetail.ts` around lines 10 - 20, `useTimelineDetail`에서 `options.enabled`가 내부 null 가드를 덮어쓰는 문제가 있습니다. `useCoreQuery`로 넘기는 옵션을 구성할 때 `orgId != null && timelineId != null` 조건을 항상 최종 `enabled` 값에 반영하도록 `useTimelineDetail`의 옵션 병합 순서를 바꾸거나, `options.enabled`와 논리적으로 결합하세요. 특히 `QUERY_KEYS.timeline.detail`와 `getTimelineDetail` 호출부가 `orgId!`, `timelineId!`에 의존하므로, `enabled`가 null 상태를 절대 통과하지 못하게 고정해야 합니다.Source: Path instructions
src/components/timeline/TimelineCreateModal.tsx (1)
108-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
reset이펙트가initialValues참조 동일성에 의존현재는 호출부(
Timeline.tsx)에서useMemo로editInitialValues를 메모이제이션해서 안전하게 동작하지만, 이 컴포넌트 자체는initialValues가 매 렌더마다 새 객체로 전달되면(향후 다른 호출부 추가 시) 폼이 계속 리셋되어 사용자가 입력한 값이 사라지는 문제가 생길 수 있습니다.isOpen이true로 바뀌는 시점(최초 오픈)에만 리셋하도록 이전isOpen값을ref로 추적하는 방식이 더 안전합니다.🤖 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/timeline/TimelineCreateModal.tsx` around lines 108 - 114, The reset effect in TimelineCreateModal is tied to initialValues identity, so the form can be reset on every render if a new object is passed. Update the effect to reset only when isOpen transitions from false to true, using a ref to track the previous isOpen state, and keep the existing reset logic for closing and first-open initialization. Reference the TimelineCreateModal component and its useEffect/reset behavior when making the change.src/pages/dashboard/timeline/Timeline.tsx (1)
58-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win페이지 컴포넌트에 편집/삭제/요약 폴링 비즈니스 로직이 과도하게 집중되어 있습니다.
수정/삭제/요약 요청 상태 관리와 폴링 로직(약 40여 줄)이
Timeline.tsx페이지 컴포넌트 안에 그대로 구현되어 있어 가독성과 재사용성이 떨어집니다.useTimelineSummaryPolling,useTimelineDeleteFlow같은 커스텀 훅으로 분리하면 페이지 컴포넌트는 조합/렌더링에만 집중할 수 있습니다. As per path instructions, "페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토."Also applies to: 172-211
🤖 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/pages/dashboard/timeline/Timeline.tsx` around lines 58 - 113, The Timeline page is carrying too much edit/delete/summary polling state and logic inside the component, making it hard to read and reuse. Move the summary polling and timeout behavior from Timeline into a dedicated hook like useTimelineSummaryPolling, and extract delete/edit flow state and handlers into hooks such as useTimelineDeleteFlow or a similar dedicated hook. Keep Timeline focused on composing useTimelineList, useTimelineDetail, and the UI, and make sure the extracted hooks own the related state setters, mutation callbacks, and polling lifecycle.Source: 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/pages/dashboard/timeline/Timeline.tsx`:
- Line 78: The edit modal flow in Timeline.tsx only reads data from
useTimelineDetail(editTimelineId), so loading/error states are ignored and a
failed fetch leaves editInitialValues unset forever. Update the
editTimelineId-driven logic around useTimelineDetail and the modal open
condition so you handle isLoading/isError explicitly, show a toast on failure,
and reset editTimelineId via setEditTimelineId(null) when the fetch fails or
cannot complete. Keep the fix centered on the edit flow symbols
useTimelineDetail, editTimelineId, editInitialValues, and the isOpen condition
so users get visible feedback and can retry.
- Around line 358-364: `Timeline`의 `onDelete`는 `onEdit`과 달리 `selectedBarId!`를
사용해 null을 단언하고 있어 처리 방식이 불일치합니다. `onDelete`가 호출되는 시점에도 안전하도록 `onEdit`과 동일하게
`selectedBarId != null` 체크를 먼저 하고, `openDeleteModal`은 유효한 id가 있을 때만 실행되도록
`selectedBarId`를 명시적으로 가드하는 방식으로 통일하세요. `openEditModal`, `openDeleteModal`,
`selectedBarId`를 기준으로 수정 위치를 찾으면 됩니다.
- Around line 75-102: `useTimelineDetail`의 `refetchInterval` 콜백에서 상태 변경과
`toast.error`를 수행하는 부분을 분리하세요. `Timeline` 컴포넌트에서 `setIsAwaitingSummary`,
`setSummaryPollStartedAt` 및 타임아웃 토스트 처리는 `useEffect`로 옮기고, `refetchInterval`은
간격(또는 중단 여부)만 반환하도록 바꾸세요. 특히 `isAwaitingSummary`, `summaryPollStartedAt`,
`selectedBarId`와 `detail`의 summary 상태를 기준으로 완료/타임아웃 판정을 별도 훅/이펙트에서 처리하도록 정리하세요.
- Around line 104-113: The edit form in Timeline currently hardcodes
comparisonPeriodType to LAST_WEEK in editInitialValues, which can overwrite an
existing timeline’s comparison period on save. Update Timeline and the
ITimelineDetail model so the edit flow restores the original
comparisonPeriodType from editDetail when available, or otherwise excludes this
field from the update payload before calling useUpdateTimeline in
TimelineCreateModal.
---
Nitpick comments:
In `@src/components/timeline/TimelineCreateModal.tsx`:
- Around line 108-114: The reset effect in TimelineCreateModal is tied to
initialValues identity, so the form can be reset on every render if a new object
is passed. Update the effect to reset only when isOpen transitions from false to
true, using a ref to track the previous isOpen state, and keep the existing
reset logic for closing and first-open initialization. Reference the
TimelineCreateModal component and its useEffect/reset behavior when making the
change.
In `@src/hooks/timeline/useTimelineDetail.ts`:
- Around line 10-20: `useTimelineDetail`에서 `options.enabled`가 내부 null 가드를 덮어쓰는
문제가 있습니다. `useCoreQuery`로 넘기는 옵션을 구성할 때 `orgId != null && timelineId != null`
조건을 항상 최종 `enabled` 값에 반영하도록 `useTimelineDetail`의 옵션 병합 순서를 바꾸거나,
`options.enabled`와 논리적으로 결합하세요. 특히 `QUERY_KEYS.timeline.detail`와
`getTimelineDetail` 호출부가 `orgId!`, `timelineId!`에 의존하므로, `enabled`가 null 상태를 절대
통과하지 못하게 고정해야 합니다.
In `@src/hooks/timeline/useUpdateTimeline.ts`:
- Around line 35-40: The repeated `(error as IApiErrorResponse).message ??
fallback` logic in useUpdateTimeline, useDeleteTimeline, and
useRequestTimelineSummary should be extracted into a shared helper. Create a
reusable function like getErrorMessage that accepts unknown error input and a
fallback string, returns the API message when present, and update each hook’s
userOnError handler to call it instead of duplicating the cast-and-fallback
pattern.
In `@src/pages/dashboard/timeline/Timeline.tsx`:
- Around line 58-113: The Timeline page is carrying too much edit/delete/summary
polling state and logic inside the component, making it hard to read and reuse.
Move the summary polling and timeout behavior from Timeline into a dedicated
hook like useTimelineSummaryPolling, and extract delete/edit flow state and
handlers into hooks such as useTimelineDeleteFlow or a similar dedicated hook.
Keep Timeline focused on composing useTimelineList, useTimelineDetail, and the
UI, and make sure the extracted hooks own the related state setters, mutation
callbacks, and polling lifecycle.
🪄 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
Run ID: 35253e2e-049d-4bce-8cc1-f4d2b7e4bd22
📒 Files selected for processing (9)
src/api/timeline/timeline.tssrc/components/timeline/TimelineCreateModal.tsxsrc/components/timeline/TimelinePerformancePanel.tsxsrc/hooks/timeline/useDeleteTimeline.tssrc/hooks/timeline/useRequestTimelineSummary.tssrc/hooks/timeline/useTimelineDetail.tssrc/hooks/timeline/useUpdateTimeline.tssrc/pages/dashboard/timeline/Timeline.tsxsrc/types/timeline/api.ts
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.
🚨 관련 이슈
Closed #282
✨ 변경사항
✏️ 작업 내용
useTimelineDetail에 query options 전달 지원 -> AI 요약 요청 후summary갱신까지 타임라인 상세 내용 GET 폴링(1.5초 간격으로 진행되고, 90초 타임아웃 걸어둠)TimelineCreateModaledit 모드 지원 및 생성/수정 모달 분리Timeline.tsx바/패널 수정/삭제 연동, 삭제 확인 Modal, AI 요약 버튼 연동TimelinePerformancePanelmock setTimeout 제거,onRequestsummary/loading props 연동😅 미완성 작업
📢 논의 사항 및 참고 사항
Summary by CodeRabbit