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

upcoming: [DI-22217] - Added the Alert Listing page with Alerting Table and added relevant api endpoints #11346

Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/api-v4": Changed
---

Type of `AlertDefinitionType` to `'system'|'user'` ([#11346](https://github.com/linode/manager/pull/11346))
17 changes: 16 additions & 1 deletion packages/api-v4/src/cloudpulse/alerts.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import { createAlertDefinitionSchema } from '@linode/validation';
import Request, { setURL, setMethod, setData } from '../request';
import Request, {
setURL,
setMethod,
setData,
setParams,
setXFilter,
} from '../request';
import { Alert, AlertServiceType, CreateAlertDefinitionPayload } from './types';
import { BETA_API_ROOT as API_ROOT } from 'src/constants';
import { Params, Filter, ResourcePage } from 'src/types';
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved

export const createAlertDefinition = (
data: CreateAlertDefinitionPayload,
Expand All @@ -16,3 +23,11 @@ export const createAlertDefinition = (
setMethod('POST'),
setData(data, createAlertDefinitionSchema)
);

export const getAlertDefinitions = (params?: Params, filters?: Filter) =>
Request<ResourcePage<Alert>>(
setURL(`${API_ROOT}/monitor/alert-definitions`),
setMethod('GET'),
setParams(params),
setXFilter(filters)
);
2 changes: 1 addition & 1 deletion packages/api-v4/src/cloudpulse/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export type MetricAggregationType = 'avg' | 'sum' | 'min' | 'max' | 'count';
export type MetricOperatorType = 'eq' | 'gt' | 'lt' | 'gte' | 'lte';
export type AlertServiceType = 'linode' | 'dbaas';
type DimensionFilterOperatorType = 'eq' | 'neq' | 'startswith' | 'endswith';
export type AlertDefinitionType = 'default' | 'custom';
export type AlertDefinitionType = 'system' | 'user';
export type AlertStatusType = 'enabled' | 'disabled';
export interface Dashboard {
id: number;
Expand Down
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11346-added-1733145106911.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Added
---

AlertListing component and AlertTableRow component with UT ([#11346](https://github.com/linode/manager/pull/11346))
santoshp210-akamai marked this conversation as resolved.
Show resolved Hide resolved
23 changes: 23 additions & 0 deletions packages/manager/src/assets/icons/entityIcons/alert.svg
Copy link
Contributor Author

@santoshp210-akamai santoshp210-akamai Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file will be replaced before the PR is merged as the icon is not looking good on the UI. There are some confirmations from the UX side.

We will omit the file and then add the icon when we receive it from the UX team.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion packages/manager/src/factories/cloudpulse/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const alertFactory = Factory.Sync.makeFactory<Alert>({
polling_interval_seconds: 0,
trigger_occurrences: 0,
},
type: 'default',
type: 'user',
updated: new Date().toISOString(),
updated_by: 'user1',
});
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { Paper, Typography } from '@linode/ui';
import * as React from 'react';
import { Route, Switch } from 'react-router-dom';

import { AlertListing } from '../AlertsListing/AlertListing';
import { CreateAlertDefinition } from '../CreateAlert/CreateAlertDefinition';

export const AlertDefinitionLanding = () => {
return (
<Switch>
<Route
component={AlertDefinition}
component={() => <AlertListing />}
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
exact
path="/monitor/alerts/definitions"
/>
Expand All @@ -19,11 +19,3 @@ export const AlertDefinitionLanding = () => {
</Switch>
);
};

const AlertDefinition = () => {
return (
<Paper>
<Typography variant="body1">Alert Definition</Typography>
</Paper>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import * as React from 'react';

import { ActionMenu } from 'src/components/ActionMenu/ActionMenu';

import type { AlertDefinitionType } from '@linode/api-v4';

export interface ActionHandlers {
// These handlers will be enhanced based on the alert type and actions required
/*
* Callback for delete action
*/
handleDelete: () => void;

/*
* Callback for show details action
*/
handleDetails: () => void;
}

export interface AlertActionMenuProps {
/*
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
* Type of the alert
*/
alertType?: AlertDefinitionType;
/*
* Handlers for alert actions like delete, show details etc.,
*/
handlers?: ActionHandlers;
}

/*
The handlers and alertType are made optional only temporarily, they will be enabled but they are dependent on another feature which will be part of next PR
*/
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
export const AlertActionMenu = () => {
return <ActionMenu actionsList={[]} ariaLabel={'Action menu for Alert'} />;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import * as React from 'react';

import { alertFactory } from 'src/factories/cloudpulse/alerts';
import { renderWithTheme } from 'src/utilities/testHelpers';

import { AlertListing } from './AlertListing';

const queryMocks = vi.hoisted(() => ({
useAllAlertDefinitionsQuery: vi.fn().mockReturnValue({}),
}));

vi.mock('src/queries/cloudpulse/alerts', async () => {
const actual = await vi.importActual('src/queries/cloudpulse/alerts');
return {
...actual,
useAllAlertDefinitionsQuery: queryMocks.useAllAlertDefinitionsQuery,
};
});

const mockResponse = alertFactory.buildList(3);

describe('Alert Listing', () => {
it('should render the error message', async () => {
queryMocks.useAllAlertDefinitionsQuery.mockReturnValue({
data: undefined,
error: 'an error happened',
isError: true,
isLoading: false,
});
const { getAllByText } = renderWithTheme(<AlertListing />);
getAllByText('Error in fetching the alerts.');
});

it('should render the alert landing table with items', async () => {
queryMocks.useAllAlertDefinitionsQuery.mockReturnValue({
data: mockResponse,
isError: false,
isLoading: false,
status: 'success',
});
const { getAllByText } = renderWithTheme(<AlertListing />);
getAllByText('Alert Name');
getAllByText('Service Type');
getAllByText('Severity');
getAllByText('Status');
getAllByText('Last Modified');
getAllByText('Created By');
});
santoshp210-akamai marked this conversation as resolved.
Show resolved Hide resolved
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { Paper } from '@linode/ui';
import { Grid } from '@mui/material';
import React from 'react';

import AlertIcon from 'src/assets/icons/entityIcons/alert.svg';
import { Table } from 'src/components/Table';
import { TableBody } from 'src/components/TableBody';
import { TableCell } from 'src/components/TableCell';
import { TableHead } from 'src/components/TableHead';
import { TableRow } from 'src/components/TableRow';
import { TableRowError } from 'src/components/TableRowError/TableRowError';
import { TableRowLoading } from 'src/components/TableRowLoading/TableRowLoading';
import { TableSortCell } from 'src/components/TableSortCell';
import { StyledPlaceholder } from 'src/features/StackScripts/StackScriptBase/StackScriptBase.styles';
import { useAllAlertDefinitionsQuery } from 'src/queries/cloudpulse/alerts';

import { AlertTableRow } from './AlertTableRow';

export const AlertListing = () => {
// These are dummy order and handlers, will replace them in the next PR
const order = 'asc';
const handleOrderChange = () => {
return 'asc';
};
const { data: alerts, isError, isLoading } = useAllAlertDefinitionsQuery();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to API paginate or client side paginate? We generally like to API paginate whenever possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going with client side paginate as decided during the API specification discussion. As per the mockups: https://www.figma.com/design/26kXnWVNU5tK2xQdDdISuC/Custom-Alerts?node-id=182-31355&node-type=canvas&t=RpnzXG6qoW63ls4N-0.

We have a searching, filtering functionality to be added for these alerts and we are not doing that from API side, so pagination also is being done client side.

if (alerts?.length === 0) {
return (
<Grid item xs={12}>
<Paper>
<StyledPlaceholder
icon={AlertIcon}
subtitle="Start Monitoring your resources."
title=""
/>
</Paper>
</Grid>
);
}
return (
<Grid marginTop={2}>
<Table colCount={7} size="small">
<TableHead>
<TableRow>
<TableSortCell
active={true}
direction={order}
handleClick={handleOrderChange}
label="alertName"
>
Alert Name
</TableSortCell>
<TableSortCell
handleClick={() => {
'asc';
}}
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
active={true}
direction={order}
label="serviceType"
size="small"
>
Service Type
</TableSortCell>
<TableSortCell
active={true}
direction={order}
handleClick={handleOrderChange}
label="severity"
>
Severity
</TableSortCell>
<TableSortCell
active={true}
direction={order}
handleClick={handleOrderChange}
label="status"
>
Status
</TableSortCell>
<TableSortCell
active={true}
direction={order}
handleClick={handleOrderChange}
label="lastModified"
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
>
Last Modified
</TableSortCell>
<TableSortCell
active={true}
direction={order}
handleClick={handleOrderChange}
label="createdBy"
size="small"
>
Created By
</TableSortCell>
santoshp210-akamai marked this conversation as resolved.
Show resolved Hide resolved
<TableCell actionCell />
</TableRow>
</TableHead>
<TableBody>
{isError === true && (
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
<TableRowError
colSpan={7}
message={'Error in fetching the alerts.'}
/>
)}
{isLoading === true && <TableRowLoading columns={7} />}
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
{alerts?.map((alert) => (
<AlertTableRow alert={alert} key={alert.id} />
))}
</TableBody>
</Table>
</Grid>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import * as React from 'react';

import { alertFactory } from 'src/factories/cloudpulse/alerts';
import { capitalize } from 'src/utilities/capitalize';
import { renderWithTheme, wrapWithTableBody } from 'src/utilities/testHelpers';

import { alertSeverityOptions } from '../constants';
import { AlertTableRow } from './AlertTableRow';

describe('Alert Row', () => {
it('should render an alert row', async () => {
const alert = alertFactory.build();
const renderedAlert = <AlertTableRow alert={alert} />;
const { getByText } = renderWithTheme(wrapWithTableBody(renderedAlert));
expect(getByText(alert.label)).toBeVisible();
});

it('should render the severity field with its label not value', async () => {
const severityValue = 0;
const alert = alertFactory.build({ severity: severityValue });
const renderedAlert = <AlertTableRow alert={alert} />;
const { getByText } = renderWithTheme(wrapWithTableBody(renderedAlert));
const severity = alertSeverityOptions.find(
(option) => option.value === severityValue
)?.label;
expect(getByText(severity!)).toBeVisible;
});

it('should render the status field in green color if status is enabled', () => {
const statusValue = 'enabled';
const alert = alertFactory.build({ status: statusValue });
const renderedAlert = <AlertTableRow alert={alert} />;
const { getByText } = renderWithTheme(wrapWithTableBody(renderedAlert));
const statusElement = getByText(capitalize(statusValue));
expect(getComputedStyle(statusElement).color).toBe('rgb(50, 205, 50)');
});
});
Loading