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.

Sign up for GitHub

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

change: [M3-8807] - Improve Linode Create VPC user experience #11188

Merged
merged 5 commits into from
Oct 30, 2024
Merged

change: [M3-8807] - Improve Linode Create VPC user experience #11188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5684856
initial improvements
bnussman Oct 30, 2024
e3458ee
fix logic and comment
bnussman Oct 30, 2024
62b73f3
update the cypress test now that the UI has less friction
bnussman Oct 30, 2024
5f71e39
add changeset
bnussman Oct 30, 2024
03609ca
reintroduce validation check @abailly-akamai
bnussman Oct 30, 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
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11188-added-1730298408749.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Added
---

Pre-select a VPC subnet's on the Linode Create page when the VPC only has one subnet ([#11188](https://github.com/linode/manager/pull/11188))
35 changes: 23 additions & 12 deletions packages/manager/cypress/e2e/core/linodes/create-linode-with-vpc.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ describe('Create Linode with VPCs', () => {
id: randomNumber(),
label: randomLabel(),
region: linodeRegion.id,
//
});

mockGetVPCs([mockVPC]).as('getVPCs');
Expand All @@ -70,20 +69,18 @@ describe('Create Linode with VPCs', () => {
linodeCreatePage.setRootPassword(randomString(32));

// Confirm that mocked VPC is shown in the Autocomplete, and then select it.
cy.findByText('Assign VPC').click().type(`${mockVPC.label}`);
cy.findByText('Assign VPC').click().type(mockVPC.label);

ui.autocompletePopper
.findByTitle(mockVPC.label)
.should('be.visible')
.click();

// Confirm that Subnet selection appears and select mock subnet.
cy.findByLabelText('Subnet').should('be.visible').type(mockSubnet.label);

ui.autocompletePopper
.findByTitle(`${mockSubnet.label} (${mockSubnet.ipv4})`)
.should('be.visible')
.click();
// Confirm that VPC's subnet gets selected
cy.findByLabelText('Subnet').should(
'have.value',
`${mockSubnet.label} (${mockSubnet.ipv4})`
);

// Confirm VPC assignment indicator is shown in Linode summary.
cy.get('[data-qa-linode-create-summary]')
Expand Down Expand Up @@ -179,20 +176,34 @@ describe('Create Linode with VPCs', () => {

// Create VPC with successful API response mocked.
mockCreateVPC(mockVPC).as('createVpc');
mockGetVPCs([mockVPC]);
vpcCreateDrawer.submit();
});

// Attempt to create Linode before selecting a VPC subnet, and confirm
// that validation error appears.
// Verify the VPC field gets populated
cy.findByLabelText('Assign VPC').should('have.value', mockVPC.label);

// Verify the subnet gets populated
cy.findByLabelText('Subnet').should(
'have.value',
`${mockSubnet.label} (${mockSubnet.ipv4})`
);

// Clear the subnet value
cy.get('[data-qa-autocomplete="Subnet"]').within(() => {
cy.findByLabelText('Clear').click();
});

// Try to submit the form without a subnet selected
ui.button
.findByTitle('Create Linode')
.should('be.visible')
.should('be.enabled')
.click();

// Verify a validation error shows
cy.findByText('Subnet is required.').should('be.visible');

// Confirm that Subnet selection appears and select mock subnet.
cy.findByLabelText('Subnet').should('be.visible').type(mockSubnet.label);
bnussman-akamai marked this conversation as resolved.
Show resolved Hide resolved

ui.autocompletePopper
Expand Down
23 changes: 22 additions & 1 deletion packages/manager/src/features/Linodes/LinodeCreate/VPC/VPC.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,19 @@ export const VPC = () => {
}
onChange={(e, vpc) => {
field.onChange(vpc?.id ?? null);

if (vpc && vpc.subnets.length === 1) {
// If the user selectes a VPC and the VPC only has one subnet,
// preselect that subnet for the user.
setValue('interfaces.0.subnet_id', vpc.subnets[0].id, {
shouldValidate: true,
});
} else {
// Otherwise, just clear the selected subnet
setValue('interfaces.0.subnet_id', null);
}

// Capture analytics
if (!vpc?.id) {
sendLinodeCreateFormInputEvent({
...vpcFormEventOptions,
Expand Down Expand Up @@ -306,7 +319,15 @@ export const VPC = () => {
</Stack>
</Stack>
<VPCCreateDrawer
handleSelectVPC={(vpcId) => setValue('interfaces.0.vpc_id', vpcId)}
onSuccess={(vpc) => {
setValue('interfaces.0.vpc_id', vpc.id);

if (vpc.subnets.length === 1) {
// If the user creates a VPC with just one subnet,
// preselect it for them
setValue('interfaces.0.subnet_id', vpc.subnets[0].id);
}
}}
onClose={() => setIsCreateDrawerOpen(false)}
open={isCreateDrawerOpen}
selectedRegion={regionId}
Expand Down
1 change: 0 additions & 1 deletion packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/InterfaceSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,6 @@ export const InterfaceSelect = (props: InterfaceSelectProps) => {
additionalIPv4RangesForVPC={additionalIPv4RangesForVPC ?? []}
assignPublicIPv4Address={nattedIPv4Address !== undefined}
autoassignIPv4WithinVPC={vpcIPv4 === undefined}
from="linodeConfig"
handleIPv4RangeChange={handleIPv4RangeChange}
handleSelectVPC={handleVPCLabelChange}
handleSubnetChange={handleSubnetChange}
Expand Down
70 changes: 3 additions & 67 deletions packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/VPCPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,17 @@ import * as React from 'react';

import { accountFactory, regionFactory } from 'src/factories';
import { makeResourcePage } from 'src/mocks/serverHandlers';
import { http, HttpResponse, server } from 'src/mocks/testServer';
import { HttpResponse, http, server } from 'src/mocks/testServer';
import { mockMatchMedia, renderWithTheme } from 'src/utilities/testHelpers';

import { VPCPanel, VPCPanelProps } from './VPCPanel';
import { VPCPanel } from './VPCPanel';

beforeAll(() => mockMatchMedia());

const props = {
additionalIPv4RangesForVPC: [],
assignPublicIPv4Address: false,
autoassignIPv4WithinVPC: true,
from: 'linodeCreate' as VPCPanelProps['from'],
handleIPv4RangeChange: vi.fn(),
handleSelectVPC: vi.fn(),
handleSubnetChange: vi.fn(),
Expand Down Expand Up @@ -100,7 +99,7 @@ describe('VPCPanel', () => {
});
});

it('should display helper text if there are no vpcs in the selected region and "from" is "linodeCreate"', async () => {
it('should not display helper text if there are no vpcs in the selected region', async () => {
server.use(
http.get('*/regions', () => {
const usEast = regionFactory.build({
Expand All @@ -116,33 +115,6 @@ describe('VPCPanel', () => {

const wrapper = renderWithTheme(<VPCPanel {...props} />);

await waitFor(() => {
expect(
wrapper.queryByText(
'No VPCs exist in the selected region. Click Create VPC to create one.'
)
).toBeInTheDocument();
});
});

it('should not display helper text if there are no vpcs in the selected region and "from" is "linodeConfig"', async () => {
server.use(
http.get('*/regions', () => {
const usEast = regionFactory.build({
capabilities: ['VPCs'],
id: 'us-east',
});
return HttpResponse.json(makeResourcePage([usEast]));
}),
http.get('*/vpcs', () => {
return HttpResponse.json(makeResourcePage([]));
})
);

const wrapper = renderWithTheme(
<VPCPanel {...props} from="linodeConfig" />
);

await waitFor(() => {
expect(
wrapper.queryByText(
Expand All @@ -151,42 +123,6 @@ describe('VPCPanel', () => {
).not.toBeInTheDocument();
});
});
it('shows helper text for when "from" = "linodeCreate" if the selected region does not support VPCs', async () => {
server.use(
http.get('*/regions', () => {
const usEast = regionFactory.build({
capabilities: [],
id: 'us-east',
});
return HttpResponse.json(makeResourcePage([usEast]));
})
);

const wrapper = renderWithTheme(<VPCPanel {...props} />);

await waitFor(() => {
expect(
wrapper.queryByText('VPC is not available in the selected region.')
).toBeInTheDocument();
});
});
it('should show the "Create VPC" drawer link when from = "linodeCreate" and a region that supports VPCs is selected', async () => {
server.use(
http.get('*/regions', () => {
const usEast = regionFactory.build({
capabilities: ['VPCs'],
id: 'us-east',
});
return HttpResponse.json(makeResourcePage([usEast]));
})
);

const wrapper = renderWithTheme(<VPCPanel {...props} />);

await waitFor(() => {
expect(wrapper.queryByText('Create VPC')).toBeInTheDocument();
});
});
it('should display an unchecked VPC IPv4 auto-assign checkbox and display the VPC IPv4 input field if there is already a value', async () => {
const _props = {
...props,
Expand Down
Loading