Skip to content
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

fix: update option list search logic to handle string and array form_data values #2701

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

shaohuzhang1
Copy link
Contributor

fix: update option list search logic to handle string and array form_data values --bug=1053919 --user=刘瑞斌 【函数库】非必填启动参数开启启动参数后再次编辑多选置空 https://www.tapd.cn/57709429/s/1677136

…data values

--bug=1053919 --user=刘瑞斌 【函数库】非必填启动参数开启启动参数后再次编辑多选置空 https://www.tapd.cn/57709429/s/1677136
Copy link

f2c-ci-robot bot commented Mar 27, 2025

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.

Instructions 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/test-infra repository.

Copy link

f2c-ci-robot bot commented Mar 27, 2025

[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.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

} else {
return form_data[item.field].indexOf([value_field]) === -1
}
})
if (find) {
return { [item.field]: form_data[item.field] }
}
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 generally correct, but there are a few things to consider for optimization:

  1. Code Style: The use of const for local variables is good practice. However, TypeScript types should be explicitly defined when using optional chaining (?.) because it can sometimes lead to unexpected undefined values.

  2. Type Checking: Ensure that the types of inputs to your function and methods are correctly handled. If 'string' is expected and you want more complex string matching logic (e.g., substring search), this could benefit from additional type checks or functions like String.includes.

  3. Performance Considerations: Since your current implementation iterates over all options to find a match based on a single criteria, this might not scale well with large lists. Consider sorting the list or creating an index if possible.

    // Example of potential performance improvement
    const optionListIndex = new Map(item.option_list.map(opt => [opt[value_field], opt]));
    
    if (optionListIndex.has(form_data[item.field])) {
      return { [item.field]: form_data[item.field] };
    }
  4. Functionality Clarification: Currently, if no match is found, the returned object will be empty {}. For users expecting some indication, you might want to add logging or display messages indicating that no valid option was found.

Based on these points, here's a slightly improved version:

/**
 * Renders data based on form inputs.
 * @param form_data - An object containing form input data.
 * @param items - An array of configuration objects defining how to render each field.
 * @returns A JavaScript object representing the rendered data.
 */
function render(form_data, items) {
  let result = {};

  items.forEach((item) => {
    if (form_data[item.field] !== undefined && item.value_field && item.option_list) {
      const value_field = item.value_field;
      const option_list = item.option_list;

      // Create index for faster lookup if needed
      const optionListIndex = new Map(option_list.map(opt => [opt[value_field], opt]));

      const findValue = () => {
        if (typeof form_data[item.field] === 'string') {
          return option_list.findIndex(i => i[value_field] === form_data[item.field]);
        } else {
          return typeof form_data[item.field][value_field] !== 'undefined';
        }
      };

      const index = findValue();
      if (index >= 0 || optionListIndex.has(form_data[item.field])) {
        result[item.field] = form_data[item.field];
      }
    }
  });

  return result;
}

This version adds basic functionality around indexing, which can improve performance. It also includes error handling for undefined indexes in case indexOf returns -1.

@liuruibin liuruibin merged commit 7145f30 into main Mar 27, 2025
4 of 5 checks passed
@liuruibin liuruibin deleted the pr@main@fix_update_option branch March 27, 2025 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants