Skip to content

Commit

Permalink
feat(highlights): Variety of fixes/changes to highlights work (#70355)
Browse files Browse the repository at this point in the history
This PR makes a variety of changes to the highlights/tags areas:
- [x] Add tests for analytics and user friendly names

- [x] Add search to the highlights modal

![image](https://github.com/getsentry/sentry/assets/35509934/75beb7c9-90c3-40a3-84d9-f4b585297de5)

- [x] Allow `replayId`, `transaction` and tags with URLs to be clickable

![image](https://github.com/getsentry/sentry/assets/35509934/bc0f4ac6-a2c2-496a-bcfa-6e14c811d151)

- [x] Adds a feedback button on Highlight section

![image](https://github.com/getsentry/sentry/assets/35509934/064edd41-1a83-48a9-ad12-b0de8102db4c)

**todo**
- [x] Add tests for search
- [x] Add tests for new tag links
- [x] Add screenshots of changes
  • Loading branch information
leeandher committed May 7, 2024
1 parent aa40406 commit 4f71b8e
Show file tree
Hide file tree
Showing 10 changed files with 406 additions and 89 deletions.
64 changes: 53 additions & 11 deletions static/app/components/events/eventTags/eventTagsTree.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import {EventFixture} from 'sentry-fixture/event';
import {OrganizationFixture} from 'sentry-fixture/organization';

import {initializeOrg} from 'sentry-test/initializeOrg';
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import {
render,
renderGlobalModal,
screen,
userEvent,
} from 'sentry-test/reactTestingLibrary';

import {EventTags} from 'sentry/components/events/eventTags';

Expand Down Expand Up @@ -54,6 +59,7 @@ describe('EventTagsTree', function () {
].concat(emptyBranchTags);

const event = EventFixture({tags});
const referrer = 'event-tags-table';

it('avoids tag tree without query param or flag', function () {
render(<EventTags projectSlug={project.slug} event={event} />, {organization});
Expand Down Expand Up @@ -160,29 +166,65 @@ describe('EventTagsTree', function () {
{
tag: {key: 'transaction', value: 'abc123'},
labelText: 'View this transaction',
validateLink: () => {
const linkElement = screen.getByRole('link', {name: 'abc123'});
const href = linkElement.attributes.getNamedItem('href');
expect(href?.value).toContain(
`/organizations/${organization.slug}/performance/summary/`
);
expect(href?.value).toContain(`project=${project.id}`);
expect(href?.value).toContain('transaction=abc123');
expect(href?.value).toContain(`referrer=${referrer}`);
},
},
{
tag: {key: 'replay_id', value: 'def456'},
labelText: 'View this replay',
validateLink: () => {
const linkElement = screen.getByRole('link', {name: 'def456'});
expect(linkElement).toHaveAttribute(
'href',
`/organizations/${organization.slug}/replays/def456/?referrer=${referrer}`
);
},
},
{
tag: {key: 'replayId', value: 'ghi789'},
labelText: 'View this replay',
validateLink: () => {
const linkElement = screen.getByRole('link', {name: 'ghi789'});
expect(linkElement).toHaveAttribute(
'href',
`/organizations/${organization.slug}/replays/ghi789/?referrer=${referrer}`
);
},
},
{
tag: {key: 'external-link', value: 'https://example.com'},
labelText: 'Visit this external link',
validateLink: async () => {
renderGlobalModal();
const linkElement = screen.getByText('https://example.com');
await userEvent.click(linkElement);
expect(screen.getByTestId('external-link-warning')).toBeInTheDocument();
},
},
])("renders unique links for '$tag.key' tag", async ({tag, labelText}) => {
const featuredOrganization = OrganizationFixture({features: ['event-tags-tree-ui']});
const uniqueTagsEvent = EventFixture({tags: [tag]});
render(<EventTags projectSlug={project.slug} event={uniqueTagsEvent} />, {
organization: featuredOrganization,
});
const dropdown = screen.getByLabelText('Tag Actions Menu');
await userEvent.click(dropdown);
expect(screen.getByLabelText(labelText)).toBeInTheDocument();
});
])(
"renders unique links for '$tag.key' tag",
async ({tag, labelText, validateLink}) => {
const featuredOrganization = OrganizationFixture({
features: ['event-tags-tree-ui'],
});
const uniqueTagsEvent = EventFixture({tags: [tag], projectID: project.id});
render(<EventTags projectSlug={project.slug} event={uniqueTagsEvent} />, {
organization: featuredOrganization,
});
const dropdown = screen.getByLabelText('Tag Actions Menu');
await userEvent.click(dropdown);
expect(screen.getByLabelText(labelText)).toBeInTheDocument();
await validateLink();
}
);

it('renders error message tooltips instead of dropdowns', function () {
const featuredOrganization = OrganizationFixture({features: ['event-tags-tree-ui']});
Expand Down
120 changes: 99 additions & 21 deletions static/app/components/events/eventTags/eventTagsTreeRow.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Fragment, useState} from 'react';
import {Link} from 'react-router';
import styled from '@emotion/styled';
import * as qs from 'query-string';

Expand All @@ -8,6 +9,7 @@ import {DropdownMenu} from 'sentry/components/dropdownMenu';
import type {TagTreeContent} from 'sentry/components/events/eventTags/eventTagsTree';
import EventTagsValue from 'sentry/components/events/eventTags/eventTagsValue';
import {AnnotatedTextErrors} from 'sentry/components/events/meta/annotatedText/annotatedTextErrors';
import ExternalLink from 'sentry/components/links/externalLink';
import Version from 'sentry/components/version';
import VersionHoverCard from 'sentry/components/versionHoverCard';
import {IconEllipsis} from 'sentry/icons';
Expand Down Expand Up @@ -43,10 +45,8 @@ export default function EventTagsTreeRow({
config = {},
...props
}: EventTagsTreeRowProps) {
const organization = useOrganization();
const originalTag = content.originalTag;
const tagMeta = content.meta?.value?.[''];
const tagErrors = tagMeta?.err ?? [];
const tagErrors = content.meta?.value?.['']?.err ?? [];
const hasTagErrors = tagErrors.length > 0 && !config?.disableActions;
const hasStem = !isLast && objectIsEmpty(content.subtree);

Expand All @@ -66,20 +66,6 @@ export default function EventTagsTreeRow({
</TreeRow>
);
}
const tagValue =
originalTag.key === 'release' && !config?.disableRichValue ? (
<VersionHoverCard
organization={organization}
projectSlug={projectSlug}
releaseVersion={content.value}
showUnderline
underlineColor="linkUnderline"
>
<Version version={content.value} truncate />
</VersionHoverCard>
) : (
<EventTagsValue tag={originalTag} meta={tagMeta} withOnlyFormattedText />
);

const tagActions = hasTagErrors ? (
<TreeValueErrors data-test-id="tag-tree-row-errors">
Expand All @@ -104,7 +90,14 @@ export default function EventTagsTreeRow({
</TreeKey>
</TreeKeyTrunk>
<TreeValueTrunk>
<TreeValue hasErrors={hasTagErrors}>{tagValue}</TreeValue>
<TreeValue hasErrors={hasTagErrors}>
<EventTagsTreeValue
config={config}
content={content}
event={event}
projectSlug={projectSlug}
/>
</TreeValue>
{!config?.disableActions && tagActions}
</TreeValueTrunk>
</TreeRow>
Expand All @@ -124,7 +117,7 @@ function EventTagsTreeRowDropdown({
return null;
}

const referrer = 'event-tags-tree';
const referrer = 'event-tags-table';
const query = generateQueryWithTag({referrer}, originalTag);
const searchQuery = `?${qs.stringify(query)}`;

Expand Down Expand Up @@ -217,6 +210,86 @@ function EventTagsTreeRowDropdown({
);
}

function EventTagsTreeValue({
config,
content,
event,
projectSlug,
}: Pick<EventTagsTreeRowProps, 'config' | 'content' | 'event' | 'projectSlug'>) {
const organization = useOrganization();
const {originalTag} = content;
const tagMeta = content.meta?.value?.[''];
if (!originalTag) {
return null;
}

const defaultValue = (
<EventTagsValue tag={originalTag} meta={tagMeta} withOnlyFormattedText />
);

if (config?.disableRichValue) {
return defaultValue;
}

let tagValue = defaultValue;
const referrer = 'event-tags-table';
switch (originalTag.key) {
case 'release':
tagValue = (
<VersionHoverCard
organization={organization}
projectSlug={projectSlug}
releaseVersion={content.value}
showUnderline
underlineColor="linkUnderline"
>
<Version version={content.value} truncate />
</VersionHoverCard>
);
break;
case 'transaction':
const transactionQuery = qs.stringify({
project: event.projectID,
transaction: content.value,
referrer,
});
const transactionDestination = `/organizations/${organization.slug}/performance/summary/?${transactionQuery}`;
tagValue = (
<TagLinkText>
<Link to={transactionDestination}>{content.value}</Link>
</TagLinkText>
);
break;
case 'replayId':
case 'replay_id':
const replayQuery = qs.stringify({referrer});
const replayDestination = `/organizations/${organization.slug}/replays/${encodeURIComponent(content.value)}/?${replayQuery}`;
tagValue = (
<TagLinkText>
<Link to={replayDestination}>{content.value}</Link>
</TagLinkText>
);
break;
default:
tagValue = defaultValue;
}

return !isUrl(content.value) ? (
tagValue
) : (
<TagLinkText>
<ExternalLink
onClick={e => {
e.preventDefault();
openNavigateToExternalLinkModal({linkText: content.value});
}}
>
{content.value}
</ExternalLink>
</TagLinkText>
);
}

const TreeRow = styled('div')<{hasErrors: boolean}>`
border-radius: ${space(0.5)};
padding-left: ${space(1)};
Expand Down Expand Up @@ -252,6 +325,7 @@ const TreeSpacer = styled('div')<{hasStem: boolean; spacerCount: number}>`
border-right: 1px solid ${p => (p.hasStem ? p.theme.border : 'transparent')};
margin-right: -1px;
height: 100%;
width: ${p => (p.spacerCount - 1) * 20 + 3}px;
`;

const TreeBranchIcon = styled('div')<{hasErrors: boolean}>`
Expand All @@ -269,8 +343,7 @@ const TreeKeyTrunk = styled('div')<{spacerCount: number}>`
display: grid;
height: 100%;
align-items: center;
grid-template-columns: ${p =>
p.spacerCount > 0 ? `${(p.spacerCount - 1) * 20 + 3}px 1rem 1fr` : '1fr'};
grid-template-columns: ${p => (p.spacerCount > 0 ? `auto 1rem 1fr` : '1fr')};
`;

const TreeValueTrunk = styled('div')`
Expand Down Expand Up @@ -321,3 +394,8 @@ const TreeValueErrors = styled('div')`
height: 20px;
margin-right: ${space(0.75)};
`;

const TagLinkText = styled('span')`
color: ${p => p.theme.linkColor};
margin: 0;
`;

0 comments on commit 4f71b8e

Please sign in to comment.