Repository navigation
feat: system and workspace homepage permission - #7867
Merged
Merged
Conversation
shaohuzhang1
commented
Nov 12, 2026
| <el-button v-if="canExport" plain :loading="exporting" :disabled="loading" @click="handleExport"> | ||
| <MkIcon name="icon_export_outlined" /> | ||
| <span>导出</span> | ||
| </el-button> |
Contributor
Author
There was a problem hiding this comment.
Here is an analysis of the provided code diff, focusing on potential issues and optimization suggestions:
1. Permission Logic Duplication & Maintainability
- Issue: The permission checks
perm.system.homepage.export()andperm.homepage.homepage.export()are hardcoded in a ternary operator within the component. - Risk: If more scopes or distinct permission paths are added later, this
computedproperty will become cluttered and harder to maintain. - Suggestion: Consider extracting the permission resolution logic into a centralized utility or a composable (e.g.,
useExportPermission(scope: string)) if this pattern repeats across components. Alternatively, ensure yourpermsobject structure supports a unified method likeperm.getExport(scope)to simplify the call.
2. Reactive Dependency on route.meta
- Context:
routeis obtained viauseRoute(). In Vue Router 4,useRoute()returns a reactive reference. Accessingroute.meta.scopeinside acomputedproperty is correctly reactive; it will re-evaluate when the route meta changes. - Note: Ensure that
route.meta.scopeis always defined as'system'for system home pages. Ifscopecan beundefined,isSystemHomewill evaluate tofalsecorrectly, which is safe but worth verifying against your router configuration.
3. Redundant Boolean Conversion
- Code:
Boolean(perm.system.homepage.export()) - Optimization: In JavaScript/TypeScript, most permission functions already return a boolean or a truthy/falsy value. Wrapping them in
Boolean()is redundant unless you have a specific reason to coerce non-boolean values (like strings or empty arrays). It adds visual noise without significant functional benefit. You can likely simplify to:const canExport = computed(() => isSystemHome.value ? perm.system.homepage.export() : perm.homepage.homepage.export(), )
4. Vue Composition API Best Practice
- Positive: Using
computedforcanExportandisSystemHomeis correct. It ensures these values are only recalculated when their dependencies (route.meta.scope) change. - Import Clarity:
import { computed } from 'vue'was added. Ensure you aren't duplicating imports ifcomputedwas already imported elsewhere in the<script setup>block (common in Vue 3 + Vite with auto-imports, but explicit is fine too).
5. UI Accessibility & UX
- Observation: The export button is now conditionally rendered with
v-if="canExport". - Suggestion: Depending on your design guidelines, consider using
v-disabled(if available in your UI library) or disabling the button with a tooltip instead of hiding it entirely. This provides better feedback to users who know they should have access but lack permissions, rather than making the feature seem "missing." However,v-ifis perfectly valid if the feature should be strictly invisible to unauthorized users.
Final Recommendation Code Snippet
If you accept the optimization in point #3:
const route = useRoute();
const isSystemHome = computed(() => route.meta.scope === 'system');
// Simplified permission check
const canExport = computed(() =>
isSystemHome.value
? perm.system.homepage.export()
: perm.homepage.homepage.export(),
);Verdict: The code is functionally sound and follows Vue 3 Composition API patterns. The primary improvement is reducing boilerplate in the permission check and considering future scalability for permission logic.
shaohuzhang1
commented
Nov 12, 2026
| */ | ||
| edit: () => can(P.HOMEPAGE_EXPORT) | ||
| export: () => can(P.HOMEPAGE_EXPORT) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
The primary irregularity is a mismatch between the code change and the comment.
- Comment Inaccuracy: The comment
工作空间「概览」导出权限translates to "Workspace 'Overview' Export Permission". However, the property key was changed fromedittoexport, which aligns with the comment, but it suggests that the comment itself may be outdated if the function's purpose has shifted. More likely, the comment accurately describes the intent, and the key rename fixes a logical consistency issue (the code now correctly exposes anexportpermission). - Potential Naming Conflict: If this object is merged with others or used in a context where
exportis a reserved keyword (though unlikely in standard JS objects as it's not a reserved word for property names), consider context. However,exportis valid as a property name. - Type Checking: Ensure
P.HOMEPAGE_EXPORTis consistently defined and thatexportis recognized elsewhere in the application logic (e.g., in permission checks, UI components, or router guards) as the new expected key. If the previous keyeditwas referenced elsewhere, those references will break.
Optimization/Recommendation:
- Verify all consumers of this permission object use the new
exportkey instead ofedit. - Update any documentation or tests referencing the old
editkey for this specific permission. - Consider if other permissions in this module follow a consistent naming convention (e.g.,
view,edit,delete,export) and ensure full consistency.
peslyn
deleted the
pr@v3@feat_system_and_workspace_homepage_permission
branch
November 12, 2026 08:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: system and workspace homepage permission