Skip to content
Merged
Changes from all commits
Commits
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
135 changes: 128 additions & 7 deletions static/app/views/issueList/pages/dynamicGrouping.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Fragment, useMemo, useState} from 'react';
import {Fragment, useCallback, useMemo, useState} from 'react';
import styled from '@emotion/styled';

import {Container, Flex} from '@sentry/scraps/layout';
Expand All @@ -9,6 +9,7 @@ import {Checkbox} from 'sentry/components/core/checkbox';
import {Disclosure} from 'sentry/components/core/disclosure';
import {NumberInput} from 'sentry/components/core/input/numberInput';
import {Link} from 'sentry/components/core/link';
import {TextArea} from 'sentry/components/core/textarea';
import EventOrGroupTitle from 'sentry/components/eventOrGroupTitle';
import EventMessage from 'sentry/components/events/eventMessage';
import TimesTag from 'sentry/components/group/inboxBadges/timesTag';
Expand All @@ -17,7 +18,14 @@ import ProjectBadge from 'sentry/components/idBadge/projectBadge';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import Redirect from 'sentry/components/redirect';
import TimeSince from 'sentry/components/timeSince';
import {IconCalendar, IconClock, IconFire, IconFix} from 'sentry/icons';
import {
IconCalendar,
IconClock,
IconClose,
IconFire,
IconFix,
IconUpload,
} from 'sentry/icons';
import {t, tn} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Group} from 'sentry/types/group';
Expand Down Expand Up @@ -331,17 +339,48 @@ function DynamicGrouping() {
const [filterByAssignedToMe, setFilterByAssignedToMe] = useState(true);
const [selectedTeamIds, setSelectedTeamIds] = useState<Set<string>>(new Set());
const [minFixabilityScore, setMinFixabilityScore] = useState(50);
const [removedClusterIds, setRemovedClusterIds] = useState<Set<number>>(new Set());
const [removedClusterIds, setRemovedClusterIds] = useState(new Set<number>());
const [showJsonInput, setShowJsonInput] = useState(false);
const [jsonInputValue, setJsonInputValue] = useState('');
const [customClusterData, setCustomClusterData] = useState<ClusterSummary[] | null>(
null
);
const [jsonError, setJsonError] = useState<string | null>(null);

// Fetch cluster data from API
const {data: topIssuesResponse, isPending} = useApiQuery<TopIssuesResponse>(
[`/organizations/${organization.slug}/top-issues/`],
{
staleTime: 60000,
enabled: customClusterData === null, // Only fetch if no custom data
}
);

const clusterData = topIssuesResponse?.data ?? [];
const handleParseJson = useCallback(() => {
try {
const parsed = JSON.parse(jsonInputValue);
// Support both {data: [...]} format and direct array format
const clusters = Array.isArray(parsed) ? parsed : parsed?.data;
if (!Array.isArray(clusters)) {
setJsonError(t('JSON must be an array or have a "data" property with an array'));
return;
}
setCustomClusterData(clusters as ClusterSummary[]);
setJsonError(null);
setShowJsonInput(false);
} catch (e) {
setJsonError(t('Invalid JSON: %s', e instanceof Error ? e.message : String(e)));
}
}, [jsonInputValue]);
Comment on lines +364 to +374
Copy link

Choose a reason for hiding this comment

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

Bug: Insufficient JSON validation in handleParseJson can cause runtime TypeError when required properties are missing from custom data.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The handleParseJson function performs insufficient validation on user-provided JSON. It only checks for valid JSON syntax and array structure, but does not verify if required properties like group_ids or cluster_id exist within each object. This can lead to runtime TypeError exceptions (e.g., Cannot read property 'length' of undefined or Cannot read property 'join' of undefined) when components like ClusterCard attempt to access these missing properties, causing the UI to crash.

💡 Suggested Fix

Implement robust schema validation for the parsed JSON objects within handleParseJson to ensure all ClusterSummary required properties (e.g., group_ids, cluster_id, title) are present before setting customClusterData.

🤖 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/dynamicGrouping.tsx#L359-L374

Potential issue: The `handleParseJson` function performs insufficient validation on
user-provided JSON. It only checks for valid JSON syntax and array structure, but does
not verify if required properties like `group_ids` or `cluster_id` exist within each
object. This can lead to runtime `TypeError` exceptions (e.g., `Cannot read property
'length' of undefined` or `Cannot read property 'join' of undefined`) when components
like `ClusterCard` attempt to access these missing properties, causing the UI to crash.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3632176


const handleClearCustomData = useCallback(() => {
setCustomClusterData(null);
setJsonInputValue('');
setJsonError(null);
}, []);

const clusterData = customClusterData ?? topIssuesResponse?.data ?? [];
const isUsingCustomData = customClusterData !== null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Team filter uses API data instead of custom data

The teamsInData memo extracts teams from topIssuesResponse?.data instead of clusterData, which includes custom JSON data. When users paste custom JSON, team filtering won't work because the team extraction logic ignores the custom data and only looks at API data. The dependency array also needs updating to track clusterData or customClusterData.

Additional Locations (1)

Fix in Cursor Fix in Web


// Extract all unique teams from the cluster data
const teamsInData = useMemo(() => {
Comment on lines 385 to 386
Copy link

Choose a reason for hiding this comment

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

Bug: Team filter is not populated when customClusterData is used because teamsInData only extracts from topIssuesResponse?.data.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The teamsInData useMemo hook only extracts teams from topIssuesResponse?.data and depends solely on it. However, the displayed clusterData uses customClusterData ?? topIssuesResponse?.data. When customClusterData is present, the API query for topIssuesResponse?.data is disabled, leading to an empty teamsInData. This causes the team filter section to be hidden, preventing users from filtering by team when using custom JSON data.

💡 Suggested Fix

Modify the teamsInData useMemo to extract teams from clusterData instead of topIssuesResponse?.data, and update its dependency array to include clusterData.

🤖 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/dynamicGrouping.tsx#L385-L386

Potential issue: The `teamsInData` `useMemo` hook only extracts teams from
`topIssuesResponse?.data` and depends solely on it. However, the displayed `clusterData`
uses `customClusterData ?? topIssuesResponse?.data`. When `customClusterData` is
present, the API query for `topIssuesResponse?.data` is disabled, leading to an empty
`teamsInData`. This causes the team filter section to be hidden, preventing users from
filtering by team when using custom JSON data.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3632176

Expand Down Expand Up @@ -417,9 +456,72 @@ function DynamicGrouping() {
return (
<PageWrapper>
<HeaderSection>
<Heading as="h1" style={{marginBottom: space(2)}}>
{t('Top Issues')}
</Heading>
<Flex align="center" gap="md" style={{marginBottom: space(2)}}>
<Heading as="h1">{t('Top Issues')}</Heading>
{isUsingCustomData && (
<CustomDataBadge>
<Text size="xs" bold>
{t('Using Custom Data')}
</Text>
<Button
size="zero"
borderless
icon={<IconClose size="xs" />}
aria-label={t('Clear custom data')}
onClick={handleClearCustomData}
/>
</CustomDataBadge>
)}
</Flex>

<Flex gap="sm" style={{marginBottom: space(2)}}>
<Button
size="sm"
icon={<IconUpload size="xs" />}
onClick={() => setShowJsonInput(!showJsonInput)}
>
{showJsonInput ? t('Hide JSON Input') : t('Paste JSON')}
</Button>
</Flex>

{showJsonInput && (
<JsonInputContainer>
<Text size="sm" variant="muted" style={{marginBottom: space(1)}}>
{t(
'Paste cluster JSON data below. Accepts either a raw array of clusters or an object with a "data" property.'
)}
</Text>
<TextArea
value={jsonInputValue}
onChange={(e: React.ChangeEvent<HTMLTextAreaElement>) => {
setJsonInputValue(e.target.value);
setJsonError(null);
}}
placeholder={t('Paste JSON here...')}
rows={8}
monospace
/>
{jsonError && (
<Text size="sm" style={{color: 'var(--red400)', marginTop: space(1)}}>
{jsonError}
</Text>
)}
<Flex gap="sm" style={{marginTop: space(1)}}>
<Button size="sm" priority="primary" onClick={handleParseJson}>
{t('Parse and Load')}
</Button>
<Button
size="sm"
onClick={() => {
setShowJsonInput(false);
setJsonError(null);
}}
>
{t('Cancel')}
</Button>
</Flex>
</JsonInputContainer>
)}

{isPending ? null : (
<Fragment>
Expand Down Expand Up @@ -686,4 +788,23 @@ const FilterLabel = styled('span')<{disabled?: boolean}>`
color: ${p => (p.disabled ? p.theme.disabled : p.theme.subText)};
`;

const JsonInputContainer = styled('div')`
margin-bottom: ${space(2)};
padding: ${space(2)};
background: ${p => p.theme.backgroundSecondary};
border: 1px solid ${p => p.theme.border};
border-radius: ${p => p.theme.borderRadius};
`;

const CustomDataBadge = styled('div')`
display: flex;
align-items: center;
gap: ${space(0.5)};
padding: ${space(0.5)} ${space(1)};
background: ${p => p.theme.yellow100};
border: 1px solid ${p => p.theme.yellow300};
border-radius: ${p => p.theme.borderRadius};
color: ${p => p.theme.yellow400};
`;

export default DynamicGrouping;
Loading