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: [M3-8903] - Improve Image Status Column #11257

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
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": Removed
---

`deleted` from the `ImageStatus` type ([#11257](https://github.com/linode/manager/pull/11257))
6 changes: 1 addition & 5 deletions packages/api-v4/src/images/types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
export type ImageStatus =
| 'available'
| 'creating'
| 'deleted'
| 'pending_upload';
export type ImageStatus = 'available' | 'creating' | 'pending_upload';

export type ImageCapabilities = 'cloud-init' | 'distributed-sites';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Changed
---

Improve the status column on the Images landing page ([#11257](https://github.com/linode/manager/pull/11257))
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const assertFailed = (label: string, id: string, message: string) => {

cy.get(`[data-qa-image-cell="${id}"]`).within(() => {
fbtVisible(label);
fbtVisible('Failed');
fbtVisible('Upload Failed');
fbtVisible('N/A');
});
};
Expand All @@ -99,7 +99,7 @@ const assertFailed = (label: string, id: string, message: string) => {
const assertProcessing = (label: string, id: string) => {
cy.get(`[data-qa-image-cell="${id}"]`).within(() => {
fbtVisible(label);
fbtVisible('Processing');
fbtVisible('Pending Upload');
fbtVisible('Pending');
});
};
Expand Down Expand Up @@ -172,7 +172,7 @@ describe('machine image', () => {

cy.get(`[data-qa-image-cell="${mockImage.id}"]`).within(() => {
cy.findByText(initialLabel).should('be.visible');
cy.findByText('Ready').should('be.visible');
cy.findByText('Available').should('be.visible');

ui.actionMenu
.findByTitle(`Action menu for Image ${initialLabel}`)
Expand Down Expand Up @@ -262,7 +262,7 @@ describe('machine image', () => {
ui.toast.assertMessage(availableMessage);
cy.get(`[data-qa-image-cell="${imageId}"]`).within(() => {
fbtVisible(label);
fbtVisible('Ready');
fbtVisible('Available');
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { StatusIcon } from 'src/components/StatusIcon/StatusIcon';
import { Typography } from 'src/components/Typography';
import { useRegionsQuery } from 'src/queries/regions/regions';

import type { ImageRegionStatus } from '@linode/api-v4';
import type { ImageRegionStatus, ImageStatus } from '@linode/api-v4';
import type { Status } from 'src/components/StatusIcon/StatusIcon';

type ExtendedImageRegionStatus = 'unsaved' | ImageRegionStatus;
Expand All @@ -34,9 +34,7 @@ export const ImageRegionRow = (props: Props) => {
</Stack>
<Stack alignItems="center" direction="row" gap={1}>
<Typography textTransform="capitalize">{status}</Typography>
<StatusIcon
status={IMAGE_REGION_STATUS_TO_STATUS_ICON_STATUS[status]}
/>
<StatusIcon status={imageStatusIconMap[status]} />
<Tooltip
title={
disableRemoveButton
Expand All @@ -60,15 +58,16 @@ export const ImageRegionRow = (props: Props) => {
);
};

const IMAGE_REGION_STATUS_TO_STATUS_ICON_STATUS: Readonly<
Record<ExtendedImageRegionStatus, Status>
export const imageStatusIconMap: Readonly<
Record<ExtendedImageRegionStatus | ImageStatus, Status>
> = {
available: 'active',
creating: 'other',
pending: 'other',
'pending deletion': 'other',
'pending replication': 'inactive',
'pending replication': 'other',
pending_upload: 'other',
replicating: 'other',
timedout: 'inactive',
timedout: 'error',
unsaved: 'inactive',
};
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Image Table Row', () => {
capabilities: ['cloud-init', 'distributed-sites'],
regions: [
{ region: 'us-east', status: 'available' },
{ region: 'us-southeast', status: 'pending' },
{ region: 'us-southeast', status: 'available' },
],
size: 300,
total_size: 600,
Expand All @@ -45,7 +45,7 @@ describe('Image Table Row', () => {
// Check to see if the row rendered some data
expect(getByText(image.label)).toBeVisible();
expect(getByText(image.id)).toBeVisible();
expect(getByText('Ready')).toBeVisible();
expect(getByText('Available')).toBeVisible();
expect(getByText('Cloud-init, Distributed')).toBeVisible();
expect(getByText('2 Regions')).toBeVisible();
expect(getByText('0.29 GB')).toBeVisible(); // Size is converted from MB to GB - 300 / 1024 = 0.292
Expand Down
60 changes: 7 additions & 53 deletions packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ import { Hidden } from 'src/components/Hidden';
import { LinkButton } from 'src/components/LinkButton';
import { TableCell } from 'src/components/TableCell';
import { TableRow } from 'src/components/TableRow';
import { Typography } from 'src/components/Typography';
import { useFlags } from 'src/hooks/useFlags';
import { useProfile } from 'src/queries/profile/profile';
import { capitalizeAllWords } from 'src/utilities/capitalize';
import { formatDate } from 'src/utilities/formatDate';
import { pluralize } from 'src/utilities/pluralize';
import { convertStorageUnit } from 'src/utilities/unitConversions';

import { ImagesActionMenu } from './ImagesActionMenu';
import { ImageStatus } from './ImageStatus';

import type { Handlers } from './ImagesActionMenu';
import type { Event, Image, ImageCapabilities } from '@linode/api-v4';
Expand Down Expand Up @@ -49,29 +48,12 @@ export const ImageRow = (props: Props) => {
const { data: profile } = useProfile();
const flags = useFlags();

const isFailed = status === 'pending_upload' && event?.status === 'failed';

const compatibilitiesList = multiRegionsEnabled
? capabilities.map((capability) => capabilityMap[capability]).join(', ')
: '';

const getStatusForImage = (status: string) => {
switch (status) {
case 'creating':
return (
<ProgressDisplay
progress={progressFromEvent(event)}
text="Creating"
/>
);
case 'available':
return 'Ready';
case 'pending_upload':
return isFailed ? 'Failed' : 'Processing';
default:
return capitalizeAllWords(status.replace('_', ' '));
}
};
const isFailedUpload =
image.status === 'pending_upload' && event?.status === 'failed';

const getSizeForImage = (
size: number,
Expand All @@ -87,7 +69,7 @@ export const ImageRow = (props: Props) => {
}).format(sizeInGB);

return `${formattedSizeInGB} GB`;
} else if (isFailed) {
} else if (isFailedUpload) {
return 'N/A';
} else {
return 'Pending';
Expand All @@ -113,7 +95,9 @@ export const ImageRow = (props: Props) => {
)}
</TableCell>
<Hidden smDown>
<TableCell>{getStatusForImage(status)}</TableCell>
<TableCell noWrap>
<ImageStatus event={event} image={image} />
</TableCell>
</Hidden>
{multiRegionsEnabled && (
<Hidden smDown>
Expand Down Expand Up @@ -170,33 +154,3 @@ export const ImageRow = (props: Props) => {
</TableRow>
);
};

export const isImageUpdating = (e?: Event) => {
// Make Typescript happy, since this function can otherwise technically return undefined
if (!e) {
return false;
}
return (
e?.action === 'disk_imagize' && ['scheduled', 'started'].includes(e.status)
);
};

const progressFromEvent = (e?: Event) => {
return e?.status === 'started' && e?.percent_complete
? e.percent_complete
: undefined;
};

const ProgressDisplay: React.FC<{
progress: number | undefined;
text: string;
}> = (props) => {
const { progress, text } = props;
const displayProgress = progress ? `${progress}%` : `scheduled`;

return (
<Typography variant="body1">
{text}: {displayProgress}
</Typography>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import React from 'react';

import { eventFactory, imageFactory } from 'src/factories';
import { renderWithTheme } from 'src/utilities/testHelpers';

import { ImageStatus } from './ImageStatus';

describe('ImageStatus', () => {
it('renders the image status', () => {
const image = imageFactory.build({ status: 'available' });

const { getByText } = renderWithTheme(
<ImageStatus event={undefined} image={image} />
);

expect(getByText('Available')).toBeVisible();
});

it('should render the first region status if any region status is not "available"', () => {
const image = imageFactory.build({
regions: [
{ region: 'us-west', status: 'available' },
{ region: 'us-east', status: 'pending replication' },
],
status: 'available',
});

const { getByText } = renderWithTheme(
<ImageStatus event={undefined} image={image} />
);

expect(getByText('Pending Replication')).toBeVisible();
});

it('should render the image status with a percent if an in progress event is happening', () => {
const image = imageFactory.build({ status: 'creating' });
const event = eventFactory.build({
percent_complete: 20,
status: 'started',
});

const { getByText } = renderWithTheme(
<ImageStatus event={event} image={image} />
);

expect(getByText('Creating (20%)')).toBeVisible();
});

it('should render "Upload Failed" if image is pending_upload, but we have a failed image upload event', () => {
const image = imageFactory.build({ status: 'pending_upload' });
const event = eventFactory.build({
action: 'image_upload',
message: 'Image too large when uncompressed',
status: 'failed',
});

const { getByLabelText, getByText } = renderWithTheme(
<ImageStatus event={event} image={image} />
);

expect(getByText('Upload Failed')).toBeVisible();
expect(getByLabelText('Image too large when uncompressed')).toBeVisible();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { Stack } from '@linode/ui';
import React from 'react';

import { StatusIcon } from 'src/components/StatusIcon/StatusIcon';
import { TooltipIcon } from 'src/components/TooltipIcon';
import { capitalizeAllWords } from 'src/utilities/capitalize';

import { imageStatusIconMap } from './ImageRegions/ImageRegionRow';

import type { Event, Image } from '@linode/api-v4';

interface Props {
/**
* The most revent event associated with this Image
*/
event: Event | undefined;
/**
* The Image object
*/
image: Image;
}

export const ImageStatus = (props: Props) => {
const { event, image } = props;

if (
event &&
event.status === 'failed' &&
event.action === 'image_upload' &&
image.status === 'pending_upload'
) {
// If we have a recent image upload failure, we show the user
// that the upload failed and why.
return (
<Stack alignItems="center" direction="row">
<StatusIcon status="error" />
Upload Failed
{event.message && (
<TooltipIcon
status="help"
sxTooltipIcon={{ ml: 1, p: 0 }}
text={event.message}
/>
)}
</Stack>
);
}

const imageRegionStatus = image.regions.find((r) => r.status !== 'available')
?.status;

if (imageRegionStatus) {
// If we have any non-available region statuses, expose the first one as the Image's status to the user
return (
<Stack direction="row">
<StatusIcon status={imageStatusIconMap[imageRegionStatus]} />
{capitalizeAllWords(imageRegionStatus)}
</Stack>
);
}

const showEventProgress =
event && event.status === 'started' && event.percent_complete !== null;

return (
<Stack direction="row">
<StatusIcon status={imageStatusIconMap[image.status]} />
{capitalizeAllWords(image.status.replace('_', ' '))}
{showEventProgress && ` (${event.percent_complete}%)`}
</Stack>
);
};