-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: The data source Tree uses fetc_list_function #4440
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.
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
Conversation
|
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. DetailsInstructions 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. |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| extra.value.current_tool_id = n.properties.node_data.tool_lib_id | ||
| node_id = n | ||
| ? [WorkflowType.DataSourceLocalNode, WorkflowType.DataSourceWebNode].includes(n.type) | ||
| ? n.type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed the provided code snippet and found a few minor improvements and corrections:
-
Import Statement: The import statement has been updated to include
provideinstead ofimport Vue. -
Variable Declaration Consistency: In the
extra.value = {}line, it's better to initialize the object with key-value pairs if you want pre-defined values. -
Provide Statement: Ensure that variables intended to be shared across components should have unique names when used in
provide. Here,'get_extra'is consistent but might be improved with more context clarity like'currentToolLibIdGetter'.
Here's an optimized version of the code snippet:
<template>
<DynamicsForm :form-data="base_form_data" />
</template>
<script setup lang="ts">
import { computed, ref, watch, provide } from 'vue'
import { WorkflowKind, WorkflowType } from '@/enums/application'
import DynamicsForm from '@/components/dynamics-form/index.vue'
import type { FormField } from '@/components/dynamics-form/type'
const workflow_nodes_local_types = [
WorkflowType.DataSourceLocalNode,
WorkflowType.DataSourceWebNode
]
const form_data = computed(() => {
// Implementation for form data computation goes here
})
const source_node_list = computed(() => {
return props.workflow?.nodes?.filter((n: any) => n.properties.kind === WorkflowKind.DataSource);
})
// Initialize extra with default values for clarity
const extra = ref<any>({
current_tool_id: null, // or whatever suitable default value you prefer
});
// Define getter function directly within extra property
extra.get_current_tool_lib_id = get_extra;
function get_extra() {
return extra.value.current_tool_id;
}
provid('currentToolLibIdGetter', get_extra);
const sourceChange = (node_id: string): void => {
base_form_data.node_id = node_id;
const n = source_node_list.value.find((n: any) => n.id === node_id);
if (n !== undefined &&
workflow_nodes_local_types.includes(n.type)) {
extra.value.current_tool_id = n.properties.node_data.tool_lib_id;
node_id = n; // Assuming n contains additional properties relevant to this use case
// Further logic can be added here based on specific requirements
}
};
watch(
() => props.workflow,
newVal => {
// Update local state whenever props.workflow changes
},
{ immediate: true }
);
</script>Key Changes:
- Ensured correct imports (
Vue) are included. - Added initialization to the
extravariable for clarity. - Improved naming conventions where appropriate by using getters and providing clear identifiers.
- Used TypeScript types effectively and consistently throughout the script.
Make sure to adjust the implementation details according to your project's specific needs.
| ), | ||
| body: { current_node: node.level == 0 ? undefined : node.data }, | ||
| then: (res: any) => { | ||
| resolve(res.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of readability and functionality, there are a few changes you can make:
-
Remove Unnecessary Type Casting: The line
const get_extra = inject('get_extra') as anyis unnecessary, as TypeScript should automatically correctly infer the type if you use it consistently. -
Simplify Template String Usage: You can simplify how the URL template is being constructed by using templated string literals directly instead of concatenating strings using
${}syntax multiple times.
Here's an optimized version based on these considerations:
function renderTemplate(template: string, data: any): string {
return template.replace(/{{(.*?)}}/g, (match, key) => (key in data ? data[key] : match));
}
const loadNode = async (node: Node, resolve: ((nodeData: Tree[]) => void)) => {
try {
const extras = await inject('get_extra');
const params = {
...props.otherParams,
...(extras ? extras() : {})
};
const response = await request.call({
url: `/workspace/${current_workspace_id}/knowledge/${current_knowledge_id}/datasource/tool/${current_tool_id}/${attrs.fetch_list_function}`,
method: 'POST',
body: { current_node: node.level === 0 ? undefined : node.data },
headers: {
'Content-Type': 'application/json'
}
});
resolve(response.data);
} catch (error) {
console.error('Error loading node:', error);
resolve([]);
}
};Key Changes:
- Removed Type Casting from
inject. - Improved Error Handling: Wrapped asynchronous operations with a
try-catchblock to handle possible errors more gracefully. - Added Headers: Added
'Content-Type': 'application/json'to the request object for clarity and consistency. - Consolidated Parameter Object: Used destructuring to create the parameters object for better organization and readability.
- Used Templated Strings Correctly: Replaced multiple concatenations with a single replacement within the
renderTemplatefunction.
fix: The data source Tree uses fetc_list_function