Skip to content

fix: The switch node does not display default values #1957

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.

Sign up for GitHub

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

Merged
shaohuzhang1 merged 1 commit into from
Dec 31, 2024
Merged

fix: The switch node does not display default values #1957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 7 additions & 3 deletions ui/src/workflow/nodes/base-node/component/UserInputFieldTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,20 @@ function refreshFieldList(data: any, index: any) {

const getDefaultValue = (row: any) => {
if (row.default_value) {
const default_value = row.option_list?.filter((v: any) => row.default_value.indexOf(v.value) > -1)
.map((v: any) => v.label).join(',')
const default_value = row.option_list
?.filter((v: any) => row.default_value.indexOf(v.value) > -1)
.map((v: any) => v.label)
.join(',')
if (default_value) {
return default_value
}
return row.default_value
}
if (row.default_value !== undefined) {
return row.default_value
}
}


onMounted(() => {
if (!props.nodeModel.properties.user_input_field_list) {
if (props.nodeModel.properties.input_field_list) {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 31, 2024

Choose a reason for hiding this comment

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

Sure, I can review this code snippet for you. Here are some suggestions:

const getDefaultValue = (row: any): string | null => {
  if (typeof row.default_value === 'string' && typeof row.option_list === 'object') {
    return row.option_list.find(option => option.value.includes(row.default_value))
      ? row.option_list.filter(option => option.value.includes(row.default_value)).map(option => option.label).join(', ')
      : null;
  } else if (typeof row.default_value !== 'undefined') {
    return String(row.default_value);
  }
  return '';
};

Changes and Suggestions:

  1. Type Checking: Added type checks to ensure default_value is a string and option_list is an object.
  2. Null Check: Replaced return '' with null when no match is found, which allows the caller to determine whether a valid value was returned or not.
  3. Simplified Filtering Logic: Used find instead of filter where possible to find only matches without creating unnecessary arrays.

This should help improve the robustness and maintainability of the getDefaultValues function. Let me know if you have any other questions!

Expand Down
13 changes: 11 additions & 2 deletions ui/src/workflow/nodes/form-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@
</div></template
>

<el-table class="border" v-if="form_data.form_field_list.length > 0" :data="form_data.form_field_list">
<el-table
class="border"
v-if="form_data.form_field_list.length > 0"
:data="form_data.form_field_list"
>
<el-table-column prop="field" label="参数">
<template #default="{ row }">
<span :title="row.field" class="ellipsis-1">{{ row.field }}</span>
Expand Down Expand Up @@ -84,7 +88,9 @@

<el-table-column prop="default_value" label="默认值">
<template #default="{ row }">
<span :title="row.default_value" class="ellipsis-1">{{ getDefaultValue(row) }}</span>
<span :title="row.default_value" class="ellipsis-1">{{
getDefaultValue(row)
}}</span>
</template>
</el-table-column>
<el-table-column label="必填">
Expand Down Expand Up @@ -208,6 +214,9 @@ const getDefaultValue = (row: any) => {
}
return row.default_value
}
if (row.default_value !== undefined) {
return row.default_value
}
}

const validate = () => {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 31, 2024

Choose a reason for hiding this comment

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

There are no immediate irregularities, potential issues, or optimization opportunities identified in the provided Vue.js template with an El Table component.

  1. Line Length: The line length of the template is slightly too long, which might not be ideal according to some coding guidelines but does not affect functionality.

  2. Code Clarity:

    • In validate function, you have duplicate checks (if (rule.value === '')). It's unnecessary because checking against an empty string will naturally fall through to the next condition when it doesn't match other conditions (like type validation).

  3. Empty Row Handling:

    • You may want to consider handling rows where neither a field nor a default value is specified more gracefully. This could involve displaying "N/A" or another placeholder depending on your UI/UX requirements. Currently, these values would default to their native JavaScript behavior.

Overall, the code appears straightforward and functional within the context of generating a table from an array of dynamic fields. Here’s a minor improvement for better clarity:

const validate = () => {
  return form_data.form_field_list.every(field => {
    if ((field.hasOwnProperty('field') && field.field.trim() === '') ||
        (!field.hasOwnProperty('default_value'))) {
      errors.push({
        dataPath: `/form_field/${index}`,
        message: 'Field name or Default value must be filled out.'
      });
    }
  });
}

This adjustment ensures that each field in the list has either a non-empty field name and/or a defined default value before proceeding with validation. Adjustments can always be made based on specific application needs regarding error messages and user interface logic.

Expand Down
Loading