Skip to content
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

Expand parent directories of opened projects #11947

Closed
wants to merge 37 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4c5729f
Expand parent directories of opened projects
somebody1234 Dec 26, 2024
edddae0
Fix expanding local directories
somebody1234 Dec 26, 2024
8269f4e
Automatically expand "Users" or "Teams" root directory as appropriate
somebody1234 Dec 30, 2024
e5259d5
Fix errors
somebody1234 Dec 30, 2024
7b96344
Merge branch 'develop' into wip/sb/expand-parent-directories
somebody1234 Dec 30, 2024
b2b361b
Only expand ancestors of initially launched projects once
somebody1234 Dec 31, 2024
e943fe1
WIP: Separate ancestor lists for each Drive sidebar category
somebody1234 Dec 31, 2024
2cee8ea
Fix errors
somebody1234 Jan 2, 2025
68000a7
Merge branch 'develop' into wip/sb/expand-parent-directories
somebody1234 Jan 3, 2025
364c3af
Fix errors
somebody1234 Jan 3, 2025
1df6478
Fix bugs
somebody1234 Jan 3, 2025
21b3489
Remove obsolete test
somebody1234 Jan 3, 2025
6adda38
Replace `category` with `categoryType` in `listDirectoryQueryOptions`
somebody1234 Jan 6, 2025
eca7931
Move logic for computing ancestors from `Dashboard.tsx` to individual…
somebody1234 Jan 7, 2025
0fa64ce
Extract computing initial expanded directories to a function
somebody1234 Jan 7, 2025
17eb1df
Compute initial expanded directories in initial value store instead o…
somebody1234 Jan 7, 2025
356cfec
Fix cyclic dependencies between stores
somebody1234 Jan 8, 2025
d01fc61
Fix errors
somebody1234 Jan 8, 2025
3091835
Expand full path to directory when clicking on breadcrumb
somebody1234 Jan 8, 2025
8f4032e
Remove `useSetExpandedDirectories`
somebody1234 Jan 8, 2025
58d59ec
Add `useIsDirectoryExpanded`
somebody1234 Jan 8, 2025
6bcfaf7
WIP: Re-add `DriveProvider.test`
somebody1234 Jan 8, 2025
98727bf
WIP: Fix `DriveProvider.test`
somebody1234 Jan 8, 2025
a7f81ae
Fix unnecessary dependencies of `DriveProvider`
somebody1234 Jan 8, 2025
4602cc6
help
somebody1234 Jan 8, 2025
afdc27b
WIP: Fix `DriveProvider.test`
somebody1234 Jan 8, 2025
71a1b8c
Fix `DriveProvider.test`
somebody1234 Jan 8, 2025
ffc6e15
Fix type errors
somebody1234 Jan 8, 2025
42fa910
Merge branch 'develop' into wip/sb/expand-parent-directories
somebody1234 Jan 9, 2025
25a05ca
Fix errors
somebody1234 Jan 9, 2025
571a60b
Fix integration tests
somebody1234 Jan 9, 2025
6b43868
Merge branch 'develop' into wip/sb/expand-parent-directories
somebody1234 Jan 13, 2025
6e6b573
Fix lint error
somebody1234 Jan 13, 2025
41ae083
Minor fixes
somebody1234 Jan 14, 2025
2d1bdbf
Merge branch 'develop' into wip/sb/expand-parent-directories
somebody1234 Jan 16, 2025
736c51e
Merge branch 'develop' into wip/sb/expand-parent-directories
somebody1234 Jan 19, 2025
4c5aef3
Fix integration tests
somebody1234 Jan 19, 2025
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
4 changes: 1 addition & 3 deletions app/common/src/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ export function browser(): Browser {
}
/**
* Returns `true` if running in Electron, else `false`.
* This is used to determine whether to use a `MemoryRouter` (stores history in an array)
* or a `BrowserRouter` (stores history in the path of the URL).
* It is also used to determine whether to send custom state to Amplify for a workaround.
* It is used to determine whether to send custom state to Amplify for a workaround.
*/
export function isOnElectron() {
return /electron/i.test(navigator.userAgent)
Expand Down
26 changes: 23 additions & 3 deletions app/common/src/services/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ export function isUserGroupId(id: string): id is UserGroupId {
/** Unique identifier for a directory. */
export type DirectoryId = newtype.Newtype<`directory-${string}`, 'DirectoryId'>
export const DirectoryId = newtype.newtypeConstructor<DirectoryId>()
/** Whether a given {@link string} is an {@link DirectoryId}. */
/** Whether a given {@link string} is a {@link DirectoryId}. */
export function isDirectoryId(id: string): id is DirectoryId {
return id.startsWith('directory-')
}

/** Unique identifier for a category. */
export type CategoryId = 'cloud' | 'local' | 'recent' | 'trash' | DirectoryId | UserGroupId | UserId

/**
* Unique identifier for an asset representing the items inside a directory for which the
* request to retrive the items has not yet completed.
Expand All @@ -66,6 +69,10 @@ export const ErrorAssetId = newtype.newtypeConstructor<ErrorAssetId>()
/** Unique identifier for a user's project. */
export type ProjectId = newtype.Newtype<string, 'ProjectId'>
export const ProjectId = newtype.newtypeConstructor<ProjectId>()
/** Whether a given {@link string} is a {@link ProjectId}. */
export function isProjectId(id: string): id is ProjectId {
return id.startsWith('project-')
}

/** Unique identifier for an uploaded file. */
export type FileId = newtype.Newtype<string, 'FileId'>
Expand Down Expand Up @@ -317,8 +324,10 @@ export interface UpdatedProject extends BaseProject {

/** A user/organization's project containing and/or currently executing code. */
export interface ProjectRaw extends ListedProjectRaw {
readonly ide_version: VersionNumber | null
readonly engine_version: VersionNumber | null
readonly ideVersion: VersionNumber | null
readonly engineVersion: VersionNumber | null
readonly parentsPath: string
readonly virtualParentsPath: string
}

/** A user/organization's project containing and/or currently executing code. */
Expand All @@ -328,6 +337,8 @@ export interface Project extends ListedProject {
readonly openedBy?: EmailAddress
/** On the Remote (Cloud) Backend, this is a S3 url that is valid for only 120 seconds. */
readonly url?: HttpsUrl
readonly parentsPath: string
readonly virtualParentsPath: string
}

/** A user/organization's project containing and/or currently executing code. */
Expand Down Expand Up @@ -1793,6 +1804,15 @@ export default abstract class Backend {
): Promise<void>
/** Download an asset. */
abstract download(assetId: AssetId, title: string): Promise<void>
/**
* The list of the asset's ancestors, if and only if the asset is in the given category.
* Note: The `null` in the return type exists to prevent accidentally implicitly returning
* `undefined`.
*/
abstract tryGetAssetAncestors(
asset: Pick<AnyAsset, 'id' | 'parentId'>,
category: CategoryId,
): Promise<readonly DirectoryId[] | null>

/**
* Get the URL for the customer portal.
Expand Down
2 changes: 1 addition & 1 deletion app/common/src/utilities/data/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function unsafeRemoveUndefined<T extends object>(
export function mapEntries<K extends PropertyKey, V, W>(
object: Record<K, V>,
map: (key: K, value: V) => W,
): Readonly<Record<K, W>> {
): Record<K, W> {
// @ts-expect-error It is known that the set of keys is the same for the input and the output,
// because the output is dynamically generated based on the input.
return Object.fromEntries(
Expand Down
41 changes: 27 additions & 14 deletions app/gui/integration-test/dashboard/actions/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ import invariant from 'tiny-invariant'
// === Constants ===
// =================

/** Return a new date that has not yet been returned. */
function newUniqueDate(): Date {
const timestamp = Number(new Date())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why's it that complex? The time always change and a new date is always unique, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newUniqueDate(); newUniqueDate() is going to often return the same date.

i'm not 100% sure it fixed anything but i think i was getting flakiness without this - note that the specific problem this was causing issues was setupAPI which is synchronous

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree it's quite complex though

Copy link
Contributor

@MrFlashAccount MrFlashAccount Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking on just doing an arbitrary delay within the function with an empty while loop. It'd be slower, but the diff will always be at leats 1ms

while (Number(new Date()) <= timestamp) {
// Busy loop
}
return new Date()
}

const __dirname = dirname(fileURLToPath(import.meta.url))

const MOCK_SVG = `
Expand Down Expand Up @@ -328,7 +337,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
)

const createUserGroupPermission = (
userGroup: backend.UserGroupInfo,
userGroup: backend.UserGroup,
permission: permissions.PermissionAction = permissions.PermissionAction.own,
rest: Partial<backend.UserGroupPermission> = {},
): backend.UserGroupPermission =>
Expand Down Expand Up @@ -358,7 +367,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
projectState: null,
extension: null,
title,
modifiedAt: dateTime.toRfc3339(new Date()),
modifiedAt: dateTime.toRfc3339(newUniqueDate()),
description: rest.description ?? '',
labels: [],
parentId,
Expand Down Expand Up @@ -411,7 +420,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
},
extension: null,
title,
modifiedAt: dateTime.toRfc3339(new Date()),
modifiedAt: dateTime.toRfc3339(newUniqueDate()),
description: rest.description ?? '',
labels: [],
parentId: defaultDirectoryId,
Expand Down Expand Up @@ -452,7 +461,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
projectState: null,
extension: '',
title: rest.title ?? '',
modifiedAt: dateTime.toRfc3339(new Date()),
modifiedAt: dateTime.toRfc3339(newUniqueDate()),
description: rest.description ?? '',
labels: [],
parentId: defaultDirectoryId,
Expand Down Expand Up @@ -494,7 +503,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
projectState: null,
extension: null,
title: rest.title ?? '',
modifiedAt: dateTime.toRfc3339(new Date()),
modifiedAt: dateTime.toRfc3339(newUniqueDate()),
description: rest.description ?? '',
labels: [],
parentId: defaultDirectoryId,
Expand Down Expand Up @@ -536,7 +545,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
projectState: null,
extension: null,
title: rest.title ?? '',
modifiedAt: dateTime.toRfc3339(new Date()),
modifiedAt: dateTime.toRfc3339(newUniqueDate()),
description: rest.description ?? '',
labels: [],
parentId: defaultDirectoryId,
Expand Down Expand Up @@ -808,9 +817,13 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
break
}
}
filteredAssets.sort(
(a, b) => backend.ASSET_TYPE_ORDER[a.type] - backend.ASSET_TYPE_ORDER[b.type],
)
filteredAssets.sort((a, b) => {
const order = backend.ASSET_TYPE_ORDER[a.type] - backend.ASSET_TYPE_ORDER[b.type]
if (order !== 0) {
return order
}
return Number(new Date(b.modifiedAt)) - Number(new Date(a.modifiedAt))
})
const json: remoteBackend.ListDirectoryResponseBody = { assets: filteredAssets }

route.fulfill({ json })
Expand Down Expand Up @@ -875,14 +888,14 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
name: 'example project name',
state: project.projectState,
packageName: 'Project_root',
// eslint-disable-next-line camelcase
ide_version: null,
// eslint-disable-next-line camelcase
engine_version: {
ideVersion: null,
engineVersion: {
value: '2023.2.1-nightly.2023.9.29',
lifecycle: backend.VersionLifecycle.development,
},
address: backend.Address('ws://localhost/'),
parentsPath: getParentPath(project.parentId),
virtualParentsPath: getVirtualParentPath(project.parentId, project.title),
} satisfies backend.ProjectRaw
})

Expand Down Expand Up @@ -1322,7 +1335,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
description: null,
id,
labels: [],
modifiedAt: dateTime.toRfc3339(new Date()),
modifiedAt: dateTime.toRfc3339(newUniqueDate()),
parentId,
permissions: [
{
Expand Down
10 changes: 8 additions & 2 deletions app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,19 @@ test('can navigate to parent directory of an asset in the Recent category', ({ p
api.addProject({ title: 'c', parentId: subDirectory.id })
},
})
.driveTable.withRows(async (rows) => {
await test.expect(rows).toHaveText([/^d/, /^b/, /^a/])
})
.driveTable.expandDirectory(0)
.driveTable.expandDirectory(1)
// Project in the nested directory (c)
.driveTable.rightClickRow(2)
.contextMenu.moveNonFolderToTrash()
.driveTable.withRows(async (rows) => {
await test.expect(rows).toHaveText([/^d/, /^e/, /^b/, /^a/])
})
// Project in the root (a)
.driveTable.rightClickRow(2)
.driveTable.rightClickRow(3)
.contextMenu.moveNonFolderToTrash()
.goToCategory.trash()
.driveTable.withPathColumnCell('a', async (cell) => {
Expand Down Expand Up @@ -174,7 +180,7 @@ test("can't run a project in browser by default", ({ page }) =>
await expect(startProjectButton).toBeDisabled()
}))

test("can't start an already running by another user", ({ page }) =>
test("can't start a project already being run by another user", ({ page }) =>
mockAllAndLogin({
page,
setupAPI: async (api) => {
Expand Down
8 changes: 4 additions & 4 deletions app/gui/src/dashboard/components/dashboard/AssetRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ export function RealAssetInternalRow(props: RealAssetRowInternalProps) {
window.setTimeout(() => {
setSelected(false)
})
toggleDirectoryExpansion(asset.id)
toggleDirectoryExpansion([asset.id], category.id)
}
}}
onContextMenu={(event) => {
Expand Down Expand Up @@ -544,7 +544,7 @@ export function RealAssetInternalRow(props: RealAssetRowInternalProps) {
}
if (asset.type === backendModule.AssetType.directory) {
dragOverTimeoutHandle.current = window.setTimeout(() => {
toggleDirectoryExpansion(asset.id, true)
toggleDirectoryExpansion([asset.id], category.id, true)
}, DRAG_EXPAND_DELAY_MS)
}
// Required because `dragover` does not fire on `mouseenter`.
Expand Down Expand Up @@ -591,7 +591,7 @@ export function RealAssetInternalRow(props: RealAssetRowInternalProps) {
event.preventDefault()
event.stopPropagation()
unsetModal()
toggleDirectoryExpansion(directoryId, true)
toggleDirectoryExpansion([directoryId], category.id, true)
const ids = payload
.filter((payloadItem) => payloadItem.asset.parentId !== directoryId)
.map((dragItem) => dragItem.key)
Expand All @@ -604,7 +604,7 @@ export function RealAssetInternalRow(props: RealAssetRowInternalProps) {
} else if (event.dataTransfer.types.includes('Files')) {
event.preventDefault()
event.stopPropagation()
toggleDirectoryExpansion(directoryId, true)
toggleDirectoryExpansion([directoryId], category.id, true)
void uploadFiles(Array.from(event.dataTransfer.files), directoryId, null)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import FolderArrowIcon from '#/assets/folder_arrow.svg'

import { backendMutationOptions } from '#/hooks/backendHooks'

import { useDriveStore, useToggleDirectoryExpansion } from '#/providers/DriveProvider'
import {
useDriveStore,
useIsDirectoryExpanded,
useToggleDirectoryExpansion,
} from '#/providers/DriveProvider'
import * as textProvider from '#/providers/TextProvider'

import type * as column from '#/components/dashboard/column'
Expand All @@ -15,7 +19,6 @@ import EditableSpan from '#/components/EditableSpan'
import * as backendModule from '#/services/Backend'

import { Button } from '#/components/AriaComponents'
import { useStore } from '#/hooks/storeHooks'
import * as eventModule from '#/utilities/event'
import * as indent from '#/utilities/indent'
import * as object from '#/utilities/object'
Expand All @@ -38,13 +41,11 @@ export interface DirectoryNameColumnProps extends column.AssetColumnProps {
*/
export default function DirectoryNameColumn(props: DirectoryNameColumnProps) {
const { item, depth, selected, state, rowState, setRowState, isEditable } = props
const { backend, nodeMap } = state
const { backend, nodeMap, category } = state
const { getText } = textProvider.useText()
const driveStore = useDriveStore()
const toggleDirectoryExpansion = useToggleDirectoryExpansion()
const isExpanded = useStore(driveStore, (storeState) =>
storeState.expandedDirectoryIds.includes(item.id),
)
const isExpanded = useIsDirectoryExpanded(item.id, category.id)

const updateDirectoryMutation = useMutation(backendMutationOptions(backend, 'updateDirectory'))

Expand Down Expand Up @@ -98,7 +99,7 @@ export default function DirectoryNameColumn(props: DirectoryNameColumnProps) {
isExpanded && 'rotate-90',
)}
onPress={() => {
toggleDirectoryExpansion(item.id)
toggleDirectoryExpansion([item.id], category.id)
}}
/>

Expand Down
Loading
Loading