Skip to content

fix: simplify chat user group deletion logic #7720

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

Merged
wxg0103 merged 1 commit into from
Nov 6, 2026
Merged

fix: simplify chat user group deletion logic #7720

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
4 changes: 1 addition & 3 deletions ui/src/views/system/chat-management/user-groups/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ function deleteGroup(group: ListItem) {
groupsLoading.value = true
return ChatGroupsApi.deleteChatUserGroup(group.id).then(() => {
MsgSuccess('删除成功')
if (currentGroup.value?.id === group.id) {
return loadChatUserGroups()
}
return loadChatUserGroups()
})
})
.catch(() => {})

@shaohuzhang1 shaohuzhang1 Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The change itself is probably an improvement: always calling loadChatUserGroups() after a successful delete avoids a stale list when a non-current group is deleted. The old code only refreshed the list when the deleted group was the current group.

However, there are several things to check/fix:

  1. currentGroup may remain stale

    • If the deleted group is the current group, you should explicitly clear or reselect currentGroup, unless loadChatUserGroups() already does that.
    • Capture the check before reloading:
    const wasCurrent = currentGroup.value?.id === group.id
await ChatGroupsApi.deleteChatUserGroup(group.id)
if (wasCurrent) currentGroup.value = null
await loadChatUserGroups()

  2. Always reloading may be unnecessary or disruptive

    • If loadChatUserGroups() resets selection, pagination, scroll position, or filters, deleting a non-current group may cause UI regressions.
    • If possible, remove the group from the local groups list and only reload when needed. Otherwise, confirm loadChatUserGroups() preserves state.

  3. Loading state is not reset on failure

    • groupsLoading.value = true is set, but there is no finally/reset in the shown code.
    • If delete or reload fails, the UI may stay stuck in a loading state.

  4. Errors are swallowed

    • .catch(() => {}) hides both delete failures and reload failures. Users get no error feedback, and callers cannot detect failure.
    • At minimum, show an error message or log it.

  5. Possible brace/parenthesis issue in the pasted snippet

    • As shown, there is an extra }) before .catch(...):
    return ChatGroupsApi.deleteChatUserGroup(group.id).then(() => {
  ...
})
})
.catch(() => {})
    • Verify the actual code. If it is a single promise chain, it should be:
    return ChatGroupsApi.deleteChatUserGroup(group.id)
  .then(() => { ... })
  .catch(() => {})

  6. Double-click / concurrent delete protection

    • Consider guarding against repeated clicks, or using a per-group deleting state.

Suggested refactor:

async function deleteGroup(group: ListItem) {
  groupsLoading.value = true
  try {
    await ChatGroupsApi.deleteChatUserGroup(group.id)
    MsgSuccess('删除成功')

    const wasCurrent = currentGroup.value?.id === group.id
    if (wasCurrent) {
      currentGroup.value = null // or select the next group
    }

    await loadChatUserGroups()
  } catch (error) {
    // MsgError('删除失败')
    console.error(error)
  } finally {
    groupsLoading.value = false
  }
}

If loadChatUserGroups() manages groupsLoading itself, use a separate flag such as deleting.value to avoid conflicting loading states.

Expand Down
Loading