Skip to content

feat : to-sing 곡 순서 변경 및 삭제 낙관적 업데이트 적용 (#230) #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GulSam00 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat : to-sing 곡 순서 변경 및 삭제 낙관적 업데이트 적용 (#230) #231

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions apps/web/src/queries/tosingSongQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function useDeleteToSingSongMutation() {
queryClient.cancelQueries({ queryKey: ['toSingSong'] });
const prev = queryClient.getQueryData(['toSingSong']);
queryClient.setQueryData(['toSingSong'], (old: ToSingSong[]) => {
old.filter(song => song.songs.id !== songId);
return old.filter(song => song.songs.id !== songId);
});
Comment on lines 60 to 62
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. old.filter without null-guard 📎 Requirement gap ☼ Reliability

The optimistic delete updater assumes old is always an array, but React Query can pass undefined
when the cache is empty/unset, causing a runtime exception. This can prevent the song from being
removed immediately and break the expected optimistic-update behavior for deletion.
Agent Prompt 
## Issue description
The optimistic delete cache updater calls `old.filter(...)` without guarding for `old` possibly being `undefined`.

## Issue Context
React Query's `setQueryData` updater may receive `undefined` if the query cache is not yet set. A runtime error here breaks the optimistic delete behavior required for the to-sing page.

## Fix Focus Areas
- apps/web/src/queries/tosingSongQuery.ts[60-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

return { prev };
},
Expand All @@ -68,15 +68,7 @@ export function useDeleteToSingSongMutation() {
queryClient.setQueryData(['toSingSong'], context?.prev);
},
onSettled: () => {
// 1초 이내에 함수가 여러 번 호출되면, 1초 뒤 트리거를 계속해서 갱신
// if (invalidateTimeout) {
// clearTimeout(invalidateTimeout);
// }
// invalidateTimeout = setTimeout(() => {
// queryClient.invalidateQueries({ queryKey: ['toSingSong'] });
// }, 1000);
queryClient.invalidateQueries({ queryKey: ['searchSong'] });
queryClient.invalidateQueries({ queryKey: ['toSingSong'] });
},
});
}
Expand Down Expand Up @@ -106,8 +98,5 @@ export function usePatchToSingSongMutation() {
alert(error.message ?? 'PATCH 실패');
queryClient.setQueryData(['toSingSong'], context?.prev);
},
onSettled: () => {
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Reorder uses stale weights 🐞 Bug ≡ Correctness

usePatchToSingSongMutation replaces the cached list with newItems but never updates the moved item’s
order_weight, and this PR removed the post-mutation invalidate of ['toSingSong'], so cached
order_weight stays stale for up to 5 minutes. Subsequent drag/drop operations compute newWeight from
these stale order_weight values and can persist incorrect weights to the server, causing ordering to
“jump” after a later refetch/reload.
Agent Prompt 
### Issue description
After a successful reorder, the cache keeps the previous `order_weight` values because the optimistic update only reorders `newItems` and `patchToSingSong` returns `void`. Since this PR removed `invalidateQueries(['toSingSong'])`, subsequent reorder operations compute `newWeight` from stale `order_weight` values and can send incorrect weights to the server.

### Issue Context
- `useToSingSong` computes `newWeight` using neighbor items’ `order_weight`.
- `usePatchToSingSongMutation` sets `['toSingSong']` cache to `newItems` as-is.
- `QueryClient` defaults prevent fast refetches (`refetchOnWindowFocus: false`, `staleTime: 5m`).

### Fix Focus Areas
- apps/web/src/queries/tosingSongQuery.ts[80-100]
- apps/web/src/hooks/useToSingSong.ts[43-64]

### Suggested fix
1. In `usePatchToSingSongMutation.onMutate`, destructure `{ songId, newWeight, newItems }` (not just `newItems`).
2. Before calling `setQueryData`, create `optimisticItems` that updates the moved item’s `order_weight` to `newWeight` (e.g., `map` by `songs.id`), then set the cache to `optimisticItems`.
3. Optionally, add a debounced `invalidateQueries(['toSingSong'])` in `onSuccess` (not `onSettled`) if you still want periodic server reconciliation without overriding in-flight optimistic interactions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

queryClient.invalidateQueries({ queryKey: ['toSingSong'] });
},
});
}