Skip to content

Commit

Permalink
change: [M3-8807] - Improve Linode Create VPC user experience (linode…
Browse files Browse the repository at this point in the history
…#11188)

* initial improvements

* fix logic and comment

* update the cypress test now that the UI has less friction

* add changeset

* reintroduce validation check @abailly-akamai

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
  • Loading branch information
bnussman-akamai and bnussman authored Oct 30, 2024
1 parent fef87a9 commit 926fca5
Show file tree
Hide file tree
Showing 9 changed files with 225 additions and 378 deletions.
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11188-added-1730298408749.md
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))
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);

ui.autocompletePopper
Expand Down
23 changes: 22 additions & 1 deletion packages/manager/src/features/Linodes/LinodeCreate/VPC/VPC.tsx
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
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
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

0 comments on commit 926fca5

Please sign in to comment.