Skip to content

Commit

Permalink
fix(issue-summary): Hide issue summary on non-error issues (#76559)
Browse files Browse the repository at this point in the history
Hides issue summary header and card if the issue is not an error issue
(i.e. it's a performance, replay, or user feedback issue).
  • Loading branch information
roaga committed Aug 26, 2024
1 parent 5771cda commit 77437b6
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 21 deletions.
182 changes: 172 additions & 10 deletions static/app/components/group/groupSummary.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import {render, screen, waitFor} from 'sentry-test/reactTestingLibrary';

import {
GroupSummary,
GroupSummaryHeader,
makeGroupSummaryQueryKey,
} from 'sentry/components/group/groupSummary';
import {IssueCategory} from 'sentry/types/group';

describe('GroupSummary', function () {
beforeEach(() => {
Expand Down Expand Up @@ -42,11 +44,9 @@ describe('GroupSummary', function () {
},
});

render(<GroupSummary groupId={groupId} />);
render(<GroupSummary groupId={groupId} groupCategory={IssueCategory.ERROR} />);

await waitFor(() => {
expect(screen.getByText('Issue Summary')).toBeInTheDocument();
});
expect(await screen.findByText('Issue Summary')).toBeInTheDocument();

expect(screen.getByText('Issue Summary')).toBeInTheDocument();
expect(screen.getByText('Test summary')).toBeInTheDocument();
Expand Down Expand Up @@ -86,18 +86,180 @@ describe('GroupSummary', function () {
},
});

render(<GroupSummary groupId={groupId} />);
render(<GroupSummary groupId={groupId} groupCategory={IssueCategory.ERROR} />);

await waitFor(() => {
expect(setupCall).toHaveBeenCalled();
});

await waitFor(
() => {
expect(setupCall).toHaveBeenCalled();
expect(screen.queryByText('Issue Summary')).not.toBeInTheDocument();
expect(screen.queryByText('Test summary')).not.toBeInTheDocument();
expect(screen.queryByText('Potential Impact')).not.toBeInTheDocument();
expect(screen.queryByText('Test impact')).not.toBeInTheDocument();
});

it('does not render the group summary if not an error', function () {
const groupId = '1';
const organizationSlug = 'org-slug';
MockApiClient.addMockResponse({
url: makeGroupSummaryQueryKey(organizationSlug, groupId)[0],
method: 'POST',
body: {
groupId,
summary: 'Test summary',
impact: 'Test impact',
},
{timeout: 5000}
);
});

MockApiClient.addMockResponse({
url: `/issues/${groupId}/autofix/setup/`,
body: {
genAIConsent: {ok: true},
integration: {ok: true},
githubWriteIntegration: {
ok: true,
repos: [
{
provider: 'integrations:github',
owner: 'getsentry',
name: 'sentry',
external_id: '123',
},
],
},
},
});

render(<GroupSummary groupId={groupId} groupCategory={IssueCategory.PERFORMANCE} />);

expect(screen.queryByText('Issue Summary')).not.toBeInTheDocument();
expect(screen.queryByText('Test summary')).not.toBeInTheDocument();
expect(screen.queryByText('Potential Impact')).not.toBeInTheDocument();
expect(screen.queryByText('Test impact')).not.toBeInTheDocument();
});
});

describe('GroupSummaryHeader', function () {
beforeEach(() => {
MockApiClient.clearMockResponses();
});

it('renders the group summary header', async function () {
const groupId = '1';
const organizationSlug = 'org-slug';
MockApiClient.addMockResponse({
url: makeGroupSummaryQueryKey(organizationSlug, groupId)[0],
method: 'POST',
body: {
groupId,
summary: 'Test summary',
impact: 'Test impact',
headline: 'Test headline',
},
});

MockApiClient.addMockResponse({
url: `/issues/${groupId}/autofix/setup/`,
body: {
genAIConsent: {ok: true},
integration: {ok: true},
githubWriteIntegration: {
ok: true,
repos: [
{
provider: 'integrations:github',
owner: 'getsentry',
name: 'sentry',
external_id: '123',
},
],
},
},
});

render(<GroupSummaryHeader groupId={groupId} groupCategory={IssueCategory.ERROR} />);

expect(await screen.findByText('Test headline')).toBeInTheDocument();
});

it('does not render the group summary headline if no consent', async function () {
const groupId = '1';
const organizationSlug = 'org-slug';
MockApiClient.addMockResponse({
url: makeGroupSummaryQueryKey(organizationSlug, groupId)[0],
method: 'POST',
body: {
groupId,
summary: 'Test summary',
impact: 'Test impact',
headline: 'Test headline',
},
});

const setupCall = MockApiClient.addMockResponse({
url: `/issues/${groupId}/autofix/setup/`,
body: {
genAIConsent: {ok: false},
integration: {ok: true},
githubWriteIntegration: {
ok: true,
repos: [
{
provider: 'integrations:github',
owner: 'getsentry',
name: 'sentry',
external_id: '123',
},
],
},
},
});

render(<GroupSummaryHeader groupId={groupId} groupCategory={IssueCategory.ERROR} />);

await waitFor(() => {
expect(setupCall).toHaveBeenCalled();
});

expect(screen.queryByText('Test headline')).not.toBeInTheDocument();
});

it('does not render the group summary headline if not an error', function () {
const groupId = '1';
const organizationSlug = 'org-slug';
MockApiClient.addMockResponse({
url: makeGroupSummaryQueryKey(organizationSlug, groupId)[0],
method: 'POST',
body: {
groupId,
summary: 'Test summary',
impact: 'Test impact',
headline: 'Test headline',
},
});

MockApiClient.addMockResponse({
url: `/issues/${groupId}/autofix/setup/`,
body: {
genAIConsent: {ok: true},
integration: {ok: true},
githubWriteIntegration: {
ok: true,
repos: [
{
provider: 'integrations:github',
owner: 'getsentry',
name: 'sentry',
external_id: '123',
},
],
},
},
});

render(
<GroupSummaryHeader groupId={groupId} groupCategory={IssueCategory.PERFORMANCE} />
);
expect(screen.queryByText('Test headline')).not.toBeInTheDocument();
});
});
32 changes: 24 additions & 8 deletions static/app/components/group/groupSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import * as SidebarSection from 'sentry/components/sidebarSection';
import {IconMegaphone} from 'sentry/icons';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {IssueCategory} from 'sentry/types/group';
import marked from 'sentry/utils/marked';
import {type ApiQueryKey, useApiQuery} from 'sentry/utils/queryClient';
import {useFeedbackForm} from 'sentry/utils/useFeedbackForm';
import useOrganization from 'sentry/utils/useOrganization';
import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';

interface GroupSummaryProps {
groupCategory: IssueCategory;
groupId: string;
}

Expand All @@ -27,6 +29,10 @@ interface GroupSummaryData {
headline?: string;
}

const isSummaryEnabled = (hasGenAIConsent: boolean, groupCategory: IssueCategory) => {
return hasGenAIConsent && groupCategory === IssueCategory.ERROR;
};

export const makeGroupSummaryQueryKey = (
organizationSlug: string,
groupId: string
Expand All @@ -35,7 +41,7 @@ export const makeGroupSummaryQueryKey = (
{method: 'POST'},
];

export function useGroupSummary(groupId: string) {
export function useGroupSummary(groupId: string, groupCategory: IssueCategory) {
const organization = useOrganization();
// We piggyback and use autofix's genai consent check for now.
const {
Expand All @@ -50,7 +56,7 @@ export function useGroupSummary(groupId: string) {
makeGroupSummaryQueryKey(organization.slug, groupId),
{
staleTime: Infinity, // Cache the result indefinitely as it's unlikely to change if it's already computed
enabled: hasGenAIConsent,
enabled: isSummaryEnabled(hasGenAIConsent, groupCategory),
}
);
return {
Expand All @@ -72,11 +78,18 @@ function GroupSummaryFeatureBadge() {
);
}

export function GroupSummaryHeader({groupId}: GroupSummaryProps) {
const {data, isLoading, isError, hasGenAIConsent} = useGroupSummary(groupId);
export function GroupSummaryHeader({groupId, groupCategory}: GroupSummaryProps) {
const {data, isLoading, isError, hasGenAIConsent} = useGroupSummary(
groupId,
groupCategory
);
const isStreamlined = useHasStreamlinedUI();

if (isError || !hasGenAIConsent || (!isLoading && !data?.headline)) {
if (
isError ||
(!isLoading && !data?.headline) ||
!isSummaryEnabled(hasGenAIConsent, groupCategory)
) {
// Don't render the summary headline if there's an error, the error is already shown in the sidebar
// If there is no headline we also don't want to render anything
return null;
Expand All @@ -98,12 +111,15 @@ export function GroupSummaryHeader({groupId}: GroupSummaryProps) {
);
}

export function GroupSummary({groupId}: GroupSummaryProps) {
const {data, isLoading, isError, hasGenAIConsent} = useGroupSummary(groupId);
export function GroupSummary({groupId, groupCategory}: GroupSummaryProps) {
const {data, isLoading, isError, hasGenAIConsent} = useGroupSummary(
groupId,
groupCategory
);

const openForm = useFeedbackForm();

if (!hasGenAIConsent) {
if (!isSummaryEnabled(hasGenAIConsent, groupCategory)) {
// TODO: Render a banner for needing genai consent
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/issueDetails/groupSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export default function GroupSidebar({
return (
<Container>
<Feature features={['organizations:ai-summary']}>
<GroupSummary groupId={group.id} />
<GroupSummary groupId={group.id} groupCategory={group.issueCategory} />
</Feature>
{hasStreamlinedUI && event && (
<ErrorBoundary mini>
Expand Down
5 changes: 4 additions & 1 deletion static/app/views/issueDetails/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,10 @@ function GroupHeader({
showUnhandled={group.isUnhandled}
/>
<Feature features={['organizations:ai-summary']}>
<GroupSummaryHeader groupId={group.id} />
<GroupSummaryHeader
groupId={group.id}
groupCategory={group.issueCategory}
/>
</Feature>
</TitleWrapper>
<StatsWrapper>
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/issueDetails/streamline/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export default function StreamlinedGroupHeader({
)}
</MessageWrapper>
<Feature features={['organizations:ai-summary']}>
<GroupSummaryHeader groupId={group.id} />
<GroupSummaryHeader groupId={group.id} groupCategory={group.issueCategory} />
</Feature>
<StyledBreak />
<InfoWrapper
Expand Down

0 comments on commit 77437b6

Please sign in to comment.