feat: Knowledge workflow edit permission

[shaohuzhang1]

feat: Knowledge workflow edit permission

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The code provided is mostly clean and well-structured. However, there are a few suggestions to improve clarity and maintainability:

1. Comments: Add comments explaining the purpose of each permission chain and their relationship. This can help other developers (or yourself at a later time) understand what each function is achieving.

2. Whitespace Consistency: Ensure consistent use of whitespace around operators and between statements for better readability.

Here's an updated version with some comments added:

const workspace = {
  // Function to check if user has permission to edit the given knowledge workspace
  workflow_edit: (source_id: string) => {
    return hasPermission(
      [
        new ComplexPermission(
          [RoleConst.USER], // Check if the role is a user
          [PermissionConst.KNOWLEDGE.getKnowledgeWorkspaceResourcePermission(source_id)], 
          [], // No additional conditions required
          'AND', // All specified permissions must be met
        ),
        RoleConst.WORKSPACE_MANAGE.getWorkspaceRole, // Check if the role allows workspace management
        PermissionConst.KNOWLEDGE_WORKFLOW_EDIT.getKnowledgeWorkspaceResourcePermission(source_id), 
        PermissionConst.KNOWLEDGE_WORKFLOW_EDIT.getWorkspacePermissionWorkspaceManageRole, 
      ], // OR condition implies any one of these conditions must be true
      'OR'
    );
  },
  
  // Placeholder for HIT test functionality
  hit_test: () => false
};

// Example usage:
console.log(workspace.workflow_edit('example_source_id')); // Use this to determine if the user can edit a specific knowledge space

Summary of Changes:

• Added comments to explain the logic and intent behind each section.
• Ensured whitespace consistency within functions for easier reading and maintenance.

These changes should enhance both the quality and maintainability of the codebase.

[shaohuzhang1]

Here are some points to consider and potential improvements:

1. Imports: The MsgError and t methods are imported but not used within the function scope.

2. Empty Function Block: If the else block is reached due to missing permissions, an unnecessary empty block can be simplified or removed if no further logic needs to be executed at that point.

3. Code Consistency: Ensure consistency in variable naming, spacing, and indentation for readability and adherence to coding standards.

4. Avoid Global Scope Variables: Minimize direct global access to variables like permissionMap. Consider encapsulating these values in a class or context where they are needed.

5. Nullish Coalescing Operator: Use the nullish coalescing operator (??) to safely handle cases where id, form_data.value[active.value], etc., might be undefined.

6. Dynamic API Calls: Consider using dependency injection mechanisms to manage dynamic API calls more cleanly, especially with loadSharedApi.

Here's a revised version of the function incorporating these suggestions:

const upload = () => {
  const permissionPrecise = computed(() => {
    return permissionMap['knowledge'][apiType.value];
  });

  // Check if document creation privilege exists before validating form
  if (permissionPrecise && permissionPrecise.doc_create(id)) {
    ActionRef.value.validate().then(() => {
      form_data.value[active.value] = ActionRef.value.get_data();
      
      loadSharedApi({ type: 'knowledge', systemType: apiType.value })
        .workflowAction(id, form_data.value, loading)
        .finally(() => {
          active.value = 'result';
        });
    });
  } else {
    MsgError(t('views.application.tip.noDocPermission'));
  }
};

By making these adjustments, you improve code safety, maintainability, and readability.

[shaohuzhang1]

The provided code has a few issues and can be optimized:

Issues:

1. Repeated Methods:

   • The Publish method is copied and pasted within the same class with minor modifications. This redundancy can be removed to improve readability and maintainability.

2. Permissions:

   • There are multiple instances of similar permission checks across different methods. Consolidate these into a single set of permissions wherever possible to avoid duplication.

3. Comments vs. Docstrings:

   • Some comments use underscores instead, which are not correct for docstring formatting (e.g., _('Knowledge Base')) should be replaced with regular double quotes ("").

4. Function Annotations:

   • Ensure that all function annotations match their types and signatures properly.

Optimizations:

1. Consolidated Permissions:

   from openai.permissions import PermissionConstants, RoleConstants, ViewPermission
   
   class KnowledgeWorkflowActionView(APIView):
       @has_permissions(
           PermissionConstants.KNOWLEDGE_DOCUMENT_CREATE.get_workspace_knowledge_permission(),
           PermissionConstants.KNOWLEDGE_DOCUMENT_CREATE.get_workspace_permission_workspace_manage_role(),
           RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
           ViewPermission([RoleConstants.USER.get_workspace_role()],
                            [PermissionConstants.KNOWLEDGE_PERMISSION.get_workspace_knowledge_permission()],
                            CompareConstants.AND)
       )
       def get(self, request):
           ...  # Existing implementation...
   
       def post(self, request):
           ...  # Existing implementation...
   
       def put(self, request):
           ...  # Existing implementation...
   
       def patch(self, request):
           ...  # Existing implementation...

2. Removed Redundant Method Copies:

   def publish(request):
       """Post an action to update workflow status."""
       return JsonResponse({"status": "success"})

By consolidating permissions and removing redundant method copies, you make the code cleaner, more maintainable, and less prone to errors.