Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ui/src/views/system/shared-resources/model/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ onMounted(() => {
:model="model"
/>
<!-- 授权工作空间 -->
<AuthorizedWorkspaceAction label="授权工作空间" :resource="model" :resource-type="RESOURCE_TYPE.MODEL" />
<AuthorizedWorkspaceAction v-if="perm.model.share.authToWorkspace()" label="授权工作空间" :resource="model" :resource-type="RESOURCE_TYPE.MODEL" />

<!-- 查看关联资源 -->
<RelatedResourcesModelAction v-if="perm.model.share.relateMap()" label="查看关联资源" :api="RelatedResourcesApi" :model="model" />

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 code snippet introduces a v-if directive with a method call: perm.model.share.authToWorkspace(). The primary issue here is performance and reliability.

Vue evaluates expressions in templates on every re-render. Calling a function inside v-if means the function executes repeatedly during template rendering cycles (e.g., when reactive dependencies change). This can lead to:

  1. Unnecessary Recomputation: If the permission check is expensive or involves complex logic, it degrades performance.
  2. Potential Side Effects/Inconsistency: If the function has side effects or depends on external state not tracked by Vue's reactivity system, the condition may not update correctly when the permission actually changes.

Recommendation

Move the permission check into a computed property so it is cached and only re-evaluated when its reactive dependencies change.

✅ Optimized Approach

Script Setup (<script setup>):

<script setup lang="ts">
import { computed } from 'vue'

// Assuming `model` and `perm` are reactive refs or part of reactive state
const canAuthToWorkspace = computed(() => {
  return perm.model.share.authToWorkspace()
})
</script>

Template:

<AuthorizedWorkspaceAction 
  v-if="canAuthToWorkspace" 
  label="授权工作空间" 
  :resource="model" 
  :resource-type="RESOURCE_TYPE.MODEL" 
/>

Benefits:

  • ✅ Caching: The computed property is cached unless its dependencies change.
  • ✅ Readability: Separates business logic from template structure.
  • ✅ Maintainability: Easier to test and debug.

Note: Ensure that perm.model.share.authToWorkspace() returns a boolean and does not have side effects. If it relies on reactive state, make sure those states are properly referenced within the computed function so Vue can track them.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/system/shared-resources/tool/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ function handleOpenWorkflow(tool: ToolItem, event: MouseEvent) {
:tool="tool"
/>
<!-- 授权工作空间 -->
<AuthorizedWorkspaceAction label="授权工作空间" :resource="tool" :resource-type="RESOURCE_TYPE.TOOL" />
<AuthorizedWorkspaceAction v-if="perm.tool.share.authToWorkspace()" label="授权工作空间" :resource="tool" :resource-type="RESOURCE_TYPE.TOOL" />

<!-- 查看关联资源 -->
<RelatedResourcesToolAction

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.

Analysis of the Code Change

1. Critical Syntax Error:
The added v-if condition is syntactically invalid and will cause a compilation or runtime error:

<!-- ❌ Invalid Vue syntax -->
v-if="perm.tool.share.authToWorkspace()"

In Vue templates, you cannot call a function by name without binding it to an object that exists in the component’s scope context. perm.tool.share.authToWorkspace appears to be an undefined reference path unless perm is explicitly defined in data, setup(), or as a prop with that exact nested structure and includes a callable method at the end. Even if valid, template expressions should not execute arbitrary methods directly; they should check state (booleans/reactive values), not invoke side-effecting functions.

2. Anti-pattern: Function Call in Template
Even if authToWorkspace() were a valid reactive getter (e.g., a computed property exposed on an object), calling methods directly in v-if/v-show is discouraged when the result is static per render cycle. If it’s expensive, it will re-evaluate on every re-render. Prefer pre-computed boolean flags.

3. Semantic Clarification
The intent appears to be conditionally rendering the “Authorize Workspace” action based on user permissions. Ensure this logic aligns with your permission model (RBAC/ABAC) and doesn’t accidentally bypass server-side authorization checks.


✅ Recommended Fix

Assuming you have a reactive permission state (e.g., from a Pinia/Vuex store or injected permission service), define a computed property or use an existing boolean flag. Here’s how to fix it properly:

Option A: Use a Computed Property (Vue 3 <script setup>)

<script setup lang="ts">
import { computed } from 'vue';
import { usePermissionStore } from '@/stores/permission'; // Example store

const permissionStore = usePermissionStore();

// Replace with actual logic returning a boolean
const canShareToolToWorkspace = computed(() => {
  return permissionStore.hasPermission('tool.share.authToWorkspace');
});
</script>

<template>
  ...
  <!-- ✅ Correct approach -->
  <AuthorizedWorkspaceAction 
    v-if="canShareToolToWorkspace" 
    label="授权工作空间" 
    :resource="tool" 
    :resource-type="RESOURCE_TYPE.TOOL" 
  />
</template>

Option B: If perm Is an Injected Permission Utility

If perm is a global/injected permission helper with boolean getters (not methods):

<!-- ✅ Only if authToWorkspace is a boolean/computed ref, NOT a function -->
v-if="perm.tool.share.authToWorkspace"

🔍 Checklist Before Shipping

  • Confirm whether authToWorkspace is a boolean state or a function. If it’s a function, move the invocation into a computed property in script.
  • Verify that hiding the UI element does not replace backend authorization (never trust frontend-only checks).
  • Ensure translation consistency for label.

Expand Down
Loading