[shaohuzhang1]

feat: tool import permission

[shaohuzhang1]

Here is the review of the provided code snippet, focusing on logic, type safety, and Vue best practices.

1. Logical & Safety Issues

Potential Runtime Error: Unsafe Property Access

The code assumes that perm.tool.system, perm.tool.share, and perm.tool.workspaceShared always exist and have an import method.

if (isSystemResource()) return Boolean(perm.tool.system[method]())

• Risk: If perm.tool.system is undefined or missing the method for a specific tool type, this will throw a TypeError.
• Suggestion: Add optional chaining or fallbacks.

  if (isSystemResource()) return Boolean(perm?.tool?.system?.[method]?.())

Type Safety with Computed Keys

While ToolImportPermissionMethod restricts method to 'import', accessing perm.tool.system[method] dynamically bypasses TypeScript's strict property checking unless perm is typed as an any or a very loose record.

• Suggestion: Explicitly cast or define the shape of perm to ensure the methods exist. Alternatively, since there is only one method ('import'), consider removing the parameter and hardcoding it to simplify the logic and avoid dynamic key lookup overhead/misuse.

2. Optimization Suggestions

Redundant Boolean() Wrapper

Permission functions typically return boolean. Wrapping the result in Boolean() is redundant unless you suspect they return non-boolean truthy/falsy values (like strings or numbers). If perm.tool.*.import() returns true/false or undefined, simply returning the value handles falsy checks correctly in JavaScript/TS.

• Suggestion:

  // Instead of: return Boolean(perm.tool.workspaceShared[method]())
  return !!perm.tool.workspaceShared?.[method]() // Or just return the value if strictly boolean

Function Name & Caching

The function toolImportPermission is called in the template via v-if. Vue re-evaluates v-if expressions reactively. Since isSystemResource(), etc., are likely reactive or state-dependent, this is fine. However, if perm is static, you could compute this once. But given typical Vue patterns, calling the function is acceptable.

3. Code Style & Best Practices

Use Computed Property Instead of Function Call in Template (Optional)

If the permissions do not change frequently after component mount, using a computed property can be cleaner than calling a function in the template.

<script setup lang="ts">
// ... imports ...

const canImport = computed(() => toolImportPermission('import'))
</script>

<template>
  <!-- Use v-if="canImport" -->
</template>

This separates logic from presentation slightly and makes the template cleaner.

Naming Convention

ToolImportPermissionMethod is verbose. Since it only represents 'import', consider simplifying:

type PermissionAction = 'import' // Future-proof if more actions are added

4. Refactored Code Example

Here is a safer, cleaner version:

import { isSystemResource, isSystemSharedResource, isWorkspaceSharedResource } from '@/utils/resource-context'
import perm from '@/permission/index.ts'

/**
 * Checks import permission based on resource context.
 */
function checkImportPermission(): boolean {
  // Use optional chaining to prevent crashes if perm structure varies
  if (isSystemResource()) return !!perm.tool.system?.import?.()
  if (isSystemSharedResource()) return !!perm.tool.share?.import?.()
  if (isWorkspaceSharedResource()) return !!perm.tool.workspaceShared?.import?.()
  
  return !!perm.tool.workspace?.import?.()
}

// Define as computed if used in template (recommended):
// const canImportTool = computed(checkImportPermission)

In the template:

<!-- If using function directly -->
v-if="checkImportPermission()"

<!-- Or if using computed (preferred) -->
v-if="canImportTool"

Summary of Recommendations:

1. Add Optional Chaining (?.) to all permission method calls to prevent runtime crashes.
2. Remove Boolean() wrapper if permissions return proper booleans; use !! if forcing boolean coercion is needed.
3. Consider computed for better template readability and performance caching.
4. Hardcode 'import' if no other methods are planned immediately, to reduce complexity.