Skip to content

Conversation

@pmakode-akamai
Copy link
Contributor

@pmakode-akamai pmakode-akamai commented Jun 18, 2025

Description 📝

Following the merge of the Toggle ADS PR, a regression is introduced in the Linode Create flow. The Legacy Mode Alerts toggles are now incorrectly enabled when they should be disabled/ready-only.

Note

  • The toggle ADS PR was merged after the release cut, so a changeset for this PR is not required
  • We'll create a separate ticket (if one doesn't already exist) to add tests ensuring that Legacy Alerts are disabled in the Create Linode flow and enabled in the Linode Details/Edit flow cc: @jdamore-linode @dmcintyr-akamai

Changes 🔄

Target release date 🗓️

1 July (Next release)

Preview 📷

On Dev branch On this branch
Screenshot 2025-06-18 at 6 10 17 PM Screenshot 2025-06-18 at 7 14 06 PM

How to test 🧪

Prerequisites

  • Enable MSW & aclpBetaServices -> alerts feature flag

Reproduction steps

  • Enable MSW & aclpBetaServices -> alerts feature flag
  • Navigate to Create Linode page
  • Select aclp supported region -> Washington DC, or Newark NJ
  • Go to Additional Option Section -> Alerts
  • For legacy mode -> toggle should have been disabled but they got enabled

Verification steps

  • In the Create Linode page:
    • Select aclp supported region -> Washington DC, or Newark , NJ
    • Go to Additional Option Section -> Legacy Alerts
    • Alerts with toggle options and input fields should remain in a disabled/read-only state when Legacy Mode is active
  • In Linode Details page -> Alerts -> Legacy Mode:
    • Alerts with toggle options and input fields should remain active (editable) in Legacy Mode
  • Ensure all related references are updated correctly
  • Ensure storybook for toggle disabled state works as expected
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

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@pmakode-akamai pmakode-akamai self-assigned this Jun 18, 2025
@pmakode-akamai pmakode-akamai added the Bug Fixes for regressions or bugs label Jun 18, 2025
@pmakode-akamai pmakode-akamai marked this pull request as ready for review June 18, 2025 13:56
@pmakode-akamai pmakode-akamai requested a review from a team as a code owner June 18, 2025 13:56
@pmakode-akamai pmakode-akamai requested review from cpathipa, dwiley-akamai and mjac0bs and removed request for a team June 18, 2025 13:56
@pmakode-akamai pmakode-akamai added Linodes Dealing with the Linodes section of the app LKE Related to Linode Kubernetes Engine offerings For Next Release Apply this label if the PR needs to be in the next release. labels Jun 18, 2025
@jaalah-akamai jaalah-akamai removed the For Next Release Apply this label if the PR needs to be in the next release. label Jun 18, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 1 failing test on test run #2 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing663 Passing4 Skipped120m 30s

Details

Failing Tests
SpecTest
smoke-linode-landing-table.spec.tsCloud Manager Cypress Tests→linode landing checks for non-empty state with restricted user » linode landing checks for non-empty state with restricted user

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts"

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linode Create --> Additional Options --> Alerts section in Legacy mode: alert toggles disabled ✅

Linode Details --> Alerts tab legacy alerts: can be toggled ✅

Code review ✅

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pmakode-akamai.

✅ Observed the bug and confirmed that Linode Create flow legacy alerts are toggled on but not editable, are editable from the Linode details page, and the LKE ACL toggle still behaves as expected for LKE and LKE-E.
✅ Changeset not needed because bug was not released.

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Jun 18, 2025
@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Jun 18, 2025
@pmakode-akamai pmakode-akamai merged commit 84e0a0e into linode:develop Jun 18, 2025
34 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Jun 18, 2025
@pmakode-akamai pmakode-akamai added the ACLP Integration CI (Cloud Interfaces) Support for CC (Core Compute) CloudPulse Integration label Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ACLP Integration CI (Cloud Interfaces) Support for CC (Core Compute) CloudPulse Integration Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs Linodes Dealing with the Linodes section of the app LKE Related to Linode Kubernetes Engine offerings

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants