upcoming: [UIE-10204] - Allow Simultaneous V1 (Legacy) and V2 (ACLP) Alerting in Linode Edit flow - #13455
Conversation
pmakode-akamai
added
the
ACLP Integration
|
|
||
| // Verify that account and region level alerts are not included in the payload | ||
| expect(mockUpdateAlerts).toHaveBeenCalledWith({ | ||
| alerts: { |
There was a problem hiding this comment.
I believe servicePayloadTransformerMap may need an update based on the latest behavior. It might make sense for the CloudPulse team to remove linode from this map as part of a separate cleanup.
For the linode service type, we probably don't need to pass the alerts wrapper to the service owners/consumers. Instead, it should just send the raw payload with system_alerts and user_alerts (which is currently working as expected). Also, this should ideally be triggered via the toggle switch, rather than on Save or during alert mutation for the linode service type.
| @@ -1,2 +1 @@ | |||
| export * from './useIsGeckoEnabled'; | |||
| export * from './useIsLinodeAclpSubscribed'; | |||
There was a problem hiding this comment.
This is no longer required due to changes in the Edit Linode Alerts flow (Linode can now support both legacy and ACLP simultaneously)
| }, | ||
| then: (schema) => schema.required(`${label} is required.`), | ||
| otherwise: (schema) => schema.notRequired(), | ||
| }); |
There was a problem hiding this comment.
This is no longer needed since the Edit Linode Alerts flow no longer has a switch to toggle between Legacy and ACLP modes and the below schema is now simplified
| <AlertInformationActionTable {...props} showConfirmationDialog /> | ||
| <AlertInformationActionTable | ||
| {...props} | ||
| serviceType="dbaas" |
There was a problem hiding this comment.
Had to use another service type (dbaas) instead of the default linode to make these tests pass since the linode service type does not have a Save button
Cloud Manager UI test results🔺 2 failing tests on test run #11 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/kubernetes/lke-create.spec.ts,cypress/e2e/core/objectStorage/object-storage.e2e.spec.ts" |
||||||||||||||||||||
dwiley-akamai
left a comment
dwiley-akamai
left a comment
There was a problem hiding this comment.
When I make a change to both legacy and ACLP alerts and try to save the changes, I get this error (happens both for linodes created w/ legacy alerts initially and w/ beta alerts initially):
@dwiley-akamai I believe this is because the API team hasn't finished updating the API to make it work. I've left a message in the project Slack channel to check when it will be available on DevCloud. |
tanushree-akamai
left a comment
tanushree-akamai
left a comment
There was a problem hiding this comment.
Changes look good! We just need to assure the API changes are in so that this doesn't cause breaking changes on prod.
pmakode-akamai
added
Approved
…Alerting in Linode Edit flow (#13455) * Save progress * More changes and some clean up... * Handle unified and standalone linode updates * Fix the ACLP alerts accordion collapse transition issue * Remove unnecessary default margins when stacking Accordions in edit flow * General error handling + some styling fixes * More changes * Handle root errors as well * Add scrollErrorIntoViewV2 for unified errors * Some more changes * Save progress * New approach * Allow simultaneous alerting in linode edit flow * Some more changes * Update mocks and remove the remaining useIsLinodeAclpSubscribed references * Fix CloudPulse ContextualView test cases * Update comments * Fix stale state: reset ACLP alerts to empty when entering ACLP mode * Added changeset: Allow simultaneous v1 (Legacy) and v2 (ACLP) alerting in Linode edit flow * Added changeset: The `useIsLinodeAclpSubscribed` hook from the shared package * Added changeset: Simplify `UpdateLinodeAlertsSchema` to support simultaneous legacy and ACLP alerting * Update comment * Don't allow unified saving if aclp reusable component is not ready * Fix unit tests * Few changes to aclp components * Update comments for clarity * Update changeset and consolidate imports * Update comments and exclude fields from payload for bare metal type * Remove Bare Metal related logic from legacy Alerts
7 participants
Description 📝
Allow Simultaneous V1 (Legacy) and V2 (ACLP) Alerting in Linode Edit flow
Changes 🔄
Edit Linode -> Alerts -> bannerisLinodeAclpSubscribedhook from thesharedpackage, along with related references, as it is no longer required due to the Edit Linode Alerts flow changesUpdateLinodeAlertsSchemaScope 🚢
Target release date 🗓️
March end release
Preview 📷
Only Legacy AlertsUI will be displayedLegacy AlertsandACLP AlertsUI withunified Savewill be displayedScreen.Recording.2026-03-05.at.2.20.02.PM.mov
How to test 🧪
Prerequisites
aclpServices -> linode -> alertsfeature flaglinodeservice type) to your Devcloud admin account to enable ACLP in the available region (customer tag is mentioned in the ticket's description)/regionsAPI endpoint and ensure thatLinodesis listed undermonitors.alertsfor that region:Verification steps
Test Using Mocks
Save AlertsbuttonSave Alerts:Save AlertsbuttonTest with Real Data (Devcloud)
Save Alerts:Save AlertsbuttonProd, as Devcloud has only one region)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 ✅