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/router/ROUTE_README.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ Workspace 共享覆盖 resourceScope,保留 Workspace scope 及知识库 activ

| 菜单 | 路径与条件 |
| ---------- | --------------------------------------------------------------------------------------------------------------------- |
| 资料库 | 空 path 分组;document、image、tag;BASE 显示图片,Workspace 自有知识库的 WORKFLOW 类型也显示图片 |
| 资料库 | 空 path 分组;document、image、tag;四种资源范围均在 BASE/WORKFLOW 类型显示图片 |
| 工作流 | workflow-entry;WORKFLOW 类型 beforeEnter 重定向对应全屏画布,其他类型显示转换页 |
| 检索优化 | retrieval/hit-test、retrieval/question、retrieval/dictionary |
| 授权与集成 | integration/chat-user、integration/external-retrieval |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function firstAccessibleDetail(params: RouteParamsGeneric) {
const system = perm.knowledge.system
const candidates = [
['document-list', system.docRead()],
['image-list', params.type === KNOWLEDGE_TYPE_KEY.BASE && system.image.read()],
['image-list', (params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW) && system.image.read()],
['hit-test', system.hitTest()],
['problem', system.problemRead()],
['tag', system.tagRead()],
Expand Down Expand Up @@ -47,7 +47,7 @@ export const systemResourceKnowledgeRoutes: RouteRecordRaw[] = [
path: 'image',
name: 'system-resource-knowledge-image-list',
component: () => import('@/views/knowledge-detail/library/image/index.vue'),
meta: { permission: () => perm.knowledge.system.image.read(), next: 'system-resource-knowledge-tag', title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE },
meta: { permission: () => perm.knowledge.system.image.read(), next: 'system-resource-knowledge-tag', title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW },
},
{
path: 'tag',

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.

Both diffs are consistent and look correct, but here are observations:

Issues / suggestions:

  1. Duplicated condition logic — params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW appears in two places (route gate + menu visibility). Extract a helper to prevent drift:

    const supportsImageList = (type?: unknown) =>
      type === KNOWLEDGE_TYPE_KEY.BASE || type === KNOWLEDGE_TYPE_KEY.WORKFLOW

    Then use supportsImageList(params.type) in both spots.

  2. Type safety — params.type is likely string | undefined. Use an array-includes check for cleaner, safer matching:

    [KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW].includes(params.type as any)

    Or a typed set: const IMAGE_SUPPORTED_TYPES = new Set<string>([...]) + IMAGE_SUPPORTED_TYPES.has(params.type).

  3. Permission vs. visibility coupling — In the route meta, permission still checks only image.read(), while detailMenuVisible now allows WORKFLOW. That's fine (permission gates access; visibility gates display), but confirm that entering the direct URL for a WORKFLOW image route without image.read() fails gracefully (it should be blocked by permission). Consider also redirecting/handling the case where type is neither BASE nor WORKFLOW on direct navigation.

  4. Potential inconsistency with next — next: 'system-resource-knowledge-tag' applies regardless of type; ensure the fallback chain in firstAccessibleDetail matches this behavior when image-list becomes accessible for WORKFLOW but tag may not be.

  5. No other irregularities — parentheses precedence is correct; template literal style/formatting unchanged. Logic-wise the change is coherent.

Minor optimization: hoist the supported-types set/constant outside the function to avoid re-allocation on every call.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const systemSharedKnowledgeRoutes: RouteRecordRaw[] = [
path: 'image',
name: 'system-shared-knowledge-image-list',
component: () => import('@/views/knowledge-detail/library/image/index.vue'),
meta: { permission: () => perm.knowledge.share.image.read(), next: 'system-shared-knowledge-tag', title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE },
meta: { permission: () => perm.knowledge.share.image.read(), next: 'system-shared-knowledge-tag', title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW },
},
{
path: 'tag',

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 change modifies the detailMenuVisible logic to include KNOWLEDGE_TYPE_KEY.WORKFLOW alongside BASE. This is semantically correct if the intent is to show this route for both types.

Observations & Suggestions:

  1. Function Name Clarity (next): The property next: 'system-shared-knowledge-tag' is somewhat ambiguous. It likely refers to a fallback route, a parent, or the next step in a wizard/list flow. Ensure this aligns with your router guard logic. If it means "redirect to," consider naming it fallback or redirectTo if possible for readability, unless next is a strict requirement of your custom router library.
  2. Arrow Function Performance: The arrow function (params) => ... is created on every render/metric evaluation. If meta objects are evaluated frequently (e.g., in loops or frequent route checks), consider extracting this logic into a constant boolean check or memoizing it if the performance profile requires it. However, for Vue Router meta objects, this overhead is typically negligible unless routes are being re-resolved extremely often.
  3. Readability: The condition params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW is clear.
  4. No Functional Issues: Assuming KNOWLEDGE_TYPE_KEY.WORKFLOW is a valid export and params.type correctly reflects the current context, the change is safe and logically sound.

Optimization (Optional):
If KNOWLEDGE_TYPE_KEY values are strings/numbers that support array inclusion checks, you could write:

detailMenuVisible: (params) => [KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW].includes(params.type)

This makes it easier to maintain if more types are added later.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/router/admin/workspace/modules/knowledge-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const sharedKnowledgeRoutes: RouteRecordRaw[] = [
path: 'image',
name: 'workspace-shared-knowledge-image-list',
component: () => import('@/views/knowledge-detail/library/image/index.vue'),
meta: { title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE },
meta: { title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW },
},
{
path: 'tag',

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:

1. Potential Logic/Safety Issue

  • Unsafe Property Access: params.type assumes that params is always defined and contains a type property. If params is undefined, null, or lacks type, this will throw a runtime error (e.g., "Cannot read property 'type' of undefined").
    • Suggestion: Use optional chaining (?.) and fallback logic if applicable, or ensure upstream code guarantees params structure. Example: (params) => params?.type === KNOWLEDGE_TYPE_KEY.BASE || ...
    • Alternative: If params.type can be other values, consider using array inclusion for readability:
    detailMenuVisible: (params) => 
        [KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW].includes(params.type)

2. Readability & Maintainability

  • Operator Precedence Clarity: While || has lower precedence than ===, adding parentheses can improve readability and prevent future bugs if more conditions are added:
    meta: { 
        // ... 
        detailMenuVisible: (params) => 
            (params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW) 
    }
  • Magic Values/Repetition: If more knowledge types are added later, this condition will grow harder to manage. Consider extracting allowed types into a constant:
    const IMAGE_VISIBLE_TYPES = [KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW];
    
    // In meta:
    detailMenuVisible: (params) => IMAGE_VISIBLE_TYPES.includes(params.type)

3. Performance/UX Note

  • The check detailMenuVisible runs whenever the menu visibility needs to be recalculated. This is generally fast, but ensure that route transitions don’t trigger excessive re-evaluations of this function unnecessarily (usually fine in Vue Router’s lifecycle).

✅ Recommended Improved Version:

{
    path: 'image',
    name: 'workspace-shared-knowledge-image-list',
    component: () => import('@/views/knowledge-detail/library/image/index.vue'),
    meta: { 
        title: '图片', 
        order: 20, 
        detailMenuVisible: (params) => 
            [KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW].includes(params.type) // Assuming params.type is guaranteed to exist
    },
},

If params.type might be missing/undefined, use:

detailMenuVisible: (params) => 
    !!params && [KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW].includes(params.type)

Let me know if you want help refactoring further!

Expand Down
Loading