Skip to content

Commit 9fcb771

Browse files
pmakode-akamaipmakode-akamai
authored
upcoming: [UIE-10204] - Allow Simultaneous V1 (Legacy) and V2 (ACLP) Alerting in Linode Edit flow (#13455)
* Save progress * More changes and some clean up... * Handle unified and standalone linode updates * Fix the ACLP alerts accordion collapse transition issue * Remove unnecessary default margins when stacking Accordions in edit flow * General error handling + some styling fixes * More changes * Handle root errors as well * Add scrollErrorIntoViewV2 for unified errors * Some more changes * Save progress * New approach * Allow simultaneous alerting in linode edit flow * Some more changes * Update mocks and remove the remaining useIsLinodeAclpSubscribed references * Fix CloudPulse ContextualView test cases * Update comments * Fix stale state: reset ACLP alerts to empty when entering ACLP mode * Added changeset: Allow simultaneous v1 (Legacy) and v2 (ACLP) alerting in Linode edit flow * Added changeset: The `useIsLinodeAclpSubscribed` hook from the shared package * Added changeset: Simplify `UpdateLinodeAlertsSchema` to support simultaneous legacy and ACLP alerting * Update comment * Don't allow unified saving if aclp reusable component is not ready * Fix unit tests * Few changes to aclp components * Update comments for clarity * Update changeset and consolidate imports * Update comments and exclude fields from payload for bare metal type * Remove Bare Metal related logic from legacy Alerts
1 parent 952f363 commit 9fcb771

22 files changed

Lines changed: 773 additions & 474 deletions

‎packages/manager/.changeset/pr-13455-upcoming-features-1772701630712.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@linode/manager": Upcoming Features
3+
---
4+
5+
Allow simultaneous v1 (Legacy) and v2 (ACLP) alerting in Linode edit flow ([#13455](https://github.com/linode/manager/pull/13455))

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

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,11 @@ describe('Alert Listing Reusable Table for contextual view', () => {
108108

109109
it('Should show confirm dialog on save button click when changes are made', async () => {
110110
renderWithTheme(
111-
<AlertInformationActionTable {...props} showConfirmationDialog />
111+
<AlertInformationActionTable
112+
{...props}
113+
serviceType="dbaas"
114+
showConfirmationDialog
115+
/>
112116
);
113117

114118
// First toggle an alert to make changes
@@ -127,7 +131,9 @@ describe('Alert Listing Reusable Table for contextual view', () => {
127131
});
128132

129133
it('Should hide confirm dialog on save button click when changes are made', async () => {
130-
renderWithTheme(<AlertInformationActionTable {...props} />);
134+
renderWithTheme(
135+
<AlertInformationActionTable {...props} serviceType="dbaas" />
136+
);
131137

132138
// First toggle an alert to make changes
133139
const alert = alerts[0];
@@ -145,14 +151,22 @@ describe('Alert Listing Reusable Table for contextual view', () => {
145151
});
146152

147153
it('Should have save button in disabled form when no changes are made', () => {
148-
renderWithTheme(<AlertInformationActionTable {...props} />);
154+
renderWithTheme(
155+
<AlertInformationActionTable {...props} serviceType="dbaas" />
156+
);
149157

150158
const saveButton = screen.getByTestId('save-alerts');
151159
expect(saveButton).toBeDisabled();
152160
});
153161

154162
it('Should send correct payload to the API when save button is clicked in edit mode', async () => {
155-
renderWithTheme(<AlertInformationActionTable {...props} alerts={alerts} />);
163+
renderWithTheme(
164+
<AlertInformationActionTable
165+
{...props}
166+
alerts={alerts}
167+
serviceType="dbaas"
168+
/>
169+
);
156170

157171
// Toggle entity-level user alert with ID 2 to enable it
158172
const userAlertRow = await screen.findByTestId('9');
@@ -164,10 +178,38 @@ describe('Alert Listing Reusable Table for contextual view', () => {
164178

165179
// Verify that account and region level alerts are not included in the payload
166180
expect(mockUpdateAlerts).toHaveBeenCalledWith({
167-
alerts: {
181+
system_alerts: [1, 2, 3, 4, 5, 6, 7],
182+
user_alerts: [9],
183+
});
184+
});
185+
186+
it('Should not render save button for linode service type', () => {
187+
// For linode, save is handled by the service owner component (e.g. LinodeAlerts unified save button).
188+
renderWithTheme(<AlertInformationActionTable {...props} />); // props.serviceType is 'linode'
189+
190+
expect(screen.queryByTestId('save-alerts')).not.toBeInTheDocument();
191+
});
192+
193+
it('Should call onToggleAlert with correct payload when a toggle is clicked for linode service type', async () => {
194+
// Even though the save button is hidden for linode, the toggle still fires onToggleAlert
195+
// so the service owner can collect the payload for its own save flow.
196+
const onToggleAlert = vi.fn();
197+
renderWithTheme(
198+
<AlertInformationActionTable {...props} onToggleAlert={onToggleAlert} />
199+
);
200+
201+
// Toggle entity-level user alert id 9 to enable it
202+
const userAlertRow = await screen.findByTestId('9');
203+
await userEvent.click(within(userAlertRow).getByRole('checkbox'));
204+
205+
// Raw payload passed to service owner: system_alerts unchanged, user_alerts with the newly toggled alert id 9,
206+
// and hasUnsavedChanges is true
207+
expect(onToggleAlert).toHaveBeenCalledWith(
208+
{
168209
system_alerts: [1, 2, 3, 4, 5, 6, 7],
169210
user_alerts: [9],
170211
},
171-
});
212+
true // hasUnsavedChanges
213+
);
172214
});
173215
});

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

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ export interface AlertRowPropsOptions {
121121
) => void;
122122
}
123123

124+
/**
125+
* Service types whose parent component handles saving alerts externally.
126+
* The internal Save button is hidden for these service types.
127+
* Add a service type here to opt out of the built-in Save button.
128+
*/
129+
const SERVICES_WITH_EXTERNAL_SAVE: CloudPulseServiceType[] = ['linode'];
130+
124131
export const AlertInformationActionTable = (
125132
props: AlertInformationActionTableProps
126133
) => {
@@ -159,6 +166,12 @@ export const AlertInformationActionTable = (
159166
const updateAlerts = useAlertsMutation(serviceType, entityId ?? '');
160167

161168
React.useEffect(() => {
169+
// To send initial state of alerts through toggle handler function in edit mode.
170+
// This ensures the service owner receives the initial enabled-alert state immediately
171+
// when the component is ready, regardless of whether any toggle action is performed.
172+
if (isEditMode && onToggleAlert) {
173+
onToggleAlert(enabledAlerts);
174+
}
162175
return () => {
163176
// Cleanup on unmount (For Edit flow)
164177
if (isEditMode && onToggleAlert) {
@@ -345,28 +358,37 @@ export const AlertInformationActionTable = (
345358
handleSizeChange={handlePageSizeChange}
346359
page={page}
347360
pageSize={pageSize}
361+
sx={{
362+
// Prevents layout breaks and enables smooth collapse when table with this footer is used inside an Accordion.
363+
// Without this, the PaginationFooter causes layout shifts during the collapse transition.
364+
contain: 'layout',
365+
}}
348366
/>
349367
</Box>
350-
{isEditMode && (
351-
<Box>
352-
<Button
353-
buttonType="primary"
354-
data-qa-buttons="true"
355-
data-testid="save-alerts"
356-
disabled={!hasUnsavedChanges || isLoading}
357-
loading={isLoading}
358-
onClick={() => {
359-
if (showConfirmationDialog) {
360-
setIsDialogOpen(true);
361-
} else {
362-
handleConfirm(enabledAlerts);
363-
}
364-
}}
365-
>
366-
Save
367-
</Button>
368-
</Box>
369-
)}
368+
{/* Show save button only in edit mode. Service types listed in
369+
SERVICES_WITH_EXTERNAL_SAVE manage their own save externally
370+
(e.g. linode handles it in the parent component). */}
371+
{isEditMode &&
372+
!SERVICES_WITH_EXTERNAL_SAVE.includes(serviceType) && (
373+
<Box>
374+
<Button
375+
buttonType="primary"
376+
data-qa-buttons="true"
377+
data-testid="save-alerts"
378+
disabled={!hasUnsavedChanges || isLoading}
379+
loading={isLoading}
380+
onClick={() => {
381+
if (showConfirmationDialog) {
382+
setIsDialogOpen(true);
383+
} else {
384+
handleConfirm(enabledAlerts);
385+
}
386+
}}
387+
>
388+
Save
389+
</Button>
390+
</Box>
391+
)}
370392
</>
371393
)}
372394
</Paginate>

‎packages/manager/src/features/CloudPulse/Alerts/ContextualView/AlertReusableComponent.test.tsx‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,33 @@ describe('Alert Resuable Component for contextual view', () => {
114114
});
115115

116116
it('Should show header for edit mode', async () => {
117-
renderWithTheme(component, {
118-
initialEntries: ['/alerts/definitions'],
119-
initialRoute: '/alerts/definitions',
120-
});
121-
await userEvent.click(screen.getByText('Manage Alerts'));
117+
// For service types not in SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW (e.g. dbaas),
118+
// the 'Alerts' heading and 'Manage Alerts' button appear together in the section header.
119+
renderWithTheme(
120+
<AlertReusableComponent
121+
entityId={entityId}
122+
entityName={entityName}
123+
onToggleAlert={onToggleAlert}
124+
regionId={region}
125+
serviceType="dbaas"
126+
/>,
127+
{
128+
initialEntries: ['/alerts/definitions'],
129+
initialRoute: '/alerts/definitions',
130+
}
131+
);
122132
expect(screen.getByText('Alerts')).toBeVisible();
133+
expect(screen.getByTestId('manage-alerts')).toBeVisible();
134+
});
135+
136+
it('Should not show Alerts heading for linode service type but still show Manage Alerts button in filter row', () => {
137+
// For service types in SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW (e.g. linode), the 'Alerts' heading
138+
// belongs to the service owner and is not rendered here. The Manage Alerts button moves to the filter row.
139+
renderWithTheme(component); // component uses serviceType='linode' with entityId
140+
141+
expect(
142+
screen.queryByRole('heading', { level: 2, name: 'Alerts' })
143+
).not.toBeInTheDocument();
144+
expect(screen.getByTestId('manage-alerts')).toBeVisible();
123145
});
124146
});

‎packages/manager/src/features/CloudPulse/Alerts/ContextualView/AlertReusableComponent.tsx‎

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type {
2424
CloudPulseAlertsPayload,
2525
CloudPulseServiceType,
2626
} from '@linode/api-v4';
27+
import type { SxProps, Theme } from '@linode/ui';
2728

2829
interface AlertReusableComponentProps {
2930
/**
@@ -41,6 +42,14 @@ interface AlertReusableComponentProps {
4142
*/
4243
isLegacyAlertAvailable?: boolean;
4344

45+
/**
46+
* Called once when this component is ready - i.e. alerts have loaded
47+
* successfully without error. Always receives `true`.
48+
* Service owners can use this to enable or disable save buttons that depend
49+
* on the readiness of this component before allowing any action.
50+
*/
51+
onStatusChange?: (isReady: boolean) => void;
52+
4453
/**
4554
* Called when an alert is toggled on or off.
4655
* @param payload enabled alerts ids
@@ -51,6 +60,11 @@ interface AlertReusableComponentProps {
5160
hasUnsavedChanges?: boolean
5261
) => void;
5362

63+
/**
64+
* Custom sx styles for the Paper wrapper component
65+
*/
66+
paperSx?: SxProps<Theme>;
67+
5468
/**
5569
* Region ID for the selected entity
5670
*/
@@ -62,11 +76,21 @@ interface AlertReusableComponentProps {
6276
serviceType: CloudPulseServiceType;
6377
}
6478

79+
/**
80+
* Service types that display the Manage Alerts button inline with the search/filter row.
81+
* For all other service types, the button appears inline with the Alerts section header.
82+
*/
83+
const SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW: CloudPulseServiceType[] = [
84+
'linode',
85+
];
86+
6587
export const AlertReusableComponent = (props: AlertReusableComponentProps) => {
6688
const {
6789
entityId,
6890
entityName,
6991
onToggleAlert,
92+
onStatusChange,
93+
paperSx,
7094
serviceType,
7195
regionId,
7296
isLegacyAlertAvailable,
@@ -77,6 +101,13 @@ export const AlertReusableComponent = (props: AlertReusableComponentProps) => {
77101
isLoading,
78102
} = useAlertDefinitionByServiceTypeQuery(serviceType);
79103

104+
React.useEffect(() => {
105+
// Only notify the parent when ready
106+
if (!isLoading && !error) {
107+
onStatusChange?.(true);
108+
}
109+
}, [isLoading, error, onStatusChange]);
110+
80111
const [searchText, setSearchText] = React.useState<string>('');
81112
const [selectedType, setSelectedType] = React.useState<
82113
AlertDefinitionType | undefined
@@ -100,24 +131,26 @@ export const AlertReusableComponent = (props: AlertReusableComponentProps) => {
100131
}
101132

102133
return (
103-
<Paper sx={{ p: entityId ? undefined : 0 }}>
134+
<Paper sx={paperSx}>
104135
<Stack gap={3}>
105-
{entityId && (
106-
<Box display="flex" justifyContent="space-between">
107-
<Box alignItems="center" display="flex" gap={0.5}>
108-
<Typography variant="h2">Alerts</Typography>
109-
{aclpServices?.[serviceType]?.alerts?.beta && <BetaChip />}
136+
{/* Not in SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW: Show Manage Alerts button inline with the Alerts section header */}
137+
{entityId &&
138+
!SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW.includes(serviceType) && (
139+
<Box display="flex" justifyContent="space-between">
140+
<Box alignItems="center" display="flex" gap={0.5}>
141+
<Typography variant="h2">Alerts</Typography>
142+
{aclpServices?.[serviceType]?.alerts?.beta && <BetaChip />}
143+
</Box>
144+
<Button
145+
buttonType="outlined"
146+
data-qa-buttons="true"
147+
data-testid="manage-alerts"
148+
onClick={() => navigate({ to: '/alerts/definitions' })}
149+
>
150+
Manage Alerts
151+
</Button>
110152
</Box>
111-
<Button
112-
buttonType="outlined"
113-
data-qa-buttons="true"
114-
data-testid="manage-alerts"
115-
onClick={() => navigate({ to: '/alerts/definitions' })}
116-
>
117-
Manage Alerts
118-
</Button>
119-
</Box>
120-
)}
153+
)}
121154
<Stack gap={2}>
122155
<Box display="flex" gap={2}>
123156
<DebouncedSearchTextField
@@ -145,6 +178,21 @@ export const AlertReusableComponent = (props: AlertReusableComponentProps) => {
145178
hideLabel: true,
146179
}}
147180
/>
181+
{/* In SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW: Show Manage Alerts button inline with the search/filter row (right-aligned) */}
182+
{entityId &&
183+
SERVICES_WITH_MANAGE_ALERTS_IN_FILTER_ROW.includes(
184+
serviceType
185+
) && (
186+
<Button
187+
buttonType="outlined"
188+
data-qa-buttons="true"
189+
data-testid="manage-alerts"
190+
onClick={() => navigate({ to: '/alerts/definitions' })}
191+
sx={{ marginLeft: 'auto' }}
192+
>
193+
Manage Alerts
194+
</Button>
195+
)}
148196
</Box>
149197

150198
<AlertInformationActionTable

‎packages/manager/src/features/Linodes/AclpPreferenceToggle.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ const preferenceConfig: Record<
7070
/**
7171
* - For Alerts, the toggle uses local state, not preferences. We do this because each Linode should manage its own alert mode individually.
7272
* - Create Linode: Toggle defaults to false (legacy mode). It's a simple UI toggle with no persistence.
73-
* - Edit Linode: Toggle defaults based on useIsLinodeAclpSubscribed (true if the Linode is already subscribed to ACLP). Still local state - not saved to preferences.
7473
*
7574
* - For Metrics, we use account-level preferences, since it's a global setting shared across all Linodes.
7675
*/

‎packages/manager/src/features/Linodes/LinodeCreate/AdditionalOptions/Alerts.tsx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { AlertsPanel } from 'src/features/Linodes/LinodesDetail/LinodeAlerts/Ale
88
import { useFlags } from 'src/hooks/useFlags';
99

1010
import { AclpPreferenceToggle } from '../../AclpPreferenceToggle';
11+
import { EMPTY_ACLP_ALERTS } from '../utilities';
1112

1213
import type { LinodeCreateFormValues } from '../utilities';
1314
import type { CloudPulseAlertsPayload } from '@linode/api-v4';
@@ -27,7 +28,7 @@ export const Alerts = ({
2728
const { field } = useController({
2829
control,
2930
name: 'alerts',
30-
defaultValue: { system_alerts: [], user_alerts: [] },
31+
defaultValue: EMPTY_ACLP_ALERTS,
3132
});
3233

3334
const handleToggleAlert = (updatedAlerts: CloudPulseAlertsPayload) => {
@@ -71,11 +72,12 @@ export const Alerts = ({
7172
// Beta ACLP Alerts View
7273
<AlertReusableComponent
7374
onToggleAlert={handleToggleAlert}
75+
paperSx={{ p: 0 }}
7476
serviceType="linode"
7577
/>
7678
) : (
77-
// Legacy Alerts View (read-only)
78-
<AlertsPanel />
79+
// Legacy Alerts View (read-only with default values)
80+
<AlertsPanel paperSx={{ p: 0 }} />
7981
)}
8082
</Accordion>
8183
);

0 commit comments

Comments
 (0)