upcoming: [DI-23778] - Added. confirmation dialog in alerts contextual view by nikhagra-akamai · Pull Request #11785 · linode/manager

nikhagra-akamai · 2025-03-05T07:11:47Z

Description 📝

Added confirmation dialog in contextual view while enabling/disabling an alert for the selected entity

Changes 🔄

List any change(s) relevant to the reviewer.

Added AlertsConfirmationDialog.tsx
Updated AlertsInformationActionTable.tsx to add confirmation dialog on click of toggle button
Added queries in alerts.ts for adding or removing entity to the alert.

Target release date 🗓️

15th March

Preview 📷

Include a screenshot or screen recording of the change.

🔒 Use the Mask Sensitive Data setting for security.

💡 Use <video src="" /> tag when including recordings in table.

Figma	After

How to test 🧪

Switch to mock user
Go to database from mega menu & select any active database
Go to Alerts tab
Click on toggle button to see confirmation dialog.

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

github-actions · 2025-03-05T07:52:13Z

Coverage Report: ✅
Base Coverage: 80.08%
Current Coverage: 80.08%

packages/manager/src/queries/cloudpulse/alerts.ts

ankita-akamai · 2025-03-05T07:58:47Z

packages/manager/src/features/CloudPulse/Alerts/ContextualView/AlertInformationActionTable.tsx

+        entityName={entityName}
+        handleCancel={handleCancel}
+        handleConfirm={handleConfirm}
+        isActive={selectedAlert?.entity_ids?.includes(entityId) ?? false}


Suggested change

isActive={selectedAlert?.entity_ids?.includes(entityId) ?? false}

isActive={selectedAlert.entity_ids.includes(entityId)}

on initial loading of action table component this object will be undefined that's why that is necessary

nikhagra-akamai · 2025-03-05T13:16:07Z

packages/manager/src/features/Databases/DatabaseDetail/index.tsx

  }))
 );
+
+const DatabaseAlert = React.lazy(() =>


I've added these changes for PR testing purpose and will remove it before merging the PR

venkymano-akamai · 2025-03-07T03:59:54Z

packages/api-v4/src/cloudpulse/alerts.ts

+    entityId: string,
+    data: { 'alert-definition-id': number }
+  ) =>
+    Request<{}>(


@nikhagra-akamai , here we are returning a json object {}, is there any valid return type like, alert with entity details like that?, API should return some response right?

same below as well

as of now no any confirmation on its return type because some discussions are going on about this update APIs

So is this PR just for the visual presentation of the confirmation dialog? When is the API response expected to be finalized?

@mjac0bs Yes!! This PR is just for preparing from the UI side. Since it's a reusable component, it won’t be visible anywhere in the UI until the service-specific APIs are finalized and approved in terms of spec and implementation. Discussions with service owners are ongoing to finalize these APIs. Once that’s done, we’ll integrate them and submit a final PR, allowing this feature to be fully integrated and visible in the UI. Until then, this remains a UI-ready code update.

@nikhagra-akamai May be you can place a comment /ToDo task in this to clear the confusion and setting the expectation.

I've added a todo comment in the query function

packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertConfirmationDialog.tsx

venkymano-akamai · 2025-03-07T04:04:42Z

packages/manager/src/features/CloudPulse/Alerts/ContextualView/AlertInformationActionTable.tsx

+          );
+        })
+        .catch(() => {
+          enqueueSnackbar(


@nikhagra-akamai , here it is a generic error? do we need to display error message from API?

will update the logic here once the API responses will be finalized

venkymano-akamai · 2025-03-07T04:07:37Z

packages/manager/src/queries/cloudpulse/alerts.ts

  });
 };
+
+export const useAddEntityToAlert = () => {


@nikhagra-akamai , for my better understanding, can you explain how this is different from invalidating the queries on success as well error, why we need this much complexities, if just invalidating works fine?

One more point to ask, since we are waiting for the API call to get completed before closing the confirmation dialog, we don't need immediate feedback in the UI, this makes one more point for just invalidating queries?

updated, thanks

venkymano-akamai

Gave some simple comments, as well as one explanation in terms of invalidating queries, Rest looks good.

nikhagra-akamai · 2025-03-07T07:53:43Z

Gave some simple comments, as well as one explanation in terms of invalidating queries, Rest looks good.

Thanks for review, I've updated the logic

venkymano-akamai

Approving, pending fix for typecheck and UT's

mjac0bs · 2025-03-07T21:58:35Z

packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertConfirmationDialog.test.tsx

+    entityName={entityName}
+    handleCancel={vi.fn()}
+    handleConfirm={confirmFunction}
+    isActive={true}


Can we add some coverage to this test so that we test both an alert that is active and not active?

We could include assertions for whether:

the title includes "Enable" vs "Disable"

the body includes "enable" vs "disable"

the entityName is included in the text

packages/manager/src/features/CloudPulse/Alerts/ContextualView/AlertInformationActionTable.tsx

mjac0bs · 2025-03-07T22:22:26Z

packages/api-v4/src/cloudpulse/alerts.ts

+    entityId: string,
+    data: { 'alert-definition-id': number }
+  ) =>
+    Request<{}>(


So is this PR just for the visual presentation of the confirmation dialog? When is the API response expected to be finalized?

…xt-view-dialog

mjac0bs

Thanks for addressing feedback.

linode-gh-bot · 2025-03-12T01:34:41Z

Cloud Manager UI test results

🔺 1 failing test on test run #14 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`1 Failing`	`537 Passing`	`3 Skipped`	`120m 6s`

Details

Failing Tests
	Spec	Test
❌	`linode-config.spec.ts`	End-to-End » Clones a config

Troubleshooting

Use this command to re-run the failing tests:

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

cypress · 2025-03-12T04:40:22Z

Cloud Manager E2E Run #7385

Run Properties: Passed #7385 • 4f9504a065: Merge pull request #11785 from nikhagra-akamai/alerts-context-view-dialog

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Passed #7385`
Run duration	`57m 50s`
Commit	`4f9504a065: Merge pull request #11785 from nikhagra-akamai/alerts-context-view-dialog`
Committer	`Nikhil Agrawal`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`6`
Pending	`3`
Skipped	`0`
Passing	`538`
View all changes introduced in this branch ↗︎

nikhagra-akamai added 3 commits March 5, 2025 12:33

upcoming: [DI-23778] - Added confirmation dialog component

f04e65f

upcoming: [DI-23778] - Added confirmation dialog to table component

a404d98

upcoming: [DI-23778] - Added queries to add/remove entities from alert

a595249

nikhagra-akamai requested a review from a team as a code owner March 5, 2025 07:11

nikhagra-akamai removed the request for review from a team March 5, 2025 07:11

github-project-automation bot added this to Cloud Manager Mar 5, 2025

nikhagra-akamai requested review from coliu-akamai and mjac0bs March 5, 2025 07:11

github-project-automation bot moved this to Review in Cloud Manager Mar 5, 2025

nikhagra-akamai self-assigned this Mar 5, 2025

nikhagra-akamai added Ready for Review Cloud Pulse Cloud Pulse - Alerting labels Mar 5, 2025

upcoming: [DI-23778] - added changeset

58c0673

ankita-akamai reviewed Mar 5, 2025

View reviewed changes

packages/manager/src/queries/cloudpulse/alerts.ts Outdated Show resolved Hide resolved

ankita-akamai reviewed Mar 5, 2025

View reviewed changes

Added index.tsx for PR testing

3d0e2c6

nikhagra-akamai commented Mar 5, 2025

View reviewed changes

upcoming: [DI-23778] - Updated on success method

6c4454c

nikhagra-akamai requested a review from jaalah-akamai March 5, 2025 13:25

ankita-akamai approved these changes Mar 5, 2025

View reviewed changes

upcoming: [DI-23778] - Added optimistic update logic

d7e296d

venkymano-akamai reviewed Mar 7, 2025

View reviewed changes

packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertConfirmationDialog.tsx Outdated Show resolved Hide resolved

venkymano-akamai reviewed Mar 7, 2025

View reviewed changes

venkymano-akamai requested changes Mar 7, 2025

View reviewed changes

github-project-automation bot moved this from Review to Changes Requested in Cloud Manager Mar 7, 2025

upcoming: [DI-23778] - Updated queries to add/remove entity to alert

b6b99b0

upcoming: [DI-23778] - Updated function name

8078b58

nikhagra-akamai requested a review from venkymano-akamai March 7, 2025 07:53

venkymano-akamai approved these changes Mar 7, 2025

View reviewed changes

upcoming: [DI-23778] - Fixed testcases

204c4b5

mjac0bs reviewed Mar 7, 2025

View reviewed changes

upcoming: [DI-23778] - Added test cases

c5bfd04

nikhagra-akamai requested a review from mjac0bs March 10, 2025 06:44

nikhagra-akamai added 5 commits March 10, 2025 19:33

upcoming: [DI-23778] - Added todo comment

c86f1d5

Merge branch 'develop' of github.com:linode/manager into alerts-conte…

6001e23

…xt-view-dialog

revert testing changes

5738b53

upcoming: [DI-23778] - Updated failing pipeline changes

13bf1c6

Merge branch 'develop' of github.com:linode/manager into alerts-conte…

84e0029

…xt-view-dialog

mjac0bs approved these changes Mar 11, 2025

View reviewed changes

github-project-automation bot moved this from Changes Requested to Approved in Cloud Manager Mar 11, 2025

mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Mar 11, 2025

Merge branch 'linode:develop' into alerts-context-view-dialog

a2b4a74

nikhagra-akamai merged commit 4f9504a into linode:develop Mar 12, 2025
26 of 27 checks passed

github-project-automation bot moved this from Approved to Merged in Cloud Manager Mar 12, 2025

This was referenced Mar 20, 2025

Release v1.138.0 - release → staging #11895

Merged

Release v1.138.0 - staging → master #11900

Merged

	isActive={selectedAlert?.entity_ids?.includes(entityId) ?? false}
	isActive={selectedAlert.entity_ids.includes(entityId)}

Conversation

nikhagra-akamai commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

As an Author, to speed up the review process, I considered 🤔

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

Uh oh!

github-actions bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ankita-akamai Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkymano-akamai Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikhagra-akamai Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkymano-akamai Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkymano-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

nikhagra-akamai commented Mar 7, 2025

Uh oh!

venkymano-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjac0bs left a comment

Choose a reason for hiding this comment

Uh oh!

linode-gh-bot commented Mar 12, 2025

Cloud Manager UI test results

🔺 1 failing test on test run #14 ↗︎

Details

Troubleshooting

Uh oh!

Uh oh!

cypress bot commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

nikhagra-akamai commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

ankita-akamai Mar 5, 2025 •

edited

Loading

venkymano-akamai Mar 7, 2025 •

edited

Loading

nikhagra-akamai Mar 7, 2025 •

edited

Loading

venkymano-akamai Mar 7, 2025 •

edited

Loading