Skip to content

fix: [DI-26675] - Alerts List cache bug fix for Edit Alert use-case - #12699

Merged
jaalah-akamai merged 5 commits into
linode:developfrom
santoshp210-akamai:bugfix/edit_alert_listing_cache
Aug 18, 2025
Merged

fix: [DI-26675] - Alerts List cache bug fix for Edit Alert use-case#12699
jaalah-akamai merged 5 commits into
linode:developfrom
santoshp210-akamai:bugfix/edit_alert_listing_cache

Conversation

@santoshp210-akamai

Copy link
Copy Markdown
Contributor

Description 📝

Conditionally setting the alert data for successful edit-alert

Changes 🔄

  • Setting the List query data with the updated alert only when the List cache is valid/available.

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 🗓️

26th August

Preview 📷

Before After
edit-alert-list-cache-before.mp4
edit-alert-list-cache-after.mp4

How to test 🧪

Prerequisites

  • Under Monitor, choose on Alerts.
    -Choose an Alert (either enabled or disabled) and click on the Action menu and choose Edit.
  • Edit the Alert and click on Submit

Verification steps

(How to verify changes)

  • Alert listing page should show all the alerts after a successful alert edit operation
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

@github-project-automation github-project-automation Bot moved this from Review to Approved in Cloud Manager Aug 14, 2025
@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Aug 15, 2025
Comment on lines +112 to +117
if (oldAlerts) {
queryClient.setQueryData<Alert[]>(
allAlertsQueryKey,
oldAlerts.map((alert) => (alert.id === data.id ? data : alert))
);
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use single setQueryData here ?

example:

queryClient.setQueryData<Alert[] | undefined>(allAlertsKey, (prev) => {
   // nothing cached yet 
 if (!prev) return prev;         

  const idx = prev.findIndex((a) => a.id === data.id);
  if (idx === -1) return prev;                

 // if no change keep referential equality
  if (prev[idx] === data) return prev;           

  const next = prev.slice();
  next[idx] = data;
  return next;
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed. Using setQueryData with an "updater" function is preferred over getQueryData + setQueryData

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will test this once and push the changes.

@santoshp210-akamai santoshp210-akamai Aug 18, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@cpathipa , @bnussman-akamai I have tested this change, will go ahead and push this code change.

"@linode/manager": Changed
---

ACLP-Alerting: Conditionally setting the query data on sucessful edit alert operation ([#12699](https://github.com/linode/manager/pull/12699))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
ACLP-Alerting: Conditionally setting the query data on sucessful edit alert operation ([#12699](https://github.com/linode/manager/pull/12699))
ACLP-Alerting: Conditionally set the query data on successful edit alert operation ([#12699](https://github.com/linode/manager/pull/12699))

@linode-gh-bot

Copy link
Copy Markdown

Cloud Manager UI test results

🎉 724 passing tests on test run #5 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing724 Passing4 Skipped117m 37s

@jaalah-akamai
jaalah-akamai merged commit 53faa5a into linode:develop Aug 18, 2025
35 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Merged in Cloud Manager Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

8 participants