Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rules): rules can be enabled/disabled #533

Merged
merged 9 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/app/Rules/CreateRule.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
GridItem,
Split,
SplitItem,
Switch,
Text,
TextInput,
TextVariants,
Expand Down Expand Up @@ -81,6 +82,7 @@ const Comp = () => {
const [name, setName] = React.useState('');
const [nameValid, setNameValid] = React.useState(ValidatedOptions.default);
const [description, setDescription] = React.useState('');
const [enabled, setEnabled] = React.useState(true);
const [matchExpression, setMatchExpression] = React.useState('');
const [matchExpressionValid, setMatchExpressionValid] = React.useState(ValidatedOptions.default);
const [templates, setTemplates] = React.useState([] as EventTemplate[]);
Expand Down Expand Up @@ -200,6 +202,7 @@ const Comp = () => {
const rule: Rule = {
name,
description,
enabled,
matchExpression,
eventSpecifier: eventSpecifierString,
archivalPeriodSeconds: archivalPeriod * archivalPeriodUnits,
Expand All @@ -226,6 +229,7 @@ const Comp = () => {
name,
nameValid,
description,
enabled,
matchExpression,
eventSpecifierString,
archivalPeriod,
Expand Down Expand Up @@ -342,6 +346,22 @@ const Comp = () => {
validated={matchExpressionValid}
/>
</FormGroup>
<FormGroup
label="Enabled"
isRequired
fieldId="rule-enabled"
helperText={`Rules take effect when created if enabled and will be matched against all
discovered target applications immediately. When new target applications appear they are
checked against all enabled rules. Disabled rules have no effect but are available to be
enabled in the future.`}
>
<Switch
id="rule-enabled"
aria-label="Apply this rule to matching targets"
isChecked={enabled}
onChange={setEnabled}
/>
</FormGroup>
<FormGroup
label="Template"
isRequired
Expand Down
39 changes: 36 additions & 3 deletions src/app/Rules/Rules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
CardHeaderMain,
EmptyState,
EmptyStateIcon,
Switch,
Text,
TextVariants,
Title,
Expand Down Expand Up @@ -83,6 +84,7 @@ export interface Rule {
name: string;
description: string;
matchExpression: string;
enabled: boolean;
eventSpecifier: string;
archivalPeriodSeconds: number;
initialDelaySeconds: number;
Expand All @@ -106,6 +108,7 @@ export const Rules = () => {
const [cleanRuleEnabled, setCleanRuleEnabled] = React.useState(true);

const tableColumns = [
{ title: 'Enabled' },
{
title: 'Name',
transforms: [sortable],
Expand Down Expand Up @@ -205,6 +208,21 @@ export const Rules = () => {
);
}, [addSubscription, context, context.notificationChannel, setRules]);

React.useEffect(() => {
addSubscription(
context.notificationChannel.messages(NotificationCategory.RuleUpdated).subscribe((msg) => {
setRules((old) => {
for (const r of old) {
if (r.name === msg.message.name) {
r.enabled = msg.message.enabled;
}
}
return [...old];
});
})
);
}, [addSubscription, context, context.notificationChannel, setRules]);

React.useEffect(() => {
if (!context.settings.autoRefreshEnabled()) {
return;
Expand All @@ -231,6 +249,13 @@ export const Rules = () => {
setIsUploadModalOpen(true);
}, [setIsUploadModalOpen]);

const handleToggle = React.useCallback(
(rule: Rule, enabled: boolean): void => {
addSubscription(context.api.updateRule({ ...rule, enabled }).subscribe());
},
[context, context.api, addSubscription]
);

const displayRules = React.useMemo(() => {
const { index, direction } = sortBy;
let sorted = [...rules];
Expand All @@ -241,6 +266,14 @@ export const Rules = () => {
sorted = direction === SortByDirection.asc ? sorted : sorted.reverse();
}
return sorted.map((r: Rule) => [
<>
<Switch
aria-label={`${r.name} is enabled`}
tthvo marked this conversation as resolved.
Show resolved Hide resolved
className={'switch-toggle-' + String(r.enabled)}
isChecked={r.enabled}
onChange={(state) => handleToggle(r, state)}
/>
</>,
r.name,
r.description,
r.matchExpression,
Expand All @@ -251,13 +284,13 @@ export const Rules = () => {
r.maxAgeSeconds,
r.maxSizeBytes,
]);
}, [rules, sortBy]);
}, [rules, sortBy, handleToggle]);

const handleDelete = React.useCallback(
(rowData: IRowData, clean: boolean = true) => {
addSubscription(
context.api
.deleteRule(rowData[0], clean)
.deleteRule(rowData[1], clean)
.pipe(first())
.subscribe(() => {} /* do nothing - notification will handle updating state */)
);
Expand All @@ -267,7 +300,7 @@ export const Rules = () => {

const handleDownload = React.useCallback(
(rowData: IRowData) => {
context.api.downloadRule(rowData[0]);
context.api.downloadRule(rowData[1]);
},
[context, context.api]
);
Expand Down
13 changes: 13 additions & 0 deletions src/app/Shared/Services/Api.service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,19 @@ export class ApiService {
);
}

updateRule(rule: Rule): Observable<boolean> {
const headers = new Headers();
headers.set('Content-Type', 'application/json');
return this.sendRequest('v2', `rules/${rule.name}`, {
method: 'PATCH',
body: JSON.stringify(rule),
headers,
}).pipe(
map((resp) => resp.ok),
first()
);
}

deleteRule(name: string, clean: boolean = true): Observable<boolean> {
return this.sendRequest('v2', `rules/${name}?clean=${clean}`, {
method: 'DELETE',
Expand Down
9 changes: 9 additions & 0 deletions src/app/Shared/Services/NotificationChannel.service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export enum NotificationCategory {
TemplateUploaded = 'TemplateUploaded',
TemplateDeleted = 'TemplateDeleted',
RuleCreated = 'RuleCreated',
RuleUpdated = 'RuleUpdated',
RuleDeleted = 'RuleDeleted',
RecordingMetadataUpdated = 'RecordingMetadataUpdated',
GrafanaConfiguration = 'GrafanaConfiguration', // generated client-side
Expand Down Expand Up @@ -210,6 +211,14 @@ export const messageKeys = new Map([
body: (evt) => `${evt.message.name} was created`,
} as NotificationMessageMapper,
],
[
NotificationCategory.RuleUpdated,
{
variant: AlertVariant.success,
title: 'Automated Rule Updated',
body: (evt) => `${evt.message.name} was ` + (evt.message.enabled ? 'enabled' : 'disabled'),
} as NotificationMessageMapper,
],
[
NotificationCategory.RuleDeleted,
{
Expand Down
1 change: 1 addition & 0 deletions src/test/Rules/CreateRule.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const mockRule: Rule = {
name: 'mockRule',
description: 'A mock rule',
matchExpression: "target.alias == 'io.cryostat.Cryostat' || target.annotations.cryostat['PORT'] == 9091",
enabled: true,
eventSpecifier: 'template=Profiling,type=TARGET',
archivalPeriodSeconds: 0,
initialDelaySeconds: 0,
Expand Down
77 changes: 72 additions & 5 deletions src/test/Rules/Rules.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,26 @@
import * as React from 'react';
import { Router } from 'react-router-dom';
import { createMemoryHistory } from 'history';
import { of } from 'rxjs';
import { of, Subject } from 'rxjs';
import '@testing-library/jest-dom';
import renderer, { act } from 'react-test-renderer';
import { render, cleanup, screen, within, waitFor } from '@testing-library/react';
import { act as doAct, render, cleanup, screen, within, waitFor } from '@testing-library/react';
import * as tlr from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Rules, Rule } from '@app/Rules/Rules';
import { ServiceContext, defaultServices } from '@app/Shared/Services/Services';
import { NotificationMessage } from '@app/Shared/Services/NotificationChannel.service';
import { ServiceContext, defaultServices, Services } from '@app/Shared/Services/Services';
import {
NotificationCategory,
NotificationChannel,
NotificationMessage,
} from '@app/Shared/Services/NotificationChannel.service';
import { DeleteAutomatedRules, DeleteWarningType } from '@app/Modal/DeleteWarningUtils';

const mockRule: Rule = {
name: 'mockRule',
description: 'A mock rule',
matchExpression: "target.alias == 'io.cryostat.Cryostat' || target.annotations.cryostat['PORT'] == 9091",
enabled: true,
eventSpecifier: 'template=Profiling,type=TARGET',
archivalPeriodSeconds: 0,
initialDelaySeconds: 0,
Expand All @@ -68,6 +73,8 @@ mockFileUpload.text = jest.fn(() => new Promise((resolve, _) => resolve(JSON.str

const mockDeleteNotification = { message: { ...mockRule } } as NotificationMessage;

const mockUpdateNotification = { message: { ...mockRule, enabled: false } } as NotificationMessage;

const history = createMemoryHistory({ initialEntries: ['/rules'] });

jest.mock('react-router-dom', () => ({
Expand All @@ -78,9 +85,10 @@ jest.mock('react-router-dom', () => ({

const downloadSpy = jest.spyOn(defaultServices.api, 'downloadRule').mockReturnValue();
const createSpy = jest.spyOn(defaultServices.api, 'createRule').mockReturnValue(of(true));
const updateSpy = jest.spyOn(defaultServices.api, 'updateRule').mockReturnValue(of(true));
jest
.spyOn(defaultServices.api, 'doGet')
.mockReturnValueOnce(of(mockRuleListEmptyResponse)) // renders correctly
.mockReturnValueOnce(of(mockRuleListEmptyResponse)) // renders correctly empty
.mockReturnValue(of(mockRuleListResponse));

jest.spyOn(defaultServices.settings, 'deletionDialogsEnabledFor').mockReturnValueOnce(true);
Expand All @@ -89,21 +97,31 @@ jest
.spyOn(defaultServices.notificationChannel, 'messages')
.mockReturnValueOnce(of()) // renders correctly
.mockReturnValueOnce(of())
.mockReturnValueOnce(of())

.mockReturnValueOnce(of()) // open view to create rules
.mockReturnValueOnce(of())
.mockReturnValueOnce(of())

.mockReturnValueOnce(of()) // opens upload modal
.mockReturnValueOnce(of())
.mockReturnValueOnce(of())

.mockReturnValueOnce(of()) // delete a rule when clicked with popup
.mockReturnValueOnce(of())
.mockReturnValueOnce(of())

.mockReturnValueOnce(of()) // delete a rule when clicked w/o popup
.mockReturnValueOnce(of())
.mockReturnValueOnce(of())

.mockReturnValueOnce(of()) // remove a rule when receiving notification
.mockReturnValueOnce(of(mockDeleteNotification))
.mockReturnValueOnce(of())

.mockReturnValueOnce(of()) // update a rule when receiving notification
.mockReturnValueOnce(of())
.mockReturnValueOnce(of(mockUpdateNotification))

.mockReturnValue(of()); // other tests

Expand Down Expand Up @@ -223,6 +241,40 @@ describe('<Rules/>', () => {
expect(screen.queryByText(mockRule.name)).not.toBeInTheDocument();
});

it('update a rule when receiving a notification', async () => {
const subj = new Subject<NotificationMessage>();
const mockNotifications = {
messages: (category: string) => (category === NotificationCategory.RuleUpdated ? subj.asObservable() : of()),
} as NotificationChannel;
const services: Services = {
...defaultServices,
notificationChannel: mockNotifications,
};
const { container } = render(
<ServiceContext.Provider value={services}>
<Router location={history.location} history={history}>
<Rules />
</Router>
</ServiceContext.Provider>
);

expect(await screen.findByText(mockRule.name)).toBeInTheDocument();

let labels = container.querySelectorAll('label');
expect(labels).toHaveLength(1);
let label = labels[0];
expect(label).toHaveClass('switch-toggle-true');
expect(label).not.toHaveClass('switch-toggle-false');

doAct(() => subj.next(mockUpdateNotification));

labels = container.querySelectorAll('label');
expect(labels).toHaveLength(1);
label = labels[0];
expect(label).not.toHaveClass('switch-toggle-true');
expect(label).toHaveClass('switch-toggle-false');
});

it('downloads a rule when Download is clicked', async () => {
render(
<ServiceContext.Provider value={defaultServices}>
Expand All @@ -239,6 +291,21 @@ describe('<Rules/>', () => {
expect(downloadSpy).toBeCalledWith(mockRule.name);
});

it('updates a rule when the switch is clicked', async () => {
render(
<ServiceContext.Provider value={defaultServices}>
<Router location={history.location} history={history}>
<Rules />
</Router>
</ServiceContext.Provider>
);

userEvent.click(screen.getByRole('checkbox'));

expect(updateSpy).toHaveBeenCalledTimes(1);
expect(updateSpy).toBeCalledWith({ ...mockRule, enabled: !mockRule.enabled });
});

it('upload a rule file when Submit is clicked', async () => {
render(
<ServiceContext.Provider value={defaultServices}>
Expand Down
Loading