Browse Source

feat(alerts): Add migration warnings on list view (#58426)

- relates to https://github.com/getsentry/sentry/issues/58139
ArthurKnaus 1 year ago
parent
commit
6f2f1c962b

+ 15 - 1
static/app/views/alerts/filterBar.tsx

@@ -6,6 +6,7 @@ import {CompactSelect} from 'sentry/components/compactSelect';
 import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilter';
 import SearchBar from 'sentry/components/searchBar';
 import {SegmentedControl} from 'sentry/components/segmentedControl';
+import {IconWarning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 
@@ -19,6 +20,7 @@ interface Props {
   hasStatusFilters?: boolean;
   onChangeDataset?: (dataset: DatasetOption) => void;
   onChangeStatus?: (status: string) => void;
+  showMigrationWarning?: boolean;
 }
 
 function FilterBar({
@@ -28,6 +30,7 @@ function FilterBar({
   onChangeStatus,
   onChangeDataset,
   hasStatusFilters,
+  showMigrationWarning,
 }: Props) {
   const selectedTeams = getTeamParams(location.query.team);
   const selectedStatus = getQueryStatus(location.query.status);
@@ -77,8 +80,12 @@ function FilterBar({
               <SegmentedControl.Item key={DatasetOption.SESSIONS}>
                 {t('Sessions')}
               </SegmentedControl.Item>
-              <SegmentedControl.Item key={DatasetOption.PERFORMANCE}>
+              <SegmentedControl.Item
+                textValue={t('Performance')}
+                key={DatasetOption.PERFORMANCE}
+              >
                 {t('Performance')}
+                {showMigrationWarning ? <StyledIconWarning /> : null}
               </SegmentedControl.Item>
             </SegmentedControl>
           </SegmentedControlWrapper>
@@ -121,3 +128,10 @@ const FilterButtons = styled(ButtonBar)`
 const SegmentedControlWrapper = styled('div')`
   width: max-content;
 `;
+
+const StyledIconWarning = styled(IconWarning)`
+  vertical-align: middle;
+  margin-top: -${space(0.5)};
+  margin-left: ${space(0.5)};
+  color: ${p => p.theme.yellow400};
+`;

+ 1 - 1
static/app/views/alerts/list/rules/alertRulesList.spec.tsx

@@ -254,7 +254,7 @@ describe('AlertRulesList', () => {
       'ascending'
     );
 
-    expect(rulesMock).toHaveBeenCalledTimes(1);
+    expect(rulesMock).toHaveBeenCalledTimes(2);
     expect(rulesMock).toHaveBeenCalledWith(
       '/organizations/org-slug/combined-rules/',
       expect.objectContaining({

+ 27 - 2
static/app/views/alerts/list/rules/alertRulesList.tsx

@@ -9,6 +9,7 @@ import {
   addMessage,
   addSuccessMessage,
 } from 'sentry/actionCreators/indicator';
+import Alert from 'sentry/components/alert';
 import * as Layout from 'sentry/components/layouts/thirds';
 import Link from 'sentry/components/links/link';
 import LoadingError from 'sentry/components/loadingError';
@@ -16,8 +17,8 @@ import PageFiltersContainer from 'sentry/components/organizations/pageFilters/co
 import Pagination from 'sentry/components/pagination';
 import PanelTable from 'sentry/components/panels/panelTable';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
-import {IconArrow} from 'sentry/icons';
-import {t} from 'sentry/locale';
+import {IconArrow, IconWarning} from 'sentry/icons';
+import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {Project} from 'sentry/types';
 import {defined} from 'sentry/utils';
@@ -35,6 +36,11 @@ import useApi from 'sentry/utils/useApi';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import useRouter from 'sentry/utils/useRouter';
+import {
+  hasMigrationFeatureFlag,
+  ruleNeedsMigration,
+  useOrgNeedsMigration,
+} from 'sentry/views/alerts/utils/migrationUi';
 
 import FilterBar from '../../filterBar';
 import {AlertRuleType, CombinedMetricIssueAlerts} from '../../types';
@@ -91,6 +97,8 @@ function AlertRulesList() {
       staleTime: 0,
     }
   );
+  const hasMigrationUIFeatureFlag = hasMigrationFeatureFlag(organization);
+  const showMigrationUI = useOrgNeedsMigration();
 
   const handleChangeFilter = (activeFilters: string[]) => {
     const {cursor: _cursor, page: _page, ...currentQuery} = location.query;
@@ -195,8 +203,17 @@ function AlertRulesList() {
         <AlertHeader router={router} activeTab="rules" />
         <Layout.Body>
           <Layout.Main fullWidth>
+            {showMigrationUI ? (
+              <Alert showIcon type="warning">
+                {tct(
+                  'Our performance alerts just got a lot more accurate, which is why we recommend you review the thresholds of all rules marked with a “[warningIcon]“',
+                  {warningIcon: <StyledIconWarning />}
+                )}
+              </Alert>
+            ) : null}
             <FilterBar
               location={location}
+              showMigrationWarning={showMigrationUI}
               onChangeDataset={handleChangeDataset}
               onChangeFilter={handleChangeFilter}
               onChangeSearch={handleChangeSearch}
@@ -261,6 +278,9 @@ function AlertRulesList() {
                         key={`${
                           isIssueAlert(rule) ? AlertRuleType.METRIC : AlertRuleType.ISSUE
                         }-${rule.id}`}
+                        showMigrationWarning={
+                          hasMigrationUIFeatureFlag && ruleNeedsMigration(rule)
+                        }
                         projectsLoaded={initiallyLoaded}
                         projects={projects as Project[]}
                         rule={rule}
@@ -325,3 +345,8 @@ const StyledPanelTable = styled(PanelTable)`
   white-space: nowrap;
   font-size: ${p => p.theme.fontSizeMedium};
 `;
+
+const StyledIconWarning = styled(IconWarning)`
+  vertical-align: middle;
+  color: ${p => p.theme.yellow400};
+`;

+ 15 - 0
static/app/views/alerts/list/rules/row.tsx

@@ -24,6 +24,7 @@ import {
   IconMute,
   IconNot,
   IconUser,
+  IconWarning,
 } from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
@@ -52,6 +53,7 @@ type Props = {
   projects: Project[];
   projectsLoaded: boolean;
   rule: CombinedMetricIssueAlerts;
+  showMigrationWarning: boolean;
 };
 
 function RuleListRow({
@@ -62,6 +64,7 @@ function RuleListRow({
   onDelete,
   onOwnerChange,
   hasEditAccess,
+  showMigrationWarning,
 }: Props) {
   const {teams: userTeams} = useUserTeams();
   const [assignee, setAssignee] = useState<string>('');
@@ -316,6 +319,13 @@ function RuleListRow({
     <ErrorBoundary>
       <AlertNameWrapper isIssueAlert={isIssueAlert(rule)}>
         <FlexCenter>
+          {showMigrationWarning && (
+            <Tooltip
+              title={t('The current thresholds for this alert could use some review')}
+            >
+              <StyledIconWarning />
+            </Tooltip>
+          )}
           <Tooltip
             title={
               isIssueAlert(rule)
@@ -519,4 +529,9 @@ const Label = styled(TextOverflow)`
   margin-left: 6px;
 `;
 
+const StyledIconWarning = styled(IconWarning)`
+  margin-right: ${space(1)};
+  color: ${p => p.theme.yellow400};
+`;
+
 export default RuleListRow;

+ 29 - 0
static/app/views/alerts/utils/migrationUi.ts

@@ -0,0 +1,29 @@
+import {Organization} from 'sentry/types';
+import {useApiQuery} from 'sentry/utils/queryClient';
+import useOrganization from 'sentry/utils/useOrganization';
+import {Dataset} from 'sentry/views/alerts/rules/metric/types';
+import {CombinedMetricIssueAlerts} from 'sentry/views/alerts/types';
+
+// TODO(telemetry-experience): remove when the migration is complete
+export const hasMigrationFeatureFlag = (organization: Organization): boolean =>
+  organization.features.includes('alert-migration-ui');
+
+// TODO(telemetry-experience): remove when the migration is complete
+export const ruleNeedsMigration = (rule: CombinedMetricIssueAlerts): boolean => {
+  return 'dataset' in rule && rule.dataset === Dataset.TRANSACTIONS;
+};
+// TODO(telemetry-experience): remove when the migration is complete
+export function useOrgNeedsMigration(): boolean {
+  const organization = useOrganization();
+  const {data = []} = useApiQuery<CombinedMetricIssueAlerts[]>(
+    [
+      `/organizations/${organization.slug}/combined-rules/`,
+      {query: {dataset: Dataset.TRANSACTIONS}},
+    ],
+    {staleTime: 0}
+  );
+
+  const hasTransactionAlerts = data.length > 0;
+  const hasFeatureFlag = hasMigrationFeatureFlag(organization);
+  return hasTransactionAlerts && hasFeatureFlag;
+}