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.

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

fix: [M3-6480] - Fix blank Node Pool plan selection #9009

Merged

Conversation

hkhalil-akamai
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai commented Apr 17, 2023

Description 📝

Fixes a bug introduced in #8840 that causes the AddNodePool drawer to not show any plans in certain situations.

Also included are basic unit tests that validate the fix and small amendments to the recent useQueryClient PR.

Preview 📷

Before After
Screenshot 2023-04-17 at 1 30 24 PM Screenshot 2023-04-17 at 1 30 07 PM

How to test 🧪

  1. Under Kubernetes, navigate to an existing cluster or create a new one.
  2. Refresh the page to clean the RQ store.
  3. Click on 'Add A Node Pool'.
  4. Verify the AddNodePoolDrawer appears and displays all plan types and details.

hkhalil-akamai and others added 5 commits April 13, 2023 15:28
Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
…k-plan-selection

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
@hkhalil-akamai hkhalil-akamai added Bug Fixes for regressions or bugs Ready for Review labels Apr 17, 2023
@hkhalil-akamai hkhalil-akamai self-assigned this Apr 17, 2023
const [_types, setNewType] = React.useState<ExtendedTypeWithCount[]>(
addCountToTypes(types)
const [typeCountMap, setTypeCountMap] = React.useState<Map<string, number>>(
new Map()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to use a Map here instead of an index signature object because those can lead to runtime errors without noUncheckedIndexedAccess enabled:

const map1: { [key: string]: number } = {"type1": 0};
const type2Count: number = map1["type2"]; # no type error

vs with Map:

const map1: Map<string, number> = new Map([[ "type1", 0 ]]);
const type2Count: number = map1.get("type2"); # type error: number | undefined is not assignable to number.

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
@cypress
Copy link

cypress bot commented Apr 17, 2023

2 flaky tests on run #3069 ↗︎

0 149 3 0 Flakiness 2

Details:

Merge remote-tracking branch 'upstream/develop' into M3-6480-fix-blank-plan-sele...
Project: Cloud Manager E2E Commit: 0b77eae74a
Status: Passed Duration: 16:21 💡
Started: Apr 19, 2023 5:45 PM Ended: Apr 19, 2023 6:01 PM
Flakiness  linodes/rescue-linode.spec.ts • 1 flaky test

View Output Video

Test Artifacts
rescue linode > rescue a linode Output Screenshots Video
Flakiness  objectStorage/object-storage.e2e.spec.ts • 1 flaky test

View Output Video

Test Artifacts
object storage end-to-end tests > can upload, access, and delete objects Output Screenshots Video

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not notice any regressions, thank you for for the fix!

@hkhalil-akamai hkhalil-akamai added the Add'tl Approval Needed Waiting on another approval! label Apr 18, 2023
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@jaalah-akamai jaalah-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Apr 18, 2023
…k-plan-selection

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
@hkhalil-akamai hkhalil-akamai merged commit f0f6a95 into linode:develop Apr 19, 2023
@hkhalil-akamai hkhalil-akamai deleted the M3-6480-fix-blank-plan-selection branch January 9, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs Ready for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants