upcoming: [M3-7131] - Add AGLB Configuration Edit Tests & General Bug Fixes

[bnussman-akamai]

Description 📝

• Adds AGLB e2es for updating configurations using only mock data 🧪
• Also improves error handling for AGLB configurations 🚫
• Disabled the "Save Configuration" button if no changes have been made ⏹️
• Fixed bug where a rule would not be creatable (we pass label to fix this) 🐛
• Only shows downstream certificates when configuring a Configuration 🔐
• Handles the certificates endpoint not returning certificate for alpha 🚫

Preview 📷

Shows button behavior when editing 👁️

Screen.Recording.2023-11-28.at.3.26.17.PM.mov

Example of how complex API errors will be shown 👁️

How to test 🧪

Verification steps

• Run yarn cy:debug
• Run the load-balancer-configurations.spec.ts tests
• Manually test the Configurations tab with the MSW on for any regressions

As an Author I have 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

[mjac0bs]

🎉 for tests and all the clean up and bug fixes you did!

✅ load-balancer-configurations.spec.ts passes locally and in CI; it covers error handling
✅ No regressions in testing the AGLB configuration flow in the UI - I can create, update, and delete a configuration
✅ The "Save Configuration" button is disabled if no changes have been made and enabled once they have
✅ The "Reset" button appears once an existing configuration is edited, and if clicked, it clears the edits to the form
❓ The exception to the above seems to be if a rule is added or edited to an existing route. This isn't a change that can be reset.
✅ Rules are creatable
✅ Only downstream certificates are shown configuring a Configuration

I'm approving this since all of the above are looking good and the other things I'll mention are minor. In testing, I did notice

• ❓ The API returns an error if a rule is created or edited to not have any service targets, though our UI allows a user to delete service targets even if there is just one listed. Do we want to continue to allow that or disable the ability to remove a service target in that case?

• A console error caused by the beta chip in the create flow. If you want to fix it here as a "general bug fix" or punt it to another PR of small bug fixes and clean up, either will work. (Maybe it better fits with #9849 changes, I don't know. 🤷🏼 )

[cpathipa]

Observation on validation behavior on Configuration label. Is this expected ?

Config_label_validation.mov

[cpathipa]

Overall LGTM!

• Verified load-balancer-configurations.spec.ts locally.
• No regressions in testing the AGLB configuration flow.
• Verified "Save Configuration" button.
• Verified "Reset" button functionality.

[cpathipa]

Nice!

[cpathipa]

🎉 for tests and all the clean up and bug fixes you did!

✅ load-balancer-configurations.spec.ts passes locally and in CI; it covers error handling ✅ No regressions in testing the AGLB configuration flow in the UI - I can create, update, and delete a configuration ✅ The "Save Configuration" button is disabled if no changes have been made and enabled once they have ✅ The "Reset" button appears once an existing configuration is edited, and if clicked, it clears the edits to the form ❓ The exception to the above seems to be if a rule is added or edited to an existing route. This isn't a change that can be reset. ✅ Rules are creatable ✅ Only downstream certificates are shown configuring a Configuration

I'm approving this since all of the above are looking good and the other things I'll mention are minor. In testing, I did notice

• ❓ The API returns an error if a rule is created or edited to not have any service targets, though our UI allows a user to delete service targets even if there is just one listed. Do we want to continue to allow that or disable the ability to remove a service target in that case?

• A console error caused by the beta chip in the create flow. If you want to fix it here as a "general bug fix" or punt it to another PR of small bug fixes and clean up, either will work. (Maybe it better fits with feat: [M3-7292] - Stepper component - Details content #9849 changes, I don't know. 🤷🏼 )

@mjac0bs I think we should handle all the places the chip is being used. IMO, considering the scope of the PR, a separate ticket make sense.

I think the usage should be like the below instead of rendering the chip within the Typography.

 <Box sx={{ alignItems: 'center', display: 'flex' }}>
       <BetaChip
             sx={(theme) => ({
             marginLeft: '0 !important',
             marginRight: `${theme.spacing(1)} !important`,
           })}
        />
       <Typography>
          After the load balancer is created, and if the protocol is HTTPS, upload TLS termination certificates. Learn more.
       </Typography>
 </Box>

[bnussman-akamai]

Pushed some changes

• Rules are required to have at least one service target
• Improved weird validation behavior @mjac0bs found

Thank you for all of the great feedback. Unless something is urgent, I will address outstanding items in the next bug-fix PR!