refactor: [M3-9211] - Refactor Linode Rebuild Dialog #11629
Conversation
bnussman-akamai
requested review from
dmcintyr-akamai
and removed request for
a team
There was a problem hiding this comment.
This is not really new code, it is just extracted from AccessPanel, which is now deleted 🗑️
| @@ -107,12 +109,14 @@ export const Security = () => { | |||
| ? DISK_ENCRYPTION_DEFAULT_DISTRIBUTED_INSTANCES | |||
| : DISK_ENCRYPTION_UNAVAILABLE_IN_REGION_COPY | |||
| } | |||
| isEncryptEntityChecked={ | |||
| isDistributedRegion || field.value === 'enabled' | |||
There was a problem hiding this comment.
Local disk encryption is on by default for Compute Instances in distributed regions, and can't be turned off.
| const error = ( | ||
| // @ts-expect-error UDFs don't abide by the form's error type. This is an api-v4 bug. | ||
| formState.errors?.[userDefinedField.name] ?? fieldState.error | ||
| )?.message?.replace('the UDF', ''); |
There was a problem hiding this comment.
The POST /v4/linode/instances endpoint breaks normal error handling patterns. This is why a // @ts-expect-error is here. I updated this to support correct error shape so that client side errors also surface.
This change was necessary because the Linode Rebuild form does client side validation on UDF fields. This is something I'll probably circle back to and add to Linode Create
| <ImageSelect | ||
| disabled={disabled} | ||
| errorText={imageFieldError} | ||
| onChange={onImageChange} | ||
| value={selectedImage} | ||
| variant="all" | ||
| /> | ||
| <StyledAccessPanel |
There was a problem hiding this comment.
I went ahead an made this change so I could delete AccessPanel.
AccessPanel was an abstraction that was getting out of hand. Now we just use PasswordInput and UserSSHKeyPanel, etc... directly
There was a problem hiding this comment.
I was confused as to why ImageAndPassword.tsx implemented restricted user support, but this whole form didn't. (This form uses ImageAndPassword)
I went ahead and moved the restricted user code up one level so that it now lives here instead of inside of ImageAndPassword.
Ultimately, I don't think it matters because I don't think restricted users can even open this drawer.
| image: string().required('An image is required.'), | ||
| root_pass: string().required('Password is required.'), | ||
| authorized_keys: array().of(SSHKeySchema), |
There was a problem hiding this comment.
Unless there is an undocumented API easter egg I don't know about, this was wrong.
https://techdocs.akamai.com/linode-api/reference/post-rebuild-linode-instance
|
Coverage Report: ❌ |
Cloud Manager UI test results🔺 6 failing tests on test run #3 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: yarn cy:run -s "cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts,cypress/e2e/core/linodes/backup-linode.spec.ts" |
||||||||||||||||||||||||||||||||
Description 📝
react-hook-form🧹recomposePreview 📷
How to test 🧪
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅