Skip to content

Commit 0e9cfa4

Browse files
tanushree-akamaitanushree-akamai
committed
Address review comments.
1 parent 2db2836 commit 0e9cfa4

File tree

3 files changed

+47
-87
lines changed

3 files changed

+47
-87
lines changed

‎packages/manager/src/features/Linodes/LinodesDetail/LinodeLock/RemoveLockDialog.test.tsx‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,21 +78,6 @@ describe('RemoveLockDialog', () => {
7878
).toBeVisible();
7979
});
8080

81-
it('should prioritize cannot_delete_with_subresources description when both lock types are present', () => {
82-
const { getByText } = renderWithTheme(
83-
<RemoveLockDialog
84-
{...defaultProps}
85-
linodeLocks={['cannot_delete', 'cannot_delete_with_subresources']}
86-
/>
87-
);
88-
89-
expect(
90-
getByText(
91-
'Unlocking will allow this Linode and all its attached resources to be deleted or rebuilt.'
92-
)
93-
).toBeVisible();
94-
});
95-
9681
it('should have Remove Lock and Cancel buttons', () => {
9782
const { getByText } = renderWithTheme(
9883
<RemoveLockDialog {...defaultProps} />

‎packages/manager/src/features/Linodes/LinodesDetail/LinodeLock/RemoveLockDialog.tsx‎

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { deleteLock, getLocks } from '@linode/api-v4';
22
import { ActionsPanel, Notice, Typography } from '@linode/ui';
3+
import { useMutation } from '@tanstack/react-query';
34
import { useSnackbar } from 'notistack';
45
import * as React from 'react';
56

67
import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog';
78
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
89

9-
import type { LockType } from '@linode/api-v4';
10+
import type { APIError, LockType } from '@linode/api-v4';
1011

1112
interface Props {
1213
linodeId: number;
@@ -30,63 +31,64 @@ export const RemoveLockDialog = (props: Props) => {
3031
const { linodeId, linodeLabel, linodeLocks, onClose, open } = props;
3132
const { enqueueSnackbar } = useSnackbar();
3233

33-
const [isLoading, setIsLoading] = React.useState(false);
34-
const [error, setError] = React.useState<string | undefined>(undefined);
35-
36-
// Reset state when dialog opens
37-
React.useEffect(() => {
38-
if (open) {
39-
setError(undefined);
40-
setIsLoading(false);
41-
}
42-
}, [open]);
43-
44-
const handleSubmit = async () => {
45-
setIsLoading(true);
46-
setError(undefined);
47-
48-
try {
49-
// Fetch lock ID
34+
const { error, isPending, mutate, reset } = useMutation<{}, APIError[]>({
35+
mutationFn: async () => {
5036
const locksResponse = await getLocks(
5137
{},
5238
{
5339
'+and': [{ 'entity.id': linodeId }, { 'entity.type': 'linode' }],
5440
}
5541
);
5642

57-
const lock = locksResponse.data[0];
43+
const locks = locksResponse.data;
5844

59-
if (!lock) {
60-
setError('No lock found for this Linode.');
61-
setIsLoading(false);
62-
return;
45+
if (locks.length === 0) {
46+
throw [{ reason: 'No lock found for this Linode.' }];
6347
}
6448

65-
// Delete lock
49+
// TODO: Currently only removes the first lock. If and when multiple locks are supported,
50+
// this dialog should be enhanced to let users select which lock to remove.
51+
const lock = locks[0];
52+
6653
await deleteLock(lock.id);
6754

55+
return {};
56+
},
57+
onSuccess() {
6858
enqueueSnackbar(`Lock removed from ${linodeLabel}.`, {
6959
variant: 'success',
7060
});
7161
onClose();
72-
} catch (err) {
73-
setError(getAPIErrorOrDefault(err, 'Failed to remove lock.')[0].reason);
74-
setIsLoading(false);
62+
},
63+
});
64+
65+
// Reset mutation state when dialog opens
66+
React.useEffect(() => {
67+
if (open) {
68+
reset();
7569
}
70+
}, [open, reset]);
71+
72+
const handleSubmit = () => {
73+
mutate();
7674
};
7775

76+
const errorMessage = error
77+
? getAPIErrorOrDefault(error, 'Failed to remove lock.')[0].reason
78+
: undefined;
79+
7880
return (
7981
<ConfirmationDialog
8082
actions={
8183
<ActionsPanel
8284
primaryButtonProps={{
83-
disabled: isLoading,
85+
disabled: isPending,
8486
label: 'Remove Lock',
85-
loading: isLoading,
87+
loading: isPending,
8688
onClick: handleSubmit,
8789
}}
8890
secondaryButtonProps={{
89-
disabled: isLoading,
91+
disabled: isPending,
9092
label: 'Cancel',
9193
onClick: onClose,
9294
}}
@@ -97,7 +99,7 @@ export const RemoveLockDialog = (props: Props) => {
9799
open={open}
98100
title="Remove Lock?"
99101
>
100-
{error && <Notice text={error} variant="error" />}
102+
{errorMessage && <Notice text={errorMessage} variant="error" />}
101103
<Typography>{getLockTypeDescription(linodeLocks)}</Typography>
102104
</ConfirmationDialog>
103105
);

‎packages/manager/src/features/Linodes/LinodesLanding/LinodeActionMenu/LinodeActionMenu.test.tsx‎

Lines changed: 14 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -47,19 +47,12 @@ const queryMocks = vi.hoisted(() => ({
4747
},
4848
})),
4949
useNavigate: vi.fn(),
50-
useFlags: vi.fn(() => ({
51-
resourceLock: { linodes: false },
52-
})),
5350
}));
5451

5552
vi.mock('src/features/IAM/hooks/usePermissions', () => ({
5653
usePermissions: queryMocks.userPermissions,
5754
}));
5855

59-
vi.mock('src/hooks/useFlags', () => ({
60-
useFlags: queryMocks.useFlags,
61-
}));
62-
6356
vi.mock('@tanstack/react-router', async () => {
6457
const actual = await vi.importActual('@tanstack/react-router');
6558
return {
@@ -289,10 +282,6 @@ describe('LinodeActionMenu', () => {
289282

290283
describe('Lock/Unlock action', () => {
291284
it('should not show Lock action when feature flag is disabled', async () => {
292-
queryMocks.useFlags.mockReturnValue({
293-
resourceLock: { linodes: false },
294-
});
295-
296285
const { getByLabelText, queryByText } = renderWithTheme(
297286
<LinodeActionMenu {...props} />
298287
);
@@ -306,12 +295,9 @@ describe('LinodeActionMenu', () => {
306295
});
307296

308297
it('should show Lock action when feature flag is enabled and Linode is not locked', async () => {
309-
queryMocks.useFlags.mockReturnValue({
310-
resourceLock: { linodes: true },
311-
});
312-
313298
const { getByLabelText, getByText } = renderWithTheme(
314-
<LinodeActionMenu {...props} linodeLocks={[]} />
299+
<LinodeActionMenu {...props} linodeLocks={[]} />,
300+
{ flags: { resourceLock: { linodes: true } } }
315301
);
316302

317303
await userEvent.click(
@@ -322,12 +308,9 @@ describe('LinodeActionMenu', () => {
322308
});
323309

324310
it('should show Unlock action when feature flag is enabled and Linode is locked', async () => {
325-
queryMocks.useFlags.mockReturnValue({
326-
resourceLock: { linodes: true },
327-
});
328-
329311
const { getByLabelText, getByText } = renderWithTheme(
330-
<LinodeActionMenu {...props} linodeLocks={['cannot_delete']} />
312+
<LinodeActionMenu {...props} linodeLocks={['cannot_delete']} />,
313+
{ flags: { resourceLock: { linodes: true } } }
331314
);
332315

333316
await userEvent.click(
@@ -338,9 +321,6 @@ describe('LinodeActionMenu', () => {
338321
});
339322

340323
it('should call onOpenRemoveLockDialog when Unlock is clicked', async () => {
341-
queryMocks.useFlags.mockReturnValue({
342-
resourceLock: { linodes: true },
343-
});
344324
queryMocks.userPermissions.mockReturnValue({
345325
data: {
346326
...queryMocks.userPermissions().data,
@@ -354,7 +334,8 @@ describe('LinodeActionMenu', () => {
354334
{...props}
355335
linodeLocks={['cannot_delete']}
356336
onOpenRemoveLockDialog={onOpenRemoveLockDialog}
357-
/>
337+
/>,
338+
{ flags: { resourceLock: { linodes: true } } }
358339
);
359340

360341
await userEvent.click(
@@ -366,9 +347,6 @@ describe('LinodeActionMenu', () => {
366347
});
367348

368349
it('should disable Lock action when user lacks create_lock permission', async () => {
369-
queryMocks.useFlags.mockReturnValue({
370-
resourceLock: { linodes: true },
371-
});
372350
queryMocks.userPermissions.mockReturnValue({
373351
data: {
374352
...queryMocks.userPermissions().data,
@@ -377,7 +355,8 @@ describe('LinodeActionMenu', () => {
377355
});
378356

379357
const { getByLabelText, getByTestId } = renderWithTheme(
380-
<LinodeActionMenu {...props} linodeLocks={[]} />
358+
<LinodeActionMenu {...props} linodeLocks={[]} />,
359+
{ flags: { resourceLock: { linodes: true } } }
381360
);
382361

383362
await userEvent.click(
@@ -388,9 +367,6 @@ describe('LinodeActionMenu', () => {
388367
});
389368

390369
it('should disable Unlock action when user lacks delete_lock permission', async () => {
391-
queryMocks.useFlags.mockReturnValue({
392-
resourceLock: { linodes: true },
393-
});
394370
queryMocks.userPermissions.mockReturnValue({
395371
data: {
396372
...queryMocks.userPermissions().data,
@@ -399,7 +375,8 @@ describe('LinodeActionMenu', () => {
399375
});
400376

401377
const { getByLabelText, getByTestId } = renderWithTheme(
402-
<LinodeActionMenu {...props} linodeLocks={['cannot_delete']} />
378+
<LinodeActionMenu {...props} linodeLocks={['cannot_delete']} />,
379+
{ flags: { resourceLock: { linodes: true } } }
403380
);
404381

405382
await userEvent.click(
@@ -410,9 +387,6 @@ describe('LinodeActionMenu', () => {
410387
});
411388

412389
it('should enable Lock action when user has create_lock permission', async () => {
413-
queryMocks.useFlags.mockReturnValue({
414-
resourceLock: { linodes: true },
415-
});
416390
queryMocks.userPermissions.mockReturnValue({
417391
data: {
418392
...queryMocks.userPermissions().data,
@@ -421,7 +395,8 @@ describe('LinodeActionMenu', () => {
421395
});
422396

423397
const { getByLabelText, getByTestId } = renderWithTheme(
424-
<LinodeActionMenu {...props} linodeLocks={[]} />
398+
<LinodeActionMenu {...props} linodeLocks={[]} />,
399+
{ flags: { resourceLock: { linodes: true } } }
425400
);
426401

427402
await userEvent.click(
@@ -432,9 +407,6 @@ describe('LinodeActionMenu', () => {
432407
});
433408

434409
it('should enable Unlock action when user has delete_lock permission', async () => {
435-
queryMocks.useFlags.mockReturnValue({
436-
resourceLock: { linodes: true },
437-
});
438410
queryMocks.userPermissions.mockReturnValue({
439411
data: {
440412
...queryMocks.userPermissions().data,
@@ -443,7 +415,8 @@ describe('LinodeActionMenu', () => {
443415
});
444416

445417
const { getByLabelText, getByTestId } = renderWithTheme(
446-
<LinodeActionMenu {...props} linodeLocks={['cannot_delete']} />
418+
<LinodeActionMenu {...props} linodeLocks={['cannot_delete']} />,
419+
{ flags: { resourceLock: { linodes: true } } }
447420
);
448421

449422
await userEvent.click(

0 commit comments

Comments
 (0)