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 apps/common/constants/permission_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -1466,7 +1466,7 @@ class PermissionConstants(Enum):
parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
)
RESOURCE_KNOWLEDGE_WORKFLOW_EDIT = Permission(
group=Group.SYSTEM_RES_KNOWLEDGE_WORKFLOW, operate=Operate.READ, role_list=[RoleConstants.ADMIN],
group=Group.SYSTEM_RES_KNOWLEDGE_WORKFLOW, operate=Operate.EDIT, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
)
RESOURCE_KNOWLEDGE_DOCUMENT_READ = Permission(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code snippet appears to be an enumeration of permission constants with different roles and access levels for users. Here are some potential issues and optimizations:

  1. Potential Duplicate: The RESOURCE_KNOWLEDGE_WORKFLOW_EDIT constant has both a READ and an EDIT operation on the same set of parents, which might not be intended if only one action (e.g., editing) should override reading.

  2. Role List Consistency: The parent_group field references Resource.Knowledge, but the RESOURCE_KNOWLEDGE_WORKFLOW_EDIT is associated with SYSTEM_RES_KNOWLEDGE_WORKFLOW. This inconsistency could potentially lead to unexpected behavior in certain applications that rely on this information.

  3. Code Readability and Maintainability: Having duplicate entries like RESOURCE_KNOWLEDGE_WORKFLOW_EDIT can make the code harder to understand and maintain. Consider removing unnecessary duplicates or using more descriptive names where applicable.

Here's a slightly optimized version with these considerations addressed:

@@ -1466,8 +1466,7 @@ class PermissionConstants(Enum):
     """
         Access to resources related to knowledge base.
     """

-    RESOURCE_KNOWLEDGE_READ = Permission(
-        group=Group.SYSTEM_RES_KNOWLEDGE, operate=Operate.READ, role_list=[RoleConstants.ADMIN],
-        parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
+    RESOURCE_KNOWLEDGE_MANAGE = Permission(
+        group=Group.SYSTEM_RES_KNOWLEDGE, operate=Operate.ALL, role_list=[RoleConstants.ADMIN],
+        parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
     )

     # Other permissions...

In this updated version:

  • The RESOURCE_KNOWLEDGE_WRITE constant is renamed to RESOURCE_KNOWLEDGE_MANAGE to reflect its broader scope rather than being limited to just read operations.
  • The operate attribute is changed to Operate.ALL to allow all specified roles ([RoleConstants.ADMIN]) to perform edit actions without needing separate READ permissions.

These changes enhance readability, maintainability, and the clarity of the permission structure.

Expand Down
68 changes: 61 additions & 7 deletions ui/src/views/knowledge-workflow/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ import { KnowledgeWorkFlowInstance } from '@/workflow/common/validate'
import { hasPermission } from '@/utils/permission'
import DebugVue from './component/DebugDrawer.vue'
import { t } from '@/locales'
import { ComplexPermission } from '@/utils/permission/type'
import { ComplexPermission, Permission } from '@/utils/permission/type'
import { EditionConst, PermissionConst, RoleConst } from '@/utils/permission/data'
import permissionMap from '@/permission'
import { WorkflowMode } from '@/enums/application'
Expand Down Expand Up @@ -464,21 +464,75 @@ function saveknowledge(bool?: boolean, back?: boolean) {
})
}
const go = () => {
if (route.path.includes('workspace')) {
if (route.path.includes('resource-management')) {
return router.push({ path: get_resource_management_route() })
} else if (route.path.includes('shared')) {
return router.push({ path: get_shared_route() })
} else {
return router.push({ path: get_route() })
}
}

const get_shared_route = () => {
if (hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_DOCUMENT_READ], 'OR')) {
return `knowledge/${id}/shared/4/document`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_PROBLEM_READ], 'OR')) {
return `/knowledge/${id}/shared/4/problem`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_HIT_TEST_READ], 'OR')) {
return `/knowledge/${id}/shared/4/hit-test`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_CHAT_USER_READ], 'OR')) {
return `/knowledge/${id}/shared/4/chat-user`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.SHARED_KNOWLEDGE_EDIT], 'OR')) {
return `/knowledge/${id}/shared/4/setting`
} else {
return router.push({ path: get_resource_management_route() })
return `/system/shared/knowledge`
}
}

const get_resource_management_route = () => {
return `/knowledge/${id}/${folderId}/4/document`

// return `/system/resource-management/knowledge` 没有权限的时候返回resource-management的列表
if (hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_READ],'OR')) {
return `/knowledge/${id}/resource-management/4/document`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_PROBLEM_READ], 'OR')) {
return `/knowledge/${id}/resource-management/4/problem`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_HIT_TEST], 'OR')) {
return `/knowledge/${id}/resource-management/4/hit-test`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_CHAT_USER_READ], 'OR')) {
return `/knowledge/${id}/resource-management/4/chat-user`
} else if (hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_EDIT], 'OR')) {
return `/knowledge/${id}/resource-management/4/setting`
} else {
return `/system/resource-management/knowledge`
}
}

const get_route = () => {
return `/knowledge/${id}/${folderId}/4/document`
const checkPermission = (permissionConst: Permission) => {
return hasPermission([
new ComplexPermission(
[RoleConst.USER],
[PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(id)],
[],
'AND'
),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
permissionConst.getWorkspacePermissionWorkspaceManageRole,
permissionConst.getKnowledgeWorkspaceResourcePermission(id),
],'OR'
)
}
if (checkPermission(PermissionConst.KNOWLEDGE_DOCUMENT_READ)) {
return `/knowledge/${id}/${folderId}/4/document`
} else if (checkPermission(PermissionConst.KNOWLEDGE_PROBLEM_READ)) {
return `/knowledge/${id}/${folderId}/4/problem`
} else if (checkPermission(PermissionConst.KNOWLEDGE_HIT_TEST_READ)) {
return `/knowledge/${id}/${folderId}/4/hit-test`
} else if (checkPermission(PermissionConst.KNOWLEDGE_CHAT_USER_READ)) {
return `/knowledge/${id}/${folderId}/4/chat-user`
} else if (checkPermission(PermissionConst.KNOWLEDGE_EDIT)) {
return `/knowledge/${id}/${folderId}/4/setting`
} else {
return `/knowledge`
}
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code has several issues that need to be addressed:

  1. Inconsistent Imports: The import statement for ComplexPermission is different from the other places where it's used, which can lead to confusion. You should update all occurrences of it to use import { ComplexPermission, Permission } from '@/utils/permission/type';.

  2. Duplicate Logic: There is duplicate logic in the get_shared_route and ge_resource_management_route functions. This could be optimized.

  3. Conditional Checks: In the go() function, you have two separate conditions for checking if the route includes '/workspace'. If these conditions are always mutually exclusive based on the usage context, you might want to simplify them.

  4. Permissions Functionality: The CheckPermission function looks repetitive. It would be more efficient to refactor this into a reusable helper function.

Here's an improved version of your code with suggestions for addressing these issues:

import { ComplexPermission, Permission } from '@/utils/permission/type';
import { KnowledgeWorkFlowInstance } from '@/workflow/common/validate';
import { hasPermission } from '@/utils/permission';
const DebugVue = './component/DebugDrawer.vue';
import { t } from '@/locales';
import { EditionConst, PermissionConst, RoleConst } from '@/utils/permission/data';
import permissionMap from '@/permission';
import { WorkflowMode } from '@/enums/application';

function saveknowledge(bool?: boolean, back?: boolean) {
    // Implementation here
}

const go = () => {
    if (route.path.includes('resource-management') || route.path.includes('shared')) {
        return router.push(get_route());
    } else {
        return router.push('/knowledge');
    }
};

const get_route = () => {
    const checkPermission = (permissionConst: Permission) => {
        return hasPermission([
            new ComplexPermission([RoleConst.USER], [permissionConst.getWorkspacePermission(workspaceId)], [], 'AND'),
            RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
            workspaceId && permissionConst.getWorkspacePermission(workspaceId).includes(RoleConst.WORKSPACE_MANAGE),
            permissionConst.getKnowledgeWorkspaceResourcePermission(workspaceId),
        ], 'OR',
        );
    };

    const knowledgePermissions = [
        PermissionConst.KNOWLEDGE_DOCUMENT_READ,
        PermissionConst.KNOWLEDGE_PROBLEM_READ,
        PermissionConst.KNOWLEDGE_HIT_TEST_READ,
        PermissionConst.KNOWLEDGE_CHAT_USER_READ,
        PermissionConst.KNOWLEDGE_EDIT,
    ];

    for (let perm of knowledgePermissions) {
        if (checkPermission(perm)) {
            return `/knowledge/${workspaceId}/document`;
        }
    }

    return `/knowledge`;
};

Key Changes Made:

  • Consistent Import Statement for ComplexPermission.
  • Simplified conditional logic in go() based on mutual exclusivity.
  • Refactored permissions check into checkPermission utility function for better readability and maintainability.
  • Ensured consistent naming conventions across references to permissions.

Expand Down
Loading