Skip to content

feat: system and workspace homepage permission #7867

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

feat: system and workspace homepage permission #7867

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
2 changes: 1 addition & 1 deletion ui/src/permission/modules/homepage/homepage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ export default {
/**
* 工作空间「概览」导出权限
*/
edit: () => can(P.HOMEPAGE_EXPORT)
export: () => can(P.HOMEPAGE_EXPORT)
}

@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.

The primary irregularity is a mismatch between the code change and the comment.

  1. Comment Inaccuracy: The comment 工作空间「概览」导出权限 translates to "Workspace 'Overview' Export Permission". However, the property key was changed from edit to export, 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 an export permission).
  2. Potential Naming Conflict: If this object is merged with others or used in a context where export is a reserved keyword (though unlikely in standard JS objects as it's not a reserved word for property names), consider context. However, export is valid as a property name.
  3. Type Checking: Ensure P.HOMEPAGE_EXPORT is consistently defined and that export is recognized elsewhere in the application logic (e.g., in permission checks, UI components, or router guards) as the new expected key. If the previous key edit was referenced elsewhere, those references will break.

Optimization/Recommendation:

  • Verify all consumers of this permission object use the new export key instead of edit.
  • Update any documentation or tests referencing the old edit key 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.

13 changes: 12 additions & 1 deletion ui/src/views/home/components/ranking/RankingDrawer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ import type HomepageApi from '@/api/admin/workspace/homepage'
import MkDateRange from '@/components/mk-date-range/index.vue'
import type { MkDateRangeValue } from '@/components/mk-date-range/types'
import { numberFormat, formatTokenNumber } from '@/utils/number'
import { computed } from 'vue'
import { useRoute } from 'vue-router'
import perm from '@/permission'

const route = useRoute()
const isSystemHome = computed(() => route.meta.scope === 'system')

/* 排行榜导出权限：系统概览走系统首页权限，工作空间概览走概览权限 */
const canExport = computed(() =>
isSystemHome.value ? Boolean(perm.system.homepage.export()) : Boolean(perm.homepage.homepage.export()),
)

const props = defineProps<{ api: typeof HomepageApi; workspaceId: string; initialKind: HomeRankingKind; initialRange: HomeDateRange }>()

Expand Down Expand Up @@ -92,7 +103,7 @@ onMounted(() => loadRanking())
<MkSearchInput v-model="searchName" class="w-55!" @change="handleSearch" />
</div>
<!-- 导出当前筛选下的完整排行榜 -->
<el-button plain :loading="exporting" :disabled="loading" @click="handleExport">
<el-button v-if="canExport" plain :loading="exporting" :disabled="loading" @click="handleExport">
<MkIcon name="icon_export_outlined" />
<span>导出</span>
</el-button>

@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 provided code diff, focusing on potential issues and optimization suggestions:

1. Permission Logic Duplication & Maintainability

  • Issue: The permission checks perm.system.homepage.export() and perm.homepage.homepage.export() are hardcoded in a ternary operator within the component.
  • Risk: If more scopes or distinct permission paths are added later, this computed property 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 your perms object structure supports a unified method like perm.getExport(scope) to simplify the call.

2. Reactive Dependency on route.meta

  • Context: route is obtained via useRoute(). In Vue Router 4, useRoute() returns a reactive reference. Accessing route.meta.scope inside a computed property is correctly reactive; it will re-evaluate when the route meta changes.
  • Note: Ensure that route.meta.scope is always defined as 'system' for system home pages. If scope can be undefined, isSystemHome will evaluate to false correctly, 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 computed for canExport and isSystemHome is 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 if computed was 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-if is 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.

Expand Down
Loading