Browse Source

feat(highlights): Error boundaries, styling fixes, empty states and add from tags (#70597)

More features/fixes for highlights:

- Error boundaries to prevent crashing the whole page if something goes
wrong:
![Screenshot 2024-05-09 at 11 31
17 AM](https://github.com/getsentry/sentry/assets/35509934/7de63bd5-8cd4-42ba-90d0-33fcbde59c91)
![Screenshot 2024-05-09 at 12 15
10 PM](https://github.com/getsentry/sentry/assets/35509934/d83dfb85-7bb5-438b-a81c-4dc76f73dedc)
![Screenshot 2024-05-09 at 11 28
25 AM](https://github.com/getsentry/sentry/assets/35509934/5bbdae65-a9ee-42a2-a607-bf0f4ff604fe)

- Fixed height for edit highlights modal + empty states on context/tag
search:


https://github.com/getsentry/sentry/assets/35509934/4f12b063-51b4-4233-a18e-9af9cda7c968

- Highlights not present in the event no longer appear as links or with
dropdowns

![image](https://github.com/getsentry/sentry/assets/35509934/62732a1a-101c-46da-82b6-c53df70ce332)

- You can add to highlights from the tag section 

![image](https://github.com/getsentry/sentry/assets/35509934/ceccf5a2-e67a-4b64-88c3-f7aaed45b16b)
(and it's omitted when already present in highlights)

![image](https://github.com/getsentry/sentry/assets/35509934/d3a98ff6-3f3c-416a-a26d-5e9af334c58a)

- Display meta annotations that aren't errors for tags

![image](https://github.com/getsentry/sentry/assets/35509934/0d05e422-4f6f-4722-ab92-6a5fd9a0771b)


**todo**
- [x] Add tests 
- [x] Add screenshots/videos
Leander Rodrigues 10 months ago
parent
commit
76e2767ea3

+ 6 - 1
static/app/components/events/contexts/contextCard.tsx

@@ -26,7 +26,12 @@ interface ContextCardProps {
 }
 
 interface ContextCardContentConfig {
+  // Omit error styling from being displayed, even if context is invalid
   disableErrors?: boolean;
+  // Displays tag value as plain text, rather than a hyperlink if applicable
+  disableRichValue?: boolean;
+  // Includes the Context Type as a prefix to the key. Useful if displaying a single Context key
+  // apart from the rest of that Context. E.g. 'Email' -> 'User: Email'
   includeAliasInSubject?: boolean;
 }
 
@@ -71,7 +76,7 @@ export function ContextCardContent({
       <ContextSubject>{contextSubject}</ContextSubject>
       <ContextValueSection hasErrors={hasErrors}>
         <ContextValueWrapper>
-          {defined(action?.link) ? (
+          {!config?.disableRichValue && defined(action?.link) ? (
             <Link to={action.link}>{dataComponent}</Link>
           ) : (
             dataComponent

+ 13 - 6
static/app/components/events/contexts/contextDataSection.tsx

@@ -1,6 +1,7 @@
 import {useRef} from 'react';
 import styled from '@emotion/styled';
 
+import ErrorBoundary from 'sentry/components/errorBoundary';
 import {getOrderedContextItems} from 'sentry/components/events/contexts';
 import ContextCard from 'sentry/components/events/contexts/contextCard';
 import {CONTEXT_DOCS_LINK} from 'sentry/components/events/contextSummary/utils';
@@ -16,7 +17,7 @@ interface ContextDataSectionProps {
   project?: Project;
 }
 
-function ContextDataSection({event, group, project}: ContextDataSectionProps) {
+function ContextData({event, group, project}: ContextDataSectionProps) {
   const containerRef = useRef<HTMLDivElement>(null);
   const columnCount = useIssueDetailsColumnCount(containerRef);
   const columns: React.ReactNode[] = [];
@@ -39,6 +40,14 @@ function ContextDataSection({event, group, project}: ContextDataSectionProps) {
   for (let i = 0; i < cards.length; i += columnSize) {
     columns.push(<CardColumn key={i}>{cards.slice(i, i + columnSize)}</CardColumn>);
   }
+  return (
+    <CardWrapper columnCount={columnCount} ref={containerRef}>
+      {columns}
+    </CardWrapper>
+  );
+}
+
+export default function ContextDataSection(props: ContextDataSectionProps) {
   return (
     <EventDataSection
       key={'context'}
@@ -52,9 +61,9 @@ function ContextDataSection({event, group, project}: ContextDataSectionProps) {
       )}
       isHelpHoverable
     >
-      <CardWrapper columnCount={columnCount} ref={containerRef}>
-        {columns}
-      </CardWrapper>
+      <ErrorBoundary mini message={t('There was a problem loading event context.')}>
+        <ContextData {...props} />
+      </ErrorBoundary>
     </EventDataSection>
   );
 }
@@ -69,5 +78,3 @@ const CardWrapper = styled('div')<{columnCount: number}>`
 const CardColumn = styled('div')`
   grid-column: span 1;
 `;
-
-export default ContextDataSection;

+ 96 - 2
static/app/components/events/eventTags/eventTagsTree.spec.tsx

@@ -7,6 +7,7 @@ import {
   renderGlobalModal,
   screen,
   userEvent,
+  within,
 } from 'sentry-test/reactTestingLibrary';
 
 import {EventTags} from 'sentry/components/events/eventTags';
@@ -60,6 +61,15 @@ describe('EventTagsTree', function () {
 
   const event = EventFixture({tags});
   const referrer = 'event-tags-table';
+  let mockDetailedProject;
+
+  beforeEach(function () {
+    MockApiClient.clearMockResponses();
+    mockDetailedProject = MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/`,
+      body: project,
+    });
+  });
 
   it('avoids tag tree without query param or flag', function () {
     render(<EventTags projectSlug={project.slug} event={event} />, {organization});
@@ -113,6 +123,9 @@ describe('EventTagsTree', function () {
       organization,
       router,
     });
+    expect(mockDetailedProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
     await assertNewTagsView();
   });
 
@@ -121,6 +134,9 @@ describe('EventTagsTree', function () {
     render(<EventTags projectSlug={project.slug} event={event} />, {
       organization: featuredOrganization,
     });
+    expect(mockDetailedProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
     await assertNewTagsView();
   });
 
@@ -147,6 +163,9 @@ describe('EventTagsTree', function () {
     render(<EventTags projectSlug={project.slug} event={releaseEvent} />, {
       organization: featuredOrganization,
     });
+    expect(mockDetailedProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
     const versionText = screen.getByText<
       HTMLElement & {parentElement: HTMLAnchorElement}
     >(releaseVersion);
@@ -219,6 +238,9 @@ describe('EventTagsTree', function () {
       render(<EventTags projectSlug={project.slug} event={uniqueTagsEvent} />, {
         organization: featuredOrganization,
       });
+      expect(mockDetailedProject).toHaveBeenCalled();
+      expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
       const dropdown = screen.getByLabelText('Tag Actions Menu');
       await userEvent.click(dropdown);
       expect(screen.getByLabelText(labelText)).toBeInTheDocument();
@@ -226,7 +248,7 @@ describe('EventTagsTree', function () {
     }
   );
 
-  it('renders error message tooltips instead of dropdowns', function () {
+  it('renders error message tooltips instead of dropdowns', async function () {
     const featuredOrganization = OrganizationFixture({features: ['event-tags-tree-ui']});
     const errorTagEvent = EventFixture({
       _meta: {
@@ -264,6 +286,8 @@ describe('EventTagsTree', function () {
     render(<EventTags projectSlug={project.slug} event={errorTagEvent} />, {
       organization: featuredOrganization,
     });
+    expect(mockDetailedProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
 
     // Should only be one dropdown, others have errors
     const dropdown = screen.getByLabelText('Tag Actions Menu');
@@ -273,7 +297,7 @@ describe('EventTagsTree', function () {
     expect(errorRows.length).toBe(2);
   });
 
-  it('avoids rendering nullish tags', function () {
+  it('avoids rendering nullish tags', async function () {
     const featuredOrganization = OrganizationFixture({features: ['event-tags-tree-ui']});
     const uniqueTagsEvent = EventFixture({
       tags: [
@@ -285,10 +309,80 @@ describe('EventTagsTree', function () {
     render(<EventTags projectSlug={project.slug} event={uniqueTagsEvent} />, {
       organization: featuredOrganization,
     });
+    expect(mockDetailedProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
 
     expect(screen.getByText('boring-tag', {selector: 'div'})).toBeInTheDocument();
     expect(screen.getByText('boring tag')).toBeInTheDocument();
     expect(screen.queryByText('null tag')).not.toBeInTheDocument();
     expect(screen.queryByText('undefined tag')).not.toBeInTheDocument();
   });
+
+  it("renders 'Add to event highlights' option based on highlights", async function () {
+    const featuredOrganization = OrganizationFixture({features: ['event-tags-tree-ui']});
+    const highlightsEvent = EventFixture({
+      tags: [
+        {key: 'useless-tag', value: 'not so much'},
+        {key: 'highlighted-tag', value: 'so important'},
+      ],
+    });
+    const highlightProject = {...project, highlightTags: ['highlighted-tag']};
+    MockApiClient.clearMockResponses();
+    const mockHighlightProject = MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/`,
+      body: highlightProject,
+    });
+    render(<EventTags projectSlug={highlightProject.slug} event={highlightsEvent} />, {
+      organization: featuredOrganization,
+    });
+    await expect(mockHighlightProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
+    const normalTagRow = screen
+      .getByText('useless-tag', {selector: 'div'})
+      .closest('div[data-test-id=tag-tree-row]') as HTMLElement;
+    const normalTagDropdown = within(normalTagRow).getByLabelText('Tag Actions Menu');
+    await userEvent.click(normalTagDropdown);
+    expect(screen.getByLabelText('Add to event highlights')).toBeInTheDocument();
+
+    const highlightTagRow = screen
+      .getByText('highlighted-tag', {selector: 'div'})
+      .closest('div[data-test-id=tag-tree-row]') as HTMLElement;
+    const highlightTagDropdown =
+      within(highlightTagRow).getByLabelText('Tag Actions Menu');
+    await userEvent.click(highlightTagDropdown);
+    expect(screen.queryByLabelText('Add to event highlights')).not.toBeInTheDocument();
+  });
+
+  it("renders 'Add to event highlights' option based on permissions", async function () {
+    const featuredOrganization = OrganizationFixture({
+      features: ['event-tags-tree-ui'],
+      access: ['org:read'],
+    });
+    const highlightsEvent = EventFixture({
+      tags: [{key: 'useless-tag', value: 'not so much'}],
+    });
+    const highlightProject = {
+      ...project,
+      access: ['project:read'],
+      highlightTags: ['highlighted-tag'],
+    };
+    MockApiClient.clearMockResponses();
+    const mockHighlightProject = MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/`,
+      body: highlightProject,
+    });
+    render(<EventTags projectSlug={highlightProject.slug} event={highlightsEvent} />, {
+      organization: featuredOrganization,
+    });
+    await expect(mockHighlightProject).toHaveBeenCalled();
+    expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
+    const normalTagRow = screen
+      .getByText('useless-tag', {selector: 'div'})
+      .closest('div[data-test-id=tag-tree-row]') as HTMLElement;
+    const normalTagDropdown = within(normalTagRow).getByLabelText('Tag Actions Menu');
+    await userEvent.click(normalTagDropdown);
+    expect(screen.queryByLabelText('Add to event highlights')).not.toBeInTheDocument();
+  });
 });

+ 34 - 7
static/app/components/events/eventTags/eventTagsTree.tsx

@@ -1,13 +1,19 @@
 import {Fragment, useMemo, useRef} from 'react';
 import styled from '@emotion/styled';
 
+import ErrorBoundary from 'sentry/components/errorBoundary';
 import EventTagsTreeRow, {
   type EventTagsTreeRowProps,
 } from 'sentry/components/events/eventTags/eventTagsTreeRow';
 import {useIssueDetailsColumnCount} from 'sentry/components/events/eventTags/util';
+import LoadingIndicator from 'sentry/components/loadingIndicator';
+import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
+import type {Project} from 'sentry/types';
 import type {Event, EventTag} from 'sentry/types/event';
 import {defined} from 'sentry/utils';
+import {useDetailedProject} from 'sentry/utils/useDetailedProject';
+import useOrganization from 'sentry/utils/useOrganization';
 
 const MAX_TREE_DEPTH = 4;
 const INVALID_BRANCH_REGEX = /\.{2,}/;
@@ -32,7 +38,7 @@ interface TagTreeColumnData {
 
 interface EventTagsTreeProps {
   event: Event;
-  projectSlug: string;
+  projectSlug: Project['slug'];
   tags: EventTag[];
   meta?: Record<any, any>;
 }
@@ -122,9 +128,22 @@ function TagTreeColumns({
   meta,
   tags,
   columnCount,
+  projectSlug,
   ...props
 }: EventTagsTreeProps & {columnCount: number}) {
+  const organization = useOrganization();
+  const {data: project, isLoading} = useDetailedProject({
+    orgSlug: organization.slug,
+    projectSlug,
+  });
   const assembledColumns = useMemo(() => {
+    if (isLoading) {
+      return <TreeLoadingIndicator hideMessage />;
+    }
+
+    if (!project) {
+      return [];
+    }
     // Create the TagTree data structure using all the given tags
     const tagTree = tags.reduce<TagTree>(
       (tree, tag, i) => addToTagTree(tree, tag, meta?.[i], tag),
@@ -134,7 +153,7 @@ function TagTreeColumns({
     // root parent so that we do not split up roots/branches when forming columns
     const tagTreeRowGroups: React.ReactNode[][] = Object.entries(tagTree).map(
       ([tagKey, content], i) =>
-        getTagTreeRows({tagKey, content, uniqueKey: `${i}`, ...props})
+        getTagTreeRows({tagKey, content, uniqueKey: `${i}`, project: project, ...props})
     );
     // Get the total number of TagTreeRow components to be rendered, and a goal size for each column
     const tagTreeRowTotal = tagTreeRowGroups.reduce(
@@ -171,7 +190,7 @@ function TagTreeColumns({
       {startIndex: 0, runningTotal: 0, columns: []}
     );
     return data.columns;
-  }, [meta, tags, props, columnCount]);
+  }, [columnCount, isLoading, project, props, tags, meta]);
 
   return <Fragment>{assembledColumns}</Fragment>;
 }
@@ -180,9 +199,11 @@ function EventTagsTree(props: EventTagsTreeProps) {
   const containerRef = useRef<HTMLDivElement>(null);
   const columnCount = useIssueDetailsColumnCount(containerRef);
   return (
-    <TreeContainer columnCount={columnCount} ref={containerRef}>
-      <TagTreeColumns columnCount={columnCount} {...props} />
-    </TreeContainer>
+    <ErrorBoundary mini message={t('There was a problem loading event tags.')}>
+      <TreeContainer columnCount={columnCount} ref={containerRef}>
+        <TagTreeColumns columnCount={columnCount} {...props} />
+      </TreeContainer>
+    </ErrorBoundary>
   );
 }
 
@@ -191,13 +212,15 @@ export const TreeContainer = styled('div')<{columnCount: number}>`
   display: grid;
   grid-template-columns: repeat(${p => p.columnCount}, 1fr);
   align-items: start;
-  margin-left: -${space(1)};
 `;
 
 export const TreeColumn = styled('div')`
   display: grid;
   grid-template-columns: minmax(auto, 175px) 1fr;
   grid-column-gap: ${space(3)};
+  &:first-child {
+    margin-left: -${space(1)};
+  }
   &:not(:first-child) {
     border-left: 1px solid ${p => p.theme.innerBorder};
     padding-left: ${space(2)};
@@ -209,4 +232,8 @@ export const TreeColumn = styled('div')`
   }
 `;
 
+export const TreeLoadingIndicator = styled(LoadingIndicator)`
+  grid-column: 1 /-1;
+`;
+
 export default EventTagsTree;

+ 39 - 10
static/app/components/events/eventTags/eventTagsTreeRow.tsx

@@ -4,6 +4,7 @@ import * as qs from 'query-string';
 
 import {openNavigateToExternalLinkModal} from 'sentry/actionCreators/modal';
 import {navigateTo} from 'sentry/actionCreators/navigation';
+import {hasEveryAccess} from 'sentry/components/acl/access';
 import {DropdownMenu} from 'sentry/components/dropdownMenu';
 import type {TagTreeContent} from 'sentry/components/events/eventTags/eventTagsTree';
 import EventTagsValue from 'sentry/components/events/eventTags/eventTagsValue';
@@ -15,20 +16,26 @@ import VersionHoverCard from 'sentry/components/versionHoverCard';
 import {IconEllipsis} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
+import type {Project} from 'sentry/types';
 import type {Event} from 'sentry/types/event';
 import {generateQueryWithTag, isUrl, objectIsEmpty} from 'sentry/utils';
+import useMutateProject from 'sentry/utils/useMutateProject';
 import useOrganization from 'sentry/utils/useOrganization';
 import useRouter from 'sentry/utils/useRouter';
 
 interface EventTagTreeRowConfig {
+  // Omits the dropdown of actions applicable to this tag
   disableActions?: boolean;
+  // Omit error styling from being displayed, even if context is invalid
+  disableErrors?: boolean;
+  // Displays tag value as plain text, rather than a hyperlink if applicable
   disableRichValue?: boolean;
 }
 
 export interface EventTagsTreeRowProps {
   content: TagTreeContent;
   event: Event;
-  projectSlug: string;
+  project: Project;
   tagKey: string;
   config?: EventTagTreeRowConfig;
   isLast?: boolean;
@@ -39,7 +46,7 @@ export default function EventTagsTreeRow({
   event,
   content,
   tagKey,
-  projectSlug,
+  project,
   spacerCount = 0,
   isLast = false,
   config = {},
@@ -47,7 +54,7 @@ export default function EventTagsTreeRow({
 }: EventTagsTreeRowProps) {
   const originalTag = content.originalTag;
   const tagErrors = content.meta?.value?.['']?.err ?? [];
-  const hasTagErrors = tagErrors.length > 0 && !config?.disableActions;
+  const hasTagErrors = tagErrors.length > 0 && !config?.disableErrors;
   const hasStem = !isLast && objectIsEmpty(content.subtree);
 
   if (!originalTag) {
@@ -72,7 +79,7 @@ export default function EventTagsTreeRow({
       <AnnotatedTextErrors errors={tagErrors} />
     </TreeValueErrors>
   ) : (
-    <EventTagsTreeRowDropdown content={content} event={event} />
+    <EventTagsTreeRowDropdown content={content} event={event} project={project} />
   );
 
   return (
@@ -95,7 +102,7 @@ export default function EventTagsTreeRow({
             config={config}
             content={content}
             event={event}
-            projectSlug={projectSlug}
+            project={project}
           />
         </TreeValue>
         {!config?.disableActions && tagActions}
@@ -105,11 +112,13 @@ export default function EventTagsTreeRow({
 }
 
 function EventTagsTreeRowDropdown({
-  event,
   content,
-}: Pick<EventTagsTreeRowProps, 'content' | 'event'>) {
+  event,
+  project,
+}: Pick<EventTagsTreeRowProps, 'content' | 'event' | 'project'>) {
   const organization = useOrganization();
   const router = useRouter();
+  const {mutate: saveTag} = useMutateProject({organization, project});
   const [isVisible, setIsVisible] = useState(false);
   const originalTag = content.originalTag;
 
@@ -118,8 +127,18 @@ function EventTagsTreeRowDropdown({
   }
 
   const referrer = 'event-tags-table';
+  const highlightTagSet = new Set(project?.highlightTags ?? []);
+  const hideAddHighlightsOption =
+    // Check for existing highlight record to prevent replacing all with a single tag if we receive a project summary (instead of a detailed project)
+    project?.highlightTags &&
+    // Skip tags already highlighted
+    highlightTagSet.has(originalTag.key);
   const query = generateQueryWithTag({referrer}, originalTag);
   const searchQuery = `?${qs.stringify(query)}`;
+  const isProjectAdmin = hasEveryAccess(['project:admin'], {
+    organization,
+    project,
+  });
 
   return (
     <TreeValueDropdown
@@ -156,6 +175,16 @@ function EventTagsTreeRowDropdown({
             );
           },
         },
+        {
+          key: 'add-to-highlights',
+          label: t('Add to event highlights'),
+          hidden: hideAddHighlightsOption || !isProjectAdmin,
+          onAction: () => {
+            saveTag({
+              highlightTags: [...(project?.highlightTags ?? []), originalTag.key],
+            });
+          },
+        },
         {
           key: 'release',
           label: t('View this release'),
@@ -214,8 +243,8 @@ function EventTagsTreeValue({
   config,
   content,
   event,
-  projectSlug,
-}: Pick<EventTagsTreeRowProps, 'config' | 'content' | 'event' | 'projectSlug'>) {
+  project,
+}: Pick<EventTagsTreeRowProps, 'config' | 'content' | 'event' | 'project'>) {
   const organization = useOrganization();
   const {originalTag} = content;
   const tagMeta = content.meta?.value?.[''];
@@ -238,7 +267,7 @@ function EventTagsTreeValue({
       tagValue = (
         <VersionHoverCard
           organization={organization}
-          projectSlug={projectSlug}
+          projectSlug={project.slug}
           releaseVersion={content.value}
           showUnderline
           underlineColor="linkUnderline"

+ 9 - 10
static/app/components/events/eventTags/eventTagsValue.tsx

@@ -20,16 +20,15 @@ function EventTagsValue({
   locationSearch,
   withOnlyFormattedText = false,
 }: Props) {
-  const content =
-    !!meta && !value ? (
-      <AnnotatedText
-        value={value}
-        meta={meta}
-        withOnlyFormattedText={withOnlyFormattedText}
-      />
-    ) : (
-      <DeviceName value={String(value)} />
-    );
+  const content = meta ? (
+    <AnnotatedText
+      value={value}
+      meta={meta}
+      withOnlyFormattedText={withOnlyFormattedText}
+    />
+  ) : (
+    <DeviceName value={String(value)} />
+  );
 
   if (!meta?.err?.length && defined(key) && streamPath && locationSearch) {
     return <Link to={{pathname: streamPath, search: locationSearch}}>{content}</Link>;

+ 3 - 2
static/app/components/events/eventTags/index.tsx

@@ -7,6 +7,7 @@ import EventTagCustomBanner from 'sentry/components/events/eventTags/eventTagCus
 import EventTagsTree from 'sentry/components/events/eventTags/eventTagsTree';
 import {TagFilter, useHasNewTagsUI} from 'sentry/components/events/eventTags/util';
 import Pills from 'sentry/components/pills';
+import type {Project} from 'sentry/types';
 import type {Event, EventTag} from 'sentry/types/event';
 import {defined, generateQueryWithTag} from 'sentry/utils';
 import {trackAnalytics} from 'sentry/utils/analytics';
@@ -20,7 +21,7 @@ import EventTagsPill from './eventTagsPill';
 
 type Props = {
   event: Event;
-  projectSlug: string;
+  projectSlug: Project['slug'];
   filteredTags?: EventTag[];
   tagFilter?: TagFilter;
 };
@@ -112,7 +113,7 @@ export function EventTags({
   return (
     <Fragment>
       {hasNewTagsUI ? (
-        <EventTagsTree event={event} tags={tags} meta={meta} projectSlug={projectSlug} />
+        <EventTagsTree event={event} meta={meta} projectSlug={projectSlug} tags={tags} />
       ) : (
         <StyledClippedBox clipHeight={150}>
           <Pills>

+ 28 - 10
static/app/components/events/eventTagsAndScreenshot/index.spec.tsx

@@ -522,7 +522,19 @@ describe('EventTagsAndScreenshot', function () {
         query: {tagsTree: '1'},
       },
     });
-    function assertNewTagsView() {
+    let mockDetailedProject;
+    beforeEach(function () {
+      MockApiClient.clearMockResponses();
+      mockDetailedProject = MockApiClient.addMockResponse({
+        url: `/projects/${organization.slug}/${project.slug}/`,
+        body: project,
+      });
+    });
+
+    async function assertNewTagsView() {
+      await expect(mockDetailedProject).toHaveBeenCalled();
+      expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
       expect(screen.getByText('Tags')).toBeInTheDocument();
       // Ensure context isn't added in tag section
       const contextItems = screen.queryByTestId('context-item');
@@ -534,7 +546,7 @@ describe('EventTagsAndScreenshot', function () {
       );
     }
 
-    function assertFlagAndQueryParamWork() {
+    async function assertFlagAndQueryParamWork() {
       const flaggedOrgTags = render(
         <EventTagsAndScreenshot
           event={EventFixture({...event, tags, contexts})}
@@ -542,7 +554,7 @@ describe('EventTagsAndScreenshot', function () {
         />,
         {organization: featuredOrganization}
       );
-      assertNewTagsView();
+      await assertNewTagsView();
       flaggedOrgTags.unmount();
 
       const flaggedOrgTagsAsShare = render(
@@ -553,7 +565,7 @@ describe('EventTagsAndScreenshot', function () {
         />,
         {organization: featuredOrganization}
       );
-      assertNewTagsView();
+      await assertNewTagsView();
       flaggedOrgTagsAsShare.unmount();
 
       const queryParamTags = render(
@@ -563,7 +575,7 @@ describe('EventTagsAndScreenshot', function () {
         />,
         {organization, router}
       );
-      assertNewTagsView();
+      await assertNewTagsView();
       queryParamTags.unmount();
 
       const queryParamTagsAsShare = render(
@@ -574,24 +586,24 @@ describe('EventTagsAndScreenshot', function () {
         />,
         {organization, router}
       );
-      assertNewTagsView();
+      await assertNewTagsView();
       queryParamTagsAsShare.unmount();
     }
 
-    it('no context, tags only', function () {
+    it('no context, tags only', async function () {
       MockApiClient.addMockResponse({
         url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
         body: [],
       });
-      assertFlagAndQueryParamWork();
+      await assertFlagAndQueryParamWork();
     });
 
-    it('no context, tags and screenshot', function () {
+    it('no context, tags and screenshot', async function () {
       MockApiClient.addMockResponse({
         url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
         body: attachments,
       });
-      assertFlagAndQueryParamWork();
+      await assertFlagAndQueryParamWork();
     });
 
     it("allows filtering with 'event-tags-tree-ui' flag", async function () {
@@ -614,6 +626,9 @@ describe('EventTagsAndScreenshot', function () {
       render(<EventTagsAndScreenshot projectSlug={project.slug} event={testEvent} />, {
         organization: featuredOrganization,
       });
+      expect(mockDetailedProject).toHaveBeenCalled();
+      expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
       let rows = screen.getAllByTestId('tag-tree-row');
       expect(rows).toHaveLength(allTags.length);
 
@@ -650,6 +665,9 @@ describe('EventTagsAndScreenshot', function () {
       render(<EventTagsAndScreenshot projectSlug={project.slug} event={testEvent} />, {
         organization: featuredOrganization,
       });
+      expect(mockDetailedProject).toHaveBeenCalled();
+      expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument();
+
       const rows = screen.getAllByTestId('tag-tree-row');
       expect(rows).toHaveLength(applicationTags.length);
 

+ 11 - 3
static/app/components/events/highlights/editHighlightsModal.spec.tsx

@@ -87,7 +87,7 @@ describe('EditHighlightsModal', function () {
     renderModal({highlightContext: {}, highlightTags: []});
     expect(screen.getByText('Edit Event Highlights')).toBeInTheDocument();
     expect(screen.getByTestId('highlights-preview-section')).toBeInTheDocument();
-    expect(screen.getByTestId('highlights-empty-message')).toBeInTheDocument();
+    expect(screen.getByTestId('highlights-empty-preview')).toBeInTheDocument();
     expect(screen.getByTestId('highlights-save-info')).toBeInTheDocument();
     expect(screen.getByTestId('highlights-tag-section')).toBeInTheDocument();
     expect(screen.getByTestId('highlights-context-section')).toBeInTheDocument();
@@ -98,7 +98,7 @@ describe('EditHighlightsModal', function () {
       'highlights.edit_modal.use_default_clicked',
       expect.anything()
     );
-    expect(screen.queryByTestId('highlights-empty-message')).not.toBeInTheDocument();
+    expect(screen.queryByTestId('highlights-empty-preview')).not.toBeInTheDocument();
 
     const updateProjectMock = MockApiClient.addMockResponse({
       url,
@@ -137,7 +137,7 @@ describe('EditHighlightsModal', function () {
 
     // Existing Tags and Context Keys should be highlighted
     const previewSection = screen.getByTestId('highlights-preview-section');
-    expect(screen.queryByTestId('highlights-empty-message')).not.toBeInTheDocument();
+    expect(screen.queryByTestId('highlights-empty-preview')).not.toBeInTheDocument();
     highlightTags.forEach(tag => {
       const tagItem = within(previewSection).getByText(tag, {selector: 'div'});
       expect(tagItem).toBeInTheDocument();
@@ -350,6 +350,10 @@ describe('EditHighlightsModal', function () {
     await userEvent.type(tagInput, 'le');
     expect(screen.getAllByTestId('highlight-tag-option')).toHaveLength(3); // handled, level, release
     await userEvent.clear(tagInput);
+    await userEvent.type(tagInput, 'gibberish');
+    expect(screen.queryAllByTestId('highlight-tag-option')).toHaveLength(0);
+    expect(screen.getByTestId('highlights-empty-tags')).toBeInTheDocument();
+    await userEvent.clear(tagInput);
     expect(screen.getAllByTestId('highlight-tag-option')).toHaveLength(tagCount);
 
     const ctxCount = Object.values(TEST_EVENT_CONTEXTS)
@@ -360,6 +364,10 @@ describe('EditHighlightsModal', function () {
     await userEvent.type(contextInput, 'name'); // client_os.name, runtime.name
     expect(screen.getAllByTestId('highlight-context-option')).toHaveLength(2);
     await userEvent.clear(contextInput);
+    await userEvent.type(contextInput, 'gibberish');
+    expect(screen.queryAllByTestId('highlight-context-option')).toHaveLength(0);
+    expect(screen.getByTestId('highlights-empty-context')).toBeInTheDocument();
+    await userEvent.clear(contextInput);
     expect(screen.getAllByTestId('highlight-context-option')).toHaveLength(ctxCount);
   });
 });

+ 41 - 59
static/app/components/events/highlights/editHighlightsModal.tsx

@@ -1,7 +1,7 @@
 import {Fragment, useState} from 'react';
+import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
-import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
 import type {ModalRenderProps} from 'sentry/actionCreators/modal';
 import {Button} from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
@@ -20,14 +20,11 @@ import {
 import type {InputProps} from 'sentry/components/input';
 import {InputGroup} from 'sentry/components/inputGroup';
 import {IconAdd, IconInfo, IconSearch, IconSubtract} from 'sentry/icons';
-import {t, tct} from 'sentry/locale';
+import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {Event, Project} from 'sentry/types';
 import {trackAnalytics} from 'sentry/utils/analytics';
-import {setApiQueryData, useMutation, useQueryClient} from 'sentry/utils/queryClient';
-import type RequestError from 'sentry/utils/requestError/requestError';
-import useApi from 'sentry/utils/useApi';
-import {makeDetailedProjectQueryKey} from 'sentry/utils/useDetailedProject';
+import useMutateProject from 'sentry/utils/useMutateProject';
 import useOrganization from 'sentry/utils/useOrganization';
 
 export interface EditHighlightsModalProps extends ModalRenderProps {
@@ -81,7 +78,11 @@ function EditPreviewHighlightSection({
             meta={meta}
             item={item}
             alias={alias}
-            config={{includeAliasInSubject: true, disableErrors: true}}
+            config={{
+              includeAliasInSubject: true,
+              disableErrors: true,
+              disableRichValue: true,
+            }}
             data-test-id="highlights-preview-ctx"
           />
         </Fragment>
@@ -105,8 +106,8 @@ function EditPreviewHighlightSection({
         content={content}
         event={event}
         tagKey={content.originalTag.key}
-        projectSlug={project.slug}
-        config={{disableActions: true, disableRichValue: true}}
+        project={project}
+        config={{disableActions: true, disableRichValue: true, disableErrors: true}}
         data-test-id="highlights-preview-tag"
       />
     </Fragment>
@@ -127,10 +128,7 @@ function EditPreviewHighlightSection({
       {columns.length > 0 ? (
         columns
       ) : (
-        <EmptyHighlightMessage
-          columnCount={previewColumnCount}
-          data-test-id="highlights-empty-message"
-        >
+        <EmptyHighlightMessage data-test-id="highlights-empty-preview">
           {t('Promote tags or context keys to highlights for quicker debugging!')}
         </EmptyHighlightMessage>
       )}
@@ -201,7 +199,13 @@ function EditTagHighlightSection({
         />
       </Subtitle>
       <EditHighlightSectionContent columnCount={columnCount}>
-        {tagColumns}
+        {tagColumns.length > 0 ? (
+          tagColumns
+        ) : (
+          <EmptyHighlightMessage extraMargin data-test-id="highlights-empty-tags">
+            {t('No matching event tags found.')}
+          </EmptyHighlightMessage>
+        )}
       </EditHighlightSectionContent>
     </EditHighlightSection>
   );
@@ -300,7 +304,13 @@ function EditContextHighlightSection({
         />
       </Subtitle>
       <EditHighlightSectionContent columnCount={columnCount}>
-        {contextColumns}
+        {contextColumns.length > 0 ? (
+          contextColumns
+        ) : (
+          <EmptyHighlightMessage extraMargin data-test-id="highlights-empty-context">
+            {t('No matching event context found.')}
+          </EmptyHighlightMessage>
+        )}
       </EditHighlightSectionContent>
     </EditHighlightSection>
   );
@@ -322,47 +332,11 @@ export default function EditHighlightsModal({
   const [highlightTags, setHighlightTags] = useState<HighlightTags>(prevHighlightTags);
 
   const organization = useOrganization();
-  const api = useApi();
-  const queryClient = useQueryClient();
-
-  const {mutate: saveHighlights, isLoading} = useMutation<
-    Project,
-    RequestError,
-    {
-      highlightContext: HighlightContext;
-      highlightTags: HighlightTags;
-    }
-  >({
-    mutationFn: data => {
-      return api.requestPromise(`/projects/${organization.slug}/${project.slug}/`, {
-        method: 'PUT',
-        data,
-      });
-    },
-    onSuccess: (updatedProject: Project) => {
-      addSuccessMessage(
-        tct(`Successfully updated highlights for '[projectName]' project`, {
-          projectName: project.name,
-        })
-      );
-      setApiQueryData<Project>(
-        queryClient,
-        makeDetailedProjectQueryKey({
-          orgSlug: organization.slug,
-          projectSlug: project.slug,
-        }),
-        data =>
-          updatedProject ? updatedProject : {...data, highlightTags, highlightContext}
-      );
-      closeModal();
-    },
-    onError: _error => {
-      addErrorMessage(
-        tct(`Failed to update highlights for '[projectName]' project`, {
-          projectName: project.name,
-        })
-      );
-    },
+
+  const {mutate: saveHighlights, isLoading} = useMutateProject({
+    organization,
+    project,
+    onSuccess: closeModal,
   });
 
   const columnCount = 3;
@@ -371,7 +345,7 @@ export default function EditHighlightsModal({
       <Header closeButton>
         <Title>{t('Edit Event Highlights')}</Title>
       </Header>
-      <Body>
+      <Body css={modalBodyCss}>
         <EditPreviewHighlightSection
           event={event}
           highlightTags={highlightTags}
@@ -480,6 +454,13 @@ function SectionFilterInput(props: InputProps) {
   );
 }
 
+const modalBodyCss = css`
+  margin: 0 -${space(4)};
+  padding: 0 ${space(4)};
+  max-height: 75vh;
+  overflow-y: scroll;
+`;
+
 const Title = styled('h3')`
   font-size: ${p => p.theme.fontSizeLarge};
 `;
@@ -517,11 +498,12 @@ const EditHighlightPreview = styled('div')<{columnCount: number}>`
   font-size: ${p => p.theme.fontSizeSmall};
 `;
 
-const EmptyHighlightMessage = styled('div')<{columnCount: number}>`
+const EmptyHighlightMessage = styled('div')<{extraMargin?: boolean}>`
   font-size: ${p => p.theme.fontSizeMedium};
   color: ${p => p.theme.subText};
-  grid-column: span ${p => p.columnCount};
+  grid-column: 1 / -1;
   text-align: center;
+  margin: ${p => (p.extraMargin ? space(3) : 0)} 0;
 `;
 
 const EditHighlightSection = styled('div')`

Some files were not shown because too many files changed in this diff