Skip to content

Commit

Permalink
sorting and ordering
Browse files Browse the repository at this point in the history
  • Loading branch information
@abailly-akamai
abailly-akamai committed Oct 23, 2024
1 parent 1a059c7 commit 5bfeb19
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 15 deletions.
93 changes: 85 additions & 8 deletions packages/manager/src/components/ImageSelect/ImageSelect.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ import { ImageSelect } from './ImageSelect';
describe('ImageSelect', () => {
it('should render a default "Images" label', () => {
const { getByLabelText } = renderWithTheme(
<ImageSelect onChange={vi.fn()} value={null} />
<ImageSelect onChange={vi.fn()} value={null} variant="public" />
);

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

it('should render default placeholder text', () => {
const { getByPlaceholderText } = renderWithTheme(
<ImageSelect onChange={vi.fn()} value={null} />
<ImageSelect onChange={vi.fn()} value={null} variant="public" />
);

expect(getByPlaceholderText('Choose an image')).toBeVisible();
Expand All @@ -36,7 +36,7 @@ describe('ImageSelect', () => {
);

const { getByPlaceholderText, getByText } = renderWithTheme(
<ImageSelect onChange={vi.fn()} value={null} />
<ImageSelect onChange={vi.fn()} value={null} variant="public" />
);

await userEvent.click(getByPlaceholderText('Choose an image'));
Expand All @@ -57,7 +57,7 @@ describe('ImageSelect', () => {
);

const { getByPlaceholderText, getByText } = renderWithTheme(
<ImageSelect onChange={onChange} value={null} />
<ImageSelect onChange={onChange} value={null} variant="public" />
);

await userEvent.click(getByPlaceholderText('Choose an image'));
Expand All @@ -81,7 +81,7 @@ describe('ImageSelect', () => {
);

const { findByDisplayValue } = renderWithTheme(
<ImageSelect onChange={vi.fn()} value={image.id} />
<ImageSelect onChange={vi.fn()} value={image.id} variant="public" />
);

await findByDisplayValue(image.label);
Expand All @@ -97,7 +97,7 @@ describe('ImageSelect', () => {
);

const { findByTestId } = renderWithTheme(
<ImageSelect onChange={vi.fn()} value={image.id} />
<ImageSelect onChange={vi.fn()} value={image.id} variant="public" />
);

await findByTestId('os-icon');
Expand Down Expand Up @@ -133,7 +133,7 @@ describe('ImageSelect', () => {
);

const { getByPlaceholderText, getByText, queryByText } = renderWithTheme(
<ImageSelect onChange={vi.fn()} value={null} />
<ImageSelect onChange={vi.fn()} value={null} variant="public" />
);

await userEvent.click(getByPlaceholderText('Choose an image'));
Expand All @@ -146,7 +146,12 @@ describe('ImageSelect', () => {

it('should display an error', () => {
const { getByText } = renderWithTheme(
<ImageSelect errorText="An error" onChange={vi.fn()} value={null} />
<ImageSelect
errorText="An error"
onChange={vi.fn()}
value={null}
variant="public"
/>
);
expect(getByText('An error')).toBeInTheDocument();
});
Expand All @@ -161,6 +166,7 @@ describe('ImageSelect', () => {
multiple
onChange={onSelect}
value={[]}
variant="public"
/>
);

Expand All @@ -172,4 +178,75 @@ describe('ImageSelect', () => {
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.build({
created: '2023-03-01T00:00:00',
is_public: true,
label: 'Public Image 3',
vendor: 'Ubuntu',
}),
];
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',
}),
];

// 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 { getAllByRole, getByLabelText, getByText } = renderWithTheme(
<ImageSelect
onChange={vi.fn()}
options={images}
value={null}
variant="all"
/>
);

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');
});
});
40 changes: 33 additions & 7 deletions packages/manager/src/components/ImageSelect/ImageSelect.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ interface BaseProps
label?: string;
placeholder?: string;
selectIfOnlyOneOption?: boolean;
variant?: ImageSelectVariant;
variant: ImageSelectVariant;
}

interface SingleProps extends BaseProps {
Expand All @@ -56,7 +56,7 @@ export const ImageSelect = (props: Props) => {
onChange,
placeholder,
selectIfOnlyOneOption,
variant = 'all',
variant,
...rest
} = props;

Expand Down Expand Up @@ -87,6 +87,35 @@ export const ImageSelect = (props: Props) => {
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) {
Expand Down Expand Up @@ -157,7 +186,7 @@ export const ImageSelect = (props: Props) => {
clearOnBlur
label={label || 'Images'}
loading={isLoading}
options={options}
options={sortedOptions}
placeholder={placeholder || 'Choose an image'}
{...rest}
disableClearable={
Expand All @@ -177,10 +206,7 @@ export const ImageSelect = (props: Props) => {
);
};

const StyledList = styled(
List,
{}
)({
const StyledList = styled(List, { label: 'ImageSelectStyledList' })({
listStyle: 'none',
padding: 0,
});
1 change: 1 addition & 0 deletions packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/ImageAndPassword.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const ImageAndPassword = (props: Props) => {
errorText={imageFieldError}
onChange={onImageChange}
value={selectedImage}
variant="all"
/>
<StyledAccessPanel
disabledReason={
Expand Down

0 comments on commit 5bfeb19

Please sign in to comment.