Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
95 changes: 95 additions & 0 deletions .claude/rules/destructive-confirmation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Destructive Action Confirmation Rule

For **irreversible destructive actions** (permanent deletion, purge, force termination, data wipe), confirmation must be collected through a **modal that requires the user to type a confirmation string** — not a `Popconfirm`, not a plain `Modal.confirm`, not a single-click dialog.

## Why

Popconfirm and one-click confirmation dialogs are appropriate for **reversible or low-impact** actions (inactivating, hiding, unassigning, canceling a draft). For actions the user cannot undo, a single misclick has permanent consequences. Requiring the user to type a specific string (typically the resource's name) forces a deliberate pause and prevents accidental destruction.

This convention was applied project-wide in FR-2479 ("standardize confirmation UX"), which replaced the legacy `PopConfirmWithInput.tsx` with the shared `BAIConfirmModalWithInput` component.

## Rules

1. **Irreversible actions → `BAIConfirmModalWithInput`** (from `backend.ai-ui`). The user must type a confirmation string (usually the resource's name) before the OK button enables. Examples: permanently delete a VFolder, terminate a model service endpoint, purge a user, delete an image, delete a resource preset, remove a shell script.
2. **Reversible / low-impact actions → `Popconfirm`** or `App.useApp().modal.confirm({ ... })`. Examples: deactivating (not deleting) a user, canceling an in-progress action, hiding an item, marking inactive.
3. **Never use `Popconfirm` for permanent deletion**, even when the action is guarded server-side. The UX contract is about *user intent*, not backend safety.
4. Do **not** reintroduce `PopConfirmWithInput` or any ad-hoc "modal with a text input" for destructive flows — use the shared `BAIConfirmModalWithInput`. This keeps the copy, layout, danger styling, and accessibility consistent.
5. The confirmation string should be something the user sees on screen and can copy unambiguously (e.g., the resource's `name` or `id`). Avoid opaque tokens.

## Pattern

### ❌ Wrong — Popconfirm for permanent deletion

```tsx
<Popconfirm
title={t('dialog.ask.DoYouWantToDeleteSomething', { name: row.name })}
onConfirm={() => deleteForever(row.id)}
>
<Button danger icon={<DeleteOutlined />} />
</Popconfirm>
```

### ❌ Wrong — single-click `modal.confirm` for permanent deletion

```tsx
modal.confirm({
title: t('dialog.ask.DoYouWantToDeleteSomething', { name: row.name }),
okButtonProps: { danger: true },
onOk: () => deleteForever(row.id),
});
```

### ✅ Correct — typed confirmation for permanent deletion

```tsx
import { BAIConfirmModalWithInput } from 'backend.ai-ui';

const [deletingTarget, setDeletingTarget] = useState<Row | null>(null);

// Trigger
<Button
danger
icon={<DeleteOutlined />}
onClick={() => setDeletingTarget(row)}
/>

// Modal
<BAIConfirmModalWithInput
open={!!deletingTarget}
title={t('dialog.ask.PermanentlyDeleteSomething', { name: deletingTarget?.name })}
content={t('dialog.warning.CannotBeUndone')}
confirmText={deletingTarget?.name ?? ''}
okText={t('button.Delete')}
onOk={async () => {
if (deletingTarget) await deleteForever(deletingTarget.id);
setDeletingTarget(null);
}}
onCancel={() => setDeletingTarget(null)}
/>
```

### ✅ Correct — `Popconfirm` for reversible actions

```tsx
<Popconfirm
title={t('dialog.ask.DoYouWantToInactivateSomething', { name: row.name })}
onConfirm={() => setInactive(row.id)}
>
<Button icon={<StopOutlined />} />
</Popconfirm>
```

## How to decide

Ask: *"If the user clicks OK by accident, can they recover the state in <30 seconds without contacting support?"*

- **Yes** → `Popconfirm` / `modal.confirm` is fine.
- **No** → `BAIConfirmModalWithInput`.

Soft-delete / trash-bin flows count as reversible **only if** the UI actually exposes a restore path the user can reach on their own. If restoration requires admin intervention or database access, treat it as irreversible.

## Related

- `BAIConfirmModalWithInput` — `packages/backend.ai-ui/src/components/BAIConfirmModalWithInput.tsx`
- `BAIDeleteConfirmModal` — higher-level convenience wrapper for common delete flows
- FR-2479 — the refactor that standardized this convention across the project
2 changes: 1 addition & 1 deletion packages/backend.ai-ui/src/locale/ko.json
Comment thread
ironAiken2 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@
"modal": {
"DeleteForeverDesc": "경고: 선택한 항목은 완전히 삭제되며 복원할 수 없습니다.",
"ItemSelectedWithCount": "{{count}}개 선택됨.",
"PleaseTypeToConfirm": "{{ confirmText }}를 입력하십시오."
"PleaseTypeToConfirm": "{{ confirmText }}를 입력해주세요."
},
"validation": {
"LetterNumber-_dot": "문자, 숫자, '-', '_', '.'만 입력할 수 있습니다.",
Expand Down
119 changes: 66 additions & 53 deletions react/src/components/EndpointList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,20 @@ import {
DeleteOutlined,
SettingOutlined,
} from '@ant-design/icons';
import { Typography, theme, App, TablePaginationConfig } from 'antd';
import { Typography, theme, App, TablePaginationConfig, Alert } from 'antd';
import type { ColumnType } from 'antd/lib/table';
import {
filterOutEmpty,
filterOutNullAndUndefined,
BAITable,
BAITableProps,
BAINameActionCell,
BAIConfirmModalWithInput,
BAIFlex,
} from 'backend.ai-ui';
import dayjs from 'dayjs';
import * as _ from 'lodash-es';
import React from 'react';
import React, { useState } from 'react';
import { useTranslation } from 'react-i18next';
import { graphql, useFragment } from 'react-relay';

Expand Down Expand Up @@ -76,10 +78,13 @@ const EndpointList: React.FC<EndpointListProps> = ({
}) => {
const { t } = useTranslation();
const { token } = theme.useToken();
const { message, modal } = App.useApp();
const { message } = App.useApp();
const [currentUser] = useCurrentUserInfo();
const baiClient = useSuspendedBackendaiClient();
const webuiNavigate = useWebUINavigate();
const [deletingEndpoint, setDeletingEndpoint] = useState<Endpoint | null>(
null,
);

const endpoints = useFragment(
graphql`
Expand Down Expand Up @@ -151,46 +156,7 @@ const EndpointList: React.FC<EndpointListProps> = ({
type: 'danger',
disabled: isEndpointInDestroyingCategory(row),
onClick: () => {
modal.confirm({
title: t('dialog.ask.DoYouWantToDeleteSomething', {
name: row.name,
}),
content: t('dialog.warning.CannotBeUndone'),
okText: t('button.Delete'),
okButtonProps: {
danger: true,
type: 'primary',
},
onOk: () => {
if (row.endpoint_id) {
return new Promise<void>((resolve) => {
terminateModelServiceMutation.mutate(row.endpoint_id!, {
onSuccess: (res) => {
onDeleted?.(row);
if (res.success) {
message.success(
t('modelService.ServiceTerminated', {
name: row?.name,
}),
);
} else {
message.error(
t('modelService.FailedToTerminateService'),
);
}
resolve();
},
onError: () => {
message.error(
t('modelService.FailedToTerminateService'),
);
resolve();
},
});
});
}
},
});
setDeletingEndpoint(row);
},
},
]}
Expand Down Expand Up @@ -282,16 +248,63 @@ const EndpointList: React.FC<EndpointListProps> = ({
]);

return (
<BAITable
size="small"
loading={loading}
scroll={{ x: 'max-content' }}
rowKey={'endpoint_id'}
dataSource={filterOutNullAndUndefined(endpoints)}
columns={columns}
pagination={pagination}
{...tableProps}
/>
<>
<BAITable
size="small"
loading={loading}
scroll={{ x: 'max-content' }}
rowKey={'endpoint_id'}
dataSource={filterOutNullAndUndefined(endpoints)}
columns={columns}
pagination={pagination}
{...tableProps}
/>
<BAIConfirmModalWithInput
open={!!deletingEndpoint}
title={t('dialog.ask.DoYouWantToDeleteSomething', {
name: deletingEndpoint?.name,
})}
content={
<BAIFlex direction="column" gap="md" align="stretch">
<Alert type="warning" title={t('dialog.warning.CannotBeUndone')} />
<BAIFlex>
<Typography.Text style={{ marginRight: token.marginXXS }}>
{t('dialog.TypeNameToConfirmDeletion')}
</Typography.Text>
(<Typography.Text code>{deletingEndpoint?.name}</Typography.Text>)
</BAIFlex>
</BAIFlex>
}
confirmText={deletingEndpoint?.name ?? ''}
inputProps={{ placeholder: deletingEndpoint?.name ?? '' }}
okText={t('button.Delete')}
okButtonProps={{ loading: terminateModelServiceMutation.isPending }}
onOk={() => {
if (deletingEndpoint?.endpoint_id) {
terminateModelServiceMutation.mutate(deletingEndpoint.endpoint_id, {
onSuccess: (res) => {
onDeleted?.(deletingEndpoint);
if (res.success) {
message.success(
t('modelService.ServiceTerminated', {
name: deletingEndpoint?.name,
}),
);
} else {
message.error(t('modelService.FailedToTerminateService'));
}
setDeletingEndpoint(null);
},
onError: () => {
message.error(t('modelService.FailedToTerminateService'));
setDeletingEndpoint(null);
},
});
Comment thread
ironAiken2 marked this conversation as resolved.
}
}}
onCancel={() => setDeletingEndpoint(null)}
/>
</>
);
};

Expand Down
51 changes: 32 additions & 19 deletions react/src/components/ManageAppsModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ import {
Form,
message,
Typography,
App,
FormInstance,
theme,
} from 'antd';
import { BAIFlex, BAIModal, BAIModalProps } from 'backend.ai-ui';
import {
BAIFlex,
BAIModal,
BAIModalProps,
BAIConfirmModalWithInput,
} from 'backend.ai-ui';
import * as _ from 'lodash-es';
import React from 'react';
import React, { useState } from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { graphql, useFragment, useMutation } from 'react-relay';

Expand All @@ -37,7 +41,10 @@ const ManageAppsModal: React.FC<ManageAppsModalProps> = ({
}) => {
const { t } = useTranslation();
const formRef = React.useRef<FormInstance>(null);
const app = App.useApp();
const [isReinstallConfirmOpen, setIsReinstallConfirmOpen] = useState(false);
const [pendingCommitRequest, setPendingCommitRequest] = useState<
(() => void) | null
>(null);

const { token } = theme.useToken();

Expand Down Expand Up @@ -157,21 +164,8 @@ const ManageAppsModal: React.FC<ManageAppsModalProps> = ({
});

if (image?.installed) {
app.modal.confirm({
title: 'Image reinstallation required',
content: (
<>
<Trans
i18nKey={
'environment.ModifyImageResourceLimitReinstallRequired'
}
/>
</>
),
onOk: commitRequest,
getContainer: () => document.body,
closable: true,
});
setPendingCommitRequest(() => commitRequest);
setIsReinstallConfirmOpen(true);
} else {
commitRequest();
}
Expand Down Expand Up @@ -329,6 +323,25 @@ const ManageAppsModal: React.FC<ManageAppsModalProps> = ({
</Form.List>
</BAIFlex>
</Form>
<BAIConfirmModalWithInput
open={isReinstallConfirmOpen}
title={t('environment.ImageReinstallationRequired')}
content={
<Trans
i18nKey={'environment.ModifyImageResourceLimitReinstallRequired'}
/>
}
confirmText={image?.name ?? image?.namespace ?? ''}
onOk={() => {
setIsReinstallConfirmOpen(false);
pendingCommitRequest?.();
setPendingCommitRequest(null);
}}
onCancel={() => {
setIsReinstallConfirmOpen(false);
setPendingCommitRequest(null);
}}
/>
</BAIModal>
);
};
Expand Down
Loading
Loading