From 0cbb976196bb288d0a4660d3de386f704947a0ee Mon Sep 17 00:00:00 2001 From: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:45:15 -0400 Subject: [PATCH] refactor: [M3-8720] - One `ImageSelect` to rule them all (#11058) * Initial commit - console errors and low hanging fruits * Renaming and moving things * Small clanup * One ImageSelect to rule them all * One ImageSelect to rule them all * fixing e2e tests * save progress * cleanup * cleanup * post rebase fix * fix the console errors with autocomplete * sorting and ordering * Added changeset: Consolidate ImageSelect components * feedback @bnussman-akamai * fix rebase gone wrong * feedback @coliu-akamai --- .../pr-11058-tech-stories-1729621320539.md | 5 + .../e2e/core/linodes/rebuild-linode.spec.ts | 57 +-- .../stackscripts/create-stackscripts.spec.ts | 8 +- .../stackscripts/update-stackscripts.spec.ts | 6 +- .../manager/src/components/Dialog/Dialog.tsx | 107 ++--- .../ImageOption.test.tsx} | 29 +- .../components/ImageSelect/ImageOption.tsx | 93 ++-- .../ImageSelect/ImageSelect.stories.tsx | 16 + .../ImageSelect/ImageSelect.test.tsx | 321 +++++++++----- .../components/ImageSelect/ImageSelect.tsx | 398 ++++++++---------- .../utilities.test.ts | 0 .../utilities.ts | 8 +- .../ImageSelectv2/ImageOptionv2.tsx | 59 --- .../ImageSelectv2/ImageSelectv2.stories.tsx | 16 - .../ImageSelectv2/ImageSelectv2.test.tsx | 144 ------- .../ImageSelectv2/ImageSelectv2.tsx | 130 ------ .../src/containers/images.container.tsx | 44 -- .../src/features/Images/ImageSelect.test.tsx | 88 ---- .../src/features/Images/ImageSelect.tsx | 100 ----- .../Linodes/LinodeCreate/Tabs/Images.tsx | 8 +- .../LinodeCreate/Tabs/OperatingSystems.tsx | 4 +- .../Tabs/StackScripts/StackScriptImages.tsx | 6 +- .../LinodeRebuild/LinodeRebuildDialog.tsx | 21 +- .../LinodeRebuild/RebuildFromImage.tsx | 41 +- .../LinodeRebuild/RebuildFromStackScript.tsx | 20 +- .../LinodeSettings/ImageAndPassword.test.tsx | 1 + .../LinodeSettings/ImageAndPassword.tsx | 19 +- .../LinodeStorage/CreateDiskDrawer.tsx | 1 + .../StackScriptCreate.test.tsx | 6 +- .../StackScriptCreate/StackScriptCreate.tsx | 17 +- .../StackScriptForm/StackScriptForm.test.tsx | 5 +- .../StackScriptForm/StackScriptForm.tsx | 31 +- packages/manager/src/utilities/images.ts | 22 - 33 files changed, 626 insertions(+), 1205 deletions(-) create mode 100644 packages/manager/.changeset/pr-11058-tech-stories-1729621320539.md rename packages/manager/src/components/{ImageSelectv2/ImageOptionv2.test.tsx => ImageSelect/ImageOption.test.tsx} (69%) create mode 100644 packages/manager/src/components/ImageSelect/ImageSelect.stories.tsx rename packages/manager/src/components/{ImageSelectv2 => ImageSelect}/utilities.test.ts (100%) rename packages/manager/src/components/{ImageSelectv2 => ImageSelect}/utilities.ts (93%) delete mode 100644 packages/manager/src/components/ImageSelectv2/ImageOptionv2.tsx delete mode 100644 packages/manager/src/components/ImageSelectv2/ImageSelectv2.stories.tsx delete mode 100644 packages/manager/src/components/ImageSelectv2/ImageSelectv2.test.tsx delete mode 100644 packages/manager/src/components/ImageSelectv2/ImageSelectv2.tsx delete mode 100644 packages/manager/src/containers/images.container.tsx delete mode 100644 packages/manager/src/features/Images/ImageSelect.test.tsx delete mode 100644 packages/manager/src/features/Images/ImageSelect.tsx delete mode 100644 packages/manager/src/utilities/images.ts diff --git a/packages/manager/.changeset/pr-11058-tech-stories-1729621320539.md b/packages/manager/.changeset/pr-11058-tech-stories-1729621320539.md new file mode 100644 index 00000000000..de8b6082d8f --- /dev/null +++ b/packages/manager/.changeset/pr-11058-tech-stories-1729621320539.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Tech Stories +--- + +Consolidate ImageSelect components ([#11058](https://github.com/linode/manager/pull/11058)) diff --git a/packages/manager/cypress/e2e/core/linodes/rebuild-linode.spec.ts b/packages/manager/cypress/e2e/core/linodes/rebuild-linode.spec.ts index c4b77519d83..157e43dda05 100644 --- a/packages/manager/cypress/e2e/core/linodes/rebuild-linode.spec.ts +++ b/packages/manager/cypress/e2e/core/linodes/rebuild-linode.spec.ts @@ -131,13 +131,13 @@ describe('rebuild linode', () => { openRebuildDialog(linode.label); findRebuildDialog(linode.label).within(() => { // "From Image" should be selected by default; no need to change the value. - ui.select.findByText('From Image').should('be.visible'); - - ui.select - .findByText('Choose an image') + ui.autocomplete + .findByLabel('From Image') .should('be.visible') - .click() - .type(`${image}{enter}`); + .should('have.value', 'From Image'); + + ui.autocomplete.findByLabel('Images').should('be.visible').click(); + ui.autocompletePopper.findByTitle(image).should('be.visible').click(); // Type to confirm. cy.findByLabelText('Linode Label').type(linode.label); @@ -186,10 +186,9 @@ describe('rebuild linode', () => { openRebuildDialog(linode.label); findRebuildDialog(linode.label).within(() => { - ui.select.findByText('From Image').click(); - - ui.select - .findItemByText('From Community StackScript') + ui.autocomplete.findByLabel('From Image').should('be.visible').click(); + ui.autocompletePopper + .findByTitle('From Community StackScript') .should('be.visible') .click(); @@ -203,13 +202,8 @@ describe('rebuild linode', () => { cy.get(`[id="${stackScriptId}"][type="radio"]`).click(); }); - ui.select - .findByText('Choose an image') - .scrollIntoView() - .should('be.visible') - .click(); - - ui.select.findItemByText(image).should('be.visible').click(); + ui.autocomplete.findByLabel('Images').should('be.visible').click(); + ui.autocompletePopper.findByTitle(image).should('be.visible').click(); cy.findByLabelText('Linode Label') .should('be.visible') @@ -229,7 +223,7 @@ describe('rebuild linode', () => { */ it('rebuilds a linode from Account StackScript', () => { cy.tag('method:e2e'); - const image = 'Alpine'; + const image = 'Alpine 3.18'; const region = 'us-east'; // Create a StackScript to rebuild a Linode. @@ -265,10 +259,9 @@ describe('rebuild linode', () => { openRebuildDialog(linode.label); findRebuildDialog(linode.label).within(() => { - ui.select.findByText('From Image').should('be.visible').click(); - - ui.select - .findItemByText('From Account StackScript') + ui.autocomplete.findByLabel('From Image').should('be.visible').click(); + ui.autocompletePopper + .findByTitle('From Account StackScript') .should('be.visible') .click(); @@ -280,13 +273,8 @@ describe('rebuild linode', () => { cy.get(`[id="${stackScript.id}"][type="radio"]`).click(); }); - ui.select - .findByText('Choose an image') - .scrollIntoView() - .should('be.visible') - .click(); - - ui.select.findItemByText(image).should('be.visible').click(); + ui.autocomplete.findByLabel('Images').should('be.visible').click(); + ui.autocompletePopper.findByTitle(image).should('be.visible').click(); cy.findByLabelText('Linode Label') .should('be.visible') @@ -319,14 +307,13 @@ describe('rebuild linode', () => { cy.visitWithLogin(`/linodes/${mockLinode.id}?rebuild=true`); findRebuildDialog(mockLinode.label).within(() => { - ui.select.findByText('From Image').should('be.visible'); - ui.select - .findByText('Choose an image') + ui.autocomplete.findByLabel('From Image').should('be.visible'); + ui.autocomplete + .findByLabel('Images') .should('be.visible') .click() - .type(`${image}`); - - ui.select.findItemByText(image).should('be.visible').click(); + .type(image); + ui.autocompletePopper.findByTitle(image).should('be.visible').click(); assertPasswordComplexity(rootPassword, 'Good'); diff --git a/packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts b/packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts index c2fa9e65557..5cfce4ac762 100644 --- a/packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts +++ b/packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts @@ -20,7 +20,7 @@ import { cleanUp } from 'support/util/cleanup'; import { createTestLinode } from 'support/util/linodes'; import { interceptGetAllImages } from 'support/intercepts/images'; import type { Image } from '@linode/api-v4'; -import { getFilteredImagesForImageSelect } from 'src/components/ImageSelectv2/utilities'; +import { getFilteredImagesForImageSelect } from 'src/components/ImageSelect/utilities'; // StackScript fixture paths. const stackscriptBasicPath = 'stackscripts/stackscript-basic.sh'; @@ -80,9 +80,9 @@ const fillOutStackscriptForm = ( .type(description); } - cy.findByText('Target Images').click().type(`${targetImage}`); - - cy.findByText(`${targetImage}`).should('be.visible').click(); + ui.autocomplete.findByLabel('Target Images').should('be.visible').click(); + ui.autocompletePopper.findByTitle(targetImage).should('be.visible').click(); + ui.autocomplete.findByLabel('Target Images').click(); // Close autocomplete popper // Insert a script. inputStackScript(script); diff --git a/packages/manager/cypress/e2e/core/stackscripts/update-stackscripts.spec.ts b/packages/manager/cypress/e2e/core/stackscripts/update-stackscripts.spec.ts index 40ca71afe7a..66d888d546c 100644 --- a/packages/manager/cypress/e2e/core/stackscripts/update-stackscripts.spec.ts +++ b/packages/manager/cypress/e2e/core/stackscripts/update-stackscripts.spec.ts @@ -55,9 +55,9 @@ const fillOutStackscriptForm = ( .type(description); } - cy.findByText('Target Images').click().type(targetImage); - - cy.findByText(targetImage).should('be.visible').click(); + ui.autocomplete.findByLabel('Target Images').should('be.visible').click(); + ui.autocompletePopper.findByTitle(targetImage).should('be.visible').click(); + ui.autocomplete.findByLabel('Target Images').click(); // Close autocomplete popper // Insert a script with invalid UDF data. cy.get('[data-qa-textfield-label="Script"]') diff --git a/packages/manager/src/components/Dialog/Dialog.tsx b/packages/manager/src/components/Dialog/Dialog.tsx index 5c4db71a634..1ab4366ca12 100644 --- a/packages/manager/src/components/Dialog/Dialog.tsx +++ b/packages/manager/src/components/Dialog/Dialog.tsx @@ -40,64 +40,67 @@ export interface DialogProps extends _DialogProps { * > A modal can only be closed by taking direct action, clicking on a button or the “X” button, or using the `esc` key. * */ -export const Dialog = (props: DialogProps) => { - const theme = useTheme(); - const { - children, - className, - error, - fullHeight, - fullWidth, - maxWidth = 'md', - onClose, - subtitle, - title, - titleBottomBorder, - ...rest - } = props; +export const Dialog = React.forwardRef( + (props: DialogProps, ref: React.Ref) => { + const theme = useTheme(); + const { + children, + className, + error, + fullHeight, + fullWidth, + maxWidth = 'md', + onClose, + subtitle, + title, + titleBottomBorder, + ...rest + } = props; - const titleID = convertForAria(title); + const titleID = convertForAria(title); - return ( - - - onClose && onClose({}, 'backdropClick')} - subtitle={subtitle} - title={title} - /> - {titleBottomBorder && } - - {error && } - {children} - - - - ); -}; + onClose && onClose({}, 'backdropClick')} + subtitle={subtitle} + title={title} + /> + {titleBottomBorder && } + + {error && } + {children} + + + + ); + } +); const StyledDialog = styled(_Dialog, { shouldForwardProp: omittedProps(['fullHeight', 'title']), diff --git a/packages/manager/src/components/ImageSelectv2/ImageOptionv2.test.tsx b/packages/manager/src/components/ImageSelect/ImageOption.test.tsx similarity index 69% rename from packages/manager/src/components/ImageSelectv2/ImageOptionv2.test.tsx rename to packages/manager/src/components/ImageSelect/ImageOption.test.tsx index 4c6ddfaab35..8b704581f62 100644 --- a/packages/manager/src/components/ImageSelectv2/ImageOptionv2.test.tsx +++ b/packages/manager/src/components/ImageSelect/ImageOption.test.tsx @@ -3,32 +3,34 @@ import React from 'react'; import { imageFactory } from 'src/factories'; import { renderWithTheme } from 'src/utilities/testHelpers'; -import { ImageOptionv2 } from './ImageOptionv2'; +import { ImageOption } from './ImageOption'; -describe('ImageOptionv2', () => { +describe('ImageOption', () => { it('renders the image label', () => { const image = imageFactory.build({ eol: null }); const { getByText } = renderWithTheme( - + ); expect(getByText(image.label)).toBeVisible(); }); + it('renders an OS icon', () => { const image = imageFactory.build(); const { getByTestId } = renderWithTheme( - + ); expect(getByTestId('os-icon')).toBeVisible(); }); + it('renders a metadata (cloud-init) icon if the flag is on and the image supports cloud-init', () => { const image = imageFactory.build({ capabilities: ['cloud-init'] }); const { getByLabelText } = renderWithTheme( - , + , { flags: { metadata: true } } ); @@ -36,11 +38,12 @@ describe('ImageOptionv2', () => { getByLabelText('This image supports our Metadata service via cloud-init.') ).toBeVisible(); }); + it('renders a distributed icon if image has the "distributed-sites" capability', () => { const image = imageFactory.build({ capabilities: ['distributed-sites'] }); const { getByLabelText } = renderWithTheme( - + ); expect( @@ -54,7 +57,7 @@ describe('ImageOptionv2', () => { const image = imageFactory.build({ deprecated: true }); const { getByText } = renderWithTheme( - + ); expect(getByText(`${image.label} (deprecated)`)).toBeVisible(); @@ -67,9 +70,19 @@ describe('ImageOptionv2', () => { }); const { getByText } = renderWithTheme( - + ); expect(getByText(`${image.label} (deprecated)`)).toBeVisible(); }); + + it('should show the selected icon when isSelected is true', () => { + const image = imageFactory.build(); + + const { getByTestId } = renderWithTheme( + + ); + + expect(getByTestId('DoneIcon')).toBeVisible(); + }); }); diff --git a/packages/manager/src/components/ImageSelect/ImageOption.tsx b/packages/manager/src/components/ImageSelect/ImageOption.tsx index f1fe9a3ac76..c1f229ca958 100644 --- a/packages/manager/src/components/ImageSelect/ImageOption.tsx +++ b/packages/manager/src/components/ImageSelect/ImageOption.tsx @@ -1,92 +1,61 @@ import { Tooltip } from '@linode/ui'; import React from 'react'; -import { makeStyles } from 'tss-react/mui'; import CloudInitIcon from 'src/assets/icons/cloud-init.svg'; import DistributedRegionIcon from 'src/assets/icons/entityIcons/distributed-region.svg'; -import { Box } from 'src/components/Box'; -import { Option } from 'src/components/EnhancedSelect/components/Option'; import { useFlags } from 'src/hooks/useFlags'; +import { SelectedIcon } from '../Autocomplete/Autocomplete.styles'; +import { OSIcon } from '../OSIcon'; import { Stack } from '../Stack'; +import { Typography } from '../Typography'; +import { isImageDeprecated } from './utilities'; -import type { ImageItem } from './ImageSelect'; -import type { Theme } from '@mui/material/styles'; -import type { OptionProps } from 'react-select'; +import type { Image } from '@linode/api-v4'; -const useStyles = makeStyles()((theme: Theme) => ({ - distroIcon: { - fontSize: '1.8em', - - margin: `0 ${theme.spacing()}`, - [theme.breakpoints.only('xs')]: { - fontSize: '1.52em', - }, - }, - focused: { - '& g': { - fill: theme.tokens.color.Neutrals.White, - }, - backgroundColor: theme.palette.primary.main, - color: theme.tokens.color.Neutrals.White, - }, - root: { - '& *': { - lineHeight: '1.2em', - }, - '& g': { - fill: - theme.name === 'dark' ? theme.tokens.color.Neutrals.White : '#888f91', - }, - display: 'flex !important', - flexDirection: 'row', - justifyContent: 'space-between', - padding: '2px 8px !important', // Revisit use of important when we refactor the Select component - }, - selected: { - '& g': { - fill: theme.palette.primary.main, - }, - }, -})); - -interface ImageOptionProps extends OptionProps { - data: ImageItem; +interface Props { + image: Image; + isSelected: boolean; + listItemProps: Omit, 'key'>; } -export const ImageOption = (props: ImageOptionProps) => { - const { classes, cx } = useStyles(); - const { data, isFocused, isSelected, label } = props; +export const ImageOption = ({ image, isSelected, listItemProps }: Props) => { const flags = useFlags(); return ( - + ); }; diff --git a/packages/manager/src/components/ImageSelect/ImageSelect.stories.tsx b/packages/manager/src/components/ImageSelect/ImageSelect.stories.tsx new file mode 100644 index 00000000000..d7d9a56a200 --- /dev/null +++ b/packages/manager/src/components/ImageSelect/ImageSelect.stories.tsx @@ -0,0 +1,16 @@ +import * as React from 'react'; + +import { ImageSelect } from './ImageSelect'; + +import type { Meta, StoryObj } from '@storybook/react'; + +export const Default: StoryObj = { + render: (args) => , +}; + +const meta: Meta = { + component: ImageSelect, + title: 'Components/Selects/Image Select', +}; + +export default meta; diff --git a/packages/manager/src/components/ImageSelect/ImageSelect.test.tsx b/packages/manager/src/components/ImageSelect/ImageSelect.test.tsx index ff31a28981a..1947fb7cc6d 100644 --- a/packages/manager/src/components/ImageSelect/ImageSelect.test.tsx +++ b/packages/manager/src/components/ImageSelect/ImageSelect.test.tsx @@ -1,121 +1,252 @@ -import { DateTime } from 'luxon'; +import userEvent from '@testing-library/user-event'; +import { Settings } from 'luxon'; import React from 'react'; import { imageFactory } from 'src/factories'; +import { makeResourcePage } from 'src/mocks/serverHandlers'; +import { HttpResponse, http, server } from 'src/mocks/testServer'; import { renderWithTheme } from 'src/utilities/testHelpers'; -import { ImageSelect, imagesToGroupedItems } from './ImageSelect'; +import { ImageSelect } from './ImageSelect'; + +describe('ImageSelect', () => { + it('should render a default "Images" label', () => { + const { getByLabelText } = renderWithTheme( + + ); + + expect(getByLabelText('Images')).toBeVisible(); + }); + + it('should render default placeholder text', () => { + const { getByPlaceholderText } = renderWithTheme( + + ); + + expect(getByPlaceholderText('Choose an image')).toBeVisible(); + }); + + it('should render items returned by the API', async () => { + const images = imageFactory.buildList(5, { eol: null }); + + server.use( + http.get('*/v4/images', () => { + return HttpResponse.json(makeResourcePage(images)); + }) + ); + + const { getByPlaceholderText, getByText } = renderWithTheme( + + ); + + await userEvent.click(getByPlaceholderText('Choose an image')); + + for (const image of images) { + expect(getByText(image.label)).toBeVisible(); + } + }); + + it('should call onChange when a value is selected', async () => { + const image = imageFactory.build({ eol: null }); + const onChange = vi.fn(); + + server.use( + http.get('*/v4/images', () => { + return HttpResponse.json(makeResourcePage([image])); + }) + ); + + const { getByPlaceholderText, getByText } = renderWithTheme( + + ); + + await userEvent.click(getByPlaceholderText('Choose an image')); + + const imageOption = getByText(image.label); + + expect(imageOption).toBeVisible(); + + await userEvent.click(imageOption); + + expect(onChange).toHaveBeenCalledWith(image); + }); + + it('should correctly initialize with a default value', async () => { + const image = imageFactory.build(); + + server.use( + http.get('*/v4/images', () => { + return HttpResponse.json(makeResourcePage([image])); + }) + ); + + const { findByDisplayValue } = renderWithTheme( + + ); + + await findByDisplayValue(image.label); + }); + + it('should render an OS icon for the selected Image', async () => { + const image = imageFactory.build(); + + server.use( + http.get('*/v4/images', () => { + return HttpResponse.json(makeResourcePage([image])); + }) + ); + + const { findByTestId } = renderWithTheme( + + ); + + await findByTestId('os-icon'); + }); + + it('does not render images that are more than 6 months past their eol', async () => { + // Mock the current date + Settings.now = () => new Date(2018, 1, 1).valueOf(); -describe('imagesToGroupedItems', () => { - it('should filter deprecated images when end of life is past beyond 6 months ', () => { const images = [ - ...imageFactory.buildList(2, { - created: '2017-06-16T20:02:29', - deprecated: true, - eol: '2022-01-01T14:05:30', - label: 'Debian 9', + imageFactory.build({ + eol: '2018-04-01T00:00:00', // should show because this image is not EOL yet + label: 'linode/image-1', }), - ...imageFactory.buildList(2, { - created: '2017-06-16T20:02:29', - deprecated: false, - eol: '1970-01-01T14:05:30', - label: 'Debian 10', + imageFactory.build({ + eol: '2017-01-01T00:00:00', // should not show because it is > 6 months past this EOL + label: 'linode/image-2', }), - ...imageFactory.buildList(2, { - created: '2022-10-20T14:05:30', - deprecated: true, - eol: null, - label: 'Slackware 14.1', + imageFactory.build({ + eol: null, // should show because this images does not have an EOL + label: 'linode/image-3', + }), + imageFactory.build({ + eol: '2017-11-01T00:00:00', // should show as deprecated because it is < 6 months past this EOL + label: 'linode/image-4', }), ]; - const expected = [ - { - label: 'My Images', - options: [ - { - className: 'fl-tux', - created: '2022-10-20T14:05:30', - isCloudInitCompatible: false, - isDistributedCompatible: false, - label: 'Slackware 14.1', - value: 'private/5', - }, - { - className: 'fl-tux', - created: '2022-10-20T14:05:30', - isCloudInitCompatible: false, - isDistributedCompatible: false, - label: 'Slackware 14.1', - value: 'private/6', - }, - ], - }, - ]; - expect(imagesToGroupedItems(images)).toStrictEqual(expected); + server.use( + http.get('*/v4/images', () => { + return HttpResponse.json(makeResourcePage(images)); + }) + ); + + const { getByPlaceholderText, getByText, queryByText } = renderWithTheme( + + ); + + await userEvent.click(getByPlaceholderText('Choose an image')); + + expect(getByText('linode/image-1')).toBeVisible(); + expect(queryByText('linode/image-2')).toBeNull(); + expect(getByText('linode/image-3')).toBeVisible(); + expect(getByText('linode/image-4 (deprecated)')).toBeVisible(); }); - it('should add suffix `deprecated` to images at end of life ', () => { - const images = [ - ...imageFactory.buildList(2, { - created: '2017-06-16T20:02:29', - deprecated: true, - eol: DateTime.now().toISODate(), - label: 'Debian 9', + it('should display an error', () => { + const { getByText } = renderWithTheme( + + ); + expect(getByText('An error')).toBeInTheDocument(); + }); + + it('should handle any/all', async () => { + const onSelect = vi.fn(); + + const { getByLabelText, getByText } = renderWithTheme( + + ); + + await userEvent.click(getByLabelText('Images')); + + await userEvent.click(getByText('Any/All')); + + expect(onSelect).toHaveBeenCalledWith([ + expect.objectContaining({ id: 'any/all' }), + ]); + }); + + it('should sort images by vendor, then by creation date, then "My Images" first', async () => { + const publicImages = [ + imageFactory.build({ + created: '2023-01-01T00:00:00', + is_public: true, + label: 'Public Image 1', + vendor: 'CentOS', + }), + imageFactory.build({ + created: '2023-02-01T00:00:00', + is_public: true, + label: 'Public Image 2', + vendor: 'Debian', }), - ...imageFactory.buildList(2, { - created: '2017-06-16T20:02:29', - deprecated: false, - eol: '1970-01-01T14:05:30', - label: 'Debian 10', + imageFactory.build({ + created: '2023-03-01T00:00:00', + is_public: true, + label: 'Public Image 3', + vendor: 'Ubuntu', }), ]; - const expected = [ - { - label: 'My Images', - options: [ - { - className: 'fl-tux', - created: '2017-06-16T20:02:29', - isCloudInitCompatible: false, - isDistributedCompatible: false, - label: 'Debian 9 (deprecated)', - value: 'private/7', - }, - { - className: 'fl-tux', - created: '2017-06-16T20:02:29', - isCloudInitCompatible: false, - isDistributedCompatible: false, - label: 'Debian 9 (deprecated)', - value: 'private/8', - }, - ], - }, + const privateImages = [ + imageFactory.build({ + created: '2023-04-01T00:00:00', + is_public: false, + label: 'Private Image 1', + }), + imageFactory.build({ + created: '2023-05-01T00:00:00', + is_public: false, + label: 'Private Image 2', + }), ]; - expect(imagesToGroupedItems(images)).toStrictEqual(expected); - }); -}); -describe('ImageSelect', () => { - it('renders a "Indicates compatibility with distributed compute regions." notice if the user has at least one image with the distributed capability', async () => { - const images = [ - imageFactory.build({ capabilities: [] }), - imageFactory.build({ capabilities: ['distributed-sites'] }), - imageFactory.build({ capabilities: [] }), - ]; + // The API returns private images first, then public images + // Granted this won't change, I don't think we need to filter them client side + // Therefore this test assumes the API initial sort order + const images = [...privateImages, ...publicImages]; - const { getByText } = renderWithTheme( + const { getAllByRole, getByLabelText, getByText } = renderWithTheme( ); - expect( - getByText('Indicates compatibility with distributed compute regions.') - ).toBeVisible(); + await userEvent.click(getByLabelText('Images')); + expect(getByText('My Images')).toBeInTheDocument(); + expect(getByText('CentOS')).toBeInTheDocument(); + expect(getByText('Debian')).toBeInTheDocument(); + expect(getByText('Ubuntu')).toBeInTheDocument(); + + const options = getAllByRole('option'); + + expect(getByText('My Images')).toBeInTheDocument(); + expect(getByText('CentOS')).toBeInTheDocument(); + expect(getByText('Debian')).toBeInTheDocument(); + expect(getByText('Ubuntu')).toBeInTheDocument(); + + // Assert that private images ("My Images") come first + expect(options[0].textContent?.trim()).toContain('Private Image 1'); + expect(options[1].textContent?.trim()).toContain('Private Image 2'); + + // Assert the order of public images by vendor + expect(options[2].textContent?.trim()).toContain('Public Image 1'); + expect(options[3].textContent?.trim()).toContain('Public Image 2'); + expect(options[4].textContent?.trim()).toContain('Public Image 3'); }); }); diff --git a/packages/manager/src/components/ImageSelect/ImageSelect.tsx b/packages/manager/src/components/ImageSelect/ImageSelect.tsx index 92e43254322..c7f2ad94e12 100644 --- a/packages/manager/src/components/ImageSelect/ImageSelect.tsx +++ b/packages/manager/src/components/ImageSelect/ImageSelect.tsx @@ -1,257 +1,189 @@ -import produce from 'immer'; -import { DateTime } from 'luxon'; -import { equals, groupBy } from 'ramda'; -import * as React from 'react'; +import React, { useMemo } from 'react'; -import DistributedRegionIcon from 'src/assets/icons/entityIcons/distributed-region.svg'; -import Select from 'src/components/EnhancedSelect'; -import { _SingleValue } from 'src/components/EnhancedSelect/components/SingleValue'; -import { ImageOption } from 'src/components/ImageSelect/ImageOption'; -import { Paper } from 'src/components/Paper'; -import { Typography } from 'src/components/Typography'; -import { MAX_MONTHS_EOL_FILTER } from 'src/constants'; +import { Autocomplete } from 'src/components/Autocomplete/Autocomplete'; +import { imageFactory } from 'src/factories/images'; import { useAllImagesQuery } from 'src/queries/images'; -import { arePropsEqual } from 'src/utilities/arePropsEqual'; -import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; -import { getSelectedOptionFromGroupedOptions } from 'src/utilities/getSelectedOptionFromGroupedOptions'; -import { Box } from '../Box'; -import { OS_ICONS } from '../OSIcon'; -import { Stack } from '../Stack'; - -import type { Image } from '@linode/api-v4/lib/images'; -import type { GroupType, Item } from 'src/components/EnhancedSelect'; -import type { BaseSelectProps } from 'src/components/EnhancedSelect/Select'; - -export type Variant = 'all' | 'private' | 'public'; - -export interface ImageItem extends Item { - className: string; - created: string; - isCloudInitCompatible: boolean; - isDistributedCompatible: boolean; -} - -interface ImageSelectProps { - classNames?: string; - disabled?: boolean; - error?: string; - handleSelectImage: ( - selection: string | undefined, - image: Image | undefined - ) => void; - images: Image[]; +import { OSIcon } from '../OSIcon'; +import { ImageOption } from './ImageOption'; +import { + getAPIFilterForImageSelect, + getFilteredImagesForImageSelect, +} from './utilities'; + +import type { Image } from '@linode/api-v4'; +import type { EnhancedAutocompleteProps } from 'src/components/Autocomplete/Autocomplete'; + +export type ImageSelectVariant = 'all' | 'private' | 'public'; + +interface BaseProps + extends Omit< + Partial>, + 'multiple' | 'onChange' | 'value' + > { + anyAllOption?: boolean; + filter?: (image: Image) => boolean; + groupBy?: (image: Image) => string; label?: string; placeholder?: string; - selectedImageID?: string; - title: string; - variant?: Variant; + selectIfOnlyOneOption?: boolean; + variant: ImageSelectVariant; } -export interface ImageProps - extends Omit, 'onChange' | 'variant'> { - disabled: boolean; - error?: string; - handleSelectImage: (selection?: string) => void; - images: Image[]; - selectedImageID?: string; +interface SingleProps extends BaseProps { + multiple?: false; + onChange: (selected: Image | null) => void; + value: ((image: Image) => boolean) | null | string; } -export const sortByImageVersion = (a: ImageItem, b: ImageItem) => { - if (a.created < b.created) { - return 1; - } - if (a.created > b.created) { - return -1; - } - return 0; -}; - -export const sortGroupsWithMyImagesAtTheBeginning = (a: string, b: string) => { - if (a === 'My Images') { - return -1; - } - if (b === 'My Images') { - return 1; - } - if (a > b) { - return 1; - } - if (a < b) { - return -1; - } - return 0; -}; - -export const imagesToGroupedItems = (images: Image[]) => { - const groupedImages = groupBy((eachImage: Image) => { - return eachImage.vendor || 'My Images'; - }, images); +interface MultiProps extends BaseProps { + multiple: true; + onChange: (selected: Image[]) => void; + value: ((image: Image) => boolean) | null | string[]; +} - return Object.keys(groupedImages) - .sort(sortGroupsWithMyImagesAtTheBeginning) - .reduce((accum: GroupType[], thisGroup) => { - const group = groupedImages[thisGroup]; - if (!group || group.length === 0) { - return accum; - } - return produce(accum, (draft) => { - draft.push({ - label: thisGroup, - options: group - .reduce((acc: ImageItem[], thisImage) => { - const { - capabilities, - created, - eol, - id, - label, - vendor, - } = thisImage; - const differenceInMonths = DateTime.now().diff( - DateTime.fromISO(eol!), - 'months' - ).months; - // if image is past its end of life, hide it, otherwise show it - if (!eol || differenceInMonths < MAX_MONTHS_EOL_FILTER) { - acc.push({ - className: vendor - ? // Use Tux as a fallback. - `fl-${OS_ICONS[vendor as keyof typeof OS_ICONS] ?? 'tux'}` - : `fl-tux`, - created, - isCloudInitCompatible: capabilities?.includes('cloud-init'), - isDistributedCompatible: capabilities?.includes( - 'distributed-sites' - ), - // Add suffix 'deprecated' to the image at end of life. - label: - differenceInMonths > 0 ? `${label} (deprecated)` : label, - value: id, - }); - } +export type Props = MultiProps | SingleProps; - return acc; - }, []) - .sort(sortByImageVersion), - }); - }); - }, []); -}; - -const isMemo = (prevProps: ImageSelectProps, nextProps: ImageSelectProps) => { - return ( - equals(prevProps.images, nextProps.images) && - arePropsEqual( - ['selectedImageID', 'error', 'disabled', 'handleSelectImage'], - prevProps, - nextProps - ) - ); -}; - -/** - * @deprecated Start using ImageSelectv2 when possible - */ -export const ImageSelect = React.memo((props: ImageSelectProps) => { +export const ImageSelect = (props: Props) => { const { - classNames, - disabled, - error: errorText, - handleSelectImage, - images, + anyAllOption, + filter, label, + multiple, + onChange, placeholder, - selectedImageID, - title, + selectIfOnlyOneOption, variant, + ...rest } = props; - // Check for loading status and request errors in React Query - const { error, isLoading: _loading } = useAllImagesQuery(); - - const imageError = error - ? getAPIErrorOrDefault(error, 'Unable to load Images')[0].reason - : undefined; + const { data: images, error, isLoading } = useAllImagesQuery( + {}, + getAPIFilterForImageSelect(variant) + ); - const filteredImages = images.filter((thisImage) => { - switch (variant) { - case 'public': - /* - * Get all public images but exclude any Kubernetes images. - * We don't want them to show up as a selectable image to deploy since - * the Kubernetes images are used behind the scenes with LKE. - */ - return ( - thisImage.is_public && - thisImage.status === 'available' && - !thisImage.label.match(/kube/i) - ); - case 'private': - return !thisImage.is_public && thisImage.status === 'available'; - case 'all': - // We don't show images with 'kube' in the label that are created by Linode - return !( - thisImage.label.match(/kube/i) && thisImage.created_by === 'linode' - ); - default: - return true; + const _options = useMemo(() => { + // We can't filter out Kubernetes images using the API so we do it client side + const filteredOptions = + getFilteredImagesForImageSelect(images, variant) ?? []; + + return filter ? filteredOptions.filter(filter) : filteredOptions; + }, [images, filter, variant]); + + const options = useMemo(() => { + if (anyAllOption) { + return [ + imageFactory.build({ + eol: undefined, + id: 'any/all', + label: 'Any/All', + }), + ..._options, + ]; } - }); - - const options = imagesToGroupedItems(filteredImages); - - const onChange = (selection: ImageItem | null) => { - if (selection === null) { - return handleSelectImage(undefined, undefined); + return _options; + }, [anyAllOption, _options]); + + // We need to sort options when grouping in order to avoid duplicate headers + // see https://mui.com/material-ui/react-autocomplete/#grouped + // We want: + // - Vendors to be sorted alphabetically + // - "My Images" to be first + // - Images to be sorted by creation date, newest first + const sortedOptions = useMemo(() => { + const myImages = options.filter((option) => !option.is_public); + const otherImages = options.filter((option) => option.is_public); + + const sortedVendors = Array.from( + new Set(otherImages.map((img) => img.vendor)) + ).sort((a, b) => (a ?? '').localeCompare(b ?? '')); + + return [ + ...myImages.sort( + (a, b) => new Date(b.created).getTime() - new Date(a.created).getTime() + ), + ...sortedVendors.flatMap((vendor) => + otherImages + .filter((img) => img.vendor === vendor) + .sort( + (a, b) => + new Date(b.created).getTime() - new Date(a.created).getTime() + ) + ), + ]; + }, [options]); + + const selected = props.value; + const value = useMemo(() => { + if (multiple) { + return options.filter((option) => + Array.isArray(selected) ? selected.includes(option.id) : false + ); } + return options.find((option) => option.id === selected) ?? null; + }, [multiple, options, selected]); - const selectedImage = images.find((i) => i.id === selection.value); - - return handleSelectImage(selection.value, selectedImage); - }; - - const showDistributedCapabilityNotice = - variant === 'private' && - filteredImages.some((image) => - image.capabilities.includes('distributed-sites') - ); + if (options.length === 1 && onChange && selectIfOnlyOneOption && !multiple) { + onChange(options[0]); + } return ( - - - {title} - - -