Skip to content

fix(web): normalize dropdown option-filter input and option values - #2874

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-option-filter-inputs
Open

fix(web): normalize dropdown option-filter input and option values#2874
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-option-filter-inputs

Conversation

@yyqdbngt

Copy link
Copy Markdown

Summary

  • components/InstanceSelect.tsx: extract the instance-selector filterOption into an exported matchesInstanceOption helper that coerces both the search string and the option label with String(...) ?? '' before lower-casing.
  • pages/settings/AiAssistantTab.tsx: extract the two AutoComplete filterOptions (model selector + API-base preset selector) into one exported matchesAiOption helper with the same double-sided coercion; the old API-base filter additionally called .toLowerCase() on option.value without any String() guard, and all three old expressions called input.toLowerCase() unguarded.
  • Add regression suites for both helpers (case-insensitive match, empty input, nullish/non-string options and input).

Why

These dropdown filters compared user input against option fields without normalizing either side: a missing search string (input.toLowerCase() on undefined) or a non-string option value ((option?.value ?? '').toLowerCase() on a number/null payload) would throw inside antd's filter path and break the whole dropdown. This is the same class of gap already fixed for the studio producer-group selector (matchesProducerGroupOption) and the component already used the one-sided String(option?.label ?? '') pattern while leaving input unguarded.

Testing

  • cd web && ./node_modules/.bin/vitest run src/pages/settings/__tests__/aiAssistantOptionFilter.test.ts src/components/__tests__/instanceSelectFilter.test.ts — 2 files, 10 new tests passed.
  • cd web && ./node_modules/.bin/vitest run src/pages/settings/__tests__/AiAssistantTab.test.tsx — 5 pre-existing tests still pass.
  • cd web && ./node_modules/.bin/vitest run src/components — 8 files, 58 tests passed (no regressions in the shared component domain).
  • cd web && ./node_modules/.bin/tsc --noEmit — clean.
  • cd web && ./node_modules/.bin/eslint src/pages/settings/AiAssistantTab.tsx src/components/InstanceSelect.tsx src/pages/settings/__tests__/aiAssistantOptionFilter.test.ts src/components/__tests__/instanceSelectFilter.test.ts — 0 errors (2 react-refresh warnings for the exported helpers, same precedent as other helper exports).

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

This PR contains moderate changes (182 lines). I have performed an initial structural review.

Observations

  • Changes appear well-scoped
  • CLA status: unknown

Maintainers: please verify the implementation details match the intended behavior.


Automated review by github-manager-bot

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

Defensive fix that improves input validation and error handling. Code looks clean and follows existing patterns.

LGTM


Automated review by "github-manager-bot"

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — defensive fix improving input validation and error handling.


Automated review by "github-manager-bot"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants