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-23081] - Add filtering, pagination and sorting for resources section in CloudPulse alerts show details page #11541

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0b59668
upcoming: [DI-22132] - Initial changes for adding resources section
venkat199501 Jan 14, 2025
3bce3fe
Merge branch 'develop' of https://github.com/linode/manager into feat…
venkat199501 Jan 14, 2025
4d47fbc
upcoming: [DI-22838] - Added unit tests
venkat199501 Jan 15, 2025
3eabcdb
Merge branch 'develop' of https://github.com/linode/manager into feat…
venkat199501 Jan 15, 2025
b5c78bc
upcoming: [DI-22838] - Error message corrections
venkat199501 Jan 15, 2025
7049000
upcoming: [DI-22838] - Add a skeletal table
venkat199501 Jan 15, 2025
be52c87
upcoming: [DI-22838] - Update comments
venkat199501 Jan 15, 2025
21f1e16
upcoming: [DI-22838] - Add UT for utils
venkat199501 Jan 15, 2025
8111a13
upcoming: [DI-22838] - Sorting, filtering and pagination support
venkat199501 Jan 15, 2025
75e8954
upcoming: [DI-22838] - Add UT for utils functions
venkat199501 Jan 15, 2025
d925670
Merge branch 'develop' of github.com:linode/manager into feature/aler…
venkat199501 Jan 21, 2025
c2b65b2
upcoming: [DI-22838] - Lint formatting fixes
venkat199501 Jan 21, 2025
efd970b
Merge branch 'develop' of github.com:linode/manager into feature/aler…
venkat199501 Jan 21, 2025
ca39c10
upcoming: [DI-22838] - Added changeset
venkat199501 Jan 21, 2025
6adc58f
upcoming: [DI-22838] - UT fixes
venkat199501 Jan 21, 2025
33e0504
upcoming: [DI-23081] - Code review and changes
venkat199501 Jan 22, 2025
ecbbbdd
upcoming: [DI-23081] - Code refactoring and UT updates
venkat199501 Jan 22, 2025
024454b
upcoming: [DI-23081] - Code refactoring and UT updates
venkat199501 Jan 22, 2025
cec726f
upcoming: [DI-23081] - Code refactoring
venkat199501 Jan 22, 2025
be92174
upcoming: [DI-23081] - Code refactoring
venkat199501 Jan 22, 2025
82d2e74
upcoming: [DI-23081] - Added comments
venkat199501 Jan 22, 2025
8aa922c
upcoming: [DI-23081] - Code optimisations
venkat199501 Jan 22, 2025
85e3d7a
upcoming: [DI-23081] - Code optimisations
venkat199501 Jan 22, 2025
476eb9b
upcoming: [DI-23081] - keys update
venkat199501 Jan 23, 2025
53c017b
upcoming: [DI-23081] -Scroll fix
venkat199501 Jan 23, 2025
67548f9
upcoming: [DI-23081] -Scroll fix
venkat199501 Jan 23, 2025
89a4fa3
upcoming: [DI-23081] - Remove unused variable
venkat199501 Jan 23, 2025
0f9c78f
upcoming: [DI-23081] - Label validations and code updates
venkat199501 Jan 23, 2025
c03c2e8
upcoming: [DI-23081] - Reusable function
venkat199501 Jan 23, 2025
45f0db2
upcoming: [DI-23081] - Code comments
venkat199501 Jan 23, 2025
c7c6ba7
upcoming: [DI-23081] - Test error fixes
venkat199501 Jan 23, 2025
cdc5288
upcoming: [DI-23081] - Mock for scroll
venkat199501 Jan 23, 2025
a2a4bd2
upcoming: [DI-23081] - Code refactoring
venkat199501 Jan 23, 2025
9bcff06
upcoming: [DI-23081] - Code refactoring
venkat199501 Jan 23, 2025
8324a6b
upcoming: [DI-23081] - Code refactoring
venkat199501 Jan 23, 2025
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/manager": Upcoming Features
---

Add filtering, pagination and sorting for resources section in CloudPulse alerts show details page ([#11541](https://github.com/linode/manager/pull/11541))
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import * as React from 'react';

import { linodeFactory, regionFactory } from 'src/factories';
Expand All @@ -22,11 +24,21 @@ const queryMocks = vi.hoisted(() => ({

const regions = regionFactory.buildList(3);

const linodes = linodeFactory.buildList(3);
const linodes = linodeFactory.buildList(3).map((value, index) => {
return {
...value,
region: regions[index].id, // lets assign the regions from region factory to linode instances here
};
});

const searchPlaceholder = 'Search for a Region or Resource';
const regionPlaceholder = 'Select Regions';

beforeAll(() => {
window.scrollTo = vi.fn(); // mock for scrollTo and scroll
window.scroll = vi.fn();
});

beforeEach(() => {
queryMocks.useResourcesQuery.mockReturnValue({
data: linodes,
Expand Down Expand Up @@ -75,4 +87,90 @@ describe('AlertResources component tests', () => {
getByText('Table data is unavailable. Please try again later.')
).toBeInTheDocument();
});
it('should handle search, region filter functionality', async () => {
const {
getByPlaceholderText,
getByRole,
getByTestId,
getByText,
queryByText,
} = renderWithTheme(
<AlertResources alertResourceIds={['1', '2', '3']} serviceType="linode" />
);
// Get the search input box
const searchInput = getByPlaceholderText(searchPlaceholder);
await userEvent.type(searchInput, linodes[1].label);
// Wait for search results to update
await waitFor(() => {
expect(queryByText(linodes[0].label)).not.toBeInTheDocument();
expect(getByText(linodes[1].label)).toBeInTheDocument();
});
// clear the search input
Copy link
Contributor

Choose a reason for hiding this comment

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

very small nitpick, but also a personal preference: Since I’m seeing some comments start with a capital letter and others with lowercase, could we make sure to keep the first letter of every comment capitalized throughout for consistency?

await userEvent.clear(searchInput);
await waitFor(() => {
expect(getByText(linodes[0].label)).toBeInTheDocument();
expect(getByText(linodes[1].label)).toBeInTheDocument();
});
// search with invalid text and a region
await userEvent.type(searchInput, 'dummy');
await userEvent.click(getByRole('button', { name: 'Open' }));
await userEvent.click(getByTestId(regions[0].id));
await userEvent.click(getByRole('button', { name: 'Close' }));
await waitFor(() => {
expect(queryByText(linodes[0].label)).not.toBeInTheDocument();
expect(queryByText(linodes[1].label)).not.toBeInTheDocument();
});
// now clear the search input and the region filter will be applied
await userEvent.clear(searchInput);
await waitFor(() => {
expect(getByText(linodes[0].label)).toBeInTheDocument();
expect(queryByText(linodes[1].label)).not.toBeInTheDocument();
});
});

it('should handle sorting correctly', async () => {
const { getByTestId } = renderWithTheme(
<AlertResources alertResourceIds={['1', '2', '3']} serviceType="linode" />
);
const resourceColumn = getByTestId('resource'); // get the resource header column
await userEvent.click(resourceColumn);

const tableBody = getByTestId('alert_resources_content');
let rows = Array.from(tableBody.querySelectorAll('tr'));
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) => {
return text?.includes(linodes[linodes.length - 1 - index].label);
})
).toBe(true);

await userEvent.click(resourceColumn); // again reverse the sorting
rows = Array.from(tableBody.querySelectorAll('tr'));
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) => text?.includes(linodes[index].label))
).toBe(true);

const regionColumn = getByTestId('region'); // get the region header column

await userEvent.click(regionColumn); // sort ascending for region
rows = Array.from(tableBody.querySelectorAll('tr')); // refetch
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) =>
text?.includes(linodes[linodes.length - 1 - index].region)
)
).toBe(true);

await userEvent.click(regionColumn); // reverse the sorting
rows = Array.from(tableBody.querySelectorAll('tr'));
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) => text?.includes(linodes[index].region)) // validation
).toBe(true);
});
Comment on lines +131 to +175
Copy link
Contributor

@pmakode-akamai pmakode-akamai Jan 24, 2025

Choose a reason for hiding this comment

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

Suggested change
it('should handle sorting correctly', async () => {
const { getByTestId } = renderWithTheme(
<AlertResources alertResourceIds={['1', '2', '3']} serviceType="linode" />
);
const resourceColumn = getByTestId('resource'); // get the resource header column
await userEvent.click(resourceColumn);
const tableBody = getByTestId('alert_resources_content');
let rows = Array.from(tableBody.querySelectorAll('tr'));
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) => {
return text?.includes(linodes[linodes.length - 1 - index].label);
})
).toBe(true);
await userEvent.click(resourceColumn); // again reverse the sorting
rows = Array.from(tableBody.querySelectorAll('tr'));
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) => text?.includes(linodes[index].label))
).toBe(true);
const regionColumn = getByTestId('region'); // get the region header column
await userEvent.click(regionColumn); // sort ascending for region
rows = Array.from(tableBody.querySelectorAll('tr')); // refetch
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) =>
text?.includes(linodes[linodes.length - 1 - index].region)
)
).toBe(true);
await userEvent.click(regionColumn); // reverse the sorting
rows = Array.from(tableBody.querySelectorAll('tr'));
expect(
rows
.map(({ textContent }) => textContent)
.every((text, index) => text?.includes(linodes[index].region)) // validation
).toBe(true);
});
it('should handle sorting correctly', () => {
const { getByTestId } = renderWithTheme(
<AlertResources alertResourceIds={['1', '2', '3']} serviceType="linode" />
);
const resourceColumn = getByTestId('resource'); // Get the resource header column
const regionColumn = getByTestId('region'); // Get the region header column
const tableBody = getByTestId('alert_resources_content');
const getRows = () => Array.from(tableBody.querySelectorAll('tr'));
const checkSorting = async (
column: HTMLElement,
dataKey: 'label' | 'region'
) => {
// Test descending order
await userEvent.click(column);
let rows = getRows();
expect(
rows.every((row, index) =>
row.textContent?.includes(
linodes[linodes.length - 1 - index][dataKey]
)
)
).toBe(true);
// Test ascending order
await userEvent.click(column);
rows = getRows();
expect(
rows.every((row, index) =>
row.textContent?.includes(linodes[index][dataKey])
)
).toBe(true);
};
// Test sorting by 'resource' column
checkSorting(resourceColumn, 'label');
// Test sorting by 'region' column
checkSorting(regionColumn, 'region');
});

nit: Just to make it a bit cleaner and more readable

});
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,29 @@ import { CircleProgress, Stack, Typography } from '@linode/ui';
import { Grid } from '@mui/material';
import React from 'react';

import EntityIcon from 'src/assets/icons/entityIcons/alerts.svg';
import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField';
import { useResourcesQuery } from 'src/queries/cloudpulse/resources';
import { useRegionsQuery } from 'src/queries/regions/regions';

import { StyledPlaceholder } from '../AlertsDetail/AlertDetail';
import {
getFilteredResources,
getRegionOptions,
getRegionsIdRegionMap,
scrollToElement,
} from '../Utils/AlertResourceUtils';
import { AlertsRegionFilter } from './AlertsRegionFilter';
import { DisplayAlertResources } from './DisplayAlertResources';

import type { AlertInstance } from './DisplayAlertResources';
import type { Region } from '@linode/api-v4';

export interface AlertResourcesProp {
/**
* The label of the alert to be displayed
*/
alertLabel?: string;

/**
* The set of resource ids associated with the alerts, that needs to be displayed
*/
Expand All @@ -35,8 +39,7 @@ export interface AlertResourcesProp {
export const AlertResources = React.memo((props: AlertResourcesProp) => {
const { alertLabel, alertResourceIds, serviceType } = props;
const [searchText, setSearchText] = React.useState<string>();

const [, setFilteredRegions] = React.useState<string[]>();
const [filteredRegions, setFilteredRegions] = React.useState<string[]>();

const {
data: regions,
Expand Down Expand Up @@ -69,55 +72,102 @@ export const AlertResources = React.memo((props: AlertResourcesProp) => {
});
}, [resources, alertResourceIds, regionsIdToRegionMap]);

const isDataLoadingError = isRegionsError || isResourcesError;

const handleSearchTextChange = (searchText: string) => {
setSearchText(searchText);
};

const handleFilteredRegionsChange = (selectedRegions: string[]) => {
setFilteredRegions(selectedRegions);
setFilteredRegions(
selectedRegions.map(
(region) =>
regionsIdToRegionMap.get(region)
? `${regionsIdToRegionMap.get(region)?.label} (${region})`
: region // Stores filtered regions in the format `region.label (region.id)` that is displayed and filtered in the table
)
);
};

/**
* Filters resources based on the provided resource IDs, search text, and filtered regions.
*/
const filteredResources: AlertInstance[] = React.useMemo(() => {
return getFilteredResources({
data: resources,
filteredRegions,
regionsIdToRegionMap,
resourceIds: alertResourceIds,
searchText,
});
}, [
resources,
alertResourceIds,
searchText,
filteredRegions,
regionsIdToRegionMap,
]);

const titleRef = React.useRef<HTMLDivElement>(null); // Reference to the component title, used for scrolling to the title when the table's page size or page number changes.

if (isResourcesFetching || isRegionsFetching) {
return <CircleProgress />;
}

const isDataLoadingError = isRegionsError || isResourcesError;
if (!isDataLoadingError && alertResourceIds.length === 0) {
return (
<Stack gap={2}>
<Typography ref={titleRef} variant="h2">
{alertLabel || 'Resources'}
{/* It can be either the passed alert label or just Resources */}
</Typography>
<StyledPlaceholder
icon={EntityIcon}
subtitle="You can assign alerts during the resource creation process."
title="No resources are currently assigned to this alert definition."
/>
</Stack>
);
}

return (
<Stack gap={2}>
<Typography ref={titleRef} variant="h2">
{alertLabel || 'Resources'}
{/* It can be either the passed alert label or just Resources */}
</Typography>

<Grid container spacing={3}>
<Grid columnSpacing={1} container item rowSpacing={3} xs={12}>
<Grid item md={3} xs={12}>
<DebouncedSearchTextField
sx={{
maxHeight: '34px',
}}
clearable
hideLabel
label="Search for a Region or Resource"
onSearch={handleSearchTextChange}
placeholder="Search for a Region or Resource"
value={searchText || ''}
/>
{(isDataLoadingError || alertResourceIds.length) && ( // if there is data loading error display error message with empty table setup
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{(isDataLoadingError || alertResourceIds.length) && ( // if there is data loading error display error message with empty table setup
{/* If there is a data loading error, display an error message with an empty table setup. */}
{(isDataLoadingError || alertResourceIds.length) && (

nit

<Grid container spacing={3}>
<Grid columnSpacing={1} container item rowSpacing={3} xs={12}>
<Grid item md={3} xs={12}>
<DebouncedSearchTextField
sx={{
maxHeight: '34px',
}}
clearable
hideLabel
label="Search for a Region or Resource"
onSearch={handleSearchTextChange}
placeholder="Search for a Region or Resource"
value={searchText || ''}
/>
</Grid>
<Grid item md={4} xs={12}>
<AlertsRegionFilter
handleSelectionChange={handleFilteredRegionsChange}
regionOptions={regionOptions}
/>
</Grid>
</Grid>
<Grid item md={4} xs={12}>
<AlertsRegionFilter
handleSelectionChange={handleFilteredRegionsChange}
regionOptions={regionOptions}
<Grid item xs={12}>
<DisplayAlertResources
filteredResources={filteredResources}
isDataLoadingError={isDataLoadingError}
scrollToElement={() => scrollToElement(titleRef.current)}
/>
</Grid>
</Grid>
<Grid item xs={12}>
<DisplayAlertResources isDataLoadingError={isDataLoadingError} />
</Grid>
</Grid>
)}
</Stack>
);
});
Loading
Loading