Skip to content

Commit f6ed73d

Browse files
authored
Confirmation before deleting in the Dashboard (#13035)
- Close enso-org/cloud-v2#1948 - Unconditionally show dialog to confirm deleting an asset, even if it is 'just' moving it to the Trash in the Cloud # Important Notes None
1 parent db1a5c6 commit f6ed73d

File tree

13 files changed

+185
-107
lines changed

13 files changed

+185
-107
lines changed

app/common/src/services/Backend.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,18 @@ export enum AssetType {
900900
specialUp = 'specialUp',
901901
}
902902

903+
export const ASSET_TYPE_TO_TEXT_ID: Readonly<Record<AssetType, TextId>> = {
904+
[AssetType.directory]: 'directoryAssetType',
905+
[AssetType.project]: 'projectAssetType',
906+
[AssetType.file]: 'fileAssetType',
907+
[AssetType.secret]: 'secretAssetType',
908+
[AssetType.specialEmpty]: 'specialEmptyAssetType',
909+
[AssetType.specialError]: 'specialErrorAssetType',
910+
[AssetType.specialLoading]: 'specialLoadingAssetType',
911+
[AssetType.specialUp]: 'specialUpAssetType',
912+
[AssetType.datalink]: 'datalinkAssetType',
913+
} satisfies { [Type in AssetType]: `${Type}AssetType` }
914+
903915
export enum ReplaceableAssetType {
904916
project = 'project',
905917
file = 'file',

app/gui/integration-test/dashboard/actions/DrivePageActions.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -371,18 +371,10 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
371371

372372
/** Create a new empty project. */
373373
newEmptyProject() {
374-
return this.step('Create empty project', (page) =>
375-
page.getByText(TEXT.newEmptyProject, { exact: true }).click(),
376-
).into(EditorPageActions<Context>)
377-
}
378-
379-
// FIXME[sb]: https://github.com/enso-org/cloud-v2/issues/1615
380-
// Delete once cloud execution in the browser is re-enabled.
381-
/** Create a new empty project. */
382-
newEmptyProjectTest() {
383-
return this.step('Create empty project', (page) =>
384-
page.getByText(TEXT.newEmptyProject, { exact: true }).click(),
385-
).into(EditorPageActions<Context>)
374+
return this.step('Create empty project', async (page) => {
375+
await page.getByText(TEXT.newEmptyProject, { exact: true }).click()
376+
await expect(page.getByTestId('editor')).toBeVisible()
377+
}).into(EditorPageActions<Context>)
386378
}
387379

388380
/** Interact with the drive view (the main container of this page). */

app/gui/integration-test/dashboard/actions/contextMenuActions.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ export interface ContextMenuActions<T extends BaseActions<Context>, Context> {
1010
readonly uploadToCloud: () => T
1111
readonly rename: () => T
1212
readonly snapshot: () => T
13-
readonly moveNonFolderToTrash: () => T
14-
readonly moveFolderToTrash: () => T
15-
readonly moveAllToTrash: (confirm?: boolean) => T
13+
readonly moveToTrash: () => T
14+
readonly moveAllToTrash: () => T
1615
readonly restoreFromTrash: () => T
1716
readonly restoreAllFromTrash: () => T
1817
readonly share: () => T
@@ -60,14 +59,7 @@ export function contextMenuActions<T extends BaseActions<Context>, Context>(
6059
.getByText(TEXT.snapshotShortcut)
6160
.click(),
6261
),
63-
moveNonFolderToTrash: () =>
64-
step('Move to trash (context menu)', async (page) => {
65-
await page
66-
.getByRole('button', { name: TEXT.moveToTrashShortcut })
67-
.getByText(TEXT.moveToTrashShortcut)
68-
.click()
69-
}),
70-
moveFolderToTrash: () =>
62+
moveToTrash: () =>
7163
step('Move folder to trash (context menu)', async (page) => {
7264
await page
7365
.getByRole('button', { name: TEXT.moveToTrashShortcut })
@@ -77,15 +69,15 @@ export function contextMenuActions<T extends BaseActions<Context>, Context>(
7769
// Confirm the deletion in the dialog
7870
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
7971
}),
80-
moveAllToTrash: (hasFolder = false) =>
72+
moveAllToTrash: () =>
8173
step('Move all to trash (context menu)', async (page) => {
8274
await page
8375
.getByRole('button', { name: TEXT.moveAllToTrashShortcut })
8476
.getByText(TEXT.moveAllToTrashShortcut)
8577
.click()
86-
if (hasFolder) {
87-
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
88-
}
78+
79+
// Confirm the deletion in the dialog
80+
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
8981
}),
9082
restoreFromTrash: () =>
9183
step('Restore from trash (context menu)', (page) =>

app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,12 @@ test('can navigate to parent directory of an asset in the Trash category', ({ pa
6969
})
7070
// Project in the root (a)
7171
.driveTable.rightClickRow('a')
72-
.contextMenu.moveNonFolderToTrash()
72+
.contextMenu.moveToTrash()
7373
.driveTable.openDirectory('d')
7474
.driveTable.openDirectory('e')
7575
// Project in the nested directory (c)
7676
.driveTable.rightClickRow('c')
77-
.contextMenu.moveNonFolderToTrash()
77+
.contextMenu.moveToTrash()
7878
.goToCategory.trash()
7979
.driveTable.withPathColumnCell('a', async (cell) => {
8080
await expect(cell).toBeVisible()

app/gui/integration-test/dashboard/copy.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/** @file Test copying, moving, cutting and pasting. */
22
import { expect, test, type Page } from '@playwright/test'
33

4-
import { mockAllAndLogin } from './actions'
4+
import { mockAllAndLogin, TEXT } from './actions'
55

66
/** Find a button for the "Trash" category. */
77
function locateTrashCategory(page: Page) {
@@ -99,6 +99,10 @@ test('move to trash', ({ page }) =>
9999
modActions.driveTable.clickRow('New Folder 1').driveTable.clickRow('New Folder 2'),
100100
)
101101
.driveTable.dragRow('New Folder 1', locateTrashCategory(page))
102+
.do(async (page) => {
103+
// Confirm the deletion in the dialog
104+
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
105+
})
102106
.driveTable.expectPlaceholderRow()
103107
.goToCategory.trash()
104108
.driveTable.withRows(async (rows) => {

app/gui/integration-test/dashboard/delete.spec.ts

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,32 @@ test('delete and restore', ({ page }) =>
1111
await expect(rows).toHaveCount(1)
1212
})
1313
.driveTable.rightClickRow(0)
14-
.contextMenu.moveFolderToTrash()
14+
.contextMenu.moveToTrash()
15+
.driveTable.expectPlaceholderRow()
16+
.goToCategory.trash()
17+
.driveTable.withRows(async (rows) => {
18+
await expect(rows).toHaveCount(1)
19+
})
20+
.driveTable.rightClickRow(0)
21+
.contextMenu.restoreFromTrash()
22+
.driveTable.expectTrashPlaceholderRow()
23+
.goToCategory.cloud()
24+
.driveTable.withRows(async (rows) => {
25+
await expect(rows).toHaveCount(1)
26+
}))
27+
28+
test('delete and restore project', ({ page }) =>
29+
mockAllAndLogin({
30+
page,
31+
setupAPI: (api) => {
32+
api.addProject()
33+
},
34+
})
35+
.driveTable.withRows(async (rows) => {
36+
await expect(rows).toHaveCount(1)
37+
})
38+
.driveTable.rightClickRow(0)
39+
.contextMenu.moveToTrash()
1540
.driveTable.expectPlaceholderRow()
1641
.goToCategory.trash()
1742
.driveTable.withRows(async (rows) => {
@@ -49,6 +74,34 @@ test('delete and restore (keyboard)', ({ page }) =>
4974
await expect(rows).toHaveCount(1)
5075
}))
5176

77+
test('delete and restore project (keyboard)', ({ page }) =>
78+
mockAllAndLogin({
79+
page,
80+
setupAPI: (api) => {
81+
api.addProject()
82+
},
83+
})
84+
.driveTable.withRows(async (rows) => {
85+
await expect(rows).toHaveCount(1)
86+
})
87+
.driveTable.clickRow(0)
88+
.press('Delete')
89+
.do(async (thePage) => {
90+
await thePage.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
91+
})
92+
.driveTable.expectPlaceholderRow()
93+
.goToCategory.trash()
94+
.driveTable.withRows(async (rows) => {
95+
await expect(rows).toHaveCount(1)
96+
})
97+
.driveTable.clickRow(0)
98+
.press('Mod+R')
99+
.driveTable.expectTrashPlaceholderRow()
100+
.goToCategory.cloud()
101+
.driveTable.withRows(async (rows) => {
102+
await expect(rows).toHaveCount(1)
103+
}))
104+
52105
test('clear trash', ({ page }) =>
53106
mockAllAndLogin({
54107
page,
@@ -74,7 +127,7 @@ test('clear trash', ({ page }) =>
74127
}
75128
})
76129
.driveTable.rightClickRow(0)
77-
.contextMenu.moveAllToTrash(true)
130+
.contextMenu.moveAllToTrash()
78131
.driveTable.expectPlaceholderRow()
79132
.goToCategory.trash()
80133
.driveTable.withRows(async (rows) => {
@@ -86,3 +139,39 @@ test('clear trash', ({ page }) =>
86139
.driveTable.withRows(async (rows) => {
87140
await expect(rows).toHaveCount(0)
88141
}))
142+
143+
test('clear trash (without directories)', ({ page }) =>
144+
mockAllAndLogin({
145+
page,
146+
setupAPI: (api) => {
147+
api.addProject()
148+
api.addProject()
149+
api.addFile()
150+
api.addSecret()
151+
api.addDatalink()
152+
},
153+
})
154+
.driveTable.withRows(async (rows) => {
155+
await expect(rows).toHaveCount(5)
156+
})
157+
.driveTable.withRows(async (rows, _nonRows, _context, page) => {
158+
const mod = await modModifier(page)
159+
// Parallelizing this using `Promise.all` makes it inconsistent.
160+
const rowEls = await rows.all()
161+
for (const row of rowEls) {
162+
await row.click({ modifiers: [mod] })
163+
}
164+
})
165+
.driveTable.rightClickRow(0)
166+
.contextMenu.moveAllToTrash()
167+
.driveTable.expectPlaceholderRow()
168+
.goToCategory.trash()
169+
.driveTable.withRows(async (rows) => {
170+
await expect(rows).toHaveCount(5)
171+
})
172+
.clearTrash()
173+
.driveTable.expectTrashPlaceholderRow()
174+
.goToCategory.cloud()
175+
.driveTable.withRows(async (rows) => {
176+
await expect(rows).toHaveCount(0)
177+
}))

app/gui/integration-test/dashboard/driveView.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ test('drive view', ({ page }) =>
3636
await locateStopProjectButton(rows.nth(0)).click()
3737
})
3838
.driveTable.rightClickRow(0)
39-
.contextMenu.moveNonFolderToTrash()
39+
.contextMenu.moveToTrash()
4040
.driveTable.withRows(async (rows) => {
4141
await expect(rows).toHaveCount(1)
4242
}))

app/gui/integration-test/dashboard/pageSwitcher.spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ test('page switcher', ({ page }) =>
2020
page,
2121
setupAPI: (api) => api.setFeatureFlags({ enableCloudExecution: true }),
2222
})
23-
// Create a new project so that the editor page can be switched to.
24-
.newEmptyProjectTest()
23+
.newEmptyProject()
2524
.do(async (thePage) => {
2625
await expect(locateDriveView(thePage)).not.toBeVisible()
2726
await expect(locateEditor(thePage)).toBeVisible()

app/gui/playwright.config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ export default defineConfig({
8989
'--headless=new',
9090
// Required for `backdrop-filter: blur` to work.
9191
'--use-angle=swiftshader',
92-
// FIXME: `--disable-gpu` disables `backdrop-filter: blur`, which is not handled by
93-
// the software (CPU) compositor. This SHOULD be fixed eventually, but this flag
94-
// MUST stay as CI does not have a GPU.
92+
// `--disable-gpu` disables `backdrop-filter: blur`, which is not handled by
93+
// the software (CPU) compositor. This flag MUST stay if screenshot testing/
94+
// visual regression testing is needed, as CI does not have a GPU.
9595
'--disable-gpu',
9696
// Fully disable GPU process.
9797
'--disable-software-rasterizer',

app/gui/src/dashboard/components/dashboard/Permission.tsx

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import * as React from 'react'
33

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

6-
import type * as text from 'enso-common/src/text'
7-
86
import { backendMutationOptions } from '#/hooks/backendHooks'
97
import * as toastAndLogHooks from '#/hooks/toastAndLogHooks'
108

@@ -18,18 +16,6 @@ import * as backendModule from '#/services/Backend'
1816
import { Text } from '#/components/AriaComponents'
1917
import * as object from '#/utilities/object'
2018

21-
const ASSET_TYPE_TO_TEXT_ID: Readonly<Record<backendModule.AssetType, text.TextId>> = {
22-
[backendModule.AssetType.directory]: 'directoryAssetType',
23-
[backendModule.AssetType.project]: 'projectAssetType',
24-
[backendModule.AssetType.file]: 'fileAssetType',
25-
[backendModule.AssetType.secret]: 'secretAssetType',
26-
[backendModule.AssetType.specialEmpty]: 'specialEmptyAssetType',
27-
[backendModule.AssetType.specialError]: 'specialErrorAssetType',
28-
[backendModule.AssetType.specialLoading]: 'specialLoadingAssetType',
29-
[backendModule.AssetType.specialUp]: 'specialUpAssetType',
30-
[backendModule.AssetType.datalink]: 'datalinkAssetType',
31-
} satisfies { [Type in backendModule.AssetType]: `${Type}AssetType` }
32-
3319
/** Props for a {@link Permission}. */
3420
export interface PermissionProps {
3521
readonly backend: Backend
@@ -51,7 +37,7 @@ export default function Permission(props: PermissionProps) {
5137
const [permission, setPermission] = React.useState(initialPermission)
5238
const permissionId = backendModule.getAssetPermissionId(permission)
5339
const isDisabled = isOnlyOwner && backendModule.getAssetPermissionId(self) === permissionId
54-
const assetTypeName = getText(ASSET_TYPE_TO_TEXT_ID[asset.type])
40+
const assetTypeName = getText(backendModule.ASSET_TYPE_TO_TEXT_ID[asset.type])
5541

5642
const createPermission = useMutation(
5743
backendMutationOptions(backend, 'createPermission'),

0 commit comments

Comments
 (0)