Skip to content

fix: overview perm #7850

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
wangdan-fit2cloud merged 1 commit into from
Nov 12, 2026
Merged

fix: overview perm #7850

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
17 changes: 11 additions & 6 deletions ui/src/views/home/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useRoute } from 'vue-router'
import HomepageApi from '@/api/admin/workspace/homepage'
import ApplicationApi from '@/api/admin/workspace/application/application'
import type { ApplicationDetail } from '@/api/types'
import perm from '@/permission'
import HomeQuickCreate from './components/HomeQuickCreate.vue'
import HomeResourceOverview from './components/HomeResourceOverview.vue'
import HomeStatistics from './components/HomeStatistics.vue'
Expand All @@ -13,24 +14,26 @@ const workspaceId = computed(() => String(route.params.workspaceId))

/* 监控智能体筛选 */
const applicationId = ref('all')
const applications = ref<ApplicationDetail[]>([])
const applicationsList = ref<ApplicationDetail[]>([])
const applicationsLoading = ref(false)
const selectedApplication = ref<ApplicationDetail>()
function loadApplications(query = '') {
if (!perm.application.workspace.isShare()) return
applicationsLoading.value = true
return ApplicationApi.getApplicationPage({ currentPage: 1, pageSize: 200 }, { name: query })
.then((result) => {
applications.value = result.records
applicationsList.value = result.records
})
.finally(() => {
applicationsLoading.value = false
})
}
function handleApplicationChange(value: string) {
selectedApplication.value = applications.value.find((application) => application.id === value)
function handleApplicationChange(value: string | undefined) {
applicationId.value = value || 'all'
selectedApplication.value = applicationsList.value.find((application) => application.id === applicationId.value)
}
onMounted(() => {
void loadApplications()
// void loadApplications()
})
</script>
<template>
Expand All @@ -43,8 +46,10 @@ onMounted(() => {
<!-- 监控 -->
<HomeStatistics class="mt-4" :workspace-id="workspaceId" :application-id="applicationId" :api="HomepageApi">
<template #application="{ loading }">
<!-- 筛选智能体，清除后恢复全部智能体 -->
<el-select
v-model="applicationId"
clearable
filterable
remote
fit-input-width
Expand All @@ -60,7 +65,7 @@ onMounted(() => {
<span>全部智能体</span>
</div>
</el-option>
<template v-for="application in applications" :key="application.id">
<template v-for="application in applicationsList" :key="application.id">
<el-option :label="application.name" :value="application.id">
<div class="flex-align-center gap-2">
<ApplicationIcon :icon="application.icon" :size="20" class="shrink-0" />

@shaohuzhang1 shaohuzhang1 Nov 12, 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.

Here is an analysis of the code changes, highlighting potential issues, irregularities, and optimization suggestions:

1. Critical Issue: Broken Initialization Logic

  • Observation: The call void loadApplications() inside onMounted has been commented out. Consequently, applicationsList will remain empty ([]) on component mount.
  • Impact: The dropdown for "Monitoring Agent Filtering" will show no options initially. Users cannot select any agent until they manually trigger a remote search (if remote-filterable allows it without initial data) or unless other lifecycle hooks are missing from the snippet.
  • Suggestion: Un-comment the line or implement an alternative initialization strategy if the permission check prevents immediate loading. If the intention is to only load after permission check or user interaction, ensure the UX supports an empty state gracefully.

2. Security & Permission Check Side Effects

  • Observation: perm.application.workspace.isShare() is added inside loadApplications.
  • Issue: loadApplications returns early if permission is denied. However, applicationsLoading is set to false only in .finally(), which is never reached due to the return statement. While technically safe (stays false), it lacks explicit clarity. More importantly, if this function is called multiple times concurrently, there's no cancellation logic for the API request.
  • Optimization: 
    function loadApplications(query = '') {
  if (!perm.application.workspace.isShare()) return; 
  applicationsLoading.value = true;
  
  // Consider using abort signals if switching routes/filters rapidly
  return ApplicationApi.getApplicationPage({ currentPage: 1, pageSize: 200 }, { name: query })
    .then((result) => {
      // Prevent overwriting with stale data if multiple requests are in flight
      applicationsList.value = result.records; 
    })
    .catch((err) => {
       console.error("Failed to load applications", err);
       // Optionally clear list or show error
    })
    .finally(() => {
      applicationsLoading.value = false;
    });
}

3. Variable Naming Consistency

  • Observation: applications -> applicationsList.
  • Comment: Good refactor. applicationsList better reflects that it holds a list, distinguishing it from potentially singular concepts or API responses. Maintain consistency across the template and script.

4. Type Safety in handleApplicationChange

  • Observation: Parameter changed from value: string to value: string | undefined. This correctly handles the clearable behavior of el-select.
  • Logic Check: 
    applicationId.value = value || 'all'; // Sets to 'all' if cleared
selectedApplication.value = applicationsList.value.find(...) // Will find 'all'? No.
    • Potential Bug: selectedApplication.value will become undefined when cleared because no application ID matches 'all'. This is likely intended behavior (resetting selected item), but ensure downstream components relying on selectedApplication can handle undefined.

5. Template Optimization (el-option rendering)

  • Observation: The loop changed to v-for="application in applicationsList".
  • Performance Note: Since filterable and remote are used, ensure that displaying all records from the page API is acceptable UX. If the dataset grows large beyond 200 items, consider using virtual scrolling or ensuring pagination logic aligns with user expectations.
  • Small Improvement: In <template v-for="...">, Vue 3 recommends placing :key on the same element as the v-for directive or inside the slot content directly within a fragment wrapper. Placing key on <template> is valid and recommended.

6. Missing Dependency Consideration

  • The loadApplications function depends on external state/APIs. It’s good practice to debounce rapid successive calls to ApplicationApi.getApplicationPage during typing/filtering to avoid excess network traffic.

Recommended Final Fixes Code Snippet:

 <script setup>
 // Existing imports...
 import perm from '@/permission'
 
-const applications = ref<ApplicationDetail[]>([])
+const applicationsList = ref<ApplicationDetail[]>([])

@@ -13,24 +14,26 @@ const workspaceId = computed(() => String(route.params.workspaceId))
 
 /* 监控智能体筛选 */
 const applicationId = ref('all')
-const applications = ref<ApplicationDetail[]>([])
+const applicationsList = ref<ApplicationDetail[]>([])
 const applicationsLoading = ref(false)
 const selectedApplication = ref<ApplicationDetail>()
 function loadApplications(query = '') {
   if (!perm.application.workspace.isShare()) return;
   
   applicationsLoading.value = true;
-  
-  return ApplicationApi.getApplicationPage({ currentPage: 1, pageSize: 200 }, { name: query })
+  const requestParams = { currentPage: 1, pageSize: 200 };
+  
+  return ApplicationApi.getApplicationPage(requestParams, { name: query })
     .then((result) => {
       applicationsList.value = result.records;
     })
     .catch(err => {
       console.error("[HomeStatistics] Failed to fetch apps:", err);
     })
     .finally(() => {
       applicationsLoading.value = false;
     });
 }
 function handleApplicationChange(value: string | undefined) {
   // Handle clearing selection
   if (!value) {
       applicationId.value = 'all';
       selectedApplication.value = undefined;
       return;
   }
   
   applicationId.value = value;
   selectedApplication.value = applicationsList.value.find(
       (application) => application.id === value
   );
 }
 
 onMounted(() => {
-  // void loadApplications()
+  // Re-enable or conditionally enable initial load based on requirements
+  // If load is needed on mount despite permission checks:
+  if (perm.application.workspace.isShare()) {
+      void loadApplications(); 
+  }
 })
 </script>
 
 <template>
@@ -43,8 +46,10 @@ onMounted(() => {
     <!-- 监控 -->
     <HomeStatistics class="mt-4" :workspace-id="workspaceId" :application-id="applicationId" :api="HomepageApi">
       <template #application="{ loading }">
-        <el-select
+        <!-- Note: Added placeholder if needed -->
+        <el-select
           v-model="applicationId"
+          :loading="applicationsLoading" 
           clearable
           filterable
-          remote
+          remote
           fit-input-width
           @change="handleApplicationChange"
+          :remote-method="loadApplications" <!-- Ensure remote-method is linked if needed -->
         >

Expand Down
44 changes: 19 additions & 25 deletions ui/src/views/system/home/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import SystemApplicationApi from '@/api/admin/system/resource-management/applica
import type { ApplicationDetail, WorkspaceItem } from '@/api/types'
import WorkspaceDropdown from '@/components/business/workspace-dropdown/index.vue'
import { useStore } from '@/stores'
import perm from '@/permission'
import HomeResourceOverview from '@/views/home/components/HomeResourceOverview.vue'
import HomeStatistics from '@/views/home/components/HomeStatistics.vue'
import HomeRankings from '@/views/home/components/ranking/HomeRankings.vue'
Expand All @@ -30,39 +31,31 @@ function handleWorkspaceChange(workspaceId: string) {
if (workspaceId === selectedWorkspaceId.value) return
selectedWorkspaceId.value = workspaceId
applicationId.value = 'all'
applications.value = []
applicationsList.value = []
selectedApplication.value = undefined
}

/* 监控智能体筛选 */
const applicationId = ref('all')
const applications = ref<ApplicationDetail[]>([])
const applicationsList = ref<ApplicationDetail[]>([])
const applicationsLoading = ref(false)
const selectedApplication = ref<ApplicationDetail>()
async function fetchApplications(query: string) {
function loadApplications(query = '') {
if (!perm.application.system.read) return
applicationsLoading.value = true
const workspaceFilter = selectedWorkspaceId.value ? { workspace_ids: JSON.stringify([selectedWorkspaceId.value]) } : {}
try {
const result = await SystemApplicationApi.getApplicationPage({ currentPage: 1, pageSize: 200 }, { name: query, ...workspaceFilter })
applications.value = result.records
} finally {
applicationsLoading.value = false
}
}
/* 远程搜索防抖，避免输入时每次按键都发请求 */
let searchTimer: ReturnType<typeof setTimeout> | undefined
function loadApplications(query = '') {
clearTimeout(searchTimer)
searchTimer = setTimeout(() => fetchApplications(query), 300)
return SystemApplicationApi.getApplicationPage({ currentPage: 1, pageSize: 200 }, { name: query, ...workspaceFilter })
.then((result) => {
applicationsList.value = result.records
})
.finally(() => {
applicationsLoading.value = false
})
}
/* 打开下拉时才按需加载智能体列表，避免进入页面就全量拉取 */
function handleApplicationVisibleChange(visible: boolean) {
if (visible && !applicationsLoading.value && !applications.value.length) {
loadApplications('')
}
}
function handleApplicationChange(value: string) {
selectedApplication.value = applications.value.find((application) => application.id === value)

function handleApplicationChange(value: string | undefined) {
applicationId.value = value || 'all'
selectedApplication.value = applicationsList.value.find((application) => application.id === applicationId.value)
}

onMounted(() => {
Expand All @@ -88,8 +81,10 @@ onMounted(() => {
:api="SystemHomepageApi"
>
<template #application="{ loading }">
<!-- 筛选智能体，清除后恢复全部智能体 -->
<el-select
v-model="applicationId"
clearable
filterable
remote
fit-input-width
Expand All @@ -98,15 +93,14 @@ onMounted(() => {
:loading="applicationsLoading"
:disabled="loading"
@change="handleApplicationChange"
@visible-change="handleApplicationVisibleChange"
>
<el-option label="全部智能体" value="all">
<div class="flex-align-center gap-2">
<MkIcon name="icon_card_outlined" :size="20" class="shrink-0 text-N600" />
<span>全部智能体</span>
</div>
</el-option>
<template v-for="application in applications" :key="application.id">
<template v-for="application in applicationsList" :key="application.id">
<el-option :label="application.name" :value="application.id">
<div class="flex-align-center gap-2">
<ApplicationIcon :icon="application.icon" :size="20" class="shrink-0" />
Expand Down
Loading