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
17 changes: 10 additions & 7 deletions ui/src/components/select-knowledge-document/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,16 @@ import type { FormInstance, FormRules } from 'element-plus'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import useStore from '@/stores'
import { t } from '@/locales'
const props = defineProps<{
data?: {
type: object
default: () => null
}
const props = withDefaults(defineProps<{
data?: any,
postKnowledgeHandler?: (knowledge_list:Array<any>) => Array<any>
apiType: 'systemShare' | 'workspace' | 'systemManage'
isApplication?: boolean,
workspaceId?: string,
}>()
}>(), {
postKnowledgeHandler: (k_l: Array<any>) => k_l,
data: () => null
})

const { user } = useStore()

Expand Down Expand Up @@ -111,8 +112,10 @@ const loadTree = async (node: any, resolve: any) => {
}
await loadSharedApi({ type: 'knowledge', systemType: props.apiType })
.getKnowledgeList(obj, optionLoading)
.then((ok: any)=>ok.data)
.then(props.postKnowledgeHandler)
.then((res: any) => {
resolve(res.data)
resolve(res)
})
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are no errors detected in this code snippet. However, there is an instance of incorrect type annotation that can be optimized:

In the defineProps statement where you define postKnowledgeHandler, the function parameter should have a more specific type than Array<any>. Given that it's intended to handle arrays but doesn't process them further, using a union type like (knowledge_list: Array<string>) => Array<string> would be more accurate, assuming knowledge points will always be represented as strings.

Also, consider initializing optionLoading variable. If loadSharedApi.getKnowledgeList(optionLoading) expects a loading indicator, make sure it's defined outside async functions or passed properly within.

Here’s an updated version with these recommendations:

const { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import useStore from '@/stores'
import { t } from '@/locales'

const props = withDefaults(defineProps<{
  data?: any,
  postKnowledgeHandler?: (knowledge_list: Array<string>) => Array<string>
  apiType: 'systemShare' | 'workspace' | 'systemManage'
  isApplication?: boolean,
  workspaceId?: string,
}>(), {
  postKnowledgeHandler: (k_l: Array<string>) => k_l,
  data: () => null
});

var optionLoading;

const { user } = useStore();

const loadTree = async (node: any, resolve: any) => {
    let obj; // Assuming obj is defined somewhere before here

    await loadSharedApi({ type: 'knowledge', systemType: props.apiType })
        .getKnowledgeList(obj, optionLoading)
        .then((ok: any) => ok.data)
        .then(props.postKnowledgeHandler)
        .then((res: any) => resolve(res))
        .catch((error) => console.error('Error fetching knowledge list:', error));
};

This makes the types more precise while maintaining readability and clarity.

Expand Down
19 changes: 18 additions & 1 deletion ui/src/views/chat-log/component/EditContentDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@
</el-form-item>
</el-form>
<SelectKnowledgeDocument
v-if="detail.workspace_id"
ref="SelectKnowledgeDocumentRef"
:post-knowledge-handler="postKnowledgeHandler"
:apiType="apiType"
@changeKnowledge="changeKnowledge"
@changeDocument="changeDocument"
Expand All @@ -75,6 +77,10 @@ import type { FormInstance, FormRules } from 'element-plus'
import imageApi from '@/api/image'
import { t } from '@/locales'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import { Permission } from '@/utils/permission/type'
import { hasPermission } from '@/utils/permission'
import { PermissionConst, RoleConst } from '@/utils/permission/data'

const route = useRoute()
const {
params: { id },
Expand All @@ -87,6 +93,18 @@ const apiType = computed(() => {
}
})

const postKnowledgeHandler = (knowledgeList: Array<any>) => {
return knowledgeList.filter(item => {
if (apiType.value === 'workspace') {
return hasPermission([RoleConst.WORKSPACE_MANAGE.getWorkspaceRole(),
new Permission("KNOWLEDGE_DOCUMENT:READ+EDIT").getWorkspacePermissionWorkspaceManageRole,
new Permission("KNOWLEDGE_DOCUMENT:READ+EDIT").getWorkspaceResourcePermission('KNOWLEDGE', item.id)], 'OR')
} else if (apiType.value === 'systemManage') {
return hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_EDIT],'OR')
}
})
}

const emit = defineEmits(['refresh'])

const formRef = ref()
Expand Down Expand Up @@ -127,7 +145,6 @@ const SelectKnowledgeDocumentRef = ref()
const dialogVisible = ref<boolean>(false)
const loading = ref(false)
const detail = ref<any>({})

const form = ref<any>({
chat_id: '',
record_id: '',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here are some general comments and suggestions for your code:

  1. Dynamic API Calls and Conditional Rendering: The condition v-if="detail.workspace_id" to determine whether SelectKnowledgeDocument should be rendered makes sense and is used correctly.

  2. Custom Filter Function (postKnowledgeHandler):

    • Ensure that hasPermission returns a boolean value.
    • Simplify the permission checks by using logical operators like || instead of multiple calls to new Permission.
  3. Event Handling:

    • Add event handlers for buttons or other controls that interact with the component's logic.
    • Ensure all emitted events (refresh, etc.) have corresponding watchers in the parent component.
  4. Validation Rules:

    • Verify that form rules are correctly defined and handle errors appropriately.
      Consider adding error feedback messages based on validation results.
  5. Logging and Debugging:

    • Use console logs to trace execution flow and identify any unexpected behavior.
    • Add alerts or notifications where applicable to improve user experience.
  6. Error Handling:
    Implement robust error handling for API requests and UI components.
    Provide informative messages to users when operations fail.

  7. Consistent Naming Conventions:
    Maintain consistent naming conventions for variables, methods, and class properties.
    This improves readability and maintainability.

  8. Code Readability:
    Break down complex functions into smaller parts for better organization and understanding.
    Inline simple functions when they add significant clarity.

  9. Security and Privacy:
    Ensure secure handling of sensitive data such as permissions and roles.
    Validate inputs before processing them to mitigate risks.

  10. Testing:
    Write comprehensive unit tests to cover all functionalities of this component.
    Test various scenarios including permission checks, conditional rendering, and API interactions.

Here is an optimized version with these considerations:

<template>
  <el-form ref="formRef" :model="form" :rules="formRules">
    <!-- ...existing fields... -->
  </el-form>
  <SelectKnowledgeDocument
    v-if="detail.workspace_id"
    ref="SelectKnowledgeDocumentRef"
    :post-knowledge-handler="postKnowledgeHandler"
    :api-type="apiType"
    @changeKnowledge="changeKnowledge"
    @changeDocument="changeDocument"
  />
</template>

<script lang="ts">
import { ref, reactive, computed } from 'vue'
import type { FormInstance, FormRules } from 'element-plus'
import SelectKnowledgeDocument from '@/components/common/SelectKnowledgeDocument.vue'
// Import other dependencies

export default {
  name: 'YourComponentName',
  setup() {
    const formRef = ref<FormInstance>()
    // Existing code...

    const apiType = computed(() => {...})

    const postKnowledgeHandler = (knowledgeList: Array<any>): Array<any> => {
      return knowledgeList.filter(item => {
        if (
          apiType.value === 'workspace' &&
          hasPermission([
            {
              roleKey: RoleConst.WORKSPACE_MANAGE.roleKey,
              resourcePermissions: ['KNOWLEDGE'],
              resourceId: item.id,
            },
            {
              roleKey: PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_READ_WRITE.roleKey,
              resourceKey: RoleConst.WORKSPACE_RESOURCE_PERMISSION.resourceKey,
              resourceName: 'KNOWLEDGE',
              resourceId: item.id,
            },
          ])
        ) {
          return true
        }
        if (
          apiType.value === 'systemManage' &&
          hasPermission([RoleConst.ADMIN.roleKey, PermissionConst.RESOURCES.KNOWLEDGE_DOCUMENT_EDIT.roleKey])
        ) {
          return true
        }
        return false
      })
    }

    const form = reactive({
      chat_id: '',
      record_id: ''
    })

    // Existing code...
    
    return {
      formRef,
      form,
      formRules,
      selectKnowledgeDialogVisible,
      loading,
      detail,
      apiType,
      postKnowledgeHandler,
      emit,
    }
  },
}
</script>

Key Improvements:

  • Used computed property apiType.
  • Combined permissions checks into a simpler structure.
  • Refactored inline function within filter method.
  • Improved security practices for roles and resources.

Make sure to adjust according to specific project requirements and existing architecture.

Expand Down
18 changes: 17 additions & 1 deletion ui/src/views/chat-log/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@
:close-on-press-escape="false"
>
<SelectKnowledgeDocument
:post-knowledge-handler="postKnowledgeHandler"
ref="SelectKnowledgeDocumentRef"
:apiType="apiType"
:workspace-id="detail.workspace_id"
Expand Down Expand Up @@ -252,6 +253,10 @@ import { t } from '@/locales'
import { ElTable } from 'element-plus'
import permissionMap from '@/permission'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import { Permission } from '@/utils/permission/type'
import { hasPermission } from '@/utils/permission'
import { PermissionConst, RoleConst } from '@/utils/permission/data'

const route = useRoute()

const apiType = computed(() => {
Expand Down Expand Up @@ -349,7 +354,18 @@ const filter = ref<any>({
min_trample: 0,
comparer: 'and',
})

const postKnowledgeHandler = (knowledgeList: Array<any>) => {
return knowledgeList.filter(item => {
if (apiType.value === 'workspace') {
return hasPermission([RoleConst.WORKSPACE_MANAGE.getWorkspaceRole(),
new Permission("KNOWLEDGE_DOCUMENT:READ+EDIT").getWorkspacePermissionWorkspaceManageRole,
new Permission("KNOWLEDGE_DOCUMENT:READ+EDIT").getWorkspaceResourcePermission('KNOWLEDGE', item.id)], 'OR')
} else if (apiType.value === 'systemManage') {
return hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_EDIT],'OR')
}
})

}
function filterChange(val: string) {
if (val === 'clear') {
filter.value = cloneDeep(defaultFilter)
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 code looks well-written, but there are a few enhancements and optimizations you can consider:

  1. Type Annotation: Add type annotations to parameters and variables to improve readability and maintainability.

  2. Destructuring: Use destructuring instead of ref for better performance and cleaner syntax.

  3. Permissions: The way permissions are checked and used can be simplified using functional components and hooks.

  4. Event Handlers: Check if the event handlers (select-knowledge-document) are properly bound or attached within the component setup.

Here's an updated version of your code with these improvements:

<script lang="ts">
import { defineComponent, ref,computed } from 'vue'
import SelectKnowledgeDocument from '@/components/SelectKnowledgeDocument.vue'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import route from '@/router'
import permissionMap from '@/permission'

interface APIType {
  workspace?: boolean;
  systemManage?: boolean;
}

export default defineComponent({
  props: {
    closeOnPressEscape: {
      type: Boolean,
      default: false,
    },
  },
  setup(props) {
    interface FilterValues {
      keyWords: string;
      doc_types: string[];
      status_list: string[];
      created_before_time: number | '';
      max_level_of_difficulty: number;
      min_trample: number;
      comparer: 'and' | 'or';
    }

    const selectKnowledgeDocumentRef = ref<SelectKnowledgeDocument>();
    const apiType = computed<APIType>(() => {
      // Determine API type logic here
    });
    const detail = ref<{ [key: string]: any }>({}); // Adjust according to your structure

    const filter = ref<FilterValues>({
      keyWords: '',
      doc_types: [],
      status_list: ['active'],
      created_before_time: '',
      max_level_of_difficulty: Infinity,
      min_trample: 0,
      comparer: 'and',
    });

    function onSelectionChange(knowledgeList: unknown[]): void {
      if (!apiType.value || !selectKnowledgeDocumentRef.value) return;

      const filteredDocuments = postKnowledgeHandler(knowledgeList);
      if (Array.isArray(filteredDocuments)) {
        // Handle_filtered_documents_here
      }
    }

    const hasPermissionWithRole = (rolesAndResources: Array<string>, action: string): boolean => {
      return rolesAndResources.some(role =>
        hasPermission(
          Object.values(permissionMap).filter(p => p.roleId.startsWith(role))[0]
            .permissions[action] ?? []
        )
      );
    };

    const postKnowledgeHandler = (
      knowledgeList: Array<{
        id: string;
        [key: string]: any; // Adjust properties based on actual data
      }>
    ): Array<{
      id: string;
      name: string;
      // Add other relevant fields for the returned object
    }> => {
      return knowledgeList.filter((item) => {
        if (apiType.value?.workspace) {
          return hasPermissionWithRole([RoleConst.WORKSPACE_MANAGE], 'WRITE KNOWLEDGE');
        } else if (apiType.value?.systemManage) {
          return hasPermissionWithRole(['admin', 'resource_knowledge_document_edit'], 'WRITE KNOWLEDGE');
        }
        return true; // Default to accepting all items when no specific conditions match
      });
    };

    function filterChange(val: string): void {
      if (val === 'clear') {
        Object.assign(filter.value, defaultValue.filterObject());
      }
    }

    return {
      selectKnowledgeDocumentRef,
      apiType,
      detail,
      filter,
      onSelectionChange,
      filterChange,
    };
  },
});
</script>

Key Changes:

  • Type Annotations: Added types for various interfaces and functions.
  • Computed Property Destructuring: Used destructuring in the setup() hook for more concise code.
  • Permissions Functionality: Improved functionality by creating a helper function hasPermissionWithRole.
  • Conditional Logic Simplification: Enhanced conditional checks for permission handling.
  • Function Refactoring: Reorganized and modularized some logic functions.

These changes should make the code more robust, readable, and potentially optimize it for production use.

Expand Down
Loading