upcoming: [M3-9495] - Disable APL for LKE-E clusters #11809
Conversation
mjac0bs
requested review from
jdamore-linode,
abailly-akamai and
harsh-akamai
and removed request for
a team
|
Coverage Report: ✅ |
| // Confirm the APL section is not shown. | ||
| cy.findByTestId('apl-label').should('not.exist'); | ||
| cy.findByTestId('apl-radio-button-yes').should('not.exist'); | ||
| cy.findByTestId('apl-radio-button-no').should('not.exist'); | ||
|
|
There was a problem hiding this comment.
Related to the above, can we add assertions to confirm that the APL section is present when LKE-E isn't selected?
There was a problem hiding this comment.
I believe this test is already accomplishing that, right? Do you mean toggling to an LKE standard tier and checking that it exists?
There was a problem hiding this comment.
Oh yeah, you're absolutely right. There is one distinction I'll call out which is that adding the assertions here gives us explicit coverage that the APL section is present when the LKE-E feature is enabled but not selected during cluster creation, whereas the other test doesn't take the state of LKE-E into account. So if there were ever a logic issue (or mocking issue in the test) involving APL/LKE-E that resulted in APL being hidden, it would get caught by this test but may not get caught by the other.
(I have no idea what the logic for the cluster create page is like, so if this comes across as a farfetched hypothetical we can definitely leave it alone, just wanted to point it out in case it's an important distinction from a dev's POV)
There was a problem hiding this comment.
I left the case of 'LKE-E feature enabled but LKE (standard) cluster tier selected' uncovered in tests because it is essentially the same flow as 'LKE-E feature disabled'. The code sets a default of standard for the selectedTier. The account beta endpoint determines whether or not the APL section is visible to users, and the section visible + the selectedTier determine whether the section is enabled or disabled.
Basically, if there were an issue with the LKE tier selection in the LKE-E flow, the first test could catch it.
There was a problem hiding this comment.
@mjac0bs What are the UX requirements here?
If the goal is to disable the panel, then that's what we should do, not hide/remove it, especially if this will come at a later time. I find it to be a better UI to disable a panel and add contextual helper text as to the reason why, advertising the feature is coming up, rather than making the user wonder why this is happening and why this isn't possible (or will ever be). This form is already quite complex, and displaying elements conditionally could add confusion IMO.
Thanks, you're right that this is unclear. I'm following up with Product for clarity on this in #dev-apl.
To an extent, we're already doing this (displaying elements conditionally) with the HA form section being hidden once the LKE-E cluster is selected. This was accepted by Product since HA is mentioned as included in the cluster tier section. I see your point that this is a different situation where we'd potentially be "making the user wonder why this is happening and why this isn't possible (or will ever be)". |
mjac0bs
added
Ready for Review
and removed
Work in Progress
UX/UI
|
Reopening this up for review with updated UX treatment, which Product has agreed upon and the APL team is aware of. The description is up to date. |
Cloud Manager UI test results🎉 538 passing tests on test run #3 ↗︎
|
Description 📝
APL won't support LKE-E clusters until later this year (currently roadmapped for Q3). We need to disable it for LKE-E clusters until then and since it's upcoming let users know it is 'coming soon'.
Changes 🔄
isAPLSupportedto checkshowAPLand theselectedTierlke-create.spec.tsto check that an LKE-E cluster tier selection results in the section being disabled@hasyed-akamai's PR #11581 will need to replicate these changes for parity in the RHF cluster create flow.
Target release date 🗓️
3/25
Preview 📷
In summary, this was the change:
A more comprehensive reference of all the other possible states:
How to test 🧪
Prerequisites
(How to setup test environment)
Reproduction steps
(How to reproduce the issue, if applicable)
Verification steps
(How to verify changes)
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 ✅