-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: Error parameter not popped up prompt during variable splitting and parameter extraction node verification #4238
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
| }" | ||
|
|
@@ -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(() => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
| }" | ||
|
|
@@ -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) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two main issues in this code:
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:
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. |
||
|
|
||
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.
The provided code appears to be generating property settings based on a list of variables with fields like
field,parameter_type,desc, andlabel. 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 accessNoneproperties directly.I suggest:
'type': variable['type'], 'description': (variable.get('desc') or ""),'type': variable['type'], 'description': str((variable.get('desc', '')),This modification ensures that if
variable['desc']isNone, it defaults to an empty string, avoiding any runtime errors.