Skip to content

Conversation

@pmakode-akamai
Copy link
Contributor

@pmakode-akamai pmakode-akamai commented Aug 4, 2025

Description 📝

Premature validation of the numeric inputs in the Linode alerts tab prevents the user from updating the value. The bug is at a Linode details page such as https://cloud.linode.com/linodes/:id/alerts

This field requires a non-null value, but in a text input the natural behavior is to either 1) select the entire current value and then hit delete/backspace, or 2) place the cursor at the end of the text string and then repeatedly hit backspace. Because this is a required field, a null value in the numeric input disables the toggle switch which then disables the numeric input that the user is attempting to edit.

If I want to change the text value from "180" to "90", i am required to update individual characters and never leave the input blank. The attached screenshot recording demonstrates the bug and the odd workaround that is necessary to replace the current value.  

Note

Addressing this in a follow-up PR

Screenshot

Screenshot 2025-08-04 at 1 14 14 PM

Changes 🔄

  • Updated the behavior to allow numeric input fields to be temporarily cleared while editing, without disabling the field or defaulting the value to 0

Scope 🚢

Upon production release, changes in this PR will be visible to:

  • All customers
  • Some customers (e.g. in Beta or Limited Availability)
  • No customers / Not applicable

Target release date 🗓️

N/A

Preview 📷

Before After
Screen.Recording.2025-08-04.at.1.03.18.PM.mov
Screen.Recording.2025-08-04.at.12.58.43.PM.mov

How to test 🧪

Reproduction steps

Verification steps

  • Go to https://cloud.linode.com/linodes/:id/alerts
  • Clear a numeric input field (e.g., CPU alert) completely
  • Ensure the toggle remains enabled while the input is empty
  • Enter a new value (e.g., change 180 to 90) and confirm it's accepted
  • Toggle the switch on/off and confirm it continues to work as expected
  • Ensure entering 0 sets the toggle to the OFF state and keeps the input field disabled
  • Save changes and verify the value persists
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 tests and CI checks are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@pmakode-akamai pmakode-akamai marked this pull request as ready for review August 4, 2025 07:55
@pmakode-akamai pmakode-akamai requested a review from a team as a code owner August 4, 2025 07:55
@pmakode-akamai pmakode-akamai requested review from cpathipa, hana-akamai and mjac0bs and removed request for a team August 4, 2025 07:55
@pmakode-akamai pmakode-akamai added Bug Fixes for regressions or bugs Linodes Dealing with the Linodes section of the app labels Aug 4, 2025
Copy link
Contributor

@dmcintyr-akamai dmcintyr-akamai left a comment

Choose a reason for hiding this comment

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

can you create a subtask for tests?

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.

Thank you for fixing!

I'm pretty sure I'd observed this behavior before and noted as an existing issue, which is probably why you tagged me on this PR. 👍🏼

✅ Observed the bug, observed these changes address it, confirmed no console errors.

@pmakode-akamai re: the non-human-readable error messages for several of the fields - is this something we can submit to the API team for intake as a bug ticket?

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Aug 4, 2025
@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Aug 4, 2025
@pmakode-akamai
Copy link
Contributor Author

pmakode-akamai commented Aug 5, 2025

@pmakode-akamai re: the non-human-readable error messages for several of the fields - is this something we can submit to the API team for intake as a bug ticket?

Yes, I think we can. The non-human-readable error messages should ideally be more descriptive, and since this is an existing API issue, I'll go ahead and raise an ARB bug ticket for the API team.

Going to merge this PR so the issue remains reproducible and easy to reference.

EDIT: After taking another look, it looks like the validation issue is actually at the Linode Alerts object validation schema level in CM, not in the API 🤦🏻‍♂️. I'll open a quick follow-up PR to address it there.

Edit-2: Resolved validation message issue in PR: #12635

@pmakode-akamai
Copy link
Contributor Author

can you create a subtask for tests?

@dmcintyr-akamai Created M3-10433 ✅

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 3 failing tests on test run #3 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
3 Failing702 Passing4 Skipped120m 37s

Details

Failing Tests
SpecTest
linode-storage.spec.tsCloud Manager Cypress Tests→linode storage tab » delete disk
qemu-reboot-upgrade-notice.spec.tsCloud Manager Cypress Tests→QEMU reboot upgrade notification » should display maintenance banner in 'Linode' landing page when one or more Linodes get impacted.
object-storage-objects-multicluster.spec.tsCloud Manager Cypress Tests→Object Storage Multicluster objects » "before all" hook for "can upload, access, and delete objects"

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/linodes/linode-storage.spec.ts,cypress/e2e/core/notificationsAndEvents/qemu-reboot-upgrade-notice.spec.ts,cypress/e2e/core/objectStorageMulticluster/object-storage-objects-multicluster.spec.ts"

@pmakode-akamai pmakode-akamai merged commit 8faf4e4 into linode:develop Aug 5, 2025
34 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs Linodes Dealing with the Linodes section of the app

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants