From 3ca5070944593091f46aa8b5ef680ae2144e7e35 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Wed, 13 Nov 2024 14:03:22 -0500 Subject: [PATCH 1/5] inital work --- .../ImageRegions/ImageRegionRow.tsx | 16 +++--- .../Images/ImagesLanding/ImageRow.tsx | 50 +++-------------- .../Images/ImagesLanding/ImageStatus.tsx | 56 +++++++++++++++++++ 3 files changed, 71 insertions(+), 51 deletions(-) create mode 100644 packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx index 0e8b07b032f..361c997363f 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx @@ -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; @@ -34,9 +34,7 @@ export const ImageRegionRow = (props: Props) => { {status} - + { ); }; -const IMAGE_REGION_STATUS_TO_STATUS_ICON_STATUS: Readonly< - Record +export const imageStatusIconMap: Readonly< + Record > = { available: 'active', creating: 'other', + deleted: 'error', pending: 'other', 'pending deletion': 'other', - 'pending replication': 'inactive', + 'pending replication': 'other', + pending_upload: 'other', replicating: 'other', - timedout: 'inactive', + timedout: 'error', unsaved: 'inactive', }; diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx index b3640a1b32a..691fc59b4c1 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx @@ -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'; @@ -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 ( - - ); - 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, @@ -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'; @@ -113,7 +95,9 @@ export const ImageRow = (props: Props) => { )} - {getStatusForImage(status)} + + + {multiRegionsEnabled && ( @@ -180,23 +164,3 @@ export const isImageUpdating = (e?: Event) => { 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 ( - - {text}: {displayProgress} - - ); -}; diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx new file mode 100644 index 00000000000..c78b8186269 --- /dev/null +++ b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx @@ -0,0 +1,56 @@ +import { Stack } from '@linode/ui'; +import React from 'react'; + +import { StatusIcon } from 'src/components/StatusIcon/StatusIcon'; +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; + + const isFailedUpload = + image.status === 'pending_upload' && event?.status === 'failed'; + + if (isFailedUpload) { + return ( + + + Upload Failed + + ); + } + + const imageRegionStatus = image.regions.find((r) => r.status !== 'available') + ?.status; + + if (imageRegionStatus) { + return ( + + + {capitalizeAllWords(imageRegionStatus)} + + ); + } + + return ( + + + {capitalizeAllWords(image.status.replace('_', ' '))} + {event && event.percent_complete && `(${event.percent_complete}%)`} + + ); +}; From 188943b6285b019141fcebb950cca99416518602 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Wed, 13 Nov 2024 15:33:09 -0500 Subject: [PATCH 2/5] update tests and refine --- .../core/images/machine-image-upload.spec.ts | 10 ++++---- .../Images/ImagesLanding/ImageRow.test.tsx | 4 ++-- .../Images/ImagesLanding/ImageRow.tsx | 10 -------- .../Images/ImagesLanding/ImageStatus.tsx | 23 ++++++++++++++----- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts b/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts index 3f265228211..07568727731 100644 --- a/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts +++ b/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts @@ -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'); }); }; @@ -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'); }); }; @@ -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}`) @@ -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'); }); }); }); @@ -313,7 +313,7 @@ describe('machine image', () => { * - Confirms that machine image is listed in landing page as expected. * - Confirms that notifications appear that describe the image's failed status. */ - it('uploads machine image, mock expired upload event', () => { + it.only('uploads machine image, mock expired upload event', () => { const label = randomLabel(); const status = 'failed'; const message = 'Upload window expired'; diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRow.test.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRow.test.tsx index e507170eb39..0440e31b822 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageRow.test.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageRow.test.tsx @@ -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, @@ -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 diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx index 691fc59b4c1..98cc5cb111b 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx @@ -154,13 +154,3 @@ export const ImageRow = (props: Props) => { ); }; - -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) - ); -}; diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx index c78b8186269..d4763374952 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx @@ -2,6 +2,7 @@ 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'; @@ -22,14 +23,18 @@ interface Props { export const ImageStatus = (props: Props) => { const { event, image } = props; - const isFailedUpload = - image.status === 'pending_upload' && event?.status === 'failed'; - - if (isFailedUpload) { + if (event && event.status === 'failed' && image.status === 'pending_upload') { return ( - + Upload Failed + {event.message && ( + + )} ); } @@ -46,11 +51,17 @@ export const ImageStatus = (props: Props) => { ); } + const showEventProgress = + event && + event.status === 'started' && + event.percent_complete !== null && + event.percent_complete > 0; + return ( {capitalizeAllWords(image.status.replace('_', ' '))} - {event && event.percent_complete && `(${event.percent_complete}%)`} + {showEventProgress && ` (${event.percent_complete}%)`} ); }; From 9d034ffada9967ca0063c32a5e8ee0c61c93bace Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Wed, 13 Nov 2024 16:44:36 -0500 Subject: [PATCH 3/5] improve more --- packages/api-v4/src/images/types.ts | 6 +- .../core/images/machine-image-upload.spec.ts | 2 +- .../ImageRegions/ImageRegionRow.tsx | 1 - .../Images/ImagesLanding/ImageStatus.test.tsx | 64 +++++++++++++++++++ .../Images/ImagesLanding/ImageStatus.tsx | 15 +++-- 5 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 packages/manager/src/features/Images/ImagesLanding/ImageStatus.test.tsx diff --git a/packages/api-v4/src/images/types.ts b/packages/api-v4/src/images/types.ts index cc1572d449b..4a707b361d8 100644 --- a/packages/api-v4/src/images/types.ts +++ b/packages/api-v4/src/images/types.ts @@ -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'; diff --git a/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts b/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts index 07568727731..49c084aef1b 100644 --- a/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts +++ b/packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts @@ -313,7 +313,7 @@ describe('machine image', () => { * - Confirms that machine image is listed in landing page as expected. * - Confirms that notifications appear that describe the image's failed status. */ - it.only('uploads machine image, mock expired upload event', () => { + it('uploads machine image, mock expired upload event', () => { const label = randomLabel(); const status = 'failed'; const message = 'Upload window expired'; diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx index 361c997363f..c7e499e4ae0 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx @@ -63,7 +63,6 @@ export const imageStatusIconMap: Readonly< > = { available: 'active', creating: 'other', - deleted: 'error', pending: 'other', 'pending deletion': 'other', 'pending replication': 'other', diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageStatus.test.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.test.tsx new file mode 100644 index 00000000000..9ddc794d746 --- /dev/null +++ b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.test.tsx @@ -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( + + ); + + 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( + + ); + + 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( + + ); + + 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( + + ); + + expect(getByText('Upload Failed')).toBeVisible(); + expect(getByLabelText('Image too large when uncompressed')).toBeVisible(); + }); +}); diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx index d4763374952..f16a9909552 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageStatus.tsx @@ -23,7 +23,14 @@ interface Props { export const ImageStatus = (props: Props) => { const { event, image } = props; - if (event && event.status === 'failed' && image.status === 'pending_upload') { + 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 ( @@ -43,6 +50,7 @@ export const ImageStatus = (props: Props) => { ?.status; if (imageRegionStatus) { + // If we have any non-available region statuses, expose the first one as the Image's status to the user return ( @@ -52,10 +60,7 @@ export const ImageStatus = (props: Props) => { } const showEventProgress = - event && - event.status === 'started' && - event.percent_complete !== null && - event.percent_complete > 0; + event && event.status === 'started' && event.percent_complete !== null; return ( From 9c0f1043b3ce7e1e094bf9a0ee0de612880c5fe2 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Thu, 14 Nov 2024 09:38:31 -0500 Subject: [PATCH 4/5] add changesets --- packages/api-v4/.changeset/pr-11257-removed-1731594995947.md | 5 +++++ .../manager/.changeset/pr-11257-changed-1731595083756.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 packages/api-v4/.changeset/pr-11257-removed-1731594995947.md create mode 100644 packages/manager/.changeset/pr-11257-changed-1731595083756.md diff --git a/packages/api-v4/.changeset/pr-11257-removed-1731594995947.md b/packages/api-v4/.changeset/pr-11257-removed-1731594995947.md new file mode 100644 index 00000000000..041f5e6375e --- /dev/null +++ b/packages/api-v4/.changeset/pr-11257-removed-1731594995947.md @@ -0,0 +1,5 @@ +--- +"@linode/api-v4": Removed +--- + +`deleted` from the `ImageStatus` type ([#11257](https://github.com/linode/manager/pull/11257)) diff --git a/packages/manager/.changeset/pr-11257-changed-1731595083756.md b/packages/manager/.changeset/pr-11257-changed-1731595083756.md new file mode 100644 index 00000000000..81f496b0900 --- /dev/null +++ b/packages/manager/.changeset/pr-11257-changed-1731595083756.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Changed +--- + +Improve the status column on the Images landing page ([#11257](https://github.com/linode/manager/pull/11257)) From ae62938f62568af54e94de91829bef8eabc0d479 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Thu, 14 Nov 2024 10:02:21 -0500 Subject: [PATCH 5/5] disable wrapping in the status column --- packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx index 98cc5cb111b..1aa8bc50842 100644 --- a/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx +++ b/packages/manager/src/features/Images/ImagesLanding/ImageRow.tsx @@ -95,7 +95,7 @@ export const ImageRow = (props: Props) => { )} - +