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
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def get_default_model_params_setting(model_id):


def generate_properties(variable_list):
return {variable['field']: {'type': variable['parameter_type'], 'description': variable['desc'],
return {variable['field']: {'type': variable['parameter_type'], 'description': (variable.get('desc') or ""),
'title': variable['label']} for variable in
variable_list}

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 appears to be generating property settings based on a list of variables with fields like field, parameter_type, desc, and label. The main issue is that if there's no description ('desc') provided for a variable, it currently raises an error because you're trying to access None properties directly.

I suggest:

  1. Replace the line: 'type': variable['type'], 'description': (variable.get('desc') or ""),
  2. With this one: 'type': variable['type'], 'description': str((variable.get('desc', '')),

This modification ensures that if variable['desc'] is None, it defaults to an empty string, avoiding any runtime errors.

Expand Down
2 changes: 2 additions & 0 deletions ui/src/locales/lang/en-US/views/application-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ You are a master of problem optimization, adept at accurately inferring user int
addVariables: 'Add Variables',
editVariables: 'Edit Variables',
variablePlaceholder: 'Please enter variable',
variableListPlaceholder: 'Please add split variables',
expression: {
label: 'Expression',
placeholder: 'Please enter expression',
Expand All @@ -446,6 +447,7 @@ You are a master of problem optimization, adept at accurately inferring user int
text: 'Use AI models to extract structured parameters',
extractParameters: {
label: 'Extract Parameters',
variableListPlaceholder: 'Please add extraction parameters',
parameterType: 'Parameter Type',
},
},
Expand Down
2 changes: 2 additions & 0 deletions ui/src/locales/lang/zh-CN/views/application-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ export default {
addVariables: '添加变量',
editVariables: '编辑变量',
variablePlaceholder: '请输入变量',
variableListPlaceholder: '请添加拆分变量',
expression: {
label: '表达式',
placeholder: '请输入表达式',
Expand All @@ -457,6 +458,7 @@ export default {
text: '利用 AI 模型提取结构化参数',
extractParameters: {
label: '提取参数',
variableListPlaceholder: '请添加提取参数',
parameterType: '参数类型',
},
},
Expand Down
2 changes: 2 additions & 0 deletions ui/src/locales/lang/zh-Hant/views/application-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ export default {
addVariables: '添加變量',
editVariables: '編輯變量',
variablePlaceholder: '請輸入變量',
variableListPlaceholder: '請添加折開變數',
expression: {
label: '表達式',
placeholder: '請輸入表達式',
Expand All @@ -432,6 +433,7 @@ export default {
text: '利用 AI 模型提取結構化參數',
extractParameters: {
label: '提取參數',
variableListPlaceholder: '請添加選取參數',
parameterType: '參數類型',
},
},
Expand Down
8 changes: 6 additions & 2 deletions ui/src/workflow/nodes/parameter-extraction-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@
<el-form-item
prop="variable_list"
:rules="{
message: $t('views.applicationWorkflow.nodes.parameterExtractionNode.extractParameters.label'),
message: $t(
'views.applicationWorkflow.nodes.parameterExtractionNode.extractParameters.variableListPlaceholder',
),
trigger: 'blur',
required: true,
}"
Expand Down Expand Up @@ -184,7 +186,9 @@ const model_change = (model_id?: string) => {

const VariableSplittingRef = ref()
const validate = async () => {
return VariableSplittingRef.value.validate()
return VariableSplittingRef.value.validate().catch((err: any) => {
return Promise.reject({ node: props.nodeModel, errMessage: err })
})
}

onMounted(() => {
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 irregularities or significant issues with the provided code snippet. The code appears to be correctly structured and contains typical practices for form validation using El-form from Element UI in Vue.js.

Suggested Optimization:

1. Error Handling in validate Function:

  • Although there's already error handling in this part of the code using .catch(), it could benefit from adding a more clear message when an error occurs. This can help debugging and improve user experience if something goes wrong.
try:
    await VariableSplittingRef.value.validate()
except ValidationErrors as e:
    throw new Error(`Validation failed for variable spli<|fim_suffix|>on will then catch and reject this promise with additional data about the validation errors, including details such as the affected node and the specific error messages. If successful, the validated result is returned normally.

Expand Down
8 changes: 6 additions & 2 deletions ui/src/workflow/nodes/variable-splitting/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
<el-form-item
prop="variable_list"
:rules="{
message: $t('views.applicationWorkflow.variable.placeholder'),
message: $t(
'views.applicationWorkflow.nodes.variableSplittingNode.variableListPlaceholder',
),
trigger: 'blur',
required: true,
}"
Expand Down Expand Up @@ -81,7 +83,9 @@ const form_data = computed({
})
const VariableSplittingRef = ref()
const validate = async () => {
return VariableSplittingRef.value.validate()
return VariableSplittingRef.value.validate().catch((err: any) => {
return Promise.reject({ node: props.nodeModel, errMessage: err })
})
}
onMounted(() => {
set(props.nodeModel, 'validate', validate)
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 two main issues in this code:

  1. The message string in the validation rule is not localized correctly when using internationalization library $t.

  2. There's an error handling block inside validate() where it catches rejection, but it incorrectly returns { node: props.nodeModel, errMessage: err }. This might prevent further actions from being processed correctly.

Suggested Changes

const rules = computed(() => ({
-  variable_list: [
+  variable_list: {
     required: true,
     message: $t(
       'views.applicationWorkflow.nodes.variableSplittingNode.variableListPlaceholder',
     ),
-    trigger: 'blur'
+    },
+    trigger: ['blur', 'change'],
  }),
}))

Explanation of changes:

  • Added braces to make the message property inline with other properties of the same level, which makes object syntax cleaner.
  • Enhanced the trigger to include both blur and change events (trigger: ['blur', 'change']) to ensure that the form item checks if all required fields have been filled without losing focus after changing values.

Regarding the second issue of returning the promise directly instead of passing it through reject (which would require additional code elsewhere to handle), I think the correct approach is to either propagate errors properly so they can be handled higher up in your application logic, or to log them appropriately within the catch block if there's no possibility of handling them further. If you're confident these errors won't need to be propagated, leaving them caught should be sufficient for debugging purposes. But usually, it would best practice to fix what caused the initial failure rather than ignore it silently.

Expand Down
Loading