Alerting: Reorder new alert and export buttons (#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
This commit is contained in:
Virginia Cepeda 2023-05-31 10:56:54 -03:00 committed by GitHub
parent c2a9d48dd6
commit e796063a1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 135 additions and 64 deletions

View File

@ -225,7 +225,7 @@ const unifiedRoutes: RouteDescriptor[] = [
), ),
}, },
{ {
path: '/alerting/new', path: '/alerting/new/:type?',
pageClass: 'page-alerting', pageClass: 'page-alerting',
roles: evaluateAccess( roles: evaluateAccess(
[AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite], [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite],

View File

@ -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>
</>
);
}

View File

@ -127,10 +127,9 @@ describe('RuleEditor recording rules', () => {
}, },
}); });
renderRuleEditor(); renderRuleEditor(undefined, true);
await waitForElementToBeRemoved(screen.getAllByTestId('Spinner')); await waitForElementToBeRemoved(screen.getAllByTestId('Spinner'));
await userEvent.type(await ui.inputs.name.find(), 'my great new recording rule'); 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(); const dataSourceSelect = ui.inputs.dataSource.get();
await userEvent.click(byRole('combobox').get(dataSourceSelect)); await userEvent.click(byRole('combobox').get(dataSourceSelect));

View File

@ -117,7 +117,8 @@ const ui = {
rulesFilterInput: byTestId('search-query-input'), rulesFilterInput: byTestId('search-query-input'),
moreErrorsButton: byRole('button', { name: /more errors/ }), moreErrorsButton: byRole('button', { name: /more errors/ }),
editCloudGroupIcon: byTestId('edit-group'), 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 }), exportButton: byRole('link', { name: /export/i }),
editGroupModal: { editGroupModal: {
dialog: byRole('dialog'), dialog: byRole('dialog'),
@ -697,11 +698,14 @@ describe('RuleList', () => {
renderRuleList(); renderRuleList();
await userEvent.click(ui.moreButton.get());
expect(ui.exportButton.get()).toBeInTheDocument(); expect(ui.exportButton.get()).toBeInTheDocument();
}); });
it('Export button should not be visible when the user has no alert provisioning read permissions', async () => { it('Export button should not be visible when the user has no alert provisioning read permissions', async () => {
enableRBAC(); enableRBAC();
grantUserPermissions([AccessControlAction.AlertingRuleCreate, AccessControlAction.FoldersRead]);
mocks.getAllDataSourcesMock.mockReturnValue([]); mocks.getAllDataSourcesMock.mockReturnValue([]);
setDataSourceSrv(new MockDataSourceSrv({})); setDataSourceSrv(new MockDataSourceSrv({}));
mocks.api.fetchRules.mockResolvedValue([]); mocks.api.fetchRules.mockResolvedValue([]);
@ -709,6 +713,7 @@ describe('RuleList', () => {
renderRuleList(); renderRuleList();
await userEvent.click(ui.moreButton.get());
expect(ui.exportButton.query()).not.toBeInTheDocument(); expect(ui.exportButton.query()).not.toBeInTheDocument();
}); });
}); });
@ -831,7 +836,7 @@ describe('RuleList', () => {
await waitFor(() => expect(mocks.api.fetchRules).toHaveBeenCalledTimes(1)); 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); button.addEventListener('click', (event) => event.preventDefault(), false);

View File

@ -1,18 +1,17 @@
import { css } from '@emotion/css'; import { css } from '@emotion/css';
import React, { useCallback, useEffect, useMemo, useState } from 'react'; import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { useLocation } from 'react-router-dom';
import { useAsyncFn, useInterval } from 'react-use'; import { useAsyncFn, useInterval } from 'react-use';
import { GrafanaTheme2, urlUtil } from '@grafana/data'; import { GrafanaTheme2 } from '@grafana/data';
import { Stack } from '@grafana/experimental'; import { Stack } from '@grafana/experimental';
import { logInfo } from '@grafana/runtime'; import { Button, useStyles2, withErrorBoundary } from '@grafana/ui';
import { Button, LinkButton, useStyles2, withErrorBoundary } from '@grafana/ui';
import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { useQueryParams } from 'app/core/hooks/useQueryParams';
import { useDispatch } from 'app/types'; import { useDispatch } from 'app/types';
import { CombinedRuleNamespace } from '../../../types/unified-alerting'; 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 { AlertingPageWrapper } from './components/AlertingPageWrapper';
import { NoRulesSplash } from './components/rules/NoRulesCTA'; import { NoRulesSplash } from './components/rules/NoRulesCTA';
import { INSTANCES_DISPLAY_LIMIT } from './components/rules/RuleDetails'; import { INSTANCES_DISPLAY_LIMIT } from './components/rules/RuleDetails';
@ -28,7 +27,6 @@ import { fetchAllPromAndRulerRulesAction } from './state/actions';
import { useRulesAccess } from './utils/accessControlHooks'; import { useRulesAccess } from './utils/accessControlHooks';
import { RULE_LIST_POLL_INTERVAL_MS } from './utils/constants'; import { RULE_LIST_POLL_INTERVAL_MS } from './utils/constants';
import { getAllRulesSourceNames } from './utils/datasource'; import { getAllRulesSourceNames } from './utils/datasource';
import { createUrl } from './utils/url';
const VIEWS = { const VIEWS = {
groups: RuleListGroupView, groups: RuleListGroupView,
@ -43,7 +41,6 @@ const RuleList = withErrorBoundary(
const dispatch = useDispatch(); const dispatch = useDispatch();
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const rulesDataSourceNames = useMemo(getAllRulesSourceNames, []); const rulesDataSourceNames = useMemo(getAllRulesSourceNames, []);
const location = useLocation();
const [expandAll, setExpandAll] = useState(false); const [expandAll, setExpandAll] = useState(false);
const onFilterCleared = useCallback(() => setExpandAll(false), []); const onFilterCleared = useCallback(() => setExpandAll(false), []);
@ -51,8 +48,6 @@ const RuleList = withErrorBoundary(
const [queryParams] = useQueryParams(); const [queryParams] = useQueryParams();
const { filterState, hasActiveFilters } = useRulesFilter(); const { filterState, hasActiveFilters } = useRulesFilter();
const { canCreateGrafanaRules, canCreateCloudRules, canReadProvisioning } = useRulesAccess();
const view = VIEWS[queryParams['view'] as keyof typeof VIEWS] const view = VIEWS[queryParams['view'] as keyof typeof VIEWS]
? (queryParams['view'] as keyof typeof VIEWS) ? (queryParams['view'] as keyof typeof VIEWS)
: 'groups'; : 'groups';
@ -96,6 +91,8 @@ const RuleList = withErrorBoundary(
const combinedNamespaces: CombinedRuleNamespace[] = useCombinedRuleNamespaces(); const combinedNamespaces: CombinedRuleNamespace[] = useCombinedRuleNamespaces();
const filteredNamespaces = useFilteredRules(combinedNamespaces, filterState); const filteredNamespaces = useFilteredRules(combinedNamespaces, filterState);
const { canCreateGrafanaRules, canCreateCloudRules, canReadProvisioning } = useRulesAccess();
return ( return (
// We don't want to show the Loading... indicator for the whole page. // We don't want to show the Loading... indicator for the whole page.
// We show separate indicators for Grafana-managed and Cloud rules // We show separate indicators for Grafana-managed and Cloud rules
@ -119,31 +116,11 @@ const RuleList = withErrorBoundary(
)} )}
<RuleStats namespaces={filteredNamespaces} /> <RuleStats namespaces={filteredNamespaces} />
</div> </div>
<Stack direction="row" gap={0.5}> {(canCreateGrafanaRules || canCreateCloudRules || canReadProvisioning) && (
{canReadProvisioning && ( <Stack direction="row" gap={0.5}>
<LinkButton <MoreActionsRuleButtons />
variant="secondary" </Stack>
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>
</div> </div>
</> </>
)} )}

View File

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

View File

@ -2,8 +2,10 @@ import { render } from '@testing-library/react';
import React from 'react'; import React from 'react';
import { FormProvider, useForm } from 'react-hook-form'; import { FormProvider, useForm } from 'react-hook-form';
import { Provider } from 'react-redux'; import { Provider } from 'react-redux';
import { Router } from 'react-router-dom';
import { byText } from 'testing-library-selector'; import { byText } from 'testing-library-selector';
import { locationService } from '@grafana/runtime';
import { contextSrv } from 'app/core/services/context_srv'; import { contextSrv } from 'app/core/services/context_srv';
import { configureStore } from 'app/store/configureStore'; import { configureStore } from 'app/store/configureStore';
import { AccessControlAction } from 'app/types'; import { AccessControlAction } from 'app/types';
@ -28,7 +30,9 @@ function renderAlertTypeStep() {
render( render(
<Provider store={store}> <Provider store={store}>
<AlertType editingExistingRule={false} /> <Router history={locationService.getHistory()}>
<AlertType editingExistingRule={false} />
</Router>
</Provider>, </Provider>,
{ wrapper: FormProviderWrapper } { wrapper: FormProviderWrapper }
); );
@ -36,7 +40,7 @@ function renderAlertTypeStep() {
describe('RuleTypePicker', () => { describe('RuleTypePicker', () => {
describe('RBAC', () => { 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) => { jest.spyOn(contextSrv, 'hasPermission').mockImplementation((action) => {
return [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite].includes( return [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite].includes(
action as AccessControlAction action as AccessControlAction
@ -47,7 +51,17 @@ describe('RuleTypePicker', () => {
expect(ui.ruleTypePicker.grafanaManagedButton.get()).toBeInTheDocument(); expect(ui.ruleTypePicker.grafanaManagedButton.get()).toBeInTheDocument();
expect(ui.ruleTypePicker.mimirOrLokiButton.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', () => { 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.grafanaManagedButton.query()).not.toBeInTheDocument();
expect(ui.ruleTypePicker.mimirOrLokiButton.get()).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', () => { it('Should hide mimir alert and mimir recording when user does not have rule external write permission', () => {

View File

@ -31,7 +31,7 @@ export const AlertType = ({ editingExistingRule }: Props) => {
return ( return (
<> <>
{!editingExistingRule && ( {!editingExistingRule && ruleFormType !== RuleFormType.cloudRecording && (
<Field error={errors.type?.message} invalid={!!errors.type?.message} data-testid="alert-type-picker"> <Field error={errors.type?.message} invalid={!!errors.type?.message} data-testid="alert-type-picker">
<InputControl <InputControl
render={({ field: { onChange } }) => ( render={({ field: { onChange } }) => (

View File

@ -186,6 +186,11 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
clearPreviewData(); clearPreviewData();
if (type === RuleFormType.cloudRecording) { if (type === RuleFormType.cloudRecording) {
const expr = getValues('expression'); const expr = getValues('expression');
if (!recordingRuleDefaultDatasource) {
return;
}
const datasourceUid = const datasourceUid =
(editingExistingRule && getDataSourceSrv().getInstanceSettings(dataSourceName)?.uid) || (editingExistingRule && getDataSourceSrv().getInstanceSettings(dataSourceName)?.uid) ||
recordingRuleDefaultDatasource.uid; recordingRuleDefaultDatasource.uid;

View File

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

View File

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

View File

@ -15,8 +15,8 @@ export const NoRulesSplash = () => {
<EmptyListCTA <EmptyListCTA
title="You haven`t created any alert rules yet" title="You haven`t created any alert rules yet"
buttonIcon="bell" buttonIcon="bell"
buttonLink={'alerting/new'} buttonLink={'alerting/new/alerting'}
buttonTitle="Create alert rule" buttonTitle="New alert rule"
proTip="you can also create alert rules from existing panels and queries." proTip="you can also create alert rules from existing panels and queries."
proTipLink="https://grafana.com/docs/" proTipLink="https://grafana.com/docs/"
proTipLinkTitle="Learn more" proTipLinkTitle="Learn more"

View File

@ -31,16 +31,19 @@ export const ui = {
// alert type buttons // alert type buttons
grafanaManagedAlert: byRole('button', { name: /Grafana managed/ }), grafanaManagedAlert: byRole('button', { name: /Grafana managed/ }),
lotexAlert: byRole('button', { name: /Mimir or Loki alert/ }), lotexAlert: byRole('button', { name: /Mimir or Loki alert/ }),
lotexRecordingRule: byRole('button', { name: /Mimir or Loki recording rule/ }),
}, },
}; };
export function renderRuleEditor(identifier?: string) { export function renderRuleEditor(identifier?: string, recording = false) {
locationService.push(identifier ? `/alerting/${identifier}/edit` : `/alerting/new`); if (identifier) {
locationService.push(`/alerting/${identifier}/edit`);
} else {
locationService.push(`/alerting/new/${recording ? 'recording' : 'alerting'}`);
}
return render( return render(
<TestProvider> <TestProvider>
<Route path={['/alerting/new', '/alerting/:id/edit']} component={RuleEditor} /> <Route path={['/alerting/new/:type', '/alerting/:id/edit']} component={RuleEditor} />
</TestProvider> </TestProvider>
); );
} }