feat(machines): update DHCP and discovery forms to use the new API #1116 #4389

petermakowski · 2022-09-06T15:19:18Z

Done

update DHCP add form to use list box
Update discovery form to use the new API and machine list box
keep previous data to prevent flashes of no data in between data requests
add e2e test for dhcp snippets
add SelectButton component
add OutsideClickHandler component

QA

MAAS deployment

To run this branch you will need access to one of the following MAAS deployments:

Karura
Bolla
A development MAAS
A local snap MAAS (this will not usually have machines)

Running the branch

You can run this branch by:

Serving with dotrun
Building in a development MAAS

QA steps

Go to MAAS/r/settings/dhcp
Select Type: Machine and pick a machine from the list
Go to network discovery
Expand the dropdown on the right hand side and select "Add discovery..."
Select a machine from the list in the Parent field and
Ensure the device.create websocket message has been sent with the correct payload (including the id of a machine which was selected as a parent)
You can look at websocket messages using developer tools
- go to network tab -> filter by WS
- look for an entry starting with ws?csrftoken=
You can verify the parent id is valid by visiting the machine details page with a given id and verifying it's the one that was requested in the form (e.g. /MAAS/r/machine/[replace-this-with-your-selected-parent-id])

Fixes

Fixes: https://github.com/canonical/app-tribe/issues/1116
Fixes: https://github.com/canonical/app-tribe/issues/1112
Fixes: https://github.com/canonical/app-tribe/issues/1340

Launchpad issue

Related Launchpad maas issue in the form lp#number.

Backports

In general, please propose fixes against main rather than release branches (e.g. 2.7), unless the fix is only applicable for that specific release. Please apply backport labels to the PR (e.g. "Backport 2.7") for the appropriate releases to target.

Only bug and security fixes should be backported, new features should only land in main.

Screenshots

webteam-app · 2022-09-06T15:19:22Z

Demo starting at https://maas-ui-4389.demos.haus

petermakowski · 2022-09-07T14:57:02Z

@rayito Decided not to implement the absolute positioning for the select dropdown - it can be tricky when there's not enough content on the page parts of the dropdown may get cut off. If you feel strongly about it I'd suggest we look into this in a separate ticket.

rayito · 2022-09-07T15:00:08Z

Totally fine

@rayito Decided not to implement the absolute positioning for the select dropdown - it can be tricky when there's not enough content on the page parts of the dropdown may get cut off. If you feel strongly about it I'd suggest we look into this in a separate ticket.

Totally fine, closing it by clicking outside was more important than that. UX+1

petermakowski · 2022-09-07T15:01:43Z

Searching by machine id doesn't currently work and will be looked at in a separate ticket https://github.com/canonical/app-tribe/issues/1339

huwshimi

Hey @petermakowski apologies for the "code -1" but I think there are big enough changes needed that this will require another review.

Happy to chat if any of my comments don't make sense.

src/app/base/components/DhcpFormFields/DhcpFormFields.tsx

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx

src/app/base/components/OutsideClickHandler/OutsideClickHandler.tsx

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox.tsx

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx

src/app/base/hooks/base.ts

src/app/store/machine/utils/hooks.ts

src/app/base/components/SelectButton/_index.scss

src/app/base/hooks/base.ts

huwshimi

Thanks for doing those changes @petermakowski. I think changing this to a FormikField is worth doing in this PR (but not the ContextualMenu parts) as a fair bit of this will be made redundant.

src/app/base/components/DebounceSearchBox/DebounceSearchBox.tsx

src/app/base/components/DhcpFormFields/DhcpFormFields.test.tsx

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx

src/app/base/components/OutsideClickHandler/OutsideClickHandler.tsx

src/app/base/hooks/base.ts

src/app/base/components/DhcpFormFields/DhcpFormFields.tsx

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.tsx

petermakowski · 2022-09-09T09:55:15Z

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx

+      <Field label={<span id={labelId}>{label}</span>} {...props}>
+        <OutsideClickHandler onClick={() => setIsOpen(false)}>
+          <SelectButton
+            aria-describedby={labelId}


This is in order to get additional description for the button element, rendered result below.

petermakowski · 2022-09-13T10:40:53Z

@huwshimi could you take another look at this please?

- refactor OutsideClickHandler

- spread SearchBox props - fix debounced typo - simplify machineFactory - add docstring comments

- handle onKeyPress within MachineSelectTable

ndv99

You can probably remove Loading from this enum since it isn't used in the file any more
https://github.com/canonical/maas-ui/pull/4389/files#diff-3fc2d13129aa52e7573026eac1c141e58e9f922cf86d851e9e56fd4902f85ba5R19-R23

ndv99

Happy with the code over all but obviously QA can't be done until that error message in the discovery form is dealt with

petermakowski · 2022-09-20T11:00:18Z

Happy with the code over all but obviously QA can't be done until that error message in the discovery form is dealt with

As it's due to a bug in MAAS (https://bugs.launchpad.net/maas/+bug/1933408) I think it's fine to QA by verifying the right websocket message is being sent. Updated the QA description.

ndv99

LGTM!

petermakowski added Review: Code needed Review: QA needed labels Sep 6, 2022

huwshimi removed Review: Code needed Review: QA needed labels Sep 6, 2022

petermakowski force-pushed the dhcp-add-form-#1116 branch 3 times, most recently from a698680 to 8adb9ba Compare September 7, 2022 12:12

petermakowski added Review: Code needed Review: QA needed labels Sep 7, 2022

petermakowski force-pushed the dhcp-add-form-#1116 branch from 8adb9ba to 26d43a1 Compare September 7, 2022 12:15

petermakowski marked this pull request as ready for review September 7, 2022 12:15

petermakowski added the Review: Design needed label Sep 7, 2022

petermakowski force-pushed the dhcp-add-form-#1116 branch from 26d43a1 to a6224a6 Compare September 7, 2022 13:05

petermakowski changed the title ~~update DHCP add form #1116~~ feat(machines): update DHCP add form #1116 Sep 7, 2022

petermakowski force-pushed the dhcp-add-form-#1116 branch from a6224a6 to 36f3ca3 Compare September 7, 2022 13:54

rayito added Review: Design +1 and removed Review: Design needed labels Sep 7, 2022

huwshimi requested changes Sep 8, 2022

View reviewed changes

huwshimi added Review: Code -1 and removed Review: Code needed labels Sep 8, 2022

This was referenced Sep 8, 2022

Update Pagination component button type #4396

Closed

Refactor MachineSelect to use ContextualMenu #4397

Open

petermakowski requested a review from huwshimi September 8, 2022 08:08

petermakowski mentioned this pull request Sep 8, 2022

Update discovery form to use the new API and machine list box #4399

Closed

huwshimi requested changes Sep 9, 2022

View reviewed changes

huwshimi reviewed Sep 9, 2022

View reviewed changes

src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.tsx Outdated Show resolved Hide resolved

petermakowski changed the title ~~feat(machines): update DHCP add form #1116~~ feat(machines): update DHCP and discovery forms to use the new API #1116 Sep 9, 2022

petermakowski commented Sep 9, 2022

View reviewed changes

petermakowski requested review from huwshimi and ndv99 September 19, 2022 11:58

petermakowski added 12 commits September 20, 2022 11:37

update DHCP add form canonical#1116

9b36238

add OutsideClickHandler component

724c694

remove select hover style

d8e6274

address code review comments

788191a

use MachineListPagination

e14cf68

add DebounceSearchBox

93707a2

- refactor OutsideClickHandler

fix lint errors

d9b7fd0

address code review comments

79d4635

- spread SearchBox props - fix debounced typo - simplify machineFactory - add docstring comments

fix lint error

fb97b44

update discovery form api

e031a9e

add Field to MachineSelect

2b47d29

- handle onKeyPress within MachineSelectTable

fix KeyboardEvent type

9897825

petermakowski force-pushed the dhcp-add-form-#1116 branch from 428c6c4 to 9897825 Compare September 20, 2022 09:37

ndv99 reviewed Sep 20, 2022

View reviewed changes

fix TS errors

a438f14

ndv99 added Review: Code +1 Review: QA +1 and removed Review: QA needed Review: Code -1 labels Sep 20, 2022

ndv99 approved these changes Sep 20, 2022

View reviewed changes

petermakowski merged commit 409fc0b into canonical:server-side-search-and-filter Sep 20, 2022

petermakowski mentioned this pull request Sep 20, 2022

refactor: remove unused Label enum #4412

Merged

petermakowski deleted the dhcp-add-form-#1116 branch May 9, 2024 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(machines): update DHCP and discovery forms to use the new API #1116 #4389

feat(machines): update DHCP and discovery forms to use the new API #1116 #4389

petermakowski commented Sep 6, 2022 •

edited

Loading

webteam-app commented Sep 6, 2022

petermakowski commented Sep 7, 2022

rayito commented Sep 7, 2022

petermakowski commented Sep 7, 2022

huwshimi left a comment

huwshimi left a comment

petermakowski Sep 9, 2022

petermakowski commented Sep 13, 2022

ndv99 left a comment

ndv99 left a comment •

edited

Loading

petermakowski commented Sep 20, 2022 •

edited

Loading

ndv99 left a comment

feat(machines): update DHCP and discovery forms to use the new API #1116 #4389

feat(machines): update DHCP and discovery forms to use the new API #1116 #4389

Conversation

petermakowski commented Sep 6, 2022 • edited Loading

Done

QA

MAAS deployment

Running the branch

QA steps

Fixes

Launchpad issue

Backports

Screenshots

webteam-app commented Sep 6, 2022

petermakowski commented Sep 7, 2022

rayito commented Sep 7, 2022

petermakowski commented Sep 7, 2022

huwshimi left a comment

Choose a reason for hiding this comment

huwshimi left a comment

Choose a reason for hiding this comment

petermakowski Sep 9, 2022

Choose a reason for hiding this comment

petermakowski commented Sep 13, 2022

ndv99 left a comment

Choose a reason for hiding this comment

ndv99 left a comment • edited Loading

Choose a reason for hiding this comment

petermakowski commented Sep 20, 2022 • edited Loading

ndv99 left a comment

Choose a reason for hiding this comment

petermakowski commented Sep 6, 2022 •

edited

Loading

ndv99 left a comment •

edited

Loading

petermakowski commented Sep 20, 2022 •

edited

Loading