refactor: Made <RegionSelect /> more dynamic

[bnussman-akamai]

Description 📝

Why ❓

• In preparation for adding new datacenters in new regions, we want our region select component to work dynamically with the API as much as possible
• For example: if the API were to start returning a France region, with our current code, a France flag would not show up 🇫🇷🚫

Changes

• Makes the flag icon get picked dynamically based on the API's data
• Automatically maps a region's country to a continent for grouping
• Cleans up logic in the <RegionSelect /> component
• Switches <RegionSelect /> to named exports
• <RegionSelect /> no longer depends on ramda 😤

How to test 🧪

• Test places in the app that use a Region select exists (for example the Linode Create page)

[bnussman-akamai]

The values in this file allow us map any country to a continent for the sake of grouping items in the <RegionSelect />. Is this approach ok?

[jaalah-akamai]

Tested and this works 👍

[cypress]

1 failed tests on run #2966 ↗︎

1

145

3

0

0

Details:

add changelog entry
Project: Cloud Manager E2E	Commit: c2ad501d24
Status: Failed	Duration: 16:28 💡
Started: Apr 13, 2023 7:06 PM	Ended: Apr 13, 2023 7:23 PM

  cypress/e2e/general/smoke-deep-link.spec.ts • 1 failed test

View Output Video

Test		Artifacts
smoke - deep link > Go to Profile/Display > by Tab		Output Screenshots Video

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

[abailly-akamai]

Looks great! Didn't pull the code locally but just a couple suggestions you may or may not consider 👍

[abailly-akamai]

Not a blocker, but why not using readonly enums?

[bnussman-akamai]

I believe if I used an enum, we wouldn't be able to do const continentCode = COUNTRY_CODE_TO_CONTINENT_CODE[country] which is important because we need to be able to access the values.

[abailly-akamai]

Unless I am misunderstanding your intent, you can

export enum COUNTRY_CODE_TO_CONTINENT_CODE {
  AD = 'EU',
  AE = 'AS',
  AF = 'AS',
  AG = 'NA',
  AI = 'NA',
  AL = 'EU',
  AM = 'AS',
  ...
};

const continentCode = COUNTRY_CODE_TO_CONTINENT_CODE['AE']

console.log(continentCode) // => AS

[bnussman-akamai]

Oh yeah, that totally works, my bad. Do you think an enum makes the most since here?

[abailly-akamai]

Since we're TS I'd say yes, but what you did works too. With a frozen object, you just need an extra type that refers to keyof the type of that object.

[bnussman-akamai]

Going to keep this format to maintain parity with the source package, but I'd be okay switching this to an enum if we need to in the future

[abailly-akamai]

not a blocker at all but using reduce could be nicer? something like that

regions.reduce((acc, region) => {
    const country = region.country.toUpperCase();
  
    const continentCode = COUNTRY_CODE_TO_CONTINENT_CODE[
      country as keyof typeof COUNTRY_CODE_TO_CONTINENT_CODE
    ];
  
    const group = continentCode
      ? CONTINENT_CODE_TO_CONTINENT[continentCode] ?? 'Other'
      : 'Other';
  
    if (!acc[group]) {
      acc[group] = [];
    }
  
    acc[group].push({
      label: `${region.label} (${region.id})`,
      value: region.id,
      flag: <Flag country={region.country} />,
      country: region.country,
    });
  
    return acc;
  }, {});

[bnussman-akamai]

I think I'll stick with the for loop for the readability and I think the time complexity is about the same