upcoming: [M3-7691] - Create PG in Linode Create Flow #10273

abailly-akamai · 2024-03-11T20:09:55Z

Description 📝

This PR bring the PG create drawer to the Linode creation flows, within the DetailsPanel

Changes 🔄

Move PlacementGroupDetailPanel to its own component and add tests
Implement PG create flow/drawer
Improve naming conventions
Improve error handling
Improve PG related data sets (mocks and factories)
Move logic away from LinodeCreate and keep it contained (CC create refactor - @bnussman-akamai)

Preview 📷

How to test 🧪

NOTE: due to MSW limitations, creating a new PG won't place it in the Detail PG select

Prerequisites

Have both the "Placement Group" flag and MSW turned on

Verification steps

Navigate to http://localhost:3000/linodes/create
Test with the following regions:
- Atlanta: (no existing PG) - can create new PG, can add linode to PG once created (with real API)
  - can't really test payload till real API is wired
- Chicago: (no capability) can't create new PG, can't add linode to PG
- Newark: can create new PG, can add linode to PG
  - (confirm placement_group: { id: 1 } in the POST linode/instances payload)
- Toronto: (no room left on PG) - can create new PG, PG option disabled
- Fremont: (reach PG capacity for region) - create button disabled, can select PG and add linode to PG
  - (confirm placement_group: { id: 2 } in the POST linode/instances payload)

These are the scenarios I have added coverage in the unit as well

As an Author I have considered 🤔

Check all that apply

abailly-akamai · 2024-03-13T16:00:03Z

packages/manager/src/__data__/regionsData.ts

massaging our mock data sets so we get a diverse pool of regions

abailly-akamai · 2024-03-13T16:01:08Z

packages/manager/src/components/DetailsPanel/DetailsPanel.tsx

+          placementGroupsSelectProps={{
+            ...placementGroupsSelectProps,
+          }}
+        />


moved to a dedicated component

Thank you! 🙏

abailly-akamai · 2024-03-13T16:04:31Z

packages/manager/src/features/Linodes/LinodesCreate/LinodeCreate.tsx

moved as much logic as possible away from LinodeCreate and moved it to the PlacementGroupDetailPanel

abailly-akamai · 2024-03-13T16:05:27Z

packages/manager/src/features/Linodes/LinodesCreate/LinodeCreateContainer.tsx

@@ -615,6 +615,7 @@ class LinodeCreateContainer extends React.PureComponent<CombinedProps, State> {
    );
    this.setState({
      disabledClasses,
+      placementGroupSelection: undefined,


we need to reset the placementGroup selection when the region changes. This is an omission from the previous implementation

abailly-akamai · 2024-03-13T16:05:49Z

packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.test.tsx

+            region: 'us-west',
+          },
+        ]}
+      />


due to factory data changes

abailly-akamai · 2024-03-13T16:13:41Z

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

+                fontSize: '0.875rem',
+                mt: -0.75,
+                p: 0,
+              })}


Had to use a regular button and style it cause i wanted to use the built in tooltip

That's fair, but now it feels different than the Create VPC and Create Firewall. Could we use a LinkButton and just wrap it a <Tooltip /> or change the others to be consistent?

What do you mean by "it feels different"?

While looking the same, the problem is a bit more complex, so I created a ticket to handle LinkButton to be based on our Button rather than a native button that won't take custom props (LinkButton is based on StyledLinkButton which renders a native button)... we'll clean that up

"it feels different" as in there is a ripple effect when you press the button and there is no underline when you hover over the text

gotcha - that's fair: M3-7892

arguably, we'll want the actual Button styles, since underlines are usually reserved for links

I'll try to prioritize this for before Placement Groups go out

we'll want the actual Button styles, since underlines are usually reserved for links

I think I agree! Thanks for ticketing it!

github-actions · 2024-03-13T17:39:08Z

Coverage Report: ✅
Base Coverage: 81.45%
Current Coverage: 81.5%

dwiley-akamai

Atlanta: (no existing PG) - can create new PG, can add linode to PG once created (with real API)
- can't really test payload till real API is wired
- confirmed success toast appears when the PG creation form is submitted ✅
Chicago: (no capability) can't create new PG, can't add linode to PG ✅
Newark: can create new PG, can add linode to PG
- (confirm placement_group: { id: 1 } in the POST linode/instances payload) ✅
Toronto: (no room left on PG) - can create new PG, can't add linode to PG, inline error when selecting PG, error when submitting the Linode create form ✅
Fremont: (reach PG capacity for region) - create button disabled, can select PG and add linode to PG ✅
- (confirm placement_group: { id: 2 } in the POST linode/instances payload) ✅

packages/manager/.changeset/pr-10273-upcoming-features-1710350268383.md

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

bnussman-akamai · 2024-03-19T13:44:50Z

packages/manager/src/components/DetailsPanel/DetailsPanel.tsx

+          placementGroupsSelectProps={{
+            ...placementGroupsSelectProps,
+          }}
+        />


Thank you! 🙏

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

bnussman-akamai · 2024-03-19T13:54:20Z

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

+                fontSize: '0.875rem',
+                mt: -0.75,
+                p: 0,
+              })}


That's fair, but now it feels different than the Create VPC and Create Firewall. Could we use a LinkButton and just wrap it a <Tooltip /> or change the others to be consistent?

hana-linode

Cancelling the drawer does not reset the state

Screen.Recording.2024-03-19.at.11.15.35.AM.mov

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

bnussman-akamai · 2024-03-19T15:57:02Z

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

+                fontSize: '0.875rem',
+                mt: -0.75,
+                p: 0,
+              })}


"it feels different" as in there is a ripple effect when you press the button and there is no underline when you hover over the text

packages/manager/src/components/PlacementGroupsSelect/PlacementGroupSelectOption.tsx

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx

packages/manager/src/features/PlacementGroups/constants.ts

abailly-akamai · 2024-03-19T22:22:25Z

thanks @hana-linode - all addressed

carrillo-erik · 2024-03-20T13:07:25Z

Very nice work. Solid refactors and I'm in agreement with the updated naming conventions you made. Looks like there's some failures with linodes/rebuild-linode.spec.ts but other than that, looks good to me.

hana-linode

Great work!

abailly-akamai self-assigned this Mar 11, 2024

abailly-akamai changed the title ~~upcoming: [M3-7691] - Creat PG in Linode Create Flow~~ upcoming: [M3-7691] - Create PG in Linode Create Flow Mar 11, 2024

abailly-akamai commented Mar 13, 2024

View reviewed changes

abailly-akamai marked this pull request as ready for review March 13, 2024 17:16

abailly-akamai requested a review from a team as a code owner March 13, 2024 17:16

abailly-akamai requested review from dwiley-akamai, hana-linode and carrillo-erik and removed request for a team March 13, 2024 17:16

abailly-akamai force-pushed the M3-7691 branch 2 times, most recently from b68a5c2 to 1c916f1 Compare March 18, 2024 20:23

dwiley-akamai added the Placement Groups label Mar 18, 2024

dwiley-akamai approved these changes Mar 18, 2024

View reviewed changes

bnussman-akamai reviewed Mar 19, 2024

View reviewed changes

abailly-akamai added 9 commits March 19, 2024 11:02

Initial commit - save work

b783d23

Update UI and concitional rendering

b9522eb

Wrap up UI

60562db

Testing and cleaning up

d81059f

Wrap up testing and cleaning

85f4ab6

mooooar cleanup

eddbca8

Added changeset: Add Create Placement Group flow in Linode Details Panel

577077e

small cleanup

bc7fafa

small cleanup

ffe2789

abailly-akamai force-pushed the M3-7691 branch from 1c916f1 to ffe2789 Compare March 19, 2024 15:02

feedback

c094a4e

abailly-akamai requested a review from bnussman-akamai March 19, 2024 15:24

hana-linode reviewed Mar 19, 2024

View reviewed changes

packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.tsx Outdated Show resolved Hide resolved

feedback 2

ef4321e

bnussman-akamai approved these changes Mar 19, 2024

View reviewed changes

abailly-akamai added 2 commits March 19, 2024 15:36

Better handling of at capacity PGs

eb1d77b

improve coverage

ca4e966

abailly-akamai requested a review from hana-linode March 19, 2024 20:22

cleanup

95e2065

hana-linode reviewed Mar 19, 2024

View reviewed changes

feedback some moar

b3d1ef5

abailly-akamai requested a review from hana-linode March 19, 2024 22:22

abailly-akamai added 2 commits March 19, 2024 18:25

oops

28e966a

aaand the test

23ca2ca

carrillo-erik approved these changes Mar 20, 2024

View reviewed changes

hana-linode approved these changes Mar 20, 2024

View reviewed changes

hana-linode added the Approved Multiple approvals and ready to merge! label Mar 20, 2024

abailly-akamai merged commit bd8054f into linode:develop Mar 20, 2024
17 of 18 checks passed

This was referenced Mar 28, 2024

Release v1.116.0 - release → staging #10327

Merged

Release v1.116.0 - staging → master #10335

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [M3-7691] - Create PG in Linode Create Flow #10273

upcoming: [M3-7691] - Create PG in Linode Create Flow #10273

abailly-akamai commented Mar 11, 2024 •

edited

abailly-akamai Mar 13, 2024

abailly-akamai Mar 13, 2024

bnussman-akamai Mar 19, 2024

abailly-akamai Mar 13, 2024

abailly-akamai Mar 13, 2024

abailly-akamai Mar 13, 2024

abailly-akamai Mar 13, 2024

bnussman-akamai Mar 19, 2024

abailly-akamai Mar 19, 2024

abailly-akamai Mar 19, 2024 •

edited

bnussman-akamai Mar 19, 2024

abailly-akamai Mar 19, 2024 •

edited

bnussman-akamai Mar 19, 2024

github-actions bot commented Mar 13, 2024 •

edited

dwiley-akamai left a comment

bnussman-akamai Mar 19, 2024

bnussman-akamai Mar 19, 2024

hana-linode left a comment

bnussman-akamai Mar 19, 2024

abailly-akamai commented Mar 19, 2024

carrillo-erik commented Mar 20, 2024 •

edited

hana-linode left a comment

upcoming: [M3-7691] - Create PG in Linode Create Flow #10273

upcoming: [M3-7691] - Create PG in Linode Create Flow #10273

Conversation

abailly-akamai commented Mar 11, 2024 • edited

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai Mar 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai Mar 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 13, 2024 • edited

dwiley-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hana-linode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai commented Mar 19, 2024

carrillo-erik commented Mar 20, 2024 • edited

hana-linode left a comment

Choose a reason for hiding this comment

abailly-akamai commented Mar 11, 2024 •

edited

abailly-akamai Mar 19, 2024 •

edited

abailly-akamai Mar 19, 2024 •

edited

github-actions bot commented Mar 13, 2024 •

edited

carrillo-erik commented Mar 20, 2024 •

edited