feat: sync progress modal#179
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough此PR为收藏夹同步引入逐步进度回调与新的进度模态框：将原先在页面内执行的同步与即时通知/导航流程替换为打开 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FavoriteDetail as "Favorite Detail\n(component)"
participant ModalStore as "Modal Store"
participant FavoriteSyncModal as "FavoriteSyncProgress\nModal"
participant Mutation as "usePlaylistSync\nmutation"
participant SyncFacade as "SyncFacade"
User->>FavoriteDetail: 点击同步
FavoriteDetail->>ModalStore: openModal('FavoriteSyncProgress', { favoriteId })
ModalStore->>FavoriteSyncModal: 渲染并挂载
FavoriteSyncModal->>Mutation: 调用 mutation({ remoteSyncId, type: 'favorite', onProgress })
Mutation->>SyncFacade: sync(remoteSyncId, 'favorite', onProgress)
SyncFacade->>FavoriteSyncModal: onProgress(stage: 'initializing', message)
FavoriteSyncModal->>FavoriteSyncModal: 更新 UI（标题/消息）
SyncFacade->>FavoriteSyncModal: onProgress(stage: 'fetching_metadata', current, total)
FavoriteSyncModal->>FavoriteSyncModal: 更新进度条
SyncFacade->>FavoriteSyncModal: onProgress(stage: 'calculating_diff' / 'fetching_details' / 'saving')
FavoriteSyncModal->>FavoriteSyncModal: 更新阶段信息
SyncFacade->>FavoriteSyncModal: onProgress(stage: 'completed', message) or onProgress(stage: 'error', message)
User->>FavoriteSyncModal: 点击关闭
FavoriteSyncModal->>ModalStore: closeModal() (可触发跳转到本地播放列表)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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: 1
🤖 Fix all issues with AI agents
In `@src/app/playlist/remote/favorite/`[id].tsx:
- Around line 95-121: The modal can become unclosable when sync fails before
onProgress runs because syncProgress stays null; to fix, initialize syncProgress
when opening the modal (e.g., setSyncProgress to a starting object) and add
null-safe fallbacks in the syncFavorite callbacks: in the onSuccess handler
inside the syncFavorite call ensure you setSyncProgress to a completed state
even if prev is null, and in onError setSyncProgress to an error state with the
error message when prev is null; update the logic around
setIsSyncModalVisible/syncFavorite (the call that supplies remoteSyncId, type,
onProgress) so SyncProgressModal always receives a non-null progress object (or
make its dismissable/close control driven by this fallback) to guarantee the
modal can be dismissed.
🧹 Nitpick comments (1)
src/app/playlist/remote/favorite/[id].tsx (1)
239-261: 建议收敛大段注释逻辑。
这里的注释体量较大且包含已弃用逻辑，建议移到 issue/ADR 或精简为一两行关键说明，避免后续维护噪音。
Comment on lines
+95
to
+121
| setIsSyncModalVisible(true) | ||
| syncFavorite( | ||
| { | ||
| remoteSyncId: Number(id), | ||
| type: 'favorite', | ||
| toastId, | ||
| onProgress: setSyncProgress, | ||
| }, | ||
| { | ||
| onSuccess: (id) => { | ||
| setSyncProgress((prev) => | ||
| prev ? { ...prev, stage: 'completed', message: '同步完成' } : null, | ||
| ) | ||
| // 等待用户点击关闭或自动关闭逻辑 | ||
| // 这里我们不自动跳转，让用户看到结果 | ||
| if (!id) return | ||
| router.replace({ | ||
| pathname: '/playlist/local/[id]', | ||
| params: { id: String(id) }, | ||
| }) | ||
| // 如果需要自动跳转，可以在 onClose 里处理，或者这里延迟处理 | ||
| }, | ||
| onError: (error) => { | ||
| setSyncProgress((prev) => | ||
| prev | ||
| ? { | ||
| ...prev, | ||
| stage: 'error', | ||
| message: `同步失败: ${error.message}`, | ||
| } | ||
| : null, | ||
| ) |
There was a problem hiding this comment.
同步任务提前失败时，Modal 可能无法关闭。
当同步在 onProgress 触发前就失败（如“任务已在进行中”）时，syncProgress 仍为 null，SyncProgressModal 处于不可关闭状态（dismissable=false 且无关闭按钮）。建议在打开 Modal 时初始化进度，或在 onSuccess/onError 中对 prev 为空的情况兜底。
🔧 建议修复（兜底初始化 + 完成/失败时强制赋值）
- setIsSyncModalVisible(true)
+ setSyncProgress({ stage: 'initializing', message: '准备中...' })
+ setIsSyncModalVisible(true)
syncFavorite(
{
remoteSyncId: Number(id),
type: 'favorite',
onProgress: setSyncProgress,
},
{
onSuccess: (id) => {
- setSyncProgress((prev) =>
- prev ? { ...prev, stage: 'completed', message: '同步完成' } : null,
- )
+ setSyncProgress((prev) => ({
+ ...(prev ?? {}),
+ stage: 'completed',
+ message: '同步完成',
+ }))
// 等待用户点击关闭或自动关闭逻辑
// 这里我们不自动跳转，让用户看到结果
if (!id) return
// 如果需要自动跳转，可以在 onClose 里处理，或者这里延迟处理
},
onError: (error) => {
- setSyncProgress((prev) =>
- prev
- ? {
- ...prev,
- stage: 'error',
- message: `同步失败: ${error.message}`,
- }
- : null,
- )
+ setSyncProgress((prev) => ({
+ ...(prev ?? {}),
+ stage: 'error',
+ message: `同步失败: ${error.message}`,
+ }))
},
},
)🤖 Prompt for AI Agents
In `@src/app/playlist/remote/favorite/`[id].tsx around lines 95 - 121, The modal
can become unclosable when sync fails before onProgress runs because
syncProgress stays null; to fix, initialize syncProgress when opening the modal
(e.g., setSyncProgress to a starting object) and add null-safe fallbacks in the
syncFavorite callbacks: in the onSuccess handler inside the syncFavorite call
ensure you setSyncProgress to a completed state even if prev is null, and in
onError setSyncProgress to an error state with the error message when prev is
null; update the logic around setIsSyncModalVisible/syncFavorite (the call that
supplies remoteSyncId, type, onProgress) so SyncProgressModal always receives a
non-null progress object (or make its dismissable/close control driven by this
fallback) to guarantee the modal can be dismissed.
SafeDep Report Summary
No dependency changes detected. Nothing to scan. This report is generated by SafeDep Github App |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/modals/playlist/FavoriteSyncProgressModal.tsx`:
- Around line 64-68: The title currently only checks for progress?.stage ===
'completed' and otherwise shows "正在同步收藏夹", so when progress?.stage === 'error'
the title is inconsistent with the error message; update the Dialog.Title
rendering in FavoriteSyncProgressModal to handle the 'error' stage explicitly
(e.g., show "同步失败" or include the error brief) when progress?.stage === 'error',
keep "同步完成" for 'completed', and "正在同步收藏夹" for the in-progress states by
branching on progress?.stage in the Dialog.Title.
🧹 Nitpick comments (1)
src/components/modals/playlist/FavoriteSyncProgressModal.tsx (1)
19-46: React StrictMode 下可能触发两次同步请求。在 React 18+ 的 StrictMode 开发模式下，
useEffect会执行两次。虽然syncFacade有防重入保护（syncingIds），但第二次调用会返回错误，导致 modal 显示 "同步失败" 的错误信息，影响开发体验。考虑添加 cleanup 或使用 ref 来防止重复调用：
♻️ 建议的修改
+ const hasSyncStarted = useRef(false) + // Auto-start sync on mount useEffect(() => { + if (hasSyncStarted.current) return + hasSyncStarted.current = true + syncFavorite( { remoteSyncId: favoriteId, type: 'favorite', onProgress: setProgress, }, // ... ) }, [favoriteId, syncFavorite])
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.
Implements sync progress modal and reverts previous faulty commit.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.