-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore(top-issues): add pasting json to single card view #105477
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
base: master
Are you sure you want to change the base?
Conversation
d228f11 to
b46d65e
Compare
|
|
||
| return filtered.sort((a, b) => (b.fixability_score ?? 0) - (a.fixability_score ?? 0)); | ||
| }, [ |
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.
Bug: The handleParseJson function unsafely casts parsed JSON to ClusterSummary[] without validating object properties, leading to a crash when required properties are accessed later.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The handleParseJson function uses a type assertion as ClusterSummary[] on user-provided JSON without validating that the objects in the array conform to the ClusterSummary interface. Subsequent code, such as the filtering logic, accesses required properties like cluster.project_ids without defensive checks. If a user pastes JSON where objects are missing this property, the application will crash with a TypeError when cluster.project_ids.some() is called. This affects a developer-facing tool within the production UI.
💡 Suggested Fix
Implement a type guard or use a validation library (like Zod) to properly validate the structure of each object in the parsed JSON array against the ClusterSummary interface before casting and setting the state with setCustomClusterData. This ensures all required properties exist before they are accessed.
🤖 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: static/app/views/issueList/pages/topIssues.tsx#L961-L963
Potential issue: The `handleParseJson` function uses a type assertion `as
ClusterSummary[]` on user-provided JSON without validating that the objects in the array
conform to the `ClusterSummary` interface. Subsequent code, such as the filtering logic,
accesses required properties like `cluster.project_ids` without defensive checks. If a
user pastes JSON where objects are missing this property, the application will crash
with a `TypeError` when `cluster.project_ids.some()` is called. This affects a
developer-facing tool within the production UI.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8050715
| </Button> | ||
| </Flex> | ||
| </JsonInputContainer> | ||
| )} |
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.
Loading indicator shown when custom data is loaded
When custom JSON data is loaded via the dev tool, the isPending check doesn't account for customClusterData being present. If a user loads custom data before the API query completes (or if the query is slow/fails), isPending may still be true from the original query, causing the LoadingIndicator to display instead of the custom cluster data. The condition needs to also check !isUsingCustomData or !customClusterData.
Add dev tool to paste json for the new top issues view