Skip to content
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

feat(related_issues): Create separate sections for each relation type #70648

Merged
merged 2 commits into from
May 13, 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
78 changes: 34 additions & 44 deletions static/app/views/issueDetails/groupRelatedIssues/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {render, screen} from 'sentry-test/reactTestingLibrary';
import {GroupRelatedIssues} from 'sentry/views/issueDetails/groupRelatedIssues';

describe('Related Issues View', function () {
let relatedIssuesMock: jest.Mock;
let sameRootIssuesMock: jest.Mock;
let traceIssuesMock: jest.Mock;
let issuesMock: jest.Mock;
const router = RouterFixture();

Expand All @@ -21,44 +22,20 @@ describe('Related Issues View', function () {
const params = {groupId: groupId};
const errorType = 'RuntimeError';
const noData = {
data: [
Copy link
Member Author

Choose a reason for hiding this comment

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

Now, the shape of the data returned by the endpoint will only contain one type of relation.

For instance:

{
  "type": "trace_connected",
  "data": [
    5290882209,
    5291204527,
    5298771767,
    4939519003,
    4808538909,
    5298771743
  ],
  "meta": {
    "event_id": "fe2c6fbf76ac4e1886e7e150a9f9b98d",
    "trace_id": "361d8fb1fb364a2693399b0a2541787e"
  }
}

{
type: 'same_root_cause',
data: [],
},
{
type: 'trace_connected',
data: [],
},
],
type: 'irrelevant',
data: [],
};
const onlySameRootData = {
data: [
{
type: 'same_root_cause',
data: [group1, group2],
},
{
type: 'trace_connected',
data: [],
},
],
type: 'same_root_cause',
data: [group1, group2],
};
const onlyTraceConnectedData = {
data: [
{
type: 'same_root_cause',
data: [],
},
{
type: 'trace_connected',
data: [group1, group2],
meta: {
event_id: 'abcd',
trace_id: '1234',
},
},
],
type: 'trace_connected',
data: [group1, group2],
meta: {
event_id: 'abcd',
trace_id: '1234',
},
};
const issuesData = [
{
Expand Down Expand Up @@ -99,8 +76,12 @@ describe('Related Issues View', function () {
});

it('renders with no data', async function () {
relatedIssuesMock = MockApiClient.addMockResponse({
Copy link
Member Author

Choose a reason for hiding this comment

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

We now need two different mocks.

url: `/issues/${groupId}/related-issues/`,
sameRootIssuesMock = MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/?type=same_root_cause`,
body: noData,
});
traceIssuesMock = MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/?type=trace_connected`,
body: noData,
});
render(
Expand All @@ -121,14 +102,19 @@ describe('Related Issues View', function () {
await screen.findByText('No trace-connected related issues were found.')
).toBeInTheDocument();

expect(relatedIssuesMock).toHaveBeenCalled();
expect(sameRootIssuesMock).toHaveBeenCalled();
expect(traceIssuesMock).toHaveBeenCalled();
});

it('renders with same root issues', async function () {
relatedIssuesMock = MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/`,
sameRootIssuesMock = MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/?type=same_root_cause`,
body: onlySameRootData,
});
MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/?type=trace_connected`,
body: [],
});
issuesMock = MockApiClient.addMockResponse({
url: orgIssuesEndpoint,
body: issuesData,
Expand All @@ -149,7 +135,7 @@ describe('Related Issues View', function () {
expect(await screen.findByText(`EARTH-${group1}`)).toBeInTheDocument();
expect(await screen.findByText(`EARTH-${group2}`)).toBeInTheDocument();

expect(relatedIssuesMock).toHaveBeenCalled();
expect(sameRootIssuesMock).toHaveBeenCalled();
expect(issuesMock).toHaveBeenCalled();
expect(
await screen.findByText('No trace-connected related issues were found.')
Expand All @@ -163,8 +149,12 @@ describe('Related Issues View', function () {
});

it('renders with trace connected issues', async function () {
relatedIssuesMock = MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/`,
MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/?type=same_root_cause`,
body: [],
});
traceIssuesMock = MockApiClient.addMockResponse({
url: `/issues/${groupId}/related-issues/?type=trace_connected`,
body: onlyTraceConnectedData,
});
issuesMock = MockApiClient.addMockResponse({
Expand All @@ -186,7 +176,7 @@ describe('Related Issues View', function () {
expect(await screen.findByText(`EARTH-${group1}`)).toBeInTheDocument();
expect(await screen.findByText(`EARTH-${group2}`)).toBeInTheDocument();

expect(relatedIssuesMock).toHaveBeenCalled();
expect(traceIssuesMock).toHaveBeenCalled();
expect(issuesMock).toHaveBeenCalled();
expect(
await screen.findByText('No same-root-cause related issues were found.')
Expand Down
199 changes: 99 additions & 100 deletions static/app/views/issueDetails/groupRelatedIssues/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,137 +19,136 @@ type RouteParams = {
type Props = RouteComponentProps<RouteParams, {}>;

type RelatedIssuesResponse = {
data: [
{
data: number[];
meta: {
event_id: string;
trace_id: string;
};
type: string;
},
];
data: number[];
meta: {
event_id: string;
trace_id: string;
};
type: string;
};

interface RelatedIssuesSectionProps {
groupId: string;
orgSlug: string;
relationType: string;
}

function GroupRelatedIssues({params}: Props) {
const {groupId} = params;

const organization = useOrganization();
const orgSlug = organization.slug;

return (
<Fragment>
<RelatedIssuesSection
Copy link
Member Author

Choose a reason for hiding this comment

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

I have created a new component. Each component will do its fetches independently.

groupId={groupId}
orgSlug={orgSlug}
relationType="same_root_cause"
/>
<RelatedIssuesSection
groupId={groupId}
orgSlug={orgSlug}
relationType="trace_connected"
/>
</Fragment>
);
}

function RelatedIssuesSection({
groupId,
orgSlug,
relationType,
}: RelatedIssuesSectionProps) {
// Fetch the list of related issues
const {
isLoading,
isError,
data: relatedIssues,
refetch,
} = useApiQuery<RelatedIssuesResponse>([`/issues/${groupId}/related-issues/`], {
staleTime: 0,
});

let traceMeta = {
trace_id: '',
event_id: '',
};
const {
same_root_cause: sameRootCauseIssues = [],
trace_connected: traceConnectedIssues = [],
} = (relatedIssues?.data ?? []).reduce(
(mapping, item) => {
if (item.type === 'trace_connected') {
traceMeta = {...item.meta};
}
const issuesList = item.data;
mapping[item.type] = issuesList;
return mapping;
},
{same_root_cause: [], trace_connected: []}
} = useApiQuery<RelatedIssuesResponse>(
[`/issues/${groupId}/related-issues/?type=${relationType}`],
Copy link
Member Author

Choose a reason for hiding this comment

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

We now fetch with ?type=${relationType}.

{
staleTime: 0,
}
);

const traceMeta = relationType === 'trace_connected' ? relatedIssues?.meta : undefined;
const issues = relatedIssues?.data ?? [];
const query = `issue.id:[${issues}]`;
// project=-1 allows ensuring that the query will show issues from any projects for the org
// This is important for traces since issues can be for any project in the org
const baseUrl = `/organizations/${orgSlug}/issues/?project=-1`;
let title;
let linkToTrace;
let openIssuesButton;
if (relationType === 'trace_connected' && traceMeta) {
Copy link
Member Author

Choose a reason for hiding this comment

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

In this block we initialize all the values specific to the trace-connected section.

title = t('Issues in the same trace');
linkToTrace = (
<small>
{t('These issues were all found within ')}
<Link
to={`/organizations/${orgSlug}/performance/trace/${traceMeta.trace_id}/?node=error-${traceMeta.event_id}`}
>
{t('this trace')}
</Link>
.
</small>
);
openIssuesButton = (
<LinkButton
to={`${baseUrl}&query=trace:${traceMeta.trace_id}`}
size="xs"
analyticsEventName="Clicked Open Issues from trace-connected related issues"
analyticsEventKey="similar_issues.trace_connected_issues_clicked_open_issues"
>
{t('Open in Issues')}
</LinkButton>
);
} else {
title = t('Issues caused by the same root cause');
openIssuesButton = (
<LinkButton
to={`${baseUrl}&query=issue.id:[${groupId},${issues}]`}
size="xs"
analyticsEventName="Clicked Open Issues from same-root related issues"
analyticsEventKey="similar_issues.same_root_cause_clicked_open_issues"
>
{t('Open in Issues')}
</LinkButton>
);
}

return (
<Fragment>
<HeaderWrapper>
<Title>{title}</Title>
{isLoading ? (
<LoadingIndicator />
) : isError ? (
<LoadingError
message={t('Unable to load related issues, please try again later')}
onRetry={refetch}
/>
) : (
) : issues.length > 0 ? (
<Fragment>
<div>
<HeaderWrapper>
<Title>{t('Issues caused by the same root cause')}</Title>
{sameRootCauseIssues.length > 0 ? (
<div>
<TextButtonWrapper>
<div />
<LinkButton
to={`${baseUrl}&query=issue.id:[${groupId},${sameRootCauseIssues}]`}
size="xs"
analyticsEventName="Clicked Open Issues from same-root related issues"
analyticsEventKey="similar_issues.same_root_cause_clicked_open_issues"
>
{t('Open in Issues')}
</LinkButton>
</TextButtonWrapper>
<GroupList
orgSlug={orgSlug}
queryParams={{query: `issue.id:[${sameRootCauseIssues}]`}}
source="similar-issues-tab"
canSelectGroups={false}
withChart={false}
/>
</div>
) : (
<small>{t('No same-root-cause related issues were found.')}</small>
)}
</HeaderWrapper>
</div>
<div>
<HeaderWrapper>
<Title>{t('Issues in the same trace')}</Title>
{traceConnectedIssues.length > 0 ? (
<div>
<TextButtonWrapper>
<small>
{t('These issues were all found within ')}
<Link
to={`/organizations/${orgSlug}/performance/trace/${traceMeta.trace_id}/?node=error-${traceMeta.event_id}`}
>
{t('this trace')}
</Link>
.
</small>
<LinkButton
to={`${baseUrl}&query=trace:${traceMeta.trace_id}`}
size="xs"
analyticsEventName="Clicked Open Issues from trace-connected related issues"
analyticsEventKey="similar_issues.trace_connected_issues_clicked_open_issues"
>
{t('Open in Issues')}
</LinkButton>
</TextButtonWrapper>
<GroupList
orgSlug={orgSlug}
queryParams={{query: `issue.id:[${traceConnectedIssues}]`}}
source="similar-issues-tab"
canSelectGroups={false}
withChart={false}
/>
</div>
) : (
<small>{t('No trace-connected related issues were found.')}</small>
)}
</HeaderWrapper>
</div>
<TextButtonWrapper>
{linkToTrace ?? null}
{openIssuesButton ?? null}
</TextButtonWrapper>
<GroupList
orgSlug={orgSlug}
queryParams={{query: query}}
source="similar-issues-tab"
canSelectGroups={false}
withChart={false}
/>
</Fragment>
) : relationType === 'trace_connected' ? (
<small>{t('No trace-connected related issues were found.')}</small>
) : (
<small>{t('No same-root-cause related issues were found.')}</small>
)}
</Fragment>
</HeaderWrapper>
);
}

Expand Down
Loading