임시저장 안내 토스트 상대 시간으로 안내 및 공동 작성 블록 레이아웃 버그 수정#298
Conversation
H-sooyeon
added
🐞 bug
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe changes add structured backend authentication errors and frontend 401 classification, implement confirmed collaborative block deletion UI, adjust mouse drag activation, improve iOS haptic detection, and revise Korean relative-time wording. ChangesStructured authentication errors
Collaborative block deletion
Pointer drag initiation
iOS haptic detection
Relative-time wording
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant fetchWithRetry
participant errorHandler
participant AuthAPI
participant Session
Client->>fetchWithRetry: make API request
fetchWithRetry->>AuthAPI: send request
AuthAPI-->>fetchWithRetry: 401 with errorCode
fetchWithRetry->>errorHandler: classify errorCode
errorHandler-->>fetchWithRetry: refreshable or terminal
fetchWithRetry->>Session: refresh token or terminate session
sequenceDiagram
participant RecordEditor
participant usePostEditorBlocks
participant Socket
RecordEditor->>usePostEditorBlocks: request block deletion
usePostEditorBlocks->>Socket: send BLOCK_DELETE
Socket-->>usePostEditorBlocks: PATCH_COMMITTED
usePostEditorBlocks-->>RecordEditor: update block list and deleting state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 2
🧹 Nitpick comments (2)
frontend/src/lib/utils/errorHandler.ts (1)
42-49: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the combined array out of
isAuthError.
isAuthErrorallocates a new array ([...REFRESHABLE_AUTH_ERRORS, ...TERMINAL_AUTH_ERRORS]) on every call. Since both source arrays are module-level constants, the combined array can be hoisted to module scope for a small efficiency gain.♻️ Proposed refactor
const REFRESHABLE_AUTH_ERRORS: string[] = [ ERROR_CODES.TOKEN_EXPIRED, ERROR_CODES.UNAUTHORIZED, ]; const TERMINAL_AUTH_ERRORS: string[] = [ ERROR_CODES.ACCESS_TOKEN_REQUIRED, ERROR_CODES.INVALID_TOKEN, ERROR_CODES.USER_NOT_FOUND, ERROR_CODES.REFRESH_TOKEN_NOT_FOUND, ERROR_CODES.REFRESH_TOKEN_REUSE_DETECTED, ERROR_CODES.REFRESH_TOKEN_EXPIRED, ERROR_CODES.SESSION_INVALID, ]; +const ALL_AUTH_ERRORS: string[] = [ + ...REFRESHABLE_AUTH_ERRORS, + ...TERMINAL_AUTH_ERRORS, +]; + /** * 인증 에러 여부 확인 */ export function isAuthError(errorCode?: string): boolean { if (!errorCode) return false; - const authErrors: string[] = [ - ...REFRESHABLE_AUTH_ERRORS, - ...TERMINAL_AUTH_ERRORS, - ]; - return authErrors.includes(errorCode); + return ALL_AUTH_ERRORS.includes(errorCode); }🤖 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 `@frontend/src/lib/utils/errorHandler.ts` around lines 42 - 49, Hoist the combined authentication-errors array from isAuthError to module scope, using the existing REFRESHABLE_AUTH_ERRORS and TERMINAL_AUTH_ERRORS constants. Update isAuthError to reuse that module-level array while preserving its current false result for missing codes and inclusion check.frontend/src/app/(post)/_hooks/usePostEditorBlocks.ts (1)
188-188: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
removeBlockidentity churns on every deletion start/finish, re-rendering every memoizedBlockItem.
deletingBlockIdsis added toremoveBlock's deps (Line 246) solely to support thedeletingBlockIds.has(id)duplicate-request guard (Line 188). SinceremoveBlockis passed as a single prop to everyBlockIteminRecordEditor.tsx(all wrapped inmemo), each state change todeletingBlockIdsrecreatesremoveBlockand forces a full re-render of every block, not just the one being deleted.
deleteTimeoutsRef.currentalready tracks the same set of in-flight ids and is a ref, so it can back the duplicate check without needing to be a dependency.- if (deletingBlockIds.has(id)) return; // 이미 삭제 요청 중 + if (deleteTimeoutsRef.current.has(id)) return; // 이미 삭제 요청 중and drop
deletingBlockIdsfrom theremoveBlockdeps array.Also applies to: 237-247
🤖 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 `@frontend/src/app/`(post)/_hooks/usePostEditorBlocks.ts at line 188, Update removeBlock to use deleteTimeoutsRef.current for the in-flight duplicate-request check instead of deletingBlockIds, then remove deletingBlockIds from removeBlock’s dependency array. Preserve the existing deletion behavior while keeping removeBlock stable across deletingBlockIds state changes.
🤖 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 `@frontend/src/app/`(post)/_components/editor/RecordEditor.tsx:
- Around line 310-317: Add a toast.error notification to the existing onError
handler so failed saves always provide user feedback after resetting the saving
and publishing state. Keep the current state-reset and navigation behavior
unchanged, and use a clear failure message distinct from the delayed-save toast.
In `@frontend/src/lib/utils/errorHandler.ts`:
- Around line 14-37: Add ERROR_CODES.INVALID_AUTH_CODE to the
TERMINAL_AUTH_ERRORS list in errorHandler.ts so invalid authorization codes from
/auth/exchange are classified as terminal authentication failures.
---
Nitpick comments:
In `@frontend/src/app/`(post)/_hooks/usePostEditorBlocks.ts:
- Line 188: Update removeBlock to use deleteTimeoutsRef.current for the
in-flight duplicate-request check instead of deletingBlockIds, then remove
deletingBlockIds from removeBlock’s dependency array. Preserve the existing
deletion behavior while keeping removeBlock stable across deletingBlockIds state
changes.
In `@frontend/src/lib/utils/errorHandler.ts`:
- Around line 42-49: Hoist the combined authentication-errors array from
isAuthError to module scope, using the existing REFRESHABLE_AUTH_ERRORS and
TERMINAL_AUTH_ERRORS constants. Update isAuthError to reuse that module-level
array while preserving its current false result for missing codes and inclusion
check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e63add7a-a821-4a9e-88cd-84e97b707770
📒 Files selected for processing (10)
backend/src/common/exceptions/auth-unauthorized.exception.tsbackend/src/modules/auth/auth.service.tsbackend/src/modules/mypage/mypage.service.tsfrontend/src/app/(post)/_components/editor/RecordEditor.tsxfrontend/src/app/(post)/_hooks/usePostEditorBlocks.tsfrontend/src/app/(post)/_hooks/useRecordEditorDnD.tsfrontend/src/hooks/useHaptic.tsfrontend/src/lib/api/api.tsfrontend/src/lib/date.tsfrontend/src/lib/utils/errorHandler.ts
- 기존 5분 전, 1469분 전으로 표시되던 것을 하루전, 한 달 전 등으로 표현
- 블록 삭제를 낙관적(즉시 로컬 삭제)에서 비관적 처리로 전환 - 텍스트 블록 등에서 포커스가 풀려 락이 해제된 뒤 삭제를 시도하면 백엔드는 락 미보유로 삭제를 거부하지만 프론트는 이를 모르고 로컬에서 먼저 지워버려 블록 목록이 어긋나고 저장 시 다른 블록과 위치가 겹치면 문제 해결 - BLOCK_DELETE 전송 후 PATCH_COMMITTED로 실제 커밋을 확인한 다음에만 로컬에서 블록 제거, 8초 이내 확인이 없으면 자동 복구 - 삭제 대기 중인 블록은 반투명 처리 + 클릭 차단
- 10초 안전망 타이머 발동시의 isSaving false 갱신 제거 - toast.error를 toast.info로 변경해, 실제 실패가 아닌 지연 상황에서 에러 메시지를 표시해 사용자에게 혼란을 주던 문제 수정
H-sooyeon
force-pushed
the
refactor/ux-ui-feedback-#287
branch
from
381fb1c to
e978b09
Compare
- /auth/exchange가 반환하는 INVALID_AUTH_CODE가 프론트 errorHandler에 분류돼 있지 않은 문제 - 해당 코드가 어디에도 분류돼 있지 않아 공용 API 클라이언트 401 처리 로직에서 복구 불가능한 인증 에러로 인식되지 못하고 일반 에러로 새어나갈 수 있는 가능성 존재 - 공용 클라이언트를 타는 다른 경로에서 발생할 경우를 대비해 안정성 있게 등록
2 participants
요약 (연관 이슈 번호 포함)
작업 내용 + 스크린샷
임시저장 안내 토스트 상대 시간 출력
date 유틸 파일에 상대 시간을 반환하는 함수를 만들어뒀어서, 그 함수를 활용하도록 수정했어요.
공동 작성 블록 삭제 버그
Note
문제 상황
텍스트 블록 등에서 포커스가 풀려 락이 해제된 뒤 삭제를 시도하면, 백엔드는 락 미보유로 삭제를 거부하지만 프론트는 이를 모르고 로컬에서 먼저 지워버려 블록 목록이 어긋나고 저장 시 다른 블록과 위치가 겹치던 문제
회의 때 전달해주신 컨텍스트가 텍스트 블록에서만이 아닌 다른 블록 타입에서도 발생 가능했던 버그로 확인했어요.
재현 조건만 맞으면 텍스트가 아니어도 똑같이 터질 수 있었죠!
핵심은 락을 한 번 잡았다가 놓은 뒤, 다시 잡지 않은 상태에서 삭제를 시도하는 모든 케이스에서 발생할 수 있었고, 텍스트 블록이 재현하는 흐름이 제일 짧고 쉬워서 빨리 발견된 것 같아요.
그래서 텍스트 블록만이 아닌, 다른 타입 블록들도 같은 로직으로 삭제가 되도록 통일했어요.
removeBlock을 낙관적 → 비관적으로 전환했어요.BLOCK_DELETE패치 전송 →PATCH_COMMITTED로 실제 커밋을 확인한 뒤에만 로컬에서 제거하도록 했어요.id를deletingBlockIds로 추적하고, 8초 안에 커밋 확인이 안 오면(백엔드가 락 없음으로 거부한 경우 포함) 자동으로 원상복구 + 에러 토스트를 출력하도록 했어요.pointer-events-none으로 클릭 차단하도록 했어요.그외 UX 피드백인
수정했습니다.
실제 걸린 시간
4h
작업하며 고민했던 점
디스코드에 공유해주셨던 방법대로
이렇게 4단계로 나눠서 진행하게 되면 서버 왕복이 두 번이라 느리다고 생각했어요.
그래서 삭제 요청 자체를 하나의 원자적 동작(필요하면 락 잠깐 점유하고 삭제 및 해제까지 한 번에)으로 처리하면 훨씬 단순해지지 않을까 싶었어요.
그래서 프론트에서는
BLOCK_DELETE를 전송하고PATCH_COMMITTED로 그 삭제가 커밋된걸 확인하면 로컬에서 삭제하도록 해줬어요.다른 패치들도 프론트에서 요청 -> 백엔드가 소유권 검증 패턴이라, 삭제도 같은 패턴으로 맞추는게 일관적일 것 같아요.
서버 쪽에서 고려할 것
(다른 사람이 들고 있을 때는 지금처럼 거부 유지!)
여러 사람이 동시에 락을 보유하려고 할 수 있는데, 이 중 한 사람만 락을 보유하도록 하면 될 것 같아요.
테스트 실행 여부
리뷰 참고사항(선택)
현재는 재현하면 블록 삭제가 통과되지는 않아요.
Lock owner only.로 거부되는 상태에요.서버 쪽 코드를 락 미보유 시에도 잠깐 락을 가져간 뒤 삭제 처리되도록 고치면 될거에요.
번외로, ios 햅틱은 18 버전 이상에서 지원하기 때문에 그 이상의 실기기에서만 테스트가 가능합니다.
참고 게시글
시각 자료(이미지/영상, 있다면)(선택)
Summary by CodeRabbit
New Features
Improvements