From c7ed12d73dd60ed01a73620001d1bf4c556049fb Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 4 Jun 2024 13:44:40 -0400 Subject: [PATCH 1/7] add notice and helper text --- .../Images/ImagesCreate/CreateImageTab.tsx | 64 ++++++++++++------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx index d83f4af3fc1..5e30a50e97b 100644 --- a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx @@ -101,16 +101,11 @@ export const CreateImageTab = () => { const isRawDisk = selectedDisk?.filesystem === 'raw'; - /* - We only want to display the notice about disk encryption if: - 1. the Disk Encryption feature is enabled - 2. the selected linode is not in an Edge region - */ const { data: regionsData } = useRegionsQuery(); const { data: linode } = useLinodeQuery( selectedLinodeId ?? -1, - Boolean(selectedLinodeId) && isDiskEncryptionFeatureEnabled + selectedLinodeId !== null ); const linodeIsInEdgeRegion = getIsEdgeRegion( @@ -118,6 +113,31 @@ export const CreateImageTab = () => { linode?.region ?? '' ); + /* + We only want to display the notice about disk encryption if: + 1. the Disk Encryption feature is enabled + 2. the selected linode is not in an Edge region + */ + const showDiskEncryptionWarning = + isDiskEncryptionFeatureEnabled && + !linodeIsInEdgeRegion && + selectedLinodeId !== null; + + const linodeSelectHelperText = [ + { + show: linodeIsInEdgeRegion, + text: `Image will be stored in the closest core site to (${linode?.region})`, + }, + { + show: grants?.linode.some((grant) => grant.permissions === 'read_only'), + text: + 'You can only create Images from Linodes you have read/write access to.', + }, + ] + .filter((item) => item.show) + .map((item) => item.text) + .join('. '); + return (
@@ -153,6 +173,12 @@ export const CreateImageTab = () => { created from a raw disk or a disk that’s formatted using a custom file system. + {linodeIsInEdgeRegion && ( + + This Linode is in a distributed compute region. Images captured + from this Linode will be stored in the closest core site. + + )} { ) : undefined } - helperText={ - grants?.linode.some( - (grant) => grant.permissions === 'read_only' - ) - ? 'You can only create Images from Linodes you have read/write access to.' - : undefined - } onSelectionChange={(linode) => { setSelectedLinodeId(linode?.id ?? null); if (linode === null) { @@ -178,21 +197,18 @@ export const CreateImageTab = () => { } }} disabled={isImageCreateRestricted} + helperText={linodeSelectHelperText} noMarginTop required value={selectedLinodeId} /> - {isDiskEncryptionFeatureEnabled && - !linodeIsInEdgeRegion && - selectedLinodeId !== null && ( - - ({ fontFamily: theme.font.normal })} - > - {DISK_ENCRYPTION_IMAGES_CAVEAT_COPY} - - - )} + {showDiskEncryptionWarning && ( + + ({ fontFamily: theme.font.normal })}> + {DISK_ENCRYPTION_IMAGES_CAVEAT_COPY} + + + )} ( Date: Tue, 4 Jun 2024 15:23:02 -0400 Subject: [PATCH 2/7] add juicy unit testing --- .../ImagesCreate/CreateImageTab.test.tsx | 173 ++++++++++++++++++ .../Images/ImagesCreate/CreateImageTab.tsx | 5 +- 2 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx new file mode 100644 index 00000000000..9dddbb96ee4 --- /dev/null +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx @@ -0,0 +1,173 @@ +import { userEvent } from '@testing-library/user-event'; +import React from 'react'; + +import { + imageFactory, + linodeDiskFactory, + linodeFactory, + regionFactory, +} from 'src/factories'; +import { makeResourcePage } from 'src/mocks/serverHandlers'; +import { HttpResponse, http, server } from 'src/mocks/testServer'; +import { renderWithTheme } from 'src/utilities/testHelpers'; + +import { CreateImageTab } from './CreateImageTab'; + +describe('CreateImageTab', () => { + it('should render fields, titles, and buttons in their default state', () => { + const { getByLabelText, getByText } = renderWithTheme(); + + expect(getByText('Select Linode & Disk')).toBeVisible(); + + expect(getByLabelText('Linode')).toBeVisible(); + + const diskSelect = getByLabelText('Disk'); + + expect(diskSelect).toBeVisible(); + expect(diskSelect).toBeDisabled(); + + expect(getByText('Select a Linode to see available disks')).toBeVisible(); + + expect(getByText('Image Details')).toBeVisible(); + + expect(getByLabelText('Label')).toBeVisible(); + expect(getByLabelText('Add Tags')).toBeVisible(); + expect(getByLabelText('Description')).toBeVisible(); + + const submitButton = getByText('Create Image').closest('button'); + + expect(submitButton).toBeVisible(); + expect(submitButton).toBeEnabled(); + }); + + it('should render client side validation errors', async () => { + const { getByText } = renderWithTheme(); + + const submitButton = getByText('Create Image').closest('button'); + + await userEvent.click(submitButton!); + + expect(getByText('Disk is required.')).toBeVisible(); + }); + + it('should allow the user to select a disk and submit the form', async () => { + const linode = linodeFactory.build(); + const disk = linodeDiskFactory.build(); + const image = imageFactory.build(); + + server.use( + http.get('*/v4/linode/instances', () => { + return HttpResponse.json(makeResourcePage([linode])); + }), + http.get('*/v4/linode/instances/:id/disks', () => { + return HttpResponse.json(makeResourcePage([disk])); + }), + http.post('*/v4/images', () => { + return HttpResponse.json(image); + }) + ); + + const { + findByText, + getByLabelText, + getByText, + queryByText, + } = renderWithTheme(); + + const linodeSelect = getByLabelText('Linode'); + + await userEvent.click(linodeSelect); + + const linodeOption = await findByText(linode.label); + + await userEvent.click(linodeOption); + + const diskSelect = getByLabelText('Disk'); + + // Once a Linode is selected, the Disk select should become enabled + expect(diskSelect).toBeEnabled(); + expect(queryByText('Select a Linode to see available disks')).toBeNull(); + + await userEvent.click(diskSelect); + + const diskOption = await findByText(disk.label); + + await userEvent.click(diskOption); + + const submitButton = getByText('Create Image').closest('button'); + + await userEvent.click(submitButton!); + + // Verify success toast shows + await findByText('Image scheduled for creation.'); + }); + + it('should render a notice if the user selects a Linode in an edge region', async () => { + const region = regionFactory.build({ site_type: 'edge' }); + const linode = linodeFactory.build({ region: region.id }); + + server.use( + http.get('*/v4/linode/instances', () => { + return HttpResponse.json(makeResourcePage([linode])); + }), + http.get('*/v4/linode/instances/:id', () => { + return HttpResponse.json(linode); + }), + http.get('*/v4/regions', () => { + return HttpResponse.json(makeResourcePage([region])); + }) + ); + + const { findByText, getByLabelText } = renderWithTheme(); + + const linodeSelect = getByLabelText('Linode'); + + await userEvent.click(linodeSelect); + + const linodeOption = await findByText(linode.label); + + await userEvent.click(linodeOption); + + // Verify edge notice renders + await findByText( + 'This Linode is in a distributed compute region. Images captured from this Linode will be stored in the closest core site.' + ); + + // Verify helper text renders + await findByText( + `Image will be stored in the closest core site to (${linode.region})` + ); + }); + + it('should render an encryption notice if disk encryption is enabled and the Linode is not in an edge region', async () => { + const region = regionFactory.build({ site_type: 'core' }); + const linode = linodeFactory.build({ region: region.id }); + + server.use( + http.get('*/v4/linode/instances', () => { + return HttpResponse.json(makeResourcePage([linode])); + }), + http.get('*/v4/linode/instances/:id', () => { + return HttpResponse.json(linode); + }), + http.get('*/v4/regions', () => { + return HttpResponse.json(makeResourcePage([region])); + }) + ); + + const { findByText, getByLabelText } = renderWithTheme(, { + flags: { linodeDiskEncryption: true }, + }); + + const linodeSelect = getByLabelText('Linode'); + + await userEvent.click(linodeSelect); + + const linodeOption = await findByText(linode.label); + + await userEvent.click(linodeOption); + + // Verify encryption notice renders + await findByText('Virtual Machine Images are not encrypted.'); + }); +}); diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx index 5e30a50e97b..1495286504b 100644 --- a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx @@ -116,12 +116,13 @@ export const CreateImageTab = () => { /* We only want to display the notice about disk encryption if: 1. the Disk Encryption feature is enabled + 2. a linode is selected 2. the selected linode is not in an Edge region */ const showDiskEncryptionWarning = isDiskEncryptionFeatureEnabled && - !linodeIsInEdgeRegion && - selectedLinodeId !== null; + selectedLinodeId !== null && + !linodeIsInEdgeRegion; const linodeSelectHelperText = [ { From 2a104dee4fdb6f55bcc31de40c0a17fb9a7116d8 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 4 Jun 2024 15:40:36 -0400 Subject: [PATCH 3/7] don't use the word `edge` --- .../features/Images/ImagesCreate/CreateImageTab.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx index 9dddbb96ee4..2aed00d6741 100644 --- a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx @@ -102,8 +102,8 @@ describe('CreateImageTab', () => { await findByText('Image scheduled for creation.'); }); - it('should render a notice if the user selects a Linode in an edge region', async () => { - const region = regionFactory.build({ site_type: 'edge' }); + it('should render a notice if the user selects a Linode in a distributed compute region', async () => { + const region = regionFactory.build({ site_type: 'distributed' }); const linode = linodeFactory.build({ region: region.id }); server.use( @@ -128,7 +128,7 @@ describe('CreateImageTab', () => { await userEvent.click(linodeOption); - // Verify edge notice renders + // Verify distributed compute region notice renders await findByText( 'This Linode is in a distributed compute region. Images captured from this Linode will be stored in the closest core site.' ); @@ -139,7 +139,7 @@ describe('CreateImageTab', () => { ); }); - it('should render an encryption notice if disk encryption is enabled and the Linode is not in an edge region', async () => { + it('should render an encryption notice if disk encryption is enabled and the Linode is not in a distributed compute region', async () => { const region = regionFactory.build({ site_type: 'core' }); const linode = linodeFactory.build({ region: region.id }); From e0d14d1fd961063fb938949f79370773f7c1e214 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 4 Jun 2024 15:42:42 -0400 Subject: [PATCH 4/7] Added changeset: Informational notice about capturing an image from a Linode in a distributed compute region --- packages/manager/.changeset/pr-10544-added-1717530161965.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-10544-added-1717530161965.md diff --git a/packages/manager/.changeset/pr-10544-added-1717530161965.md b/packages/manager/.changeset/pr-10544-added-1717530161965.md new file mode 100644 index 00000000000..f1a78d8ef7e --- /dev/null +++ b/packages/manager/.changeset/pr-10544-added-1717530161965.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Added +--- + +Informational notice about capturing an image from a Linode in a distributed compute region ([#10544](https://github.com/linode/manager/pull/10544)) From ba513a7a6890e767ebba021e7d828f8f48c74cac Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Fri, 7 Jun 2024 12:57:11 -0400 Subject: [PATCH 5/7] increace overall spacing @hkhalil-akamai --- .../src/features/Images/ImagesCreate/CreateImageTab.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx index 50dd95da200..053cede60ae 100644 --- a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx @@ -156,7 +156,7 @@ export const CreateImageTab = () => { variant="error" /> )} - + Select Linode & Disk By default, Linode images are limited to 6144 MB of data per disk. @@ -252,7 +252,7 @@ export const CreateImageTab = () => { - + Image Details ( From 842798847a7b1c3f7c6ce6189c72e814939db3e8 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Wed, 12 Jun 2024 10:24:15 -0400 Subject: [PATCH 6/7] remove helper text as requested by ux --- .../Images/ImagesCreate/CreateImageTab.tsx | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx index 053cede60ae..b88e8a6d97a 100644 --- a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.tsx @@ -124,20 +124,11 @@ export const CreateImageTab = () => { selectedLinodeId !== null && !linodeIsInDistributedRegion; - const linodeSelectHelperText = [ - { - show: linodeIsInDistributedRegion, - text: `Image will be stored in the closest core site to (${linode?.region})`, - }, - { - show: grants?.linode.some((grant) => grant.permissions === 'read_only'), - text: - 'You can only create Images from Linodes you have read/write access to.', - }, - ] - .filter((item) => item.show) - .map((item) => item.text) - .join('. '); + const linodeSelectHelperText = grants?.linode.some( + (grant) => grant.permissions === 'read_only' + ) + ? 'You can only create Images from Linodes you have read/write access to.' + : undefined; return ( From 87cbd107e3d1291aea462882c719d1fa052cd953 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Wed, 12 Jun 2024 10:48:55 -0400 Subject: [PATCH 7/7] remove unit test for helper text --- .../src/features/Images/ImagesCreate/CreateImageTab.test.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx index 2aed00d6741..cdda9cd1340 100644 --- a/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx +++ b/packages/manager/src/features/Images/ImagesCreate/CreateImageTab.test.tsx @@ -132,11 +132,6 @@ describe('CreateImageTab', () => { await findByText( 'This Linode is in a distributed compute region. Images captured from this Linode will be stored in the closest core site.' ); - - // Verify helper text renders - await findByText( - `Image will be stored in the closest core site to (${linode.region})` - ); }); it('should render an encryption notice if disk encryption is enabled and the Linode is not in a distributed compute region', async () => {