Skip to content

Commit

Permalink
upcoming: [M3-7872] - Linode Create Refactor - StackScripts (#10367)
Browse files Browse the repository at this point in the history
* initial work

* filter options based on stackscript compatibility

* implement complex preselection logic

* clean up a bit

* implement details dialog

* fix some query param logic

* lots of behavior changes to match production

* fix image select clear behavior and fix table waypoint console error

* improve validation for when image is `null`

* first unit tests

* add lots of unit testing

* add stackscript event handler

* hook up validation packages for realtime validation

* use default validation behavior

* Revert "hook up validation packages for realtime validation"

This reverts commit f56e4ad.

* handle resets when switching tabs

* add changesets

* add some comments

* bold label and pre-select if only one option

* improve ux

* fix unit tests

* add comment

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
  • Loading branch information
bnussman-akamai and bnussman authored Apr 18, 2024
1 parent 7f2cf90 commit cffa916
Show file tree
Hide file tree
Showing 29 changed files with 1,088 additions and 72 deletions.
5 changes: 5 additions & 0 deletions packages/api-v4/.changeset/pr-10367-changed-1712868469030.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/api-v4": Changed
---

Allow `stackscript_id` to be `null` in `CreateLinodeRequest` ([#10367](https://github.com/linode/manager/pull/10367))
2 changes: 1 addition & 1 deletion packages/api-v4/src/linodes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ export interface CreateLinodeRequest {
*
* This field cannot be used when deploying from a Backup or a Private Image.
*/
stackscript_id?: number;
stackscript_id?: number | null;
/**
* A Backup ID from another Linode’s available backups.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Linode Create Refactor - StackScripts ([#10367](https://github.com/linode/manager/pull/10367))
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,7 @@ describe('ImageSelectv2', () => {

await userEvent.click(imageOption);

expect(onChange).toHaveBeenCalledWith(
expect.anything(),
image,
'selectOption',
expect.anything()
);
expect(onChange).toHaveBeenCalledWith(image);
});

it('should correctly initialize with a default value', async () => {
Expand Down
41 changes: 38 additions & 3 deletions packages/manager/src/components/ImageSelectv2/ImageSelectv2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,22 @@ import type { EnhancedAutocompleteProps } from 'src/components/Autocomplete/Auto
export type ImageSelectVariant = 'all' | 'private' | 'public';

interface Props
extends Omit<Partial<EnhancedAutocompleteProps<Image>>, 'value'> {
extends Omit<
Partial<EnhancedAutocompleteProps<Image>>,
'onChange' | 'value'
> {
/**
* Optional filter function applied to the options.
*/
filter?: (image: Image) => boolean;
/**
* Called when the value is changed
*/
onChange?: (image: Image | null) => void;
/**
* If there is only one avaiblable option, selected it by default.
*/
selectIfOnlyOneOption?: boolean;
/**
* The ID of the selected image
*/
Expand All @@ -30,7 +45,7 @@ interface Props
}

export const ImageSelectv2 = (props: Props) => {
const { variant, ...rest } = props;
const { filter, onChange, selectIfOnlyOneOption, variant, ...rest } = props;

const { data: images, error, isLoading } = useAllImagesQuery(
{},
Expand All @@ -40,8 +55,18 @@ export const ImageSelectv2 = (props: Props) => {
// We can't filter out Kubernetes images using the API so we filter them here
const options = getFilteredImagesForImageSelect(images, variant);

const filteredOptions = filter ? options?.filter(filter) : options;

const value = images?.find((i) => i.id === props.value);

if (
filteredOptions?.length === 1 &&
props.onChange &&
selectIfOnlyOneOption
) {
props.onChange(filteredOptions[0]);
}

return (
<Autocomplete
renderOption={(props, option, state) => (
Expand All @@ -52,12 +77,22 @@ export const ImageSelectv2 = (props: Props) => {
listItemProps={props}
/>
)}
clearOnBlur
groupBy={(option) => option.vendor ?? 'My Images'}
label="Images"
loading={isLoading}
options={options ?? []}
options={filteredOptions ?? []}
placeholder="Choose an image"
{...rest}
disableClearable={
rest.disableClearable ??
(selectIfOnlyOneOption && filteredOptions?.length === 1)
}
onChange={(e, image) => {
if (onChange) {
onChange(image);
}
}}
errorText={rest.errorText ?? error?.[0].reason}
value={value ?? null}
/>
Expand Down
6 changes: 3 additions & 3 deletions packages/manager/src/components/ImageSelectv2/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const getFilteredImagesForImageSelect = (
images: Image[] | undefined,
variant: ImageSelectVariant | undefined
) => {
return variant === 'public'
? images?.filter((image) => !image.id.includes('kube'))
: images;
return variant === 'private'
? images
: images?.filter((image) => !image.id.includes('kube'));
};
4 changes: 2 additions & 2 deletions packages/manager/src/components/PrimaryNav/PrimaryNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
useObjectStorageClusters,
} from 'src/queries/objectStorage';
import { useRegionsQuery } from 'src/queries/regions/regions';
import { useStackScriptsOCA } from 'src/queries/stackscripts';
import { useMarketplaceAppsQuery } from 'src/queries/stackscripts';
import { isFeatureEnabled } from 'src/utilities/accountCapabilities';

import useStyles from './PrimaryNav.styles';
Expand Down Expand Up @@ -116,7 +116,7 @@ export const PrimaryNav = (props: PrimaryNavProps) => {
data: oneClickApps,
error: oneClickAppsError,
isLoading: oneClickAppsLoading,
} = useStackScriptsOCA(enableMarketplacePrefetch);
} = useMarketplaceAppsQuery(enableMarketplacePrefetch);

const {
data: clusters,
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/containers/withMarketplaceApps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useLocation } from 'react-router-dom';

import { baseApps } from 'src/features/StackScripts/stackScriptUtils';
import { useFlags } from 'src/hooks/useFlags';
import { useStackScriptsOCA } from 'src/queries/stackscripts';
import { useMarketplaceAppsQuery } from 'src/queries/stackscripts';
import { getQueryParamFromQueryString } from 'src/utilities/queryParams';

const trimOneClickFromLabel = (script: StackScript) => {
Expand All @@ -31,7 +31,7 @@ export const withMarketplaceApps = <Props>(
// Only enable the query when the user is on the Marketplace page
const enabled = type === 'One-Click';

const { data, error, isLoading } = useStackScriptsOCA(enabled);
const { data, error, isLoading } = useMarketplaceAppsQuery(enabled);

const newApps = flags.oneClickApps || [];
const allowedApps = Object.keys({ ...baseApps, ...newApps });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,20 @@ import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers';
import { Access } from './Access';

describe('Access', () => {
it('should render a root password input', async () => {
const { findByLabelText } = renderWithThemeAndHookFormContext({
component: <Access />,
});

const rootPasswordInput = await findByLabelText('Root Password');

expect(rootPasswordInput).toBeVisible();
expect(rootPasswordInput).toBeEnabled();
});
it(
'should render a root password input',
async () => {
const { findByLabelText } = renderWithThemeAndHookFormContext({
component: <Access />,
});

const rootPasswordInput = await findByLabelText('Root Password');

expect(rootPasswordInput).toBeVisible();
expect(rootPasswordInput).toBeEnabled();
},
{ timeout: 5_000 }
);

it('should render a SSH Keys heading', async () => {
const { getAllByText } = renderWithThemeAndHookFormContext({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const Distributions = () => {
disabled={isCreateLinodeRestricted}
errorText={fieldState.error?.message}
onBlur={field.onBlur}
onChange={(_, image) => field.onChange(image?.id ?? null)}
onChange={(image) => field.onChange(image?.id ?? null)}
value={field.value}
variant="public"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const Images = () => {
disabled={isCreateLinodeRestricted}
errorText={fieldState.error?.message}
onBlur={field.onBlur}
onChange={(_, image) => field.onChange(image?.id ?? null)}
onChange={(image) => field.onChange(image?.id ?? null)}
value={field.value}
variant="private"
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from 'react';

import { stackScriptFactory } from 'src/factories';
import { HttpResponse, http, server } from 'src/mocks/testServer';
import { renderWithTheme } from 'src/utilities/testHelpers';

import { StackScriptDetailsDialog } from './StackScriptDetailsDialog';

describe('StackScriptDetailsDialog', () => {
it('should render StackScript data from the API', async () => {
const stackscript = stackScriptFactory.build();

server.use(
http.get('*/v4/linode/stackscripts/:id', () => {
return HttpResponse.json(stackscript);
})
);

const { findByText } = renderWithTheme(
<StackScriptDetailsDialog
id={stackscript.id}
onClose={vi.fn()}
open={true}
/>
);

await findByText(stackscript.id);
await findByText(stackscript.label);
await findByText(stackscript.username);
await findByText(stackscript.description);
await findByText(stackscript.script);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from 'react';

import { CircleProgress } from 'src/components/CircleProgress';
import { Dialog } from 'src/components/Dialog/Dialog';
import { ErrorState } from 'src/components/ErrorState/ErrorState';
import { StackScript } from 'src/components/StackScript/StackScript';
import { useStackScriptQuery } from 'src/queries/stackscripts';

interface Props {
/**
* The id of the StackScript
*/
id: number | undefined;
/**
* Function called when when the dialog is closed
*/
onClose: () => void;
/**
* Controls the open/close state of the dialog
*/
open: boolean;
}

export const StackScriptDetailsDialog = (props: Props) => {
const { id, onClose, open } = props;

const { data: stackscript, error, isLoading } = useStackScriptQuery(
id ?? -1,
id !== undefined
);

const title = stackscript
? `${stackscript.username} / ${stackscript.label}`
: 'StackScript';

return (
<Dialog
fullHeight
fullWidth
maxWidth="md"
onClose={onClose}
open={open}
title={title}
>
{isLoading && <CircleProgress />}
{error && <ErrorState errorText={error[0].reason} />}
{stackscript && <StackScript data={stackscript} userCanModify={false} />}
</Dialog>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import userEvent from '@testing-library/user-event';
import React from 'react';

import { imageFactory, stackScriptFactory } from 'src/factories';
import { makeResourcePage } from 'src/mocks/serverHandlers';
import { HttpResponse, http, server } from 'src/mocks/testServer';
import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers';

import { StackScriptImages } from './StackScriptImages';

describe('Images', () => {
it('should render a heading', () => {
const { getByText } = renderWithThemeAndHookFormContext({
component: <StackScriptImages />,
});

expect(getByText('Select an Image')).toBeVisible();
});

it('should render an Image Select', () => {
const { getByLabelText } = renderWithThemeAndHookFormContext({
component: <StackScriptImages />,
});

expect(getByLabelText('Images')).toBeVisible();
});

it('should only render images that are compatible with the selected StackScript', async () => {
const images = imageFactory.buildList(5);

// For the sake of this test, we pretend this image is the only compatible image.
const compatibleImage = images[2];

const stackscript = stackScriptFactory.build({
images: [compatibleImage.id],
});

server.use(
http.get('*/v4/images', () => {
return HttpResponse.json(makeResourcePage(images));
}),
http.get('*/v4/linode/stackscripts/:id', () => {
return HttpResponse.json(stackscript);
})
);

const {
findByText,
getByLabelText,
queryByText,
} = renderWithThemeAndHookFormContext({
component: <StackScriptImages />,
useFormOptions: {
defaultValues: { stackscript_id: stackscript.id },
},
});

const imageSelect = getByLabelText('Images');

await userEvent.click(imageSelect);

// Verify that the compabile image is show in the dropdown.
await findByText(compatibleImage.label);

// Verify that the images returned by the API that are NOT compatible
// with this StackScript are *not* shown in the dropdown.
for (const image of images) {
if (image !== compatibleImage) {
expect(queryByText(image.label)).toBeNull();
}
}
});
});
Loading

0 comments on commit cffa916

Please sign in to comment.