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-21119] - Handle jwe token limit in ACLP UI and Bug fix for resources component #11309

Merged
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/manager": Upcoming Features
---

Handle JWE token limit of 250 in ACLP UI ([#11309](https://github.com/linode/manager/pull/11309))
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const CloudPulseDashboard = (props: DashboardProperties) => {

const getJweTokenPayload = (): JWETokenPayLoad => {
return {
resource_ids: resourceList?.map((resource) => Number(resource.id)) ?? [],
resource_ids: resources?.map((resource) => Number(resource)) ?? [],
};
};

Expand Down Expand Up @@ -100,11 +100,11 @@ export const CloudPulseDashboard = (props: DashboardProperties) => {
const {
data: jweToken,
isError: isJweTokenError,
isLoading: isJweTokenLoading,
isFetching: isJweTokenFetching,
} = useCloudPulseJWEtokenQuery(
dashboard?.service_type,
getJweTokenPayload(),
Boolean(resourceList)
Boolean(resources) && !isDashboardLoading && !isDashboardApiError
);

if (isDashboardApiError) {
Expand All @@ -123,12 +123,7 @@ export const CloudPulseDashboard = (props: DashboardProperties) => {
return renderErrorState('Error loading the definitions of metrics.');
}

if (
isMetricDefinitionLoading ||
isDashboardLoading ||
isResourcesLoading ||
isJweTokenLoading
) {
if (isMetricDefinitionLoading || isDashboardLoading || isResourcesLoading) {
return <CircleProgress />;
}

Expand All @@ -137,6 +132,7 @@ export const CloudPulseDashboard = (props: DashboardProperties) => {
additionalFilters={additionalFilters}
dashboard={dashboard}
duration={duration}
isJweTokenFetching={isJweTokenFetching}
jweToken={jweToken}
manualRefreshTimeStamp={manualRefreshTimeStamp}
metricDefinitions={metricDefinitions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface CloudPulseWidgetProperties {
/**
* token to fetch metrics data
*/
authToken: string;
authToken?: string;

/**
* metrics defined of this widget
Expand All @@ -68,6 +68,11 @@ export interface CloudPulseWidgetProperties {
*/
errorLabel?: string;

/**
* Jwe token fetching status check
*/
isJweTokenFetching: boolean;

/**
* resources ids selected by user to show metrics for
*/
Expand Down Expand Up @@ -136,6 +141,7 @@ export const CloudPulseWidget = (props: CloudPulseWidgetProperties) => {
authToken,
availableMetrics,
duration,
isJweTokenFetching,
resourceIds,
resources,
savePref,
Expand Down Expand Up @@ -232,7 +238,7 @@ export const CloudPulseWidget = (props: CloudPulseWidgetProperties) => {
},
{
authToken,
isFlags: Boolean(flags),
isFlags: Boolean(flags && !isJweTokenFetching),
label: widget.label,
timeStamp,
url: flags.aclpReadEndpoint!,
Expand Down Expand Up @@ -326,13 +332,17 @@ export const CloudPulseWidget = (props: CloudPulseWidgetProperties) => {
? metricsApiCallError ?? 'Error while rendering graph'
: undefined
}
loading={
isLoading ||
metricsApiCallError === jweTokenExpiryError ||
isJweTokenFetching
} // keep loading until we are trying to fetch the refresh token
areas={areas}
ariaLabel={ariaLabel ? ariaLabel : ''}
data={data}
dotRadius={1.5}
height={424}
legendRows={legendRows}
loading={isLoading || metricsApiCallError === jweTokenExpiryError} // keep loading until we fetch the refresh token
showDot
showLegend={data.length !== 0}
timezone={timezone}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ interface WidgetProps {
additionalFilters?: CloudPulseMetricsAdditionalFilters[];
dashboard?: Dashboard | undefined;
duration: TimeDuration;
isJweTokenFetching: boolean;
jweToken?: JWEToken | undefined;
manualRefreshTimeStamp?: number;
metricDefinitions: MetricDefinitions | undefined;
Expand All @@ -55,6 +56,7 @@ export const RenderWidgets = React.memo(
additionalFilters,
dashboard,
duration,
isJweTokenFetching,
jweToken,
manualRefreshTimeStamp,
metricDefinitions,
Expand All @@ -74,6 +76,7 @@ export const RenderWidgets = React.memo(
availableMetrics: undefined,
duration,
errorLabel: 'Error occurred while loading data.',
isJweTokenFetching: false,
resourceIds: resources,
resources: [],
serviceType: dashboard?.service_type ?? '',
Expand Down Expand Up @@ -123,7 +126,7 @@ export const RenderWidgets = React.memo(
if (
!dashboard.service_type ||
!Boolean(resources.length > 0) ||
!jweToken?.token ||
(!isJweTokenFetching && !jweToken?.token) ||
!Boolean(resourceList?.length)
) {
return renderPlaceHolder(
Expand Down Expand Up @@ -162,6 +165,7 @@ export const RenderWidgets = React.memo(
{...cloudPulseWidgetProperties}
authToken={jweToken?.token}
availableMetrics={availMetrics}
isJweTokenFetching={isJweTokenFetching}
resources={resourceList!}
savePref={savePref}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo(
container
display={showFilter ? 'flex' : 'none'}
item
maxHeight={theme.spacing(22)}
maxHeight={theme.spacing(23)}
overflow={'auto'}
rowGap={theme.spacing(2)}
xs={12}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ describe('CloudPulseResourcesSelect component tests', () => {
expect(screen.getByText('Failed to fetch Resources.')).toBeInTheDocument();
});

it('should be able to select limited resources', async () => {
it('should be able to select limited resources and select/deselect all will not be available if resource are more than max resource selection limit', async () => {
const user = userEvent.setup();

queryMocks.useResourcesQuery.mockReturnValue({
Expand Down Expand Up @@ -298,4 +298,36 @@ describe('CloudPulseResourcesSelect component tests', () => {

expect(queryByRole('option', { name: SELECT_ALL })).not.toBeInTheDocument();
});

it('should be able to select all and deselect all the resources when number of resources are equal to resource limit', async () => {
const user = userEvent.setup();

queryMocks.useResourcesQuery.mockReturnValue({
data: linodeFactory.buildList(10),
isError: false,
isLoading: false,
status: 'success',
});

renderWithTheme(
<CloudPulseResourcesSelect
handleResourcesSelection={mockResourceHandler}
label="Resources"
region={'us-east'}
resourceType={'linode'}
/>
);

await user.click(screen.getByRole('button', { name: 'Open' }));
await user.click(screen.getByRole('option', { name: SELECT_ALL }));
await user.click(screen.getByRole('option', { name: 'Deselect All' }));

expect(screen.getByLabelText('Resources')).toBeInTheDocument();

for (let i = 26; i <= 35; i++) {
expect(
screen.getByRole('option', { name: `linode-${i}` })
).toHaveAttribute(ARIA_SELECTED, 'false');
}
});
});
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { Box, ListItem } from '@mui/material';
import { Box } from '@mui/material';
import React from 'react';

import { Autocomplete } from 'src/components/Autocomplete/Autocomplete';
import { SelectedIcon } from 'src/components/Autocomplete/Autocomplete.styles';
import {
SelectedIcon,
StyledListItem,
} from 'src/components/Autocomplete/Autocomplete.styles';
import { useFlags } from 'src/hooks/useFlags';
import { useResourcesQuery } from 'src/queries/cloudpulse/resources';
import { themes } from 'src/utilities/theme';
Expand Down Expand Up @@ -146,10 +149,20 @@ export const CloudPulseResourcesSelect = React.memo(
const isResourceSelected = selectedResources?.some(
(item) => item.label === option.label
);

const isSelectAllORDeslectAllOption =
option.label === 'Select All ' || option.label === 'Deselect All ';

const isMaxSelectionsReached =
selectedResources &&
selectedResources.length >= maxResourceSelectionLimit &&
!isResourceSelected;
!isResourceSelected &&
!isSelectAllORDeslectAllOption;

const ListItem = isSelectAllORDeslectAllOption
? StyledListItem
: 'li';

return (
<ListItem
{...rest}
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/queries/cloudpulse/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const useCloudPulseMetricsQuery = (
serviceType: string,
request: CloudPulseMetricsRequest,
obj: {
authToken: string;
authToken?: string;
isFlags: boolean;
label: string;
timeStamp: number | undefined;
Expand All @@ -29,7 +29,7 @@ export const useCloudPulseMetricsQuery = (

const query = useQuery<CloudPulseMetricsResponse, APIError[]>({
...queryFactory.metrics(
obj.authToken,
obj.authToken ?? '',
obj.url,
serviceType,
request,
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/queries/cloudpulse/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,6 @@ export const queryFactory = createQueryKeys(key, {

token: (serviceType: string | undefined, request: JWETokenPayLoad) => ({
queryFn: () => getJWEToken(request, serviceType!),
queryKey: [serviceType],
queryKey: [serviceType, { resource_ids: request.resource_ids.sort() }],
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved
}),
});