Skip to content

fix: workspace application and knowledge folder create permission #7869

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

fix: workspace application and knowledge folder create permission #7869

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
4 changes: 4 additions & 0 deletions ui/src/permission/modules/application/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ const workspace = {
debug: (id: string) => canRes(P.APPLICATION_READ, id),

// —— 文件夹 ——
/**
* 工作空间「智能体」「文件夹」创建权限
*/
folderCreateWorkspace: () => can(P.APPLICATION_FOLDER_CREATE),
/**
* 工作空间「智能体」「文件夹」创建权限
*/

@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 snippet:

1. Critical Irregularity: Duplicate/Orphaned Documentation

The main issue in this patch is a mismatch between the added code and the existing (or unchanged) code context.

  • The diff adds folderCreateWorkspace: with a JSDoc comment block above it.
  • Immediately after the new line, there appears to be another JSDoc comment block (/** ... */) which seems to be either dormant, misplaced, or part of a previous version that wasn't properly cleaned up.
  • If the second comment block belongs to another property (which might be deleted or not shown), it will now erroneously apply to whatever comes next — potentially causing misleading documentation.

✅ Fix: Ensure each JSDoc comment directly precedes its corresponding property and is not duplicated or orphaned.

2. Code Style & Consistency

  • The added method folderCreateWorkspace follows the naming convention used by other methods like read, debug, etc., so it’s stylistically consistent.
  • However, note that canRes vs can:
    • read and debug use canRes(P.APPLICATION_READ, id) → checks permission for a specific resource ID.
    • New folderCreateWorkspace uses can(P.APPLICATION_FOLDER_CREATE) → checks a global/workspace-level permission without an ID.
      This is likely correct if folder creation doesn’t depend on a specific folder ID upfront, but ensure this semantic distinction is intentional.

3. Potential Optimization / Suggestion

If multiple similar “create” permissions exist (e.g., agent create, folder create), consider grouping them under a common pattern or factory function to reduce repetition:

// Optional refactoring example
const canCreate = (permissionKey: string) => () => can(permissionKey);

// Then:
agentCreate: canCreate(P.APPLICATION_AGENT_CREATE),
folderCreate: canCreate(P.APPLICATION_FOLDER_CREATE),

This avoids boilerplate if more such methods are added later.

✅ Final Recommendation

  • Remove or relocate the duplicate/orphaned JSDoc comment.
  • Verify that using can() instead of canRes() is correct for your auth model.
  • Consider extracting repeated patterns if similar methods grow in number.

Let me know if you’d like a cleaned-up version of the entire file section.

Expand Down
4 changes: 4 additions & 0 deletions ui/src/permission/modules/knowledge/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ const workspace = {
},

// —— 文件夹 ——
/**
* 工作空间「知识库」「文件夹」只读权限
*/
folderCreateWorkspace: () => can(P.KNOWLEDGE_FOLDER_CREATE),
/**
* 工作空间「知识库」「文件夹」只读权限
*/

@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 shown diff is syntactically valid, but there are a few issues and suggestions:

  1. Comment / behavior mismatch (main issue)

    • The new property is named folderCreateWorkspace and calls P.KNOWLEDGE_FOLDER_CREATE, which clearly implies create permission.
    • The comment says “只读权限” (“read-only permission”), which contradicts the code.
    • Fix: either update the comment to describe create permission, or if read-only was intended, use the correct read permission constant and rename the property.

  2. Ambiguous naming

    • folderCreateWorkspace can be read as “create a workspace folder” or as “folder-create workspace permission”.
    • Prefer a clearer boolean-permission name such as canCreateKnowledgeFolder, createKnowledgeFolder, or follow the existing naming convention in the file.
    • Prefixing permission-check methods with can... makes their boolean return value obvious.

  3. Possible copy-paste issue

    • The added comment appears to be copied from the following existing property, which also says “只读权限”.
    • This increases the chance that the new function was pasted in the wrong place or with the wrong comment. Double-check the intended permission mapping.

  4. Workspace context for can(...)

    • If can(...) depends on the currently selected workspace, make sure this method is evaluated in the correct workspace context.
    • If the object is reused across multiple workspaces, consider passing a workspace ID or binding the correct context explicitly.

  5. Type/interface updates

    • If this object is covered by TypeScript types, an interface, or a permission schema, remember to add the new property there as well.

  6. Optional optimization

    • If there are many methods of the form () => can(P.X), you could consider a small factory such as permission(P.X) to reduce repetition, but only if it improves readability.
    • Otherwise, the current inline arrow function is fine.

  7. Tests

    • Add or update a test asserting that folderCreateWorkspace() returns the expected value for users with and without P.KNOWLEDGE_FOLDER_CREATE.

Overall: the code will run, but the comment/name/permission semantics should be corrected to avoid confusion.

Expand Down
3 changes: 1 addition & 2 deletions ui/src/views/application/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import ButtonCreateApplication from './components/ButtonCreateApplication.vue'
import ButtonTemplateStore from './components/ButtonTemplateStore.vue'
import ButtonBatchCleanStrategy from './components/ButtonBatchCleanStrategy.vue'
import perm from '@/permission/index.ts'
import { getWorkspaceId } from '@/utils/resource-context.ts'

const route = useRoute()
const router = useRouter()
Expand Down Expand Up @@ -201,7 +200,7 @@ function handleBatchDelete() {
<template #aside="{ title, Header }">
<component :is="Header">
<h4>{{ title }}</h4>
<MkTooltip v-if="perm.application.workspace.folderCreate(getWorkspaceId() || '')" content="创建文件夹" placement="top">
<MkTooltip v-if="perm.application.workspace.folderCreateWorkspace()" content="创建文件夹" placement="top">
<el-button @click="handleCreateFolder" text type="primary" class="-mr-1">
<MkIcon name="icon_add-folder_outlined" :size="18" />
</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's my review of the diff:

1. Removed import — verify no remaining usages

-import { getWorkspaceId } from '@/utils/resource-context.ts'

Issue: If getWorkspaceId is still referenced anywhere else in this file (script or template), this will throw ReferenceError at runtime / fail the build. Please grep the whole file for getWorkspaceId to confirm the only usage was the one you changed.

2. Permission API change — semantics may differ

-<MkTooltip v-if="perm.application.workspace.folderCreate(getWorkspaceId() || '')" ...>
+<MkTooltip v-if="perm.application.workspace.folderCreateWorkspace()" ...>

Potential issues to verify:

  • Does folderCreateWorkspace() exist? Confirm it's actually defined in @/permission/index.ts (or the namespace it re-exports). A typo here fails silently in a v-if (just hides the button) or throws at render time.
  • Scope change: The old call checked permission for a specific workspace ID; the new one presumably resolves the workspace context internally. Confirm the two are functionally equivalent for the current route/context. If folderCreateWorkspace() resolves the workspace differently (e.g., from a store instead of the route/resource context), the button may appear/disappear incorrectly.
  • Empty-ID fallback removed: Old code did getWorkspaceId() || '' to guard against a null/empty workspace ID. Make sure folderCreateWorkspace() handles the "no workspace selected" case explicitly; otherwise a missing workspace could yield an unintended true.
  • Reactivity: getWorkspaceId() was presumably reading a reactive store/route — that dependency was tracked by Vue. If the new method no longer touches that reactive source, the v-if may not re-evaluate when the workspace changes. Verify the method internally reads the same reactive state.

3. Minor / optimizations

  • Since the v-if is on MkTooltip (which wraps the button), the whole create-folder button is hidden when the permission is false — that's the intended UX, but it means no visual hint at all. If you want a disabled state instead, consider :disabled on the el-button with a v-if on the tooltip content only.
  • Calling the permission method inline in the template runs on every re-render. For a cheap boolean this is fine, but if permission checks become non-trivial (iterating roles, etc.), consider a computed: 
    const canCreateFolder = computed(() => perm.application.workspace.folderCreateWorkspace())
    This also makes the template cleaner and easier to test.

4. Test checklist

  • No remaining getWorkspaceId references in the file (build passes).
  • folderCreateWorkspace() is exported and behaves identically to folderCreate(workspaceId) for the current workspace.
  • Button visibility updates correctly when switching workspaces / roles.
  • Behavior with no workspace selected (empty ID case) is correct.

Overall the change looks like a sensible refactor toward encapsulation, but the two biggest risks are the orphaned import and subtle behavior changes in the permission check.

Expand Down
3 changes: 1 addition & 2 deletions ui/src/views/knowledge/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
} from './knowledge-card/action-dropdown'
import MoveToDialog from '@/components/business/folder-tree/MoveToDialog.vue'
import { MsgConfirm, MsgSuccess } from '@/utils/message'
import { getWorkspaceId } from '@/utils/resource-context.ts'

/* 知识库详情入口 */
const route = useRoute()
Expand Down Expand Up @@ -208,7 +207,7 @@ function handleBatchDelete() {
<template #aside="{ title, Header }">
<component :is="Header">
<h4>{{ title }}</h4>
<MkTooltip v-if="perm.knowledge.workspace.folderCreate(getWorkspaceId() || '')" content="创建文件夹" placement="top">
<MkTooltip v-if="perm.knowledge.workspace.folderCreateWorkspace()" content="创建文件夹" placement="top">
<el-button text type="primary" class="-mr-1" @click="handleCreateFolder">
<MkIcon name="icon_add-folder_outlined" :size="18" />
</el-button>
Expand Down
Loading