Browse Source

feat(issue-details): Enable a toggle for setting the new UI as preferred (#76283)

Requires https://github.com/getsentry/sentry/pull/76282 for saving user
prefs.


This PR adds back the 'New Experience' header button to the UI if:
- `organization` has the `issue-details-streamline` set to `true`

The new UI will be used if any of the following are true:
- `streamline` query param is set to `1`
- User viewing the issue has `prefersIssueDetailsStreamlinedUI` option
set to `true`

Also adds a feedback button if available:

![image](https://github.com/user-attachments/assets/b7424c1f-3d36-4140-a32e-d28c8a9765a2)


https://github.com/user-attachments/assets/ffee1105-6199-4593-a737-5bf05855be83

**todo**
- [x] Add test for conditions to make button appear
- [x] Add tests for `useHasStreamlinedUI` changes
- [x] Add test for navigation on click
- [x] Add analytics for toggles
Leander Rodrigues 6 months ago
parent
commit
e1f07a1169

+ 1 - 1
static/app/types/user.tsx

@@ -46,8 +46,8 @@ export interface User extends Omit<AvatarUser, 'options'> {
     avatarType: Avatar['avatarType'];
     clock24Hours: boolean;
     defaultIssueEvent: 'recommended' | 'latest' | 'oldest';
-    issueDetailsNewExperienceQ42023: boolean;
     language: string;
+    prefersIssueDetailsStreamlinedUI: boolean;
     stacktraceOrder: number;
     theme: 'system' | 'light' | 'dark';
     timezone: string;

+ 4 - 0
static/app/utils/analytics/issueAnalyticsEvents.tsx

@@ -139,6 +139,9 @@ export type IssueEventParameters = {
   'issue_details.sourcemap_wizard_copy': SourceMapWizardParam;
   'issue_details.sourcemap_wizard_dismiss': SourceMapWizardParam;
   'issue_details.sourcemap_wizard_learn_more': SourceMapWizardParam;
+  'issue_details.streamline_ui_toggle': {
+    isEnabled: boolean;
+  };
   'issue_details.view_hierarchy.hover_rendering_system': {
     platform?: string;
     user_org_role?: string;
@@ -327,6 +330,7 @@ export const issueEventMap: Record<IssueEventKey, string | null> = {
     'Issue Details: Similar Issues: Diff Clicked',
   'issue_details.similar_issues.similarity_embeddings_feedback_recieved':
     'Issue Details: Similar Issues: Similarity Embeddings Feedback Recieved',
+  'issue_details.streamline_ui_toggle': 'Streamline: UI Toggle Clicked',
   'issue_details.view_hierarchy.hover_rendering_system':
     'View Hierarchy: Hovered rendering system icon',
   'issue_details.view_hierarchy.select_from_tree': 'View Hierarchy: Selection from tree',

+ 2 - 2
static/app/utils/useMutateProject.tsx

@@ -42,7 +42,7 @@ export default function useMutateProject({
       }),
     onSuccess: (updatedProject: Project) => {
       addSuccessMessage(
-        tct(`Successfully updated highlights for '[projectName]' project`, {
+        tct(`Successfully updated '[projectName]' project`, {
           projectName: project.name,
         })
       );
@@ -58,7 +58,7 @@ export default function useMutateProject({
     },
     onError: error => {
       addErrorMessage(
-        tct(`Failed to update highlights for '[projectName]' project`, {
+        tct(`Failed to update '[projectName]' project`, {
           projectName: project.name,
         })
       );

+ 39 - 0
static/app/utils/useMutateUserOptions.tsx

@@ -0,0 +1,39 @@
+import merge from 'lodash/merge';
+
+import {addErrorMessage} from 'sentry/actionCreators/indicator';
+import ConfigStore from 'sentry/stores/configStore';
+import type {User} from 'sentry/types/user';
+import {useMutation} from 'sentry/utils/queryClient';
+import type RequestError from 'sentry/utils/requestError/requestError';
+import useApi from 'sentry/utils/useApi';
+import {useUser} from 'sentry/utils/useUser';
+
+type UpdateUserOptionsVariables = Partial<User['options']>;
+interface UseMutateProjectProps {
+  onError?: (error: RequestError) => void;
+  onSuccess?: () => void;
+}
+
+export default function useMutateUserOptions({
+  onSuccess,
+  onError,
+}: UseMutateProjectProps = {}) {
+  const user = useUser();
+  const api = useApi({persistInFlight: false});
+  return useMutation<User, RequestError, UpdateUserOptionsVariables>({
+    mutationFn: (options: UpdateUserOptionsVariables) => {
+      return api.requestPromise('/users/me/', {
+        method: 'PUT',
+        data: {options},
+      });
+    },
+    onMutate: (options: UpdateUserOptionsVariables) => {
+      ConfigStore.set('user', merge({}, user, {options}));
+      return onSuccess?.();
+    },
+    onError: error => {
+      addErrorMessage('Failed to save user preference');
+      return onError?.(error);
+    },
+  });
+}

+ 2 - 5
static/app/views/issueDetails/actions/index.tsx

@@ -378,9 +378,7 @@ export function Actions(props: Props) {
               onClick={() =>
                 onUpdate({
                   status: GroupStatus.UNRESOLVED,
-
                   statusDetails: {},
-
                   substatus: GroupSubstatus.ONGOING,
                 })
               }
@@ -427,6 +425,7 @@ export function Actions(props: Props) {
             />
           </Fragment>
         ))}
+      {hasStreamlinedUI && <NewIssueExperienceButton />}
       <DropdownMenu
         triggerProps={{
           'aria-label': t('More Actions'),
@@ -508,9 +507,7 @@ export function Actions(props: Props) {
       />
       {!hasStreamlinedUI && (
         <Fragment>
-          {organization.features.includes('issue-details-new-experience-toggle') ? (
-            <NewIssueExperienceButton />
-          ) : null}
+          <NewIssueExperienceButton />
           <SubscribeAction
             className="hidden-xs"
             disabled={disabled}

+ 75 - 5
static/app/views/issueDetails/actions/newIssueExperienceButton.spec.tsx

@@ -1,15 +1,75 @@
-import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
+import {LocationFixture} from 'sentry-fixture/locationFixture';
+import {OrganizationFixture} from 'sentry-fixture/organization';
+import {UserFixture} from 'sentry-fixture/user';
 
+import {act, render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
+
+import ConfigStore from 'sentry/stores/configStore';
+import {trackAnalytics} from 'sentry/utils/analytics';
 import {NewIssueExperienceButton} from 'sentry/views/issueDetails/actions/newIssueExperienceButton';
 
+const mockUseNavigate = jest.fn();
+jest.mock('sentry/utils/useNavigate', () => ({
+  useNavigate: () => mockUseNavigate,
+}));
+jest.mock('sentry/utils/analytics');
+
 describe('NewIssueExperienceButton', function () {
-  it('triggers changes to the user config', async function () {
+  const organization = OrganizationFixture({features: ['issue-details-streamline']});
+  const user = UserFixture();
+  user.options.prefersIssueDetailsStreamlinedUI = true;
+  const location = LocationFixture({query: {streamline: '1'}});
+
+  beforeEach(() => {
+    ConfigStore.init();
+  });
+
+  it('does not appear by default', function () {
+    render(
+      <div data-test-id="test-id">
+        <NewIssueExperienceButton />
+      </div>
+    );
+    expect(screen.getByTestId('test-id')).toBeEmptyDOMElement();
+  });
+
+  it('appears when organization has flag', function () {
+    render(
+      <div data-test-id="test-id">
+        <NewIssueExperienceButton />
+      </div>,
+      {organization}
+    );
+    expect(screen.getByTestId('test-id')).not.toBeEmptyDOMElement();
+  });
+
+  it('does not appear even if user prefers this UI', function () {
+    act(() => ConfigStore.set('user', user));
+    render(
+      <div data-test-id="test-id">
+        <NewIssueExperienceButton />
+      </div>
+    );
+    expect(screen.getByTestId('test-id')).toBeEmptyDOMElement();
+  });
+
+  it('does not appear when query param is set', function () {
+    render(
+      <div data-test-id="test-id">
+        <NewIssueExperienceButton />
+      </div>,
+      {router: {location}}
+    );
+    expect(screen.getByTestId('test-id')).toBeEmptyDOMElement();
+  });
+
+  it('triggers changes to the user config and location', async function () {
     const mockChangeUserSettings = MockApiClient.addMockResponse({
       url: '/users/me/',
       method: 'PUT',
     });
 
-    render(<NewIssueExperienceButton />);
+    render(<NewIssueExperienceButton />, {organization});
 
     const button = screen.getByRole('button', {
       name: 'Switch to the new issue experience',
@@ -27,12 +87,17 @@ describe('NewIssueExperienceButton', function () {
         expect.objectContaining({
           data: {
             options: {
-              issueDetailsNewExperienceQ42023: true,
+              prefersIssueDetailsStreamlinedUI: true,
             },
           },
         })
       );
     });
+    // Location should update
+    expect(mockUseNavigate).toHaveBeenCalledWith(
+      expect.objectContaining({query: {streamline: '1'}})
+    );
+    expect(trackAnalytics).toHaveBeenCalledTimes(1);
 
     // Clicking again toggles it off
     await userEvent.click(button);
@@ -47,11 +112,16 @@ describe('NewIssueExperienceButton', function () {
         expect.objectContaining({
           data: {
             options: {
-              issueDetailsNewExperienceQ42023: false,
+              prefersIssueDetailsStreamlinedUI: false,
             },
           },
         })
       );
     });
+    // Location should update again
+    expect(mockUseNavigate).toHaveBeenCalledWith(
+      expect.objectContaining({query: {streamline: '0'}})
+    );
+    expect(trackAnalytics).toHaveBeenCalledTimes(2);
   });
 });

+ 61 - 50
static/app/views/issueDetails/actions/newIssueExperienceButton.tsx

@@ -1,69 +1,80 @@
-import {css} from '@emotion/react';
 import styled from '@emotion/styled';
-import merge from 'lodash/merge';
 
-import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {Button} from 'sentry/components/button';
+import ButtonBar from 'sentry/components/buttonBar';
 import {IconLab} from 'sentry/icons';
 import {t} from 'sentry/locale';
-import ConfigStore from 'sentry/stores/configStore';
-import type {User} from 'sentry/types/user';
-import {useMutation} from 'sentry/utils/queryClient';
-import useApi from 'sentry/utils/useApi';
-import {useUser} from 'sentry/utils/useUser';
-
-type UpdateUserOptionsVariables = Partial<User['options']>;
+import {trackAnalytics} from 'sentry/utils/analytics';
+import {useFeedbackForm} from 'sentry/utils/useFeedbackForm';
+import {useLocation} from 'sentry/utils/useLocation';
+import useMutateUserOptions from 'sentry/utils/useMutateUserOptions';
+import {useNavigate} from 'sentry/utils/useNavigate';
+import useOrganization from 'sentry/utils/useOrganization';
+import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';
 
 export function NewIssueExperienceButton() {
-  const user = useUser();
-  const api = useApi();
+  const organization = useOrganization();
+  const location = useLocation();
+  const navigate = useNavigate();
+  const hasStreamlinedUIFlag = organization.features.includes('issue-details-streamline');
+  const hasStreamlinedUI = useHasStreamlinedUI();
+  const openForm = useFeedbackForm();
+  const {mutate} = useMutateUserOptions();
 
-  const newIssueExperienceEnabled =
-    user?.options?.issueDetailsNewExperienceQ42023 ?? false;
+  if (!hasStreamlinedUIFlag) {
+    return null;
+  }
 
-  const {mutate} = useMutation({
-    mutationFn: (options: UpdateUserOptionsVariables) => {
-      return api.requestPromise('/users/me/', {
-        method: 'PUT',
-        data: {
-          options,
-        },
-      });
-    },
-    onMutate: (options: UpdateUserOptionsVariables) => {
-      ConfigStore.set('user', merge({}, user, {options}));
-    },
-    onError: () => {
-      addErrorMessage('Failed to save new issue experience preference');
-    },
-  });
+  const feedbackButton = openForm ? (
+    <Button
+      size="sm"
+      aria-label={t('Give feedback on new UI')}
+      onClick={() =>
+        openForm({
+          messagePlaceholder: t('How can we make this new UI work for you?'),
+          tags: {
+            ['feedback.source']: 'issue_details_streamline_ui',
+            ['feedback.owner']: 'issues',
+          },
+        })
+      }
+    >
+      {t('Give Feedback')}
+    </Button>
+  ) : null;
 
-  const label = newIssueExperienceEnabled
+  const label = hasStreamlinedUI
     ? t('Switch to the old issue experience')
     : t('Switch to the new issue experience');
 
   return (
-    <StyledButton
-      enabled={newIssueExperienceEnabled}
-      size="sm"
-      icon={<IconLab isSolid={newIssueExperienceEnabled} />}
-      title={label}
-      aria-label={label}
-      onClick={() =>
-        mutate({['issueDetailsNewExperienceQ42023']: !newIssueExperienceEnabled})
-      }
-    />
+    <ButtonBar merged>
+      <StyledButton
+        enabled={hasStreamlinedUI}
+        size="sm"
+        icon={<IconLab isSolid={hasStreamlinedUI} />}
+        title={label}
+        aria-label={label}
+        onClick={() => {
+          mutate({['prefersIssueDetailsStreamlinedUI']: !hasStreamlinedUI});
+          trackAnalytics('issue_details.streamline_ui_toggle', {
+            isEnabled: !hasStreamlinedUI,
+            organization: organization,
+          });
+          navigate({
+            ...location,
+            query: {...location.query, streamline: hasStreamlinedUI ? '0' : '1'},
+          });
+        }}
+      />
+      {hasStreamlinedUI && feedbackButton}
+    </ButtonBar>
   );
 }
 
 const StyledButton = styled(Button)<{enabled: boolean}>`
-  ${p =>
-    p.enabled &&
-    css`
-      color: ${p.theme.button.primary.background};
-
-      :hover {
-        color: ${p.theme.button.primary.background};
-      }
-    `}
+  color: ${p => (p.enabled ? p.theme.button.primary.background : 'inherit')};
+  :hover {
+    color: ${p => (p.enabled ? p.theme.button.primary.background : 'inherit')};
+  }
 `;

+ 2 - 2
static/app/views/issueDetails/groupDetails.tsx

@@ -58,7 +58,7 @@ import {useUser} from 'sentry/utils/useUser';
 import GroupHeader from 'sentry/views/issueDetails//header';
 import {ERROR_TYPES} from 'sentry/views/issueDetails/constants';
 import SampleEventAlert from 'sentry/views/issueDetails/sampleEventAlert';
-import StreamlinedGroupHeader from 'sentry/views/issueDetails/streamlinedHeader';
+import StreamlinedGroupHeader from 'sentry/views/issueDetails/streamline/header';
 import {Tab, TabPaths} from 'sentry/views/issueDetails/types';
 import {
   getGroupDetailsQueryData,
@@ -572,7 +572,7 @@ function useTrackView({
     has_hierarchical_grouping:
       !!organization.features?.includes('grouping-stacktrace-ui') &&
       !!(event?.metadata?.current_tree_label || event?.metadata?.finest_tree_label),
-    new_issue_experience: user?.options?.issueDetailsNewExperienceQ42023 ?? false,
+    prefers_streamlined_ui: user?.options?.prefersIssueDetailsStreamlinedUI ?? false,
   });
   // Set default values for properties that may be updated in subcomponents.
   // Must be separate from the above values, otherwise the actual values filled in

+ 42 - 13
static/app/views/issueDetails/streamlinedHeader.spec.tsx → static/app/views/issueDetails/streamline/header.spec.tsx

@@ -1,4 +1,5 @@
 import {GroupFixture} from 'sentry-fixture/group';
+import {LocationFixture} from 'sentry-fixture/locationFixture';
 import {OrganizationFixture} from 'sentry-fixture/organization';
 import {ProjectFixture} from 'sentry-fixture/project';
 import {ReleaseFixture} from 'sentry-fixture/release';
@@ -10,17 +11,18 @@ import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import type {TeamParticipant, UserParticipant} from 'sentry/types/group';
 import {IssueCategory} from 'sentry/types/group';
-import StreamlinedGroupHeader from 'sentry/views/issueDetails/streamlinedHeader';
+import StreamlinedGroupHeader from 'sentry/views/issueDetails/streamline/header';
 import {ReprocessingStatus} from 'sentry/views/issueDetails/utils';
 
 describe('UpdatedGroupHeader', () => {
   const baseUrl = 'BASE_URL/';
-  const organization = OrganizationFixture({features: ['issue-details-streamline']});
+  const organization = OrganizationFixture();
   const project = ProjectFixture({
     platform: 'javascript',
     teams: [TeamFixture()],
   });
   const group = GroupFixture({issueCategory: IssueCategory.ERROR, isUnhandled: true});
+  const location = LocationFixture({query: {streamline: '1'}});
 
   describe('JS Project Error Issue', () => {
     const defaultProps = {
@@ -93,30 +95,33 @@ describe('UpdatedGroupHeader', () => {
         />,
         {
           organization,
+          router: {location},
         }
       );
 
-      expect(await screen.findByText('RequestError')).toBeInTheDocument();
-
-      expect(await screen.findByText('Warning')).toBeInTheDocument();
-      expect(await screen.findByText('Unhandled')).toBeInTheDocument();
+      expect(screen.getByText('RequestError')).toBeInTheDocument();
+      expect(screen.getByText('Warning')).toBeInTheDocument();
+      expect(screen.getByText('Unhandled')).toBeInTheDocument();
 
       expect(
         await screen.findByText(textWithMarkupMatcher('Releases'))
       ).toBeInTheDocument();
 
       expect(
-        await screen.findByRole('button', {name: 'Modify issue priority'})
+        screen.getByRole('button', {name: 'Modify issue priority'})
       ).toBeInTheDocument();
       expect(
-        await screen.findByRole('button', {name: 'Modify issue assignee'})
+        screen.getByRole('button', {name: 'Modify issue assignee'})
       ).toBeInTheDocument();
 
-      expect(await screen.findByText('Participants')).toBeInTheDocument();
-      expect(await screen.findByText('Viewers')).toBeInTheDocument();
+      expect(screen.getByText('Participants')).toBeInTheDocument();
+      expect(screen.getByText('Viewers')).toBeInTheDocument();
 
-      expect(await screen.findByRole('button', {name: 'Resolve'})).toBeInTheDocument();
-      expect(await screen.findByRole('button', {name: 'Archive'})).toBeInTheDocument();
+      expect(
+        screen.queryByRole('button', {name: 'Switch to the old issue experience'})
+      ).not.toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'Resolve'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'Archive'})).toBeInTheDocument();
     });
 
     it('only shows one release if possible', async function () {
@@ -128,11 +133,35 @@ describe('UpdatedGroupHeader', () => {
       });
       render(
         <StreamlinedGroupHeader {...defaultProps} group={group} project={project} />,
-        {organization}
+        {
+          organization,
+          router: {location},
+        }
       );
       expect(
         await screen.findByText(textWithMarkupMatcher('Release'))
       ).toBeInTheDocument();
     });
+
+    it('displays new experience button if flag is set', async () => {
+      MockApiClient.addMockResponse({
+        url: `/organizations/${organization.slug}/issues/${group.id}/first-last-release/`,
+        method: 'GET',
+        body: {firstRelease, lastRelease},
+      });
+      const flaggedOrganization = OrganizationFixture({
+        features: ['issue-details-streamline'],
+      });
+      render(
+        <StreamlinedGroupHeader {...defaultProps} group={group} project={project} />,
+        {
+          organization: flaggedOrganization,
+          router: {location},
+        }
+      );
+      expect(
+        await screen.findByRole('button', {name: 'Switch to the old issue experience'})
+      ).toBeInTheDocument();
+    });
   });
 });

+ 0 - 0
static/app/views/issueDetails/streamlinedHeader.tsx → static/app/views/issueDetails/streamline/header.tsx


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