Skip to content

Commit

Permalink
Even more dashboard fixes (#10541)
Browse files Browse the repository at this point in the history
- Fix enso-org/cloud-v2#1383
- Fix file download - both on Electron, and in browser
- Refresh versions list when uploading neww file version
- Fix app crashing when asset is opened in asset panel while switching to Local Backend
- Don't show asset id when asset is opened in asset panel, but user is on the Local backend, resulting in the internal Asset ID being shown
- Fix drag-n-drop
- ⚠️ `npm run dev` is NOT fixed in this PR - however it should already be fixed in another PR which has already been merged. This needs testing to confirm whether it is fixed though.

Other changes:
- Add support for "duplicate project" endpoint on Local Backend
- Fix downloading project from nested directory on Local Backend (not working)
- Refactor more E2E tests to use the "new" architecture
- Simplify "new" E2E architecture to minimize boilerplate

# Important Notes
- When testing downloads, both Electron and browser should be tested as they use completely separate implementations for how files are downloaded.
  • Loading branch information
somebody1234 authored Jul 16, 2024
1 parent a30b0c6 commit cf9d757
Show file tree
Hide file tree
Showing 66 changed files with 2,162 additions and 1,452 deletions.
2 changes: 1 addition & 1 deletion app/ide-desktop/lib/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"@babel/plugin-syntax-import-attributes": "^7.24.7",
"@electron/notarize": "2.1.0",
"@types/node": "^20.11.21",
"electron": "25.7.0",
"electron": "31.2.0",
"electron-builder": "^24.13.3",
"enso-common": "workspace:*",
"enso-gui2": "workspace:*",
Expand Down
12 changes: 10 additions & 2 deletions app/ide-desktop/lib/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ import * as urlAssociations from 'url-associations'
const logger = contentConfig.logger

if (process.env.ELECTRON_DEV_MODE === 'true' && process.env.NODE_MODULES_PATH != null) {
require.main?.paths.unshift(process.env.NODE_MODULES_PATH)
console.log(require.main?.paths)
module.paths.unshift(process.env.NODE_MODULES_PATH)
}

// ===========
Expand Down Expand Up @@ -451,6 +450,15 @@ class App {
event.reply(ipc.Channel.importProjectFromPath, path, info)
}
)
electron.ipcMain.on(
ipc.Channel.downloadURL,
(_event, url: string, headers?: Record<string, string>) => {
electron.BrowserWindow.getFocusedWindow()?.webContents.downloadURL(
url,
headers ? { headers } : {}
)
}
)
electron.ipcMain.on(ipc.Channel.showItemInFolder, (_event, fullPath: string) => {
electron.shell.showItemInFolder(fullPath)
})
Expand Down
2 changes: 2 additions & 0 deletions app/ide-desktop/lib/client/src/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,7 @@ export enum Channel {
openFileBrowser = 'open-file-browser',
/** Show a file or folder in the system file browser. */
showItemInFolder = 'show-item-in-folder',
/** Download a file using its URL. */
downloadURL = 'download-url',
showAboutModal = 'show-about-modal',
}
3 changes: 3 additions & 0 deletions app/ide-desktop/lib/client/src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ electron.contextBridge.exposeInMainWorld(MENU_API_KEY, MENU_API)
// ==================

const SYSTEM_API = {
downloadURL: (url: string, headers?: Record<string, string>) => {
electron.ipcRenderer.send(ipc.Channel.downloadURL, url, headers)
},
showItemInFolder: (fullPath: string) => {
electron.ipcRenderer.send(ipc.Channel.showItemInFolder, fullPath)
},
Expand Down
5 changes: 5 additions & 0 deletions app/ide-desktop/lib/client/src/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ const TRUSTED_HOSTS = [
'production-enso-domain.auth.eu-west-1.amazoncognito.com',
'production-enso-organizations-files.s3.amazonaws.com',
'pb-enso-domain.auth.eu-west-1.amazoncognito.com',
'7aqkn3tnbc.execute-api.eu-west-1.amazonaws.com',
'lkxuay3ha1.execute-api.eu-west-1.amazonaws.com',
'8rf1a7iy49.execute-api.eu-west-1.amazonaws.com',
'opk1cxpwec.execute-api.eu-west-1.amazonaws.com',
'xw0g8j3tsb.execute-api.eu-west-1.amazonaws.com',
's3.eu-west-1.amazonaws.com',
// This (`localhost`) is required to access Project Manager HTTP endpoints.
// This should be changed appropriately if the Project Manager's port number becomes dynamic.
Expand Down
19 changes: 3 additions & 16 deletions app/ide-desktop/lib/common/src/services/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,6 @@ export interface ProjectStateType {
readonly ec2PublicIpAddress?: string
readonly currentSessionId?: string
readonly openedBy?: EmailAddress
/** Only present on the Local backend. */
readonly path?: Path
}

export const IS_OPENING: Readonly<Record<ProjectState, boolean>> = {
Expand Down Expand Up @@ -242,7 +240,7 @@ export const IS_OPENING_OR_OPENED: Readonly<Record<ProjectState, boolean>> = {

/** Common `Project` fields returned by all `Project`-related endpoints. */
export interface BaseProject {
readonly organizationId: string
readonly organizationId: OrganizationId
readonly projectId: ProjectId
readonly name: string
}
Expand Down Expand Up @@ -1053,15 +1051,11 @@ export interface UpdateFileRequestBody {
export interface UpdateAssetRequestBody {
readonly parentDirectoryId: DirectoryId | null
readonly description: string | null
/** Only present on the Local backend. */
readonly projectPath?: Path
}

/** HTTP request body for the "delete asset" endpoint. */
export interface DeleteAssetRequestBody {
readonly force: boolean
/** Only used by the Local backend. */
readonly parentId: DirectoryId
}

/** HTTP request body for the "create project" endpoint. */
Expand All @@ -1078,8 +1072,6 @@ export interface UpdateProjectRequestBody {
readonly projectName: string | null
readonly ami: Ami | null
readonly ideVersion: VersionNumber | null
/** Only used by the Local backend. */
readonly parentId: DirectoryId
}

/** HTTP request body for the "open project" endpoint. */
Expand Down Expand Up @@ -1463,11 +1455,6 @@ export default abstract class Backend {
projectId?: string | null,
metadata?: object | null
): Promise<void>
/** Return a {@link Promise} that resolves only when a project is ready to open. */
abstract waitUntilProjectIsReady(
projectId: ProjectId,
directory: DirectoryId | null,
title: string,
abortSignal?: AbortSignal
): Promise<Project>
/** Download from an arbitrary URL that is assumed to originate from this backend. */
abstract download(url: string, name?: string): Promise<void>
}
52 changes: 37 additions & 15 deletions app/ide-desktop/lib/dashboard/e2e/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,7 @@ export function locateNewUserGroupModal(page: test.Page) {

/** Find a user menu (if any) on the current page. */
export function locateUserMenu(page: test.Page) {
// This has no identifying features.
return page.getByTestId('user-menu')
return page.getByAltText('User Settings').locator('visible=true')
}

/** Find a "set username" panel (if any) on the current page. */
Expand Down Expand Up @@ -463,7 +462,8 @@ export namespace settings {
/** Navigate so that the "user account" settings section is visible. */
export async function go(page: test.Page) {
await test.test.step('Go to "user account" settings section', async () => {
await press(page, 'Mod+,')
await locateUserMenu(page).click()
await page.getByRole('button', { name: 'Settings' }).getByText('Settings').click()
})
}

Expand All @@ -482,7 +482,8 @@ export namespace settings {
/** Navigate so that the "change password" settings section is visible. */
export async function go(page: test.Page) {
await test.test.step('Go to "change password" settings section', async () => {
await press(page, 'Mod+,')
await locateUserMenu(page).click()
await page.getByRole('button', { name: 'Settings' }).getByText('Settings').click()
})
}

Expand Down Expand Up @@ -516,7 +517,8 @@ export namespace settings {
/** Navigate so that the "profile picture" settings section is visible. */
export async function go(page: test.Page) {
await test.test.step('Go to "profile picture" settings section', async () => {
await press(page, 'Mod+,')
await locateUserMenu(page).click()
await page.getByRole('button', { name: 'Settings' }).getByText('Settings').click()
})
}

Expand All @@ -535,7 +537,8 @@ export namespace settings {
/** Navigate so that the "organization" settings section is visible. */
export async function go(page: test.Page) {
await test.test.step('Go to "organization" settings section', async () => {
await press(page, 'Mod+,')
await locateUserMenu(page).click()
await page.getByRole('button', { name: 'Settings' }).getByText('Settings').click()
await settings.tab.organization.locate(page).click()
})
}
Expand Down Expand Up @@ -571,7 +574,8 @@ export namespace settings {
/** Navigate so that the "organization profile picture" settings section is visible. */
export async function go(page: test.Page) {
await test.test.step('Go to "organization profile picture" settings section', async () => {
await press(page, 'Mod+,')
await locateUserMenu(page).click()
await page.getByRole('button', { name: 'Settings' }).getByText('Settings').click()
await settings.tab.organization.locate(page).click()
})
}
Expand All @@ -591,7 +595,8 @@ export namespace settings {
/** Navigate so that the "members" settings section is visible. */
export async function go(page: test.Page, force = false) {
await test.test.step('Go to "members" settings section', async () => {
await press(page, 'Mod+,')
await locateUserMenu(page).click()
await page.getByRole('button', { name: 'Settings' }).getByText('Settings').click()
await settings.tab.members.locate(page).click({ force })
})
}
Expand Down Expand Up @@ -876,11 +881,10 @@ export const mockApi = apiModule.mockApi
/** Set up all mocks, without logging in. */
// This syntax is required for Playwright to work properly.
// eslint-disable-next-line no-restricted-syntax
export async function mockAll({ page, setupAPI }: MockParams) {
return await test.test.step('Execute all mocks', async () => {
const api = await mockApi({ page, setupAPI })
export function mockAll({ page, setupAPI }: MockParams) {
return new LoginPageActions(page).step('Execute all mocks', async () => {
await mockApi({ page, setupAPI })
await mockDate({ page, setupAPI })
return { api, pageActions: new LoginPageActions(page) }
})
}

Expand All @@ -891,10 +895,28 @@ export async function mockAll({ page, setupAPI }: MockParams) {
/** Set up all mocks, and log in with dummy credentials. */
// This syntax is required for Playwright to work properly.
// eslint-disable-next-line no-restricted-syntax
export async function mockAllAndLogin({ page, setupAPI }: MockParams) {
export function mockAllAndLogin({ page, setupAPI }: MockParams) {
return new DrivePageActions(page)
.step('Execute all mocks', async () => {
await mockApi({ page, setupAPI })
await mockDate({ page, setupAPI })
})
.do(thePage => login({ page: thePage, setupAPI }))
}

// ===================================
// === mockAllAndLoginAndExposeAPI ===
// ===================================

/** Set up all mocks, and log in with dummy credentials.
* @deprecated Prefer {@link mockAllAndLogin}. */
// This syntax is required for Playwright to work properly.
// eslint-disable-next-line no-restricted-syntax
export async function mockAllAndLoginAndExposeAPI({ page, setupAPI }: MockParams) {
return await test.test.step('Execute all mocks and login', async () => {
const mocks = await mockAll({ page, setupAPI })
const api = await mockApi({ page, setupAPI })
await mockDate({ page, setupAPI })
await login({ page, setupAPI })
return { ...mocks, pageActions: new DrivePageActions(page) }
return api
})
}
58 changes: 53 additions & 5 deletions app/ide-desktop/lib/dashboard/e2e/actions/BaseActions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
/** @file The base class from which all `Actions` classes are derived. */
import * as test from '@playwright/test'

import type * as inputBindings from '#/utilities/inputBindings'

import * as actions from '../actions'

// ====================
// === PageCallback ===
// ====================
Expand Down Expand Up @@ -29,13 +33,19 @@ export interface LocatorCallback {
*
* [`thenable`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#thenables
*/
export default class BaseActions implements PromiseLike<void> {
export default class BaseActions implements Promise<void> {
/** Create a {@link BaseActions}. */
constructor(
protected readonly page: test.Page,
private readonly promise = Promise.resolve()
) {}

/** Get the string name of the class of this instance. Required for this class to implement
* {@link Promise}. */
get [Symbol.toStringTag]() {
return this.constructor.name
}

/** Press a key, replacing the text `Mod` with `Meta` (`Cmd`) on macOS, and `Control`
* on all other platforms. */
static press(page: test.Page, keyOrShortcut: string): Promise<void> {
Expand All @@ -55,7 +65,6 @@ export default class BaseActions implements PromiseLike<void> {
}
})
}

/** Proxies the `then` method of the internal {@link Promise}. */
async then<T, E>(
// The following types are copied almost verbatim from the type definitions for `Promise`.
Expand All @@ -72,10 +81,17 @@ export default class BaseActions implements PromiseLike<void> {
* to treat this class as a {@link Promise}. */
// The following types are copied almost verbatim from the type definitions for `Promise`.
// eslint-disable-next-line no-restricted-syntax
async catch<T>(onrejected?: ((reason: unknown) => T) | null | undefined) {
async catch<T>(onrejected?: ((reason: unknown) => PromiseLike<T> | T) | null | undefined) {
return await this.promise.catch(onrejected)
}

/** Proxies the `catch` method of the internal {@link Promise}.
* This method is not required for this to be a `thenable`, but it is still useful
* to treat this class as a {@link Promise}. */
async finally(onfinally?: (() => void) | null | undefined): Promise<void> {
await this.promise.finally(onfinally)
}

/** Return a {@link BaseActions} with the same {@link Promise} but a different type. */
into<
T extends new (page: test.Page, promise: Promise<void>, ...args: Args) => InstanceType<T>,
Expand All @@ -98,13 +114,45 @@ export default class BaseActions implements PromiseLike<void> {
}

/** Perform an action on the current page. */
step(name: string, callback: PageCallback): this {
step(name: string, callback: PageCallback) {
return this.do(() => test.test.step(name, () => callback(this.page)))
}

/** Press a key, replacing the text `Mod` with `Meta` (`Cmd`) on macOS, and `Control`
* on all other platforms. */
press(keyOrShortcut: string): this {
press<Key extends string>(keyOrShortcut: inputBindings.AutocompleteKeybind<Key>) {
return this.do(page => BaseActions.press(page, keyOrShortcut))
}

/** Perform actions until a predicate passes. */
retry(
callback: (actions: this) => this,
predicate: (page: test.Page) => Promise<boolean>,
options: { retries?: number; delay?: number } = {}
) {
// eslint-disable-next-line @typescript-eslint/no-magic-numbers
const { retries = 3, delay = 1_000 } = options
return this.step('Perform actions with retries', async thePage => {
for (let i = 0; i < retries; i += 1) {
await callback(this)
if (await predicate(thePage)) {
// eslint-disable-next-line no-restricted-syntax
return
}
await thePage.waitForTimeout(delay)
}
throw new Error('This action did not succeed.')
})
}

/** Perform actions with the "Mod" modifier key pressed. */
withModPressed<R extends BaseActions>(callback: (actions: this) => R) {
return callback(
this.step('Press "Mod"', async page => {
await page.keyboard.down(await actions.modModifier(page))
})
).step('Release "Mod"', async page => {
await page.keyboard.up(await actions.modModifier(page))
})
}
}
Loading

0 comments on commit cf9d757

Please sign in to comment.