Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f04e65f
upcoming: [DI-23778] - Added confirmation dialog component
nikhagra-akamai Mar 5, 2025
a404d98
upcoming: [DI-23778] - Added confirmation dialog to table component
nikhagra-akamai Mar 5, 2025
a595249
upcoming: [DI-23778] - Added queries to add/remove entities from alert
nikhagra-akamai Mar 5, 2025
58c0673
upcoming: [DI-23778] - added changeset
nikhagra-akamai Mar 5, 2025
3d0e2c6
Added index.tsx for PR testing
nikhagra-akamai Mar 5, 2025
6c4454c
upcoming: [DI-23778] - Updated on success method
nikhagra-akamai Mar 5, 2025
d7e296d
upcoming: [DI-23778] - Added optimistic update logic
nikhagra-akamai Mar 6, 2025
b6b99b0
upcoming: [DI-23778] - Updated queries to add/remove entity to alert
nikhagra-akamai Mar 7, 2025
8078b58
upcoming: [DI-23778] - Updated function name
nikhagra-akamai Mar 7, 2025
204c4b5
upcoming: [DI-23778] - Fixed testcases
nikhagra-akamai Mar 7, 2025
c5bfd04
upcoming: [DI-23778] - Added test cases
nikhagra-akamai Mar 10, 2025
c86f1d5
upcoming: [DI-23778] - Added todo comment
nikhagra-akamai Mar 10, 2025
6001e23
Merge branch 'develop' of github.com:linode/manager into alerts-conte…
nikhagra-akamai Mar 11, 2025
5738b53
revert testing changes
nikhagra-akamai Mar 11, 2025
13bf1c6
upcoming: [DI-23778] - Updated failing pipeline changes
nikhagra-akamai Mar 11, 2025
84e0029
Merge branch 'develop' of github.com:linode/manager into alerts-conte…
nikhagra-akamai Mar 11, 2025
a2b4a74
Merge branch 'linode:develop' into alerts-context-view-dialog
nikhagra-akamai Mar 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/api-v4": Upcoming Features
---

add `EntityAlertUpdatePayload` cloudpulse types.ts ([#11785](https://github.com/linode/manager/pull/11785))
31 changes: 31 additions & 0 deletions packages/api-v4/src/cloudpulse/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,34 @@ export const getAlertDefinitionByServiceType = (serviceType: string) =>
),
setMethod('GET')
);

export const addEntityToAlert = (
serviceType: string,
entityId: string,
data: { 'alert-definition-id': number }
) =>
Request<{}>(
Copy link
Copy Markdown
Contributor

@venkymano-akamai venkymano-akamai Mar 7, 2025

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Contributor Author

@nikhagra-akamai nikhagra-akamai Mar 7, 2025

Choose a reason for hiding this comment

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

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

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.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

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.

I've added a todo comment in the query function

setURL(
`${API_ROOT}/monitor/service/${encodeURIComponent(
serviceType
)}/entity/${encodeURIComponent(entityId)}/alert-definition`
),
setMethod('POST'),
setData(data)
);

export const deleteEntityFromAlert = (
serviceType: string,
entityId: string,
alertId: number
) =>
Request<{}>(
setURL(
`${API_ROOT}/monitor/service/${encodeURIComponent(
serviceType
)}/entity/${encodeURIComponent(
entityId
)}/alert-definition/${encodeURIComponent(alertId)}`
),
setMethod('DELETE')
);
6 changes: 6 additions & 0 deletions packages/api-v4/src/cloudpulse/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,3 +328,9 @@ export interface EditAlertPayloadWithService
}

export type AlertStatusUpdateType = 'Enable' | 'Disable';

export interface EntityAlertUpdatePayload {
serviceType: string;
entityId: string;
alertId: number;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Add `AlertsConfirmationDialog.tsx`, update `AlertInformationActionTable` to show `confirmation dialog` on toggle ([#11785](https://github.com/linode/manager/pull/11785))
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import userEvent from '@testing-library/user-event';
import React from 'react';

import { renderWithTheme } from 'src/utilities/testHelpers';

import { AlertConfirmationDialog } from './AlertConfirmationDialog';

const alertId = 1;
const alertName = 'alert-1';
const entityName = 'entity-1';
const serviceType = 'linode';
const confirmFunction = vi.fn();
const component = (
<AlertConfirmationDialog
alertId={alertId}
alertName={alertName}
entityName={entityName}
handleCancel={vi.fn()}
handleConfirm={confirmFunction}
isActive={true}
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 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

isOpen={true}
serviceType={serviceType}
/>
);

describe('Alert confirmation dialog', () => {
it('should show confirmation dialog', () => {
const { getByTestId, getByText } = renderWithTheme(component);

expect(getByTestId('confirmation-dialog')).toBeInTheDocument();
expect(getByText(`Disable ${alertName} Alert?`)).toBeVisible();
});
it('should click confirm button', async () => {
const { getByText } = renderWithTheme(component);

const cancelButton = getByText('Disable');

await userEvent.click(cancelButton);

expect(confirmFunction).toBeCalledWith(alertId, serviceType, true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { Typography } from '@linode/ui';
import React from 'react';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog';

interface AlertConfirmationDialogProps {
/**
* Id of the selected alert row
*/
alertId: number;

/**
* Name of the selected alert row
*/
alertName: string;

/**
* Name of the selected entity
*/
entityName: string;

/**
* Handler function for cancel button
*/
handleCancel: () => void;

/**
* Handler function for enable/disable button
* @param alertId id of the alert for the selected row
* @param serviceType service type of the selected entity
* @param currentStatus current state of the toggle button
*/
handleConfirm: (
alertId: number,
serviceType: string,
currentStatus: boolean
) => void;

/**
* Current state of the toggle button whether active or not
*/
isActive: boolean;

/**
* Loading state of the confirmation dialog
*/
isLoading?: boolean;

/**
* Current state of the confirmation dialoge whether open or not
*/
isOpen: boolean;

/**
* Service type of the selected entity
*/
serviceType: string;
}

export const AlertConfirmationDialog = React.memo(
(props: AlertConfirmationDialogProps) => {
const {
alertId,
alertName,
entityName,
handleCancel,
handleConfirm,
isActive,
isLoading = false,
isOpen,
serviceType,
} = props;

const actionsPanel = React.useCallback(() => {
return (
<ActionsPanel
primaryButtonProps={{
label: isActive ? 'Disable' : 'Enable',
loading: isLoading,
onClick: () => handleConfirm(alertId, serviceType, isActive),
}}
secondaryButtonProps={{
label: 'Cancel',
onClick: handleCancel,
Comment thread
nikhagra-akamai marked this conversation as resolved.
Outdated
}}
/>
);
}, [alertId, handleCancel, handleConfirm, isActive, serviceType]);
return (
<ConfirmationDialog
actions={actionsPanel}
data-testid="confirmation-dialog"
onClose={handleCancel}
open={isOpen}
title={`${isActive ? 'Disable' : 'Enable'} ${alertName} Alert?`}
>
<Typography variant="subtitle1">
Are you sure you want to {isActive ? 'disable' : 'enable'} the alert
for {entityName}?
</Typography>
</ConfirmationDialog>
);
}
);
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';

import { alertFactory } from 'src/factories/cloudpulse/alerts';
Expand Down Expand Up @@ -64,4 +65,19 @@ describe('Alert Listing Reusable Table for contextual view', () => {

expect(checkbox).toHaveProperty('checked');
});

it('Should show confirm dialog on checkbox click', async () => {
const { findByTestId, findByText } = renderWithTheme(
<AlertInformationActionTable {...props} />
);
const alert = alerts[0];
const row = await findByTestId(alert.id);

const checkbox = await within(row).findByRole('checkbox');

await userEvent.click(checkbox);

const text = await findByText(`Disable ${alert.label} Alert?`);
expect(text).toBeInTheDocument();
});
});
Loading