feat: 계획 관리 페이지 플랜 수정 기능, 페이지네이션, 상단 카드 stat 연동 구현#53
Merged
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough플랜 페이지에 클라이언트 페이지네이션, 병렬 데이터 로드 기반 플랜별 통계 계산, 인라인 플랜 편집(제목/설명) 및 서비스 API( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as Plans Page
participant Service as Plan Service
participant DB as Firestore
User->>Page: 페이지 진입 / 로그인
activate Page
Page->>Service: fetchPlans(uid) (병렬)
Page->>Service: fetchAllPlanItems(uid) (병렬)
activate Service
Service->>DB: 쿼리: plans
Service->>DB: 쿼리: plan items
DB-->>Service: Plans + PlanItems
Service-->>Page: 반환 (Plans, PlanItems)
Page->>Page: fetchAndCalculate(uid) (플랜별 통계 계산)
Page->>Page: paginate & render (Pagination + PlanSection)
deactivate Page
sequenceDiagram
participant User
participant PlanSection
participant Page as Plans Page
participant Service as Plan Service
participant DB as Firestore
User->>PlanSection: 편집 클릭 -> 제목/설명 수정 -> 저장
activate PlanSection
PlanSection->>Page: onUpdatePlan(planId, newTitle, newDesc)
activate Page
Page->>Service: updatePlan(uid, planId, {title, description})
activate Service
Service->>DB: Update plan document
DB-->>Service: 성공 응답
Service-->>Page: 성공
Page->>Service: fetchPlans(uid)
Service->>DB: 쿼리: 업데이트된 plans
DB-->>Service: Updated plans
Service-->>Page: 반환
Page->>Page: fetchAndCalculate(uid) (통계 재계산)
Page->>PlanSection: Re-render with updated data
deactivate Page
deactivate PlanSection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/plans/PlanSection.tsx (1)
153-168: 삭제 취소 시에도 통계 갱신이 호출됩니다.
사용자가 취소한 경우에도onChangeStats()가 실행됩니다. 확인 후 실제 삭제 성공 시에만 호출되도록 이동해주세요.🛠️ 수정 제안
- if (confirm(`'${title}' 하위 항목을 정말 삭제하시겠습니까?`)) { + if (confirm(`'${title}' 하위 항목을 정말 삭제하시겠습니까?`)) { try { await deletePlanItem(userId, itemId); // 목록 새로고침 const fetchedPlanItems = await fetchPlanItems(userId, planId); setTasks(fetchedPlanItems); } catch (err) { console.error(err); } - } - - onChangeStats(); + onChangeStats(); + }
🤖 Fix all issues with AI agents
In `@app/`(with-sidebar)/plans/page.tsx:
- Around line 41-83: fetchAndCalculate currently re-queries Firestore with
fetchPlans/fetchAllPlanItems causing duplicate reads; change it to accept
optional preloaded data (e.g., parameters preloadedPlans?: Plan[],
preloadedItems?: PlanItem[]) and use those when provided, otherwise fall back to
calling fetchPlans/fetchAllPlanItems; update internal logic that currently calls
fetchPlans/fetchAllPlanItems so callers pass existing state (setPlans/plans)
into fetchAndCalculate instead of letting it refetch; ensure setPlans is only
called once by the place that originally loads plans and remove redundant
fetches at the other call sites (the other occurrences you flagged at the later
ranges).
In `@services/plans/planManageService.service.ts`:
- Around line 215-226: fetchAllPlanItems returns Firestore Timestamps for date
fields causing type mismatch; update fetchAllPlanItems to convert
doc.data().createdAt and doc.data().deadline (if present/non-null) from
Timestamp to JS Date objects before returning, mirror the behavior used in
fetchPlanItems, and ensure the returned PlanItem has deadline set to null or a
Date as appropriate so the function signature and runtime usage align.
🧹 Nitpick comments (3)
components/plans/PlanSection.tsx (2)
142-151: 플랜 삭제 시 통계 갱신이 중복 호출됩니다.
상위 핸들러에서 이미 통계를 재계산하는데 여기서도onChangeStats()를 호출해 중복 조회/깜빡임이 생길 수 있습니다. 삭제는 상위에서만 갱신하도록 정리하는 편이 좋습니다.♻️ 정리 제안
- onDeletePlan(planId, title); - - setShowPlanMenu(false); - - onChangeStats(); + onDeletePlan(planId, title); + setShowPlanMenu(false);
181-189: 플랜 제목 공백 저장 방지 처리가 필요합니다.
추가 폼과 동일하게 공백 입력을 막지 않으면 빈 제목이 저장될 수 있습니다.🛠️ 수정 제안
const handleSaveEdit = (e: React.MouseEvent) => { e.stopPropagation(); + if (!editTitle.trim()) { + alert('플랜 제목을 입력해주세요.'); + return; + } // 부모 컴포넌트에게 변경된 내용 전달 onUpdatePlan(planId, editTitle, editDesc); setIsEditingPlan(false); // 수정 모드 종료 };app/(with-sidebar)/plans/page.tsx (1)
160-171: 페이지 보정 계산이 현재 상태와 어긋날 수 있습니다.
삭제 직후plans.length는 이전 값이라 페이지 계산이 불안정합니다.fetchedPlans.length를 쓰거나, 아래의 페이지네이션useEffect하나로 일원화하는 편이 안전합니다.♻️ 간단 수정 예시
- if ( - currentPage > Math.ceil((plans.length - 1) / itemsPerPage) && - Math.ceil((plans.length - 1) / itemsPerPage) > 0 - ) { - setCurrentPage(Math.ceil((plans.length - 1) / itemsPerPage)); - } else if (Math.ceil((plans.length - 1) / itemsPerPage) === 0) { - setCurrentPage(1); - } +const totalAfterDelete = Math.ceil(fetchedPlans.length / itemsPerPage); +if (currentPage > totalAfterDelete && totalAfterDelete > 0) { + setCurrentPage(totalAfterDelete); +} else if (totalAfterDelete === 0) { + setCurrentPage(1); +}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@services/plans/planManageService.service.ts`:
- Around line 215-229: fetchAllPlanItems returns null for missing deadline
(using "deadline: data.deadline?.toDate() ?? null") which mismatches the
PlanItem type (deadline?: Date) and is inconsistent with fetchPlanItems; also
the query lacks ordering so results aren’t deterministic. Update
fetchAllPlanItems to mirror fetchPlanItems: use "deadline:
data.deadline?.toDate()" (so undefined when absent), keep createdAt conversion
consistent, and add the same orderBy clause used in fetchPlanItems to the query
(referencing fetchAllPlanItems, fetchPlanItems, PlanItem, and orderBy) so
ordering and types match.
🧹 Nitpick comments (1)
app/(with-sidebar)/plans/page.tsx (1)
211-217: 페이지 조정 로직이 중복될 수 있습니다.
handleDeletePlan의 페이지 조정 로직과 이useEffect가 동일한 역할을 합니다.handleDeletePlan에서 페이지 조정을 제거하고 이useEffect에만 의존하거나, 둘 중 하나만 유지하는 것이 좋습니다.
Comment on lines
+215
to
229
| // 10. 하위 항목 모두 가져오기 | ||
| export const fetchAllPlanItems = async (uid: string): Promise<PlanItem[]> => { | ||
| const allPlanItemsRef = collection(db, 'users', uid, 'planItems'); | ||
| const q = query(allPlanItemsRef); | ||
| const snapshot = await getDocs(q); | ||
|
|
||
| return snapshot.docs.map((docSnap) => { | ||
| const data = docSnap.data(); | ||
| return snapshot.docs.map((doc) => { | ||
| const data = doc.data(); | ||
| return { | ||
| id: docSnap.id, | ||
| id: doc.id, | ||
| ...data, | ||
| createdAt: data.createdAt?.toDate(), | ||
| deadline: data.deadline?.toDate(), | ||
| deadline: data.deadline?.toDate() ?? null, | ||
| }; | ||
| }) as PlanItem[]; |
There was a problem hiding this comment.
deadline 반환 값이 타입과 불일치합니다.
Line 227에서 ?? null을 사용하면 deadline이 없을 때 null을 반환하지만, PlanItem 타입은 deadline?: Date (즉, Date | undefined)로 정의되어 있습니다. fetchPlanItems(line 84)는 deadline: data.deadline?.toDate()로 undefined를 반환하므로 동작이 일관되지 않습니다.
또한, fetchPlanItems와 달리 정렬 기준(orderBy)이 없어 결과 순서가 보장되지 않습니다.
🔧 수정 제안
export const fetchAllPlanItems = async (uid: string): Promise<PlanItem[]> => {
const allPlanItemsRef = collection(db, 'users', uid, 'planItems');
- const q = query(allPlanItemsRef);
+ const q = query(allPlanItemsRef, orderBy('createdAt', 'asc'));
const snapshot = await getDocs(q);
return snapshot.docs.map((doc) => {
const data = doc.data();
return {
id: doc.id,
...data,
createdAt: data.createdAt?.toDate(),
- deadline: data.deadline?.toDate() ?? null,
+ deadline: data.deadline?.toDate(),
};
}) as PlanItem[];
};📝 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.
Suggested change
| // 10. 하위 항목 모두 가져오기 | |
| export const fetchAllPlanItems = async (uid: string): Promise<PlanItem[]> => { | |
| const allPlanItemsRef = collection(db, 'users', uid, 'planItems'); | |
| const q = query(allPlanItemsRef); | |
| const snapshot = await getDocs(q); | |
| return snapshot.docs.map((docSnap) => { | |
| const data = docSnap.data(); | |
| return snapshot.docs.map((doc) => { | |
| const data = doc.data(); | |
| return { | |
| id: docSnap.id, | |
| id: doc.id, | |
| ...data, | |
| createdAt: data.createdAt?.toDate(), | |
| deadline: data.deadline?.toDate(), | |
| deadline: data.deadline?.toDate() ?? null, | |
| }; | |
| }) as PlanItem[]; | |
| // 10. 하위 항목 모두 가져오기 | |
| export const fetchAllPlanItems = async (uid: string): Promise<PlanItem[]> => { | |
| const allPlanItemsRef = collection(db, 'users', uid, 'planItems'); | |
| const q = query(allPlanItemsRef, orderBy('createdAt', 'asc')); | |
| const snapshot = await getDocs(q); | |
| return snapshot.docs.map((doc) => { | |
| const data = doc.data(); | |
| return { | |
| id: doc.id, | |
| ...data, | |
| createdAt: data.createdAt?.toDate(), | |
| deadline: data.deadline?.toDate(), | |
| }; | |
| }) as PlanItem[]; | |
| }; |
🤖 Prompt for AI Agents
In `@services/plans/planManageService.service.ts` around lines 215 - 229,
fetchAllPlanItems returns null for missing deadline (using "deadline:
data.deadline?.toDate() ?? null") which mismatches the PlanItem type (deadline?:
Date) and is inconsistent with fetchPlanItems; also the query lacks ordering so
results aren’t deterministic. Update fetchAllPlanItems to mirror fetchPlanItems:
use "deadline: data.deadline?.toDate()" (so undefined when absent), keep
createdAt conversion consistent, and add the same orderBy clause used in
fetchPlanItems to the query (referencing fetchAllPlanItems, fetchPlanItems,
PlanItem, and orderBy) so ordering and types match.
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
1 participant
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.
1) 작업한 이슈번호
#38
2) 변경 요약 (What & Why)
3) 스크린샷/동영상 (UI 변경 시)
4) 상세 변경사항
5) 참고사항
하위 항목 수정 기능, 검색 기능 추가 예정
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.