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

upcoming: [M3-7872] - Linode Create Refactor - StackScripts #10367

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fcf7827
initial work
bnussman Apr 9, 2024
0b82b26
filter options based on stackscript compatibility
bnussman Apr 9, 2024
e02f1c0
implement complex preselection logic
bnussman Apr 9, 2024
c656c1a
clean up a bit
bnussman Apr 9, 2024
9a5b510
implement details dialog
bnussman Apr 10, 2024
1b93698
fix some query param logic
bnussman Apr 10, 2024
325ef5f
lots of behavior changes to match production
bnussman Apr 10, 2024
8f1d6d6
fix image select clear behavior and fix table waypoint console error
bnussman Apr 10, 2024
c694197
improve validation for when image is `null`
bnussman Apr 10, 2024
5148d52
first unit tests
bnussman Apr 10, 2024
94fa5f4
add lots of unit testing
bnussman Apr 10, 2024
e4e71f6
add stackscript event handler
bnussman Apr 10, 2024
f56e4ad
hook up validation packages for realtime validation
bnussman Apr 10, 2024
26a5e44
Merge branch 'develop' into M3-7872-linode-create-refactor-stackscripts
bnussman Apr 10, 2024
c464078
use default validation behavior
bnussman Apr 10, 2024
de158af
Revert "hook up validation packages for realtime validation"
bnussman Apr 11, 2024
e7f668b
handle resets when switching tabs
bnussman Apr 11, 2024
5ed5f57
add changesets
bnussman Apr 11, 2024
658e7ec
add some comments
bnussman Apr 12, 2024
fe974cb
bold label and pre-select if only one option
bnussman Apr 12, 2024
d53719a
improve ux
bnussman Apr 12, 2024
884b4b4
fix unit tests
bnussman Apr 12, 2024
09188d4
add comment
bnussman Apr 15, 2024
3a8cf7a
Merge branch 'develop' into M3-7872-linode-create-refactor-stackscripts
bnussman Apr 17, 2024
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": 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't filteredOptions?.length === 1 & selectIfOnlyOneOption doing the same thing?

Do we need the selectIfOnlyOneOption optional prop?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the selectIfOnlyOneOption prop so that the functionality is op-in. I don't think it should be the default behavior

) {
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
Loading