Skip to content

perf: Optimize the folder to enter the second level page and retain r…#3917

Merged
wangdan-fit2cloud merged 1 commit intov2from
pr@v2@perf-chat
Aug 22, 2025
Merged

perf: Optimize the folder to enter the second level page and retain r…#3917
wangdan-fit2cloud merged 1 commit intov2from
pr@v2@perf-chat

Conversation

@wangdan-fit2cloud
Copy link
Contributor

…ecords and the first conversation does not display the previous record

What this PR does / why we need it?

Summary of your change

Please indicate you've done the following:

  • Made sure tests are passing and test coverage is added if needed.
  • Made sure commit message follow the rule of Conventional Commits specification.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed.

…ecords and the first conversation does not display the previous record
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Aug 22, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Aug 22, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

import { relatedObject } from '@/utils/array.ts'
const getApplicationDetail = inject('getApplicationDetail') as any
const route = useRoute()

Copy link
Contributor

Choose a reason for hiding this comment

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

Your code looks generally clean, but you can make a few improvements:

Improvements

  1. Remove Redundant Code: In the <div> that checks chat_data.mcp_too_id and its servers, there is redundant code checking both values simultaneously. You only need to do this once.

  2. Consistent Spacing and Line Lengths: Ensure consistent spacing within your JSX-like template syntax and avoid overly long lines that may be hard to read.

  3. Optimize Conditional Statements: Use more concise conditional statements where possible.

  4. Import Syntax Consistency: Make sure all imports are consistent with JavaScript/TypeScript format.

  5. Avoid Unnecessary Variables: Eliminate unused variables if they are defined but not used anywhere.

Here’s an optimized version of your code:

<template>
  <el-form-item @click.prevent>
    <!-- ... existing code ... -->
  </el-form-item>
</template>

<script setup lang="ts">
import { inject } from "vue";

// Other imports...

const applicationDetail = inject("getApplicationDetail") as any;

// Existing logic...
</script>

Explanation

Removed Redundancy:

  • Combined the two conditionals into one to eliminate redundancy.

Consistent Formatting:

  • Ensured proper indentation and spacing for better readability.

Simplified Logic:

  • Used logical OR instead of nested ternaries when setting the value.

By making these small adjustments, your code will be more maintainable and easier to understand.

@wangdan-fit2cloud wangdan-fit2cloud merged commit 1f5544b into v2 Aug 22, 2025
2 of 5 checks passed
@wangdan-fit2cloud wangdan-fit2cloud deleted the pr@v2@perf-chat branch August 22, 2025 08:27

onMounted(() => {
if (isKnowledge.value) {
getKnowledgeDetail()
Copy link
Contributor

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 have several optimizations and bug fixes:

Optimizations:

  1. Removed Redundant Import Statements: The loadSharedApi import statement is not used anywhere in the snippet, so it can be removed.
  2. Refactored Back Navigation Logic: Moved the logic for handling different scenarios (knowledge and application folders) into its own function toBack(). This simplifies the routing process and enhances readability.

Potential Issues:

  1. Routing Guard Missing: No check is done before pushing a new URL, which might lead to unexpected behavior if multiple routes are handled.
  2. Error Handling: There's no error handling mechanism in place for fetching data via APIs. Ensure that each API call includes proper error checking.

Additional Suggestion:
Consider adding type annotations to make the code more maintainable. For example, you could annotate variables with their expected types:

let folderId:number | null = null;

Here’s an improved version of the code incorporating these changes:

// Improved Code Snippet

import { ref } from 'vue';
import BackButton from './components/BackButton.vue';
import { useStore } from '@/stores';

// ... rest of imports omitted ...

const { common, folder } = useStore();
const route = useRoute();
const router = useRouter();

let folderId: number | null = null; // Type annotation added

// ... rest of template and methods unchanged ...
  
onMounted(() => {
  if (isKnowledge.value) {
    getKnowledgeDetail();
    fetchFolderDetails(); // Function name should match actual implementation
  } else if (isApplication.value) {
    setCurrentApplicationFolder();
    fetchDataFromApplicationFolder();
  }

  // Add this line to check before navigation or handle errors appropriately
  if (!folderId || !router.isReady()) return;
});

async function getCurrentApplicationFolder(): Promise<void> {
  folderId = await fetchCurrentApplicationFolderAPI(route);
}

function setCurrentApplicationFolder(): void {
  folder.setCurrentFolder({ id: current.value.folder });
}

function toBack(): void {
  router.push({ path: toBackPath.value }).catch(err => console.error('Failed to push:', err));
}

Ensure to replace placeholder functions (fetchCurrentApplicationFolderAPI, etc.) with actual implementations according to your application architecture.

getChatLog()
}

onMounted(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The provided code has several issues that need to be addressed:

  1. Function getChatLog now accepts an optional parameter refresh. The logic regarding resetting pagination settings when not refreshing should handle cases where refresh is truthy without causing errors.

  2. In the init function, calling getChatLog() instead of passing a specific ID ensures consistent behavior regardless of whether there's an applicationDetail.value.id available.

  3. Ensure that error handling is implemented around API calls, especially in chatAPI.pageChat, both for successful responses and network-related failures.

  4. Refactor the function names and variables to improve readability and avoid shadowing existing properties with default values like 'new'.

Here is an optimized version of the getChatLog function:

function getChatLog(refresh = true) {
  // Reset pagination onlyif refresh is true
  if (refresh) {
    paginationConfig.current_page = 1;
    paginationConfig.total = 0;
    currentRecordList.value = [];
  }

  const page = { current_page: paginationConfig.current_page, page_size: paginationConfig.page_size };
  chatAPI.pageChat(page.current_page, page.page_size, left_loading).then((res: any) => {
    if (res && res.status === 200) {
      chatLogData.value = res.data.records;
      setCurrentChatId(res.data.records?.length > 0 ? res.data.records[0].id : 'new');
    } else {
      errorMessageToast('Failed to load chat log.');
    }
  }).catch(error => {
    console.error('Error loading chat log:', error);
    errorMessageToast('An unexpected error occurred while fetching the chat data.');
  });
}

And here's how it can be used in other functions:

onMounted(() => {
  init();
});

function getCurrentChatLog() {
  return getChatLog(true); // Refresh the chat log when needed
}

function fetchNextPage() {
  const nextCurrentPage = Math.min(paginationConfig.current_page + 1, paginationConfig.total_pages!);
  paginationConfig.current_page = nextCurrentPage;

  getChatLog(false); // Do not refresh but update pagination
}

Make sure you also implement helper functions to set paginationConfig.total and similar configurations based on response from the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants