Skip to content

Confirmation before deleting in the Dashboard #13035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 14, 2025
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
12 changes: 12 additions & 0 deletions app/common/src/services/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,18 @@ export enum AssetType {
specialUp = 'specialUp',
}

export const ASSET_TYPE_TO_TEXT_ID: Readonly<Record<AssetType, TextId>> = {
[AssetType.directory]: 'directoryAssetType',
[AssetType.project]: 'projectAssetType',
[AssetType.file]: 'fileAssetType',
[AssetType.secret]: 'secretAssetType',
[AssetType.specialEmpty]: 'specialEmptyAssetType',
[AssetType.specialError]: 'specialErrorAssetType',
[AssetType.specialLoading]: 'specialLoadingAssetType',
[AssetType.specialUp]: 'specialUpAssetType',
[AssetType.datalink]: 'datalinkAssetType',
} satisfies { [Type in AssetType]: `${Type}AssetType` }

export enum ReplaceableAssetType {
project = 'project',
file = 'file',
Expand Down
16 changes: 4 additions & 12 deletions app/gui/integration-test/dashboard/actions/DrivePageActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,18 +371,10 @@ export default class DrivePageActions<Context> extends PageActions<Context> {

/** Create a new empty project. */
newEmptyProject() {
return this.step('Create empty project', (page) =>
page.getByText(TEXT.newEmptyProject, { exact: true }).click(),
).into(EditorPageActions<Context>)
}

// FIXME[sb]: https://github.com/enso-org/cloud-v2/issues/1615
// Delete once cloud execution in the browser is re-enabled.
/** Create a new empty project. */
newEmptyProjectTest() {
return this.step('Create empty project', (page) =>
page.getByText(TEXT.newEmptyProject, { exact: true }).click(),
).into(EditorPageActions<Context>)
return this.step('Create empty project', async (page) => {
await page.getByText(TEXT.newEmptyProject, { exact: true }).click()
await expect(page.getByTestId('editor')).toBeVisible()
}).into(EditorPageActions<Context>)
}

/** Interact with the drive view (the main container of this page). */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ export interface ContextMenuActions<T extends BaseActions<Context>, Context> {
readonly uploadToCloud: () => T
readonly rename: () => T
readonly snapshot: () => T
readonly moveNonFolderToTrash: () => T
readonly moveFolderToTrash: () => T
readonly moveAllToTrash: (confirm?: boolean) => T
readonly moveToTrash: () => T
readonly moveAllToTrash: () => T
readonly restoreFromTrash: () => T
readonly restoreAllFromTrash: () => T
readonly share: () => T
Expand Down Expand Up @@ -60,14 +59,7 @@ export function contextMenuActions<T extends BaseActions<Context>, Context>(
.getByText(TEXT.snapshotShortcut)
.click(),
),
moveNonFolderToTrash: () =>
step('Move to trash (context menu)', async (page) => {
await page
.getByRole('button', { name: TEXT.moveToTrashShortcut })
.getByText(TEXT.moveToTrashShortcut)
.click()
}),
moveFolderToTrash: () =>
moveToTrash: () =>
step('Move folder to trash (context menu)', async (page) => {
await page
.getByRole('button', { name: TEXT.moveToTrashShortcut })
Expand All @@ -77,15 +69,15 @@ export function contextMenuActions<T extends BaseActions<Context>, Context>(
// Confirm the deletion in the dialog
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
}),
moveAllToTrash: (hasFolder = false) =>
moveAllToTrash: () =>
step('Move all to trash (context menu)', async (page) => {
await page
.getByRole('button', { name: TEXT.moveAllToTrashShortcut })
.getByText(TEXT.moveAllToTrashShortcut)
.click()
if (hasFolder) {
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
}

// Confirm the deletion in the dialog
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
}),
restoreFromTrash: () =>
step('Restore from trash (context menu)', (page) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ test('can navigate to parent directory of an asset in the Trash category', ({ pa
})
// Project in the root (a)
.driveTable.rightClickRow('a')
.contextMenu.moveNonFolderToTrash()
.contextMenu.moveToTrash()
.driveTable.openDirectory('d')
.driveTable.openDirectory('e')
// Project in the nested directory (c)
.driveTable.rightClickRow('c')
.contextMenu.moveNonFolderToTrash()
.contextMenu.moveToTrash()
.goToCategory.trash()
.driveTable.withPathColumnCell('a', async (cell) => {
await expect(cell).toBeVisible()
Expand Down
6 changes: 5 additions & 1 deletion app/gui/integration-test/dashboard/copy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @file Test copying, moving, cutting and pasting. */
import { expect, test, type Page } from '@playwright/test'

import { mockAllAndLogin } from './actions'
import { mockAllAndLogin, TEXT } from './actions'

/** Find a button for the "Trash" category. */
function locateTrashCategory(page: Page) {
Expand Down Expand Up @@ -99,6 +99,10 @@ test('move to trash', ({ page }) =>
modActions.driveTable.clickRow('New Folder 1').driveTable.clickRow('New Folder 2'),
)
.driveTable.dragRow('New Folder 1', locateTrashCategory(page))
.do(async (page) => {
// Confirm the deletion in the dialog
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
})
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
Expand Down
93 changes: 91 additions & 2 deletions app/gui/integration-test/dashboard/delete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,32 @@ test('delete and restore', ({ page }) =>
await expect(rows).toHaveCount(1)
})
.driveTable.rightClickRow(0)
.contextMenu.moveFolderToTrash()
.contextMenu.moveToTrash()
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
})
.driveTable.rightClickRow(0)
.contextMenu.restoreFromTrash()
.driveTable.expectTrashPlaceholderRow()
.goToCategory.cloud()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))

test('delete and restore project', ({ page }) =>
mockAllAndLogin({
page,
setupAPI: (api) => {
api.addProject()
},
})
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
})
.driveTable.rightClickRow(0)
.contextMenu.moveToTrash()
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
Expand Down Expand Up @@ -49,6 +74,34 @@ test('delete and restore (keyboard)', ({ page }) =>
await expect(rows).toHaveCount(1)
}))

test('delete and restore project (keyboard)', ({ page }) =>
mockAllAndLogin({
page,
setupAPI: (api) => {
api.addProject()
},
})
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
})
.driveTable.clickRow(0)
.press('Delete')
.do(async (thePage) => {
await thePage.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
})
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
})
.driveTable.clickRow(0)
.press('Mod+R')
.driveTable.expectTrashPlaceholderRow()
.goToCategory.cloud()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))

test('clear trash', ({ page }) =>
mockAllAndLogin({
page,
Expand All @@ -74,7 +127,7 @@ test('clear trash', ({ page }) =>
}
})
.driveTable.rightClickRow(0)
.contextMenu.moveAllToTrash(true)
.contextMenu.moveAllToTrash()
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
Expand All @@ -86,3 +139,39 @@ test('clear trash', ({ page }) =>
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(0)
}))

test('clear trash (without directories)', ({ page }) =>
mockAllAndLogin({
page,
setupAPI: (api) => {
api.addProject()
api.addProject()
api.addFile()
api.addSecret()
api.addDatalink()
},
})
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(5)
})
.driveTable.withRows(async (rows, _nonRows, _context, page) => {
const mod = await modModifier(page)
// Parallelizing this using `Promise.all` makes it inconsistent.
const rowEls = await rows.all()
for (const row of rowEls) {
await row.click({ modifiers: [mod] })
}
})
.driveTable.rightClickRow(0)
.contextMenu.moveAllToTrash()
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(5)
})
.clearTrash()
.driveTable.expectTrashPlaceholderRow()
.goToCategory.cloud()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(0)
}))
2 changes: 1 addition & 1 deletion app/gui/integration-test/dashboard/driveView.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ test('drive view', ({ page }) =>
await locateStopProjectButton(rows.nth(0)).click()
})
.driveTable.rightClickRow(0)
.contextMenu.moveNonFolderToTrash()
.contextMenu.moveToTrash()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))
3 changes: 1 addition & 2 deletions app/gui/integration-test/dashboard/pageSwitcher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ test('page switcher', ({ page }) =>
page,
setupAPI: (api) => api.setFeatureFlags({ enableCloudExecution: true }),
})
// Create a new project so that the editor page can be switched to.
.newEmptyProjectTest()
.newEmptyProject()
.do(async (thePage) => {
await expect(locateDriveView(thePage)).not.toBeVisible()
await expect(locateEditor(thePage)).toBeVisible()
Expand Down
6 changes: 3 additions & 3 deletions app/gui/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ export default defineConfig({
'--headless=new',
// Required for `backdrop-filter: blur` to work.
'--use-angle=swiftshader',
// FIXME: `--disable-gpu` disables `backdrop-filter: blur`, which is not handled by
// the software (CPU) compositor. This SHOULD be fixed eventually, but this flag
// MUST stay as CI does not have a GPU.
// `--disable-gpu` disables `backdrop-filter: blur`, which is not handled by
// the software (CPU) compositor. This flag MUST stay if screenshot testing/
// visual regression testing is needed, as CI does not have a GPU.
'--disable-gpu',
// Fully disable GPU process.
'--disable-software-rasterizer',
Expand Down
16 changes: 1 addition & 15 deletions app/gui/src/dashboard/components/dashboard/Permission.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import * as React from 'react'

import { useMutation } from '@tanstack/react-query'

import type * as text from 'enso-common/src/text'

import { backendMutationOptions } from '#/hooks/backendHooks'
import * as toastAndLogHooks from '#/hooks/toastAndLogHooks'

Expand All @@ -19,18 +17,6 @@ import * as backendModule from '#/services/Backend'
import { Text } from '#/components/AriaComponents'
import * as object from '#/utilities/object'

const ASSET_TYPE_TO_TEXT_ID: Readonly<Record<backendModule.AssetType, text.TextId>> = {
[backendModule.AssetType.directory]: 'directoryAssetType',
[backendModule.AssetType.project]: 'projectAssetType',
[backendModule.AssetType.file]: 'fileAssetType',
[backendModule.AssetType.secret]: 'secretAssetType',
[backendModule.AssetType.specialEmpty]: 'specialEmptyAssetType',
[backendModule.AssetType.specialError]: 'specialErrorAssetType',
[backendModule.AssetType.specialLoading]: 'specialLoadingAssetType',
[backendModule.AssetType.specialUp]: 'specialUpAssetType',
[backendModule.AssetType.datalink]: 'datalinkAssetType',
} satisfies { [Type in backendModule.AssetType]: `${Type}AssetType` }

/** Props for a {@link Permission}. */
export interface PermissionProps {
readonly backend: Backend
Expand All @@ -52,7 +38,7 @@ export default function Permission(props: PermissionProps) {
const [permission, setPermission] = React.useState(initialPermission)
const permissionId = backendModule.getAssetPermissionId(permission)
const isDisabled = isOnlyOwner && backendModule.getAssetPermissionId(self) === permissionId
const assetTypeName = getText(ASSET_TYPE_TO_TEXT_ID[asset.type])
const assetTypeName = getText(backendModule.ASSET_TYPE_TO_TEXT_ID[asset.type])

const createPermission = useMutation(
backendMutationOptions(backend, 'createPermission'),
Expand Down
39 changes: 14 additions & 25 deletions app/gui/src/dashboard/layouts/AssetContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -337,31 +337,20 @@ export default function AssetContextMenu(props: AssetContextMenuProps) {
action="delete"
label={isCloud ? getText('moveToTrashShortcut') : getText('deleteShortcut')}
doAction={() => {
if (isCloud) {
if (asset.type === backendModule.AssetType.directory) {
setModal(
<ConfirmDeleteModal
defaultOpen
actionText={getText('trashTheAssetTypeTitle', asset.type, asset.title)}
onConfirm={async () => {
await deleteAssetsMutation([[asset.id], false])
}}
/>,
)
} else {
void deleteAssetsMutation([[asset.id], false])
}
} else {
setModal(
<ConfirmDeleteModal
defaultOpen
actionText={getText('deleteTheAssetTypeTitle', asset.type, asset.title)}
onConfirm={async () => {
await deleteAssetsMutation([[asset.id], false])
}}
/>,
)
}
const textId = isCloud ? 'trashTheAssetTypeTitle' : 'deleteTheAssetTypeTitle'
setModal(
<ConfirmDeleteModal
defaultOpen
actionText={getText(
textId,
getText(backendModule.ASSET_TYPE_TO_TEXT_ID[asset.type]),
asset.title,
)}
onConfirm={async () => {
await deleteAssetsMutation([[asset.id], false])
}}
/>,
)
}}
/>
)}
Expand Down
Loading
Loading