Alerting: Reorder new alert and export buttons (grafana#68418)

* Add component for rule creation and dropdown * Make each route type have a different path * Remove unneeded import * Fix tests * Rename CreateRuleButton to MoreActionRuleButtons * Remove Recording Rule option from new rule form * Use alerting and recording for path params on new rules * Fix tests * Only show new button if permissions are granted * Fix tests
InfluxCommunity · May 31, 2023 · e796063 · e796063
1 parent c2a9d48
commit e796063
Show file tree

Hide file tree

Showing 13 changed files with 135 additions and 64 deletions.
diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx
@@ -225,7 +225,7 @@ const unifiedRoutes: RouteDescriptor[] = [
     ),
   },
   {
-    path: '/alerting/new',
+    path: '/alerting/new/:type?',
     pageClass: 'page-alerting',
     roles: evaluateAccess(
       [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite],

diff --git a/public/app/features/alerting/unified/MoreActionsRuleButtons.tsx b/public/app/features/alerting/unified/MoreActionsRuleButtons.tsx
@@ -0,0 +1,59 @@
+import React from 'react';
+import { useLocation } from 'react-router-dom';
+
+import { urlUtil } from '@grafana/data';
+import { Button, Dropdown, Icon, LinkButton, Menu, MenuItem } from '@grafana/ui';
+
+import { logInfo, LogMessages } from './Analytics';
+import { useRulesAccess } from './utils/accessControlHooks';
+import { createUrl } from './utils/url';
+
+interface Props {}
+
+export function MoreActionsRuleButtons({}: Props) {
+  const { canCreateGrafanaRules, canCreateCloudRules, canReadProvisioning } = useRulesAccess();
+  const location = useLocation();
+  const newMenu = (
+    <Menu>
+      {(canCreateGrafanaRules || canCreateCloudRules) && (
+        <MenuItem
+          url={urlUtil.renderUrl(`alerting/new/recording`, {
+            returnTo: location.pathname + location.search,
+          })}
+          label="New recording rule"
+        />
+      )}
+      {canReadProvisioning && (
+        <MenuItem
+          url={createUrl('/api/v1/provisioning/alert-rules/export', {
+            download: 'true',
+            format: 'yaml',
+          })}
+          label="Export all"
+          target="_blank"
+        />
+      )}
+    </Menu>
+  );
+
+  return (
+    <>
+      {(canCreateGrafanaRules || canCreateCloudRules) && (
+        <LinkButton
+          href={urlUtil.renderUrl('alerting/new/alerting', { returnTo: location.pathname + location.search })}
+          icon="plus"
+          onClick={() => logInfo(LogMessages.alertRuleFromScratch)}
+        >
+          New alert rule
+        </LinkButton>
+      )}
+
+      <Dropdown overlay={newMenu}>
+        <Button variant="secondary">
+          More
+          <Icon name="angle-down" />
+        </Button>
+      </Dropdown>
+    </>
+  );
+}
diff --git a/public/app/features/alerting/unified/RuleEditorRecordingRule.test.tsx b/public/app/features/alerting/unified/RuleEditorRecordingRule.test.tsx
@@ -127,10 +127,9 @@ describe('RuleEditor recording rules', () => {
       },
     });
 
-    renderRuleEditor();
+    renderRuleEditor(undefined, true);
     await waitForElementToBeRemoved(screen.getAllByTestId('Spinner'));
     await userEvent.type(await ui.inputs.name.find(), 'my great new recording rule');
-    await userEvent.click(await ui.buttons.lotexRecordingRule.get());
 
     const dataSourceSelect = ui.inputs.dataSource.get();
     await userEvent.click(byRole('combobox').get(dataSourceSelect));

diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx
@@ -117,7 +117,8 @@ const ui = {
   rulesFilterInput: byTestId('search-query-input'),
   moreErrorsButton: byRole('button', { name: /more errors/ }),
   editCloudGroupIcon: byTestId('edit-group'),
-  newRuleButton: byRole('link', { name: 'Create alert rule' }),
+  newRuleButton: byRole('link', { name: 'New alert rule' }),
+  moreButton: byRole('button', { name: 'More' }),
   exportButton: byRole('link', { name: /export/i }),
   editGroupModal: {
     dialog: byRole('dialog'),
@@ -697,18 +698,22 @@ describe('RuleList', () => {
 
         renderRuleList();
 
+        await userEvent.click(ui.moreButton.get());
         expect(ui.exportButton.get()).toBeInTheDocument();
       });
       it('Export button should not be visible when the user has no alert provisioning read permissions', async () => {
         enableRBAC();
 
+        grantUserPermissions([AccessControlAction.AlertingRuleCreate, AccessControlAction.FoldersRead]);
+
         mocks.getAllDataSourcesMock.mockReturnValue([]);
         setDataSourceSrv(new MockDataSourceSrv({}));
         mocks.api.fetchRules.mockResolvedValue([]);
         mocks.api.fetchRulerRules.mockResolvedValue({});
 
         renderRuleList();
 
+        await userEvent.click(ui.moreButton.get());
         expect(ui.exportButton.query()).not.toBeInTheDocument();
       });
     });
@@ -831,7 +836,7 @@ describe('RuleList', () => {
 
       await waitFor(() => expect(mocks.api.fetchRules).toHaveBeenCalledTimes(1));
 
-      const button = screen.getByText('Create alert rule');
+      const button = screen.getByText('New alert rule');
 
       button.addEventListener('click', (event) => event.preventDefault(), false);
 

diff --git a/public/app/features/alerting/unified/RuleList.tsx b/public/app/features/alerting/unified/RuleList.tsx
@@ -1,18 +1,17 @@
 import { css } from '@emotion/css';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
-import { useLocation } from 'react-router-dom';
 import { useAsyncFn, useInterval } from 'react-use';
 
-import { GrafanaTheme2, urlUtil } from '@grafana/data';
+import { GrafanaTheme2 } from '@grafana/data';
 import { Stack } from '@grafana/experimental';
-import { logInfo } from '@grafana/runtime';
-import { Button, LinkButton, useStyles2, withErrorBoundary } from '@grafana/ui';
+import { Button, useStyles2, withErrorBoundary } from '@grafana/ui';
 import { useQueryParams } from 'app/core/hooks/useQueryParams';
 import { useDispatch } from 'app/types';
 
 import { CombinedRuleNamespace } from '../../../types/unified-alerting';
 
-import { LogMessages, trackRuleListNavigation } from './Analytics';
+import { trackRuleListNavigation } from './Analytics';
+import { MoreActionsRuleButtons } from './MoreActionsRuleButtons';
 import { AlertingPageWrapper } from './components/AlertingPageWrapper';
 import { NoRulesSplash } from './components/rules/NoRulesCTA';
 import { INSTANCES_DISPLAY_LIMIT } from './components/rules/RuleDetails';
@@ -28,7 +27,6 @@ import { fetchAllPromAndRulerRulesAction } from './state/actions';
 import { useRulesAccess } from './utils/accessControlHooks';
 import { RULE_LIST_POLL_INTERVAL_MS } from './utils/constants';
 import { getAllRulesSourceNames } from './utils/datasource';
-import { createUrl } from './utils/url';
 
 const VIEWS = {
   groups: RuleListGroupView,
@@ -43,16 +41,13 @@ const RuleList = withErrorBoundary(
     const dispatch = useDispatch();
     const styles = useStyles2(getStyles);
     const rulesDataSourceNames = useMemo(getAllRulesSourceNames, []);
-    const location = useLocation();
     const [expandAll, setExpandAll] = useState(false);
 
     const onFilterCleared = useCallback(() => setExpandAll(false), []);
 
     const [queryParams] = useQueryParams();
     const { filterState, hasActiveFilters } = useRulesFilter();
 
-    const { canCreateGrafanaRules, canCreateCloudRules, canReadProvisioning } = useRulesAccess();
-
     const view = VIEWS[queryParams['view'] as keyof typeof VIEWS]
       ? (queryParams['view'] as keyof typeof VIEWS)
       : 'groups';
@@ -96,6 +91,8 @@ const RuleList = withErrorBoundary(
     const combinedNamespaces: CombinedRuleNamespace[] = useCombinedRuleNamespaces();
     const filteredNamespaces = useFilteredRules(combinedNamespaces, filterState);
 
+    const { canCreateGrafanaRules, canCreateCloudRules, canReadProvisioning } = useRulesAccess();
+
     return (
       // We don't want to show the Loading... indicator for the whole page.
       // We show separate indicators for Grafana-managed and Cloud rules
@@ -119,31 +116,11 @@ const RuleList = withErrorBoundary(
                 )}
                 <RuleStats namespaces={filteredNamespaces} />
               </div>
-              <Stack direction="row" gap={0.5}>
-                {canReadProvisioning && (
-                  <LinkButton
-                    variant="secondary"
-                    href={createUrl('/api/v1/provisioning/alert-rules/export', {
-                      download: 'true',
-                      format: 'yaml',
-                    })}
-                    icon="download-alt"
-                    target="_blank"
-                    rel="noopener"
-                  >
-                    Export
-                  </LinkButton>
-                )}
-                {(canCreateGrafanaRules || canCreateCloudRules) && (
-                  <LinkButton
-                    href={urlUtil.renderUrl('alerting/new', { returnTo: location.pathname + location.search })}
-                    icon="plus"
-                    onClick={() => logInfo(LogMessages.alertRuleFromScratch)}
-                  >
-                    Create alert rule
-                  </LinkButton>
-                )}
-              </Stack>
+              {(canCreateGrafanaRules || canCreateCloudRules || canReadProvisioning) && (
+                <Stack direction="row" gap={0.5}>
+                  <MoreActionsRuleButtons />
+                </Stack>
+              )}
             </div>
           </>
         )}

diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx
@@ -1,7 +1,7 @@
 import { css } from '@emotion/css';
 import React, { useEffect, useMemo, useState } from 'react';
 import { DeepMap, FieldError, FormProvider, useForm, useFormContext, UseFormWatch } from 'react-hook-form';
-import { Link } from 'react-router-dom';
+import { Link, useParams } from 'react-router-dom';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { config, logInfo } from '@grafana/runtime';
@@ -28,6 +28,7 @@ import { NotificationsStep } from './NotificationsStep';
 import { RuleEditorSection } from './RuleEditorSection';
 import { RuleInspector } from './RuleInspector';
 import { QueryAndExpressionsStep } from './query-and-alert-condition/QueryAndExpressionsStep';
+import { translateRouteParamToRuleType } from './util';
 
 const recordingRuleNameValidationPattern = {
   message:
@@ -81,6 +82,9 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
   const [showEditYaml, setShowEditYaml] = useState(false);
   const [evaluateEvery, setEvaluateEvery] = useState(existing?.group.interval ?? MINUTE);
 
+  const routeParams = useParams<{ type: string }>();
+  const ruleType = translateRouteParamToRuleType(routeParams.type);
+
   const returnTo: string = (queryParams['returnTo'] as string | undefined) ?? '/alerting/list';
   const [showDeleteModal, setShowDeleteModal] = useState<boolean>(false);
 
@@ -101,10 +105,10 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
       queries: getDefaultQueries(),
       condition: 'C',
       ...(queryParams['defaults'] ? JSON.parse(queryParams['defaults'] as string) : {}),
-      type: RuleFormType.grafana,
+      type: ruleType || RuleFormType.grafana,
       evaluateEvery: evaluateEvery,
     };
-  }, [existing, prefill, queryParams, evaluateEvery]);
+  }, [existing, prefill, queryParams, evaluateEvery, ruleType]);
 
   const formAPI = useForm<RuleFormValues>({
     mode: 'onSubmit',

diff --git a/...ures/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.test.tsx b/...ures/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.test.tsx
@@ -2,8 +2,10 @@ import { render } from '@testing-library/react';
 import React from 'react';
 import { FormProvider, useForm } from 'react-hook-form';
 import { Provider } from 'react-redux';
+import { Router } from 'react-router-dom';
 import { byText } from 'testing-library-selector';
 
+import { locationService } from '@grafana/runtime';
 import { contextSrv } from 'app/core/services/context_srv';
 import { configureStore } from 'app/store/configureStore';
 import { AccessControlAction } from 'app/types';
@@ -28,15 +30,17 @@ function renderAlertTypeStep() {
 
   render(
     <Provider store={store}>
-      <AlertType editingExistingRule={false} />
+      <Router history={locationService.getHistory()}>
+        <AlertType editingExistingRule={false} />
+      </Router>
     </Provider>,
     { wrapper: FormProviderWrapper }
   );
 }
 
 describe('RuleTypePicker', () => {
   describe('RBAC', () => {
-    it('Should display grafana, mimir alert and mimir recording buttons when user has rule create and write permissions', async () => {
+    it('Should display grafana and mimir alert when user has rule create and write permissions', async () => {
       jest.spyOn(contextSrv, 'hasPermission').mockImplementation((action) => {
         return [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite].includes(
           action as AccessControlAction
@@ -47,7 +51,17 @@ describe('RuleTypePicker', () => {
 
       expect(ui.ruleTypePicker.grafanaManagedButton.get()).toBeInTheDocument();
       expect(ui.ruleTypePicker.mimirOrLokiButton.get()).toBeInTheDocument();
-      expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.get()).toBeInTheDocument();
+    });
+
+    it('Should not display the recording rule button', async () => {
+      jest.spyOn(contextSrv, 'hasPermission').mockImplementation((action) => {
+        return [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite].includes(
+          action as AccessControlAction
+        );
+      });
+
+      renderAlertTypeStep();
+      expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.query()).not.toBeInTheDocument();
     });
 
     it('Should hide grafana button when user does not have rule create permission', () => {
@@ -59,7 +73,7 @@ describe('RuleTypePicker', () => {
 
       expect(ui.ruleTypePicker.grafanaManagedButton.query()).not.toBeInTheDocument();
       expect(ui.ruleTypePicker.mimirOrLokiButton.get()).toBeInTheDocument();
-      expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.get()).toBeInTheDocument();
+      expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.query()).not.toBeInTheDocument();
     });
 
     it('Should hide mimir alert and mimir recording when user does not have rule external write permission', () => {

diff --git a/.../features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.tsx b/.../features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.tsx
@@ -31,7 +31,7 @@ export const AlertType = ({ editingExistingRule }: Props) => {
 
   return (
     <>
-      {!editingExistingRule && (
+      {!editingExistingRule && ruleFormType !== RuleFormType.cloudRecording && (
         <Field error={errors.type?.message} invalid={!!errors.type?.message} data-testid="alert-type-picker">
           <InputControl
             render={({ field: { onChange } }) => (

diff --git a/...ting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/...ting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx
@@ -186,6 +186,11 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
     clearPreviewData();
     if (type === RuleFormType.cloudRecording) {
       const expr = getValues('expression');
+
+      if (!recordingRuleDefaultDatasource) {
+        return;
+      }
+
       const datasourceUid =
         (editingExistingRule && getDataSourceSrv().getInstanceSettings(dataSourceName)?.uid) ||
         recordingRuleDefaultDatasource.uid;

diff --git a/public/app/features/alerting/unified/components/rule-editor/rule-types/RuleTypePicker.tsx b/public/app/features/alerting/unified/components/rule-editor/rule-types/RuleTypePicker.tsx
@@ -13,8 +13,6 @@ import { RuleFormType } from '../../../types/rule-form';
 
 import { GrafanaManagedRuleType } from './GrafanaManagedAlert';
 import { MimirFlavoredType } from './MimirOrLokiAlert';
-import { RecordingRuleType } from './MimirOrLokiRecordingRule';
-
 interface RuleTypePickerProps {
   onChange: (value: RuleFormType) => void;
   selected: RuleFormType;
@@ -31,23 +29,20 @@ const RuleTypePicker = ({ selected, onChange, enabledTypes }: RuleTypePickerProp
 
   const styles = useStyles2(getStyles);
 
+  const handleChange = (type: RuleFormType) => {
+    onChange(type);
+  };
+
   return (
     <>
       <Stack direction="row" gap={2}>
         {enabledTypes.includes(RuleFormType.grafana) && (
-          <GrafanaManagedRuleType selected={selected === RuleFormType.grafana} onClick={onChange} />
+          <GrafanaManagedRuleType selected={selected === RuleFormType.grafana} onClick={handleChange} />
         )}
         {enabledTypes.includes(RuleFormType.cloudAlerting) && (
           <MimirFlavoredType
             selected={selected === RuleFormType.cloudAlerting}
-            onClick={onChange}
-            disabled={!hasLotexDatasources}
-          />
-        )}
-        {enabledTypes.includes(RuleFormType.cloudRecording) && (
-          <RecordingRuleType
-            selected={selected === RuleFormType.cloudRecording}
-            onClick={onChange}
+            onClick={handleChange}
             disabled={!hasLotexDatasources}
           />
         )}

diff --git a/public/app/features/alerting/unified/components/rule-editor/util.ts b/public/app/features/alerting/unified/components/rule-editor/util.ts
@@ -8,6 +8,8 @@ import { isExpressionQuery } from 'app/features/expressions/guards';
 import { ClassicCondition, ExpressionQueryType } from 'app/features/expressions/types';
 import { AlertQuery } from 'app/types/unified-alerting-dto';
 
+import { RuleFormType } from '../../types/rule-form';
+
 import { createDagFromQueries, getOriginOfRefId } from './dag';
 
 export function queriesWithUpdatedReferences(
@@ -293,3 +295,11 @@ export function getStatusMessage(data: PanelData): string | undefined {
 
   return data.error?.message ?? genericErrorMessage;
 }
+
+export function translateRouteParamToRuleType(param = ''): RuleFormType {
+  if (param === 'recording') {
+    return RuleFormType.cloudRecording;
+  }
+
+  return RuleFormType.grafana;
+}