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 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
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))
19 changes: 17 additions & 2 deletions 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 { BETA_API_ROOT as API_ROOT } from '../constants';
import { Params, Filter, ResourcePage } from '../types';

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 Unit Tests ([#11346](https://github.com/linode/manager/pull/11346))
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,29 +1,21 @@
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}
exact
path="/monitor/alerts/definitions"
/>
<Route
component={() => <CreateAlertDefinition />}
component={CreateAlertDefinition}
path="/monitor/alerts/definitions/create"
/>
</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 {
/**
* 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
*/
export const AlertActionMenu = () => {
return <ActionMenu actionsList={[]} ariaLabel={'Action menu for Alert'} />;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
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 { getByText } = renderWithTheme(<AlertListing />);
expect(getByText('Alert Name')).toBeInTheDocument();
expect(getByText('Service')).toBeInTheDocument();
expect(getByText('Status')).toBeInTheDocument();
expect(getByText('Last Modified')).toBeInTheDocument();
expect(getByText('Created By')).toBeInTheDocument();
});

it('should render the alert row', async () => {
queryMocks.useAllAlertDefinitionsQuery.mockReturnValue({
data: mockResponse,
isError: false,
isLoading: false,
status: 'success',
});

const { getByText } = renderWithTheme(<AlertListing />);
expect(getByText(mockResponse[0].label)).toBeInTheDocument();
expect(getByText(mockResponse[1].label)).toBeInTheDocument();
expect(getByText(mockResponse[2].label)).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { Paper } from '@linode/ui';
import { Grid } from '@mui/material';
import React from 'react';

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';
import { AlertListingTableLabelMap } from './constants';

export const AlertListing = () => {
// These are dummy order value and handleOrder methods, 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
subtitle="Start Monitoring your resources."
title=""
/>
</Paper>
</Grid>
);
}
return (
<Grid marginTop={2}>
<Table colCount={7} size="small">
<TableHead>
<TableRow>
{AlertListingTableLabelMap.map((value) => (
<TableSortCell
active={true}
direction={order}
handleClick={handleOrderChange}
key={value.label}
label={value.label}
>
{value.colName}
</TableSortCell>
))}
<TableCell actionCell />
</TableRow>
</TableHead>
<TableBody>
{isError && (
<TableRowError
colSpan={7}
message={'Error in fetching the alerts.'}
/>
)}
{isLoading && <TableRowLoading columns={7} />}
{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,29 @@
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 { 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();
});

/**
* As of now the styling for the status 'enabled' is decided, in the future if they decide on the
other styles possible status values, will update them and test them accordingly.
*/
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(0, 176, 80)');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { Typography } from '@linode/ui';
import { useTheme } from '@mui/material';
import * as React from 'react';

import { DateTimeDisplay } from 'src/components/DateTimeDisplay';
import { TableCell } from 'src/components/TableCell';
import { TableRow } from 'src/components/TableRow';
import { capitalize } from 'src/utilities/capitalize';

import { AlertActionMenu } from './AlertActionMenu';

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

interface Props {
/**
* alert details used by the component to fill the row details
*/
alert: Alert;
}

export const AlertTableRow = (props: Props) => {
const { alert } = props;
const { created_by, id, label, service_type, status, updated } = alert;
const theme = useTheme();
return (
<TableRow data-qa-alert-cell={id} key={`alert-row-${id}`}>
<TableCell>{label}</TableCell>
<TableCell>{service_type}</TableCell>
<TableCell>
<Typography
color={
status === 'enabled'
? theme.tokens.color.Green[70]
: theme.tokens.color.Neutrals[60]
}
>
{capitalize(status)}
</Typography>
</TableCell>
Comment on lines +29 to +39
Copy link
Member

Choose a reason for hiding this comment

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

This is fine but I'm wondering why UX didn't just use our Status Icon

<TableCell>
<DateTimeDisplay value={updated} />
</TableCell>
<TableCell>{created_by}</TableCell>
<TableCell actionCell>
{/* handlers are supposed to be passed to this AlertActionMenu,
it is dependent on other feature and will added as that feature in the next PR
*/}
<AlertActionMenu />
</TableCell>
</TableRow>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export const AlertListingTableLabelMap = [
{
colName: 'Alert Name',
label: 'alertName',
},
{
colName: 'Service',
label: 'service',
},
{
colName: 'Status',
label: 'status',
},
{
colName: 'Last Modified',
label: 'lastModified',
},
{
colName: 'Created By',
label: 'createdBy',
},
];
Loading
Loading