Skip to content

fix(merged): Always show a link for latests event of a merged group #81947

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

Merged
merged 5 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions static/app/views/issueDetails/groupMerged/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,8 @@ describe('Issues -> Merged View', function () {
expect(title.parentElement).toHaveTextContent(
'Fingerprints included in this issue (2)'
);

const links = await screen.findAllByRole('button', {name: 'View latest event'});
expect(links).toHaveLength(mockData.merged.length);
});
});
203 changes: 105 additions & 98 deletions static/app/views/issueDetails/groupMerged/mergedItem.tsx
Original file line number Diff line number Diff line change
@@ -1,44 +1,37 @@
import {Component} from 'react';
import {useEffect, useState} from 'react';
import styled from '@emotion/styled';

import {Button} from 'sentry/components/button';
import {Button, LinkButton} from 'sentry/components/button';
import Checkbox from 'sentry/components/checkbox';
import {Flex} from 'sentry/components/container/flex';
import EventOrGroupHeader from 'sentry/components/eventOrGroupHeader';
import {Tooltip} from 'sentry/components/tooltip';
import {IconChevron} from 'sentry/icons';
import {IconChevron, IconLink} from 'sentry/icons';
import {t} from 'sentry/locale';
import type {Fingerprint} from 'sentry/stores/groupingStore';
import GroupingStore from 'sentry/stores/groupingStore';
import {space} from 'sentry/styles/space';
import type {Organization} from 'sentry/types/organization';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {createIssueLink} from 'sentry/views/issueList/utils';

type Props = {
interface Props {
fingerprint: Fingerprint;
organization: Organization;
totalFingerprint: number;
};

type State = {
busy: boolean;
checked: boolean;
collapsed: boolean;
};

class MergedItem extends Component<Props, State> {
state: State = {
collapsed: false,
checked: false,
busy: false,
};
}

listener = GroupingStore.listen(data => this.onGroupChange(data), undefined);
export function MergedItem({fingerprint, totalFingerprint}: Props) {
const organization = useOrganization();
const location = useLocation();
const [busy, setBusy] = useState(false);
const [collapsed, setCollapsed] = useState(false);
const [checked, setChecked] = useState(false);

onGroupChange = ({unmergeState}) => {
function onGroupChange({unmergeState}) {
if (!unmergeState) {
return;
}

const {fingerprint} = this.props;
const stateForId = unmergeState.has(fingerprint.id)
? unmergeState.get(fingerprint.id)
: undefined;
Expand All @@ -48,42 +41,37 @@ class MergedItem extends Component<Props, State> {
}

Object.keys(stateForId).forEach(key => {
if (stateForId[key] === this.state[key]) {
return;
if (key === 'collapsed') {
setCollapsed(Boolean(stateForId[key]));
} else if (key === 'checked') {
setChecked(Boolean(stateForId[key]));
} else if (key === 'busy') {
setBusy(Boolean(stateForId[key]));
}

this.setState(prevState => ({...prevState, [key]: stateForId[key]}));
});
};
}

handleToggleEvents = () => {
const {fingerprint} = this.props;
function handleToggleEvents() {
GroupingStore.onToggleCollapseFingerprint(fingerprint.id);
};

// Disable default behavior of toggling checkbox
handleLabelClick(event: React.MouseEvent) {
event.preventDefault();
}

handleToggle = () => {
const {fingerprint} = this.props;
function handleToggle() {
const {latestEvent} = fingerprint;

if (this.state.busy) {
if (busy) {
return;
}

// clicking anywhere in the row will toggle the checkbox
GroupingStore.onToggleUnmerge([fingerprint.id, latestEvent.id]);
};
}

handleCheckClick() {
function handleCheckClick() {
// noop because of react warning about being a controlled input without `onChange`
// we handle change via row click
}

renderFingerprint(id: string, label?: string) {
function renderFingerprint(id: string, label?: string) {
if (!label) {
return id;
}
Expand All @@ -95,55 +83,77 @@ class MergedItem extends Component<Props, State> {
);
}

render() {
const {fingerprint, organization, totalFingerprint} = this.props;
const {latestEvent, id, label} = fingerprint;
const {collapsed, busy, checked} = this.state;
const checkboxDisabled = busy || totalFingerprint === 1;

// `latestEvent` can be null if last event w/ fingerprint is not within retention period
return (
<MergedGroup busy={busy}>
<Controls expanded={!collapsed}>
<FingerprintLabel onClick={this.handleToggle}>
<Tooltip
containerDisplayMode="flex"
disabled={!checkboxDisabled}
title={
checkboxDisabled && totalFingerprint === 1
? t('To check, the list must contain 2 or more items')
: undefined
}
>
<Checkbox
value={id}
checked={checked}
disabled={checkboxDisabled}
onChange={this.handleCheckClick}
useEffect(() => {
const teardown = GroupingStore.listen(data => onGroupChange(data), undefined);
return () => {
teardown();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const {latestEvent, id, label} = fingerprint;
const checkboxDisabled = busy || totalFingerprint === 1;

const issueLink = latestEvent
? createIssueLink({
organization,
location,
data: latestEvent,
eventId: latestEvent.id,
referrer: 'merged-item',
})
: null;

// `latestEvent` can be null if last event w/ fingerprint is not within retention period
return (
<MergedGroup busy={busy}>
<Controls expanded={!collapsed}>
<FingerprintLabel onClick={handleToggle}>
<Tooltip
containerDisplayMode="flex"
disabled={!checkboxDisabled}
title={
checkboxDisabled && totalFingerprint === 1
? t('To check, the list must contain 2 or more items')
: undefined
}
>
<Checkbox
value={id}
checked={checked}
disabled={checkboxDisabled}
onChange={handleCheckClick}
size="xs"
/>
</Tooltip>
{renderFingerprint(id, label)}
</FingerprintLabel>

<Button
aria-label={
collapsed ? t('Show %s fingerprints', id) : t('Collapse %s fingerprints', id)
}
size="zero"
borderless
icon={<IconChevron direction={collapsed ? 'down' : 'up'} size="xs" />}
onClick={handleToggleEvents}
/>
</Controls>

{!collapsed && (
<MergedEventList>
{issueLink ? (
<Flex align="center" gap={space(0.5)}>
<LinkButton
to={issueLink}
icon={<IconLink color={'linkColor'} />}
title={t('View latest event')}
aria-label={t('View latest event')}
borderless
size="xs"
style={{marginLeft: space(1)}}
/>
</Tooltip>

{this.renderFingerprint(id, label)}
</FingerprintLabel>

<Button
aria-label={
collapsed
? t('Show %s fingerprints', id)
: t('Collapse %s fingerprints', id)
}
size="zero"
borderless
icon={<IconChevron direction={collapsed ? 'down' : 'up'} size="xs" />}
onClick={this.handleToggleEvents}
/>
</Controls>

{!collapsed && (
<MergedEventList className="event-list">
{latestEvent && (
<EventDetails className="event-details">
<EventDetails>
<EventOrGroupHeader
data={latestEvent}
organization={organization}
Expand All @@ -152,12 +162,12 @@ class MergedItem extends Component<Props, State> {
source="merged-item"
/>
</EventDetails>
)}
</MergedEventList>
)}
</MergedGroup>
);
}
</Flex>
) : null}
</MergedEventList>
)}
</MergedGroup>
);
}

const MergedGroup = styled('div')<{busy: boolean}>`
Expand Down Expand Up @@ -202,10 +212,7 @@ const MergedEventList = styled('div')`
const EventDetails = styled('div')`
display: flex;
justify-content: space-between;

.event-list & {
padding: ${space(1)};
}
padding: ${space(1)};
`;

export default MergedItem;
1 change: 0 additions & 1 deletion static/app/views/issueDetails/groupMerged/mergedList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ function MergedList({
{fingerprintsWithLatestEvent.map(fingerprint => (
<MergedItem
key={fingerprint.id}
organization={organization}
fingerprint={fingerprint}
totalFingerprint={fingerprintsWithLatestEvent.length}
/>
Expand Down
Loading