feat: Use names instead of id for component dropdown #3955
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
Comment on lines
47
to
+48
| const componentNames = componentsData?.components?.map( | ||
| (component) => component?.id | ||
| (component) => component?.name |
There was a problem hiding this comment.
Bug: Type mismatch in MultiSelect component: items are strings, selectedItemsOverride are objects, causing isEqual to fail.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The MultiSelect component receives an array of strings (component?.name) for its items prop, but an array of Component objects ({name, id}) for its selectedItemsOverride prop. The internal isItemSelected function uses lodash.isEqual to compare these, which returns false when comparing a string (e.g., 'Component 1') with an object (e.g., {name: 'Component 1', id: 'component1'}). This type mismatch prevents correct item selection matching, causing all dropdown items to appear as unselected, leading to visual duplicates and a broken selection UI. A related test in Header.test.tsx will also fail as it expects to find the component's id.
💡 Suggested Fix
Ensure the items prop passed to MultiSelect is an array of Component objects ({name, id}) to match the type of selectedItemsOverride and the onChange handler's expected input. This can be achieved by mapping componentNames to the full Component objects instead of just their name property.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/pages/RepoPage/CoverageTab/ComponentsTab/Header/Header.tsx#L47-L48
Potential issue: The `MultiSelect` component receives an array of strings
(`component?.name`) for its `items` prop, but an array of `Component` objects (`{name,
id}`) for its `selectedItemsOverride` prop. The internal `isItemSelected` function uses
`lodash.isEqual` to compare these, which returns `false` when comparing a string (e.g.,
'Component 1') with an object (e.g., `{name: 'Component 1', id: 'component1'}`). This
type mismatch prevents correct item selection matching, causing all dropdown items to
appear as unselected, leading to visual duplicates and a broken selection UI. A related
test in `Header.test.tsx` will also fail as it expects to find the component's `id`.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3339232
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Prevent Tests Dashboard |
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| const componentNames = componentsData?.components?.map( | ||
| (component) => component?.id | ||
| (component) => component?.name |
There was a problem hiding this comment.
Bug: Component filter uses names instead of IDs for API
The change from component?.id to component?.name updates what's displayed in the dropdown, but the selected values are also used for API filtering via updateParams({ components }). The selected names flow to useRepoComponentsTable where they're passed as filters: { components: params?.components } to useRepoComponents. Test data shows id and name are distinct values (e.g., { name: 'foo', id: '1' }). If the GraphQL ComponentMeasurementsSetFilters expects component IDs for filtering, passing names will break the filter functionality. The dropdown needs to display names while still passing IDs for filtering.
Contributor
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
Bundle ReportChanges will increase total bundle size by 4 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: gazebo-production-systemAssets Changed:
Files in
view changes for bundle: gazebo-production-esmAssets Changed:
Files in
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes https://linear.app/getsentry/issue/CCMRG-918/components-drop-down-in-components-tab-displays-the-list-of-component
Changes from id names to pretty names.
Before:
After:
Note
Switches ComponentsTab dropdown to display component names instead of IDs.
src/pages/RepoPage/CoverageTab/ComponentsTab/Header/Header.tsx, map dropdown items tocomponent.nameinstead ofcomponent.idfor the multi-select.Written by Cursor Bugbot for commit 61cca7a. This will update automatically on new commits. Configure here.