Skip to content

fix: Share chat record link #4850

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
zhanweizhang7 merged 1 commit into from
Mar 9, 2026
Merged

fix: Share chat record link #4850

Show file tree
Hide file tree
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
29 changes: 18 additions & 11 deletions ui/src/components/ai-chat/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
<el-checkbox-group v-model="multipleSelectionChat" @change="handleCheckedChatChange">
<template v-for="(item, index) in chatList" :key="index">
<div class="flex-between w-full">
<el-checkbox :value="item.id" v-if="selection" />
<el-checkbox :value="item.record_id" v-if="selection" />
<div
class="w-full border-r-8"
:class="selection ? 'is-selected p-12 mt-8 mb-8 cursor' : 'mt-24'"
@click="toggleSelect(item.id)"
@click="toggleSelect(item.record_id)"
>
<!-- 问题 -->
<QuestionContent
Expand All @@ -59,11 +59,11 @@
</div>
</div>
<div class="flex align-center w-full">
<el-checkbox :value="item.id" v-if="selection" />
<el-checkbox :value="item.record_id" v-if="selection" />
<div
class="w-full border-r-8"
:class="selection ? 'is-selected p-12 cursor' : ''"
@click="toggleSelect(item.id)"
@click="toggleSelect(item.record_id)"
>
<!-- 回答 -->
<AnswerContent
Expand Down Expand Up @@ -304,28 +304,35 @@ watch(
},
)

// 选择对话分享
const checkAll = ref(false)
const multipleSelectionChat = ref<any[]>([])
const shareLoading = ref(false)

watch(
() => props.selection,
(value) => {
if (value) {
if (value && multipleSelectionChat.value.length === 0) {
multipleSelectionChat.value = chatList.value.map((v) => v.id)
multipleSelectionChat.value = chatList.value.map((v) => v.record_id)
checkAll.value = true
}
} else {
checkAll.value = false
multipleSelectionChat.value = []
}
},
{
immediate: true,
},
)

// 选择对话分享
const checkAll = ref(false)
const multipleSelectionChat = ref<any[]>([])
const shareLoading = ref(false)
function shareChatHandle() {
const validIds = new Set(chatList.value.map((v) => v.record_id))
const selectedIds = multipleSelectionChat.value.filter((id) => validIds.has(id))

const obj = {
chat_record_ids: multipleSelectionChat.value,
chat_record_ids: selectedIds,
is_current_all: checkAll.value,
}
chatAPI.postShareChat(id || props.appId, chartOpenId.value, obj, shareLoading).then((res) => {
Expand All @@ -336,7 +343,7 @@ function shareChatHandle() {
}

const handleCheckAllChange = (val: CheckboxValueType) => {
multipleSelectionChat.value = val ? chatList.value.map((v) => v.id) : []
multipleSelectionChat.value = val ? chatList.value.map((v) => v.record_id) : []
checkAll.value = val as boolean
}
const handleCheckedChatChange = (value: CheckboxValueType[]) => {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Mar 9, 2026

Choose a reason for hiding this comment

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

The provided code appears to be functional but has some inconsistencies and could be optimized in several ways:

Inconsistencies:

  1. Checkbox Value Binding: The el-checkbox component's :value attribute is bound to different properties (item.id, item.record_id, etc.). This should be consistent across all uses of the checkbox.
  2. Event Handling: There is a logical error where checking one item does not affect other items that are already checked with selection.
  3. Selection Management: Multiple selection handling needs improvement, especially when switching from manual selection mode to automatic selection.

Potential Issues:

  1. Security Risks: Unchecked access to record IDs can expose sensitive data.
  2. Performance: Iterating over arrays multiple times within the same event handler can impact performance.

Suggestions for Optimization and Improvement:

  1. Consistent Checkboxes:
<template v-for="(item, index) in chatList" :key="index">
  <div class="flex-between w-full">
    <el-checkbox :value="item.record_id" v-if="selection" />
  1. Refactor Selection Logic:
watch(
  () => props.selection,
  (newVal, oldVal) => {
    if (oldVal && !newVal) {
      // Clear existing selections on deselection
      // ...
    } else if (!oldVal && newVal) {
      // Initialize and select all items on first selection
      [...multipleSelectionChat.value] = chatList.value.map((v) => v.record_id);
      checkAll.value = true;
    }
    // Keep track of shared status separately from selected states
    const isValidId = new Set(chatList.value.map((v) => v.record_id));
    multipleSelectionChat.value.forEach((id) => {
      if (!isValidId.has(id)) {
        multipleSelectionChat.value.splice(multipleSelectionChat.value.indexOf(id), 1);
      }
    });
  },
  { deep: true, immediate: true },  
),
  1. Optimize Share Functionality:
    Avoid unnecessary operations like filtering during each click update. Instead, prepare the list once per change and then use it for both display and sharing:
function shareChatHandle() {
  const idsToShare = multipleSelectionChat.value.filter(validRecordIds.has); // Avoid recalculating validIDs every time
  const payload = {
    chat_record_ids: idsToShare,
    is_current_all: checkAll.value,
  };
  // Proceed with network request as before
}

Additionally, consider debouncing API calls for better responsiveness or optimizing bulk uploads.

Overall, ensure consistency in how checkboxes manage their values and improve logic around managing selection state to avoid redundant checks unnecessarily affecting UI updates.

Expand Down
2 changes: 2 additions & 0 deletions ui/src/views/chat/pc/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ function handleScroll(event: any) {
}

function newChat() {
showSelection.value = false
if (!chatLogData.value.some((v) => v.id === 'new')) {
paginationConfig.value.current_page = 1
paginationConfig.value.total = 0
Expand Down Expand Up @@ -458,6 +459,7 @@ function getChatRecord() {

const clickListHandle = (item: any) => {
if (item.id !== currentChatId.value) {
showSelection.value = false
paginationConfig.value.current_page = 1
paginationConfig.value.total = 0
currentRecordList.value = []
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Mar 9, 2026

Choose a reason for hiding this comment

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

The provided code snippet appears to be part of an application that handles chat logging and navigation. Here are some potential issues, optimizations, and comments:

Potential Issues:

  1. Redundant showSelection.value = false: The line showSelection.value = false is redundant because it's already set at the beginning of both functions (handleScroll and getChatRecord). This can be removed.

  2. Unnecessary Resetting of paginationConfig: If the condition if (!chatLogData.value.some((v) => v.id === 'new')) always results in setting paginationConfig.current_page and paginationConfig.total, you might want to optimize this logic.

  3. Logic for Click List Handling: It’s important to ensure that when a different chat is selected, the previous state is reset properly, especially if there are side effects associated with showing selection or navigating records.

Optimization Suggestions:

  1. Single Condition Check: Instead of checking multiple conditions in consecutive lines, consider consolidating them into fewer operations. For example, after resetting paginationConfig, you could directly use the logical OR operator to simplify the loop:

    if (condition && !chatLogData.value.some((v) => v.id === 'new') || otherCondition) {
    // do something
}

  2. Avoid Redundant Initialization: Once certain configurations have been initialized once within a function, ensure they are not re-initialized unless absolutely necessary to avoid unnecessary processing.

  3. Refactor Logic: Consider refactoring repeated logic across similar methods such as handleScroll, especially regarding initializing state variables before primary business logic kicks off.

Here’s a refined version of your code based on these suggestions:

function handleScroll(event: any): void {
    // Existing logic...
}

function newChat(): void {
    if (!chatLogData.value.some((v) => v.id === 'new')) {
        paginationConfig.value.current_page = 1;
        paginationConfig.value.total = 0;
        currentRecordList.value = [];
    } else if (otherConditionMet()) {  // Example of another conditional case where config needs updating
        updatePagination();  // Implement logic here to update configs safely
    }

    // More scrolling-related initialization and processing...
}

function getChatRecord(item: any): void {
    // Existing logic...

    if (item.id !== currentChatId.value) {
        updatePagination();
        resetRecordsState();
    }

    // Continue with fetching chat record...
}

// Helper function to update only needed parts of the configuration without overwriting everything
const updatePagination = () => {
    paginationConfig.value.current_page = 1;
    paginationConfig.value.total = 0;
};

// Helper function to reset specific states of the record list related state
const resetRecordsState = (): void => {
    currentRecordList.value = [];  // Clearing the record list
};

In summary, the main focus should be minimizing duplicated setup calls and keeping logic clean while efficiently managing changes in user interactions like switching between chats.

Expand Down