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

Drive bugs #11925

Merged
merged 34 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7196b9e
Fix remove button tooltip in sidebar
MrFlashAccount Nov 22, 2024
be206bc
Implementation of the Path column
MrFlashAccount Dec 18, 2024
f51109c
Dispay group names properly
MrFlashAccount Dec 19, 2024
df7c3c5
Fixes
MrFlashAccount Dec 22, 2024
c7af0cb
Small fixes
MrFlashAccount Dec 22, 2024
70d953d
Prettier
MrFlashAccount Dec 22, 2024
61da295
FIx tests
MrFlashAccount Dec 22, 2024
376d868
Fix lint
MrFlashAccount Dec 22, 2024
4c5f6a9
Improvements
MrFlashAccount Dec 22, 2024
553967c
Merge branch 'develop' into wip/sergeigarin/drive-bugs
MrFlashAccount Dec 22, 2024
c7b2470
Fix tests
MrFlashAccount Dec 22, 2024
d745b3e
fix prettier
MrFlashAccount Dec 22, 2024
f7560c8
Small fixes
MrFlashAccount Dec 22, 2024
b0b8b12
Fix tests, small refactor
MrFlashAccount Dec 23, 2024
32dd8bf
Revert Await component in the Drive
MrFlashAccount Dec 23, 2024
f12fb87
Fix eslint
MrFlashAccount Dec 23, 2024
4bcd6fb
Add tests, fix some bugs
MrFlashAccount Dec 23, 2024
a66a0b2
Try fix unit tests
MrFlashAccount Dec 23, 2024
074c668
Fix types
MrFlashAccount Dec 23, 2024
13ffbfe
Fix eslint
MrFlashAccount Dec 23, 2024
62febdd
Merge branch 'develop' into wip/sergeigarin/drive-bugs
MrFlashAccount Dec 25, 2024
ab9c645
Address issues from QA
MrFlashAccount Dec 25, 2024
13ce203
Fix prettier
MrFlashAccount Dec 25, 2024
f8ee925
Fix prettier x2
MrFlashAccount Dec 25, 2024
48baffe
Fix navigation
MrFlashAccount Dec 25, 2024
5f89849
Remove console.log
MrFlashAccount Dec 25, 2024
d5d01fe
Fix headers
MrFlashAccount Dec 26, 2024
401a5b9
Fix nits and comments
MrFlashAccount Dec 30, 2024
1d1638c
Fix comment
MrFlashAccount Dec 30, 2024
cd228a1
Remove await
MrFlashAccount Dec 30, 2024
618e579
Add issue for todo
MrFlashAccount Dec 30, 2024
f53e1b9
Remove TODO
MrFlashAccount Dec 30, 2024
96ef9fd
Merge branch 'develop' into wip/sergeigarin/drive-bugs
MrFlashAccount Dec 30, 2024
95f6bbe
Fix types
MrFlashAccount Dec 30, 2024
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
26 changes: 15 additions & 11 deletions app/common/src/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,21 @@ export function isOnUnknownBrowser() {

let detectedArchitecture: string | null = null
// Only implemented by Chromium.
// @ts-expect-error This API exists, but no typings exist for it yet.
navigator.userAgentData?.getHighEntropyValues(['architecture']).then((values: unknown) => {
if (
typeof values === 'object' &&
values != null &&
'architecture' in values &&
typeof values.architecture === 'string'
) {
detectedArchitecture = String(values.architecture)
}
})
// navigator is undefined in Node.js, e.g. in integration tests(mock server).
// So we need to check if it is defined before using it.
if (typeof navigator !== 'undefined' && 'userAgentData' in navigator) {
// @ts-expect-error This API exists, but no typings exist for it yet.
navigator.userAgentData.getHighEntropyValues(['architecture']).then((values: unknown) => {
if (
typeof values === 'object' &&
values != null &&
'architecture' in values &&
typeof values.architecture === 'string'
) {
detectedArchitecture = String(values.architecture)
}
})
}

/** Possible processor architectures. */
export enum Architecture {
Expand Down
40 changes: 23 additions & 17 deletions app/common/src/services/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,36 @@ export const S3_CHUNK_SIZE_BYTES = 10_000_000
// ================

/** Unique identifier for an organization. */
export type OrganizationId = newtype.Newtype<string, 'OrganizationId'>
export type OrganizationId = newtype.Newtype<`organization-${string}`, 'OrganizationId'>
export const OrganizationId = newtype.newtypeConstructor<OrganizationId>()
/** Whether a given {@link string} is an {@link OrganizationId}. */
export function isOrganizationId(id: string): id is OrganizationId {
return id.startsWith('organization-')
}

/** Unique identifier for a user in an organization. */
export type UserId = newtype.Newtype<string, 'UserId'>
export const UserId = newtype.newtypeConstructor<UserId>()
/** Whether a given {@link string} is an {@link UserId}. */
export function isUserId(id: string): id is UserId {
return id.startsWith('user-')
}

/** Unique identifier for a user group. */
export type UserGroupId = newtype.Newtype<string, 'UserGroupId'>
export type UserGroupId = newtype.Newtype<`usergroup-${string}`, 'UserGroupId'>
export const UserGroupId = newtype.newtypeConstructor<UserGroupId>()
/** Whether a given {@link string} is an {@link UserGroupId}. */
export function isUserGroupId(id: string): id is UserGroupId {
return id.startsWith('usergroup-')
}

/** Unique identifier for a directory. */
export type DirectoryId = newtype.Newtype<string, 'DirectoryId'>
export type DirectoryId = newtype.Newtype<`directory-${string}`, 'DirectoryId'>
export const DirectoryId = newtype.newtypeConstructor<DirectoryId>()
/** Whether a given {@link string} is an {@link DirectoryId}. */
export function isDirectoryId(id: string): id is DirectoryId {
return id.startsWith('directory-')
}

/**
* Unique identifier for an asset representing the items inside a directory for which the
Expand Down Expand Up @@ -118,16 +134,6 @@ export type UserPermissionIdentifier = UserGroupId | UserId
export type Path = newtype.Newtype<string, 'Path'>
export const Path = newtype.newtypeConstructor<Path>()

/** Whether a given {@link string} is an {@link UserId}. */
export function isUserId(id: string): id is UserId {
return id.startsWith('user-')
}

/** Whether a given {@link string} is an {@link UserGroupId}. */
export function isUserGroupId(id: string): id is UserGroupId {
return id.startsWith('usergroup-')
}

const PLACEHOLDER_USER_GROUP_PREFIX = 'usergroup-placeholder-'

/**
Expand All @@ -143,7 +149,7 @@ export function isPlaceholderUserGroupId(id: string) {
* being created on the backend.
*/
export function newPlaceholderUserGroupId() {
return UserGroupId(`${PLACEHOLDER_USER_GROUP_PREFIX}${uniqueString.uniqueString()}`)
return UserGroupId(`${PLACEHOLDER_USER_GROUP_PREFIX}${uniqueString.uniqueString()}` as const)
}

// =============
Expand Down Expand Up @@ -830,7 +836,7 @@ export function createRootDirectoryAsset(directoryId: DirectoryId): DirectoryAss
title: '(root)',
id: directoryId,
modifiedAt: dateTime.toRfc3339(new Date()),
parentId: DirectoryId(''),
parentId: DirectoryId('directory-'),
permissions: [],
projectState: null,
extension: null,
Expand Down Expand Up @@ -905,7 +911,7 @@ export function createPlaceholderDirectoryAsset(
): DirectoryAsset {
return {
type: AssetType.directory,
id: DirectoryId(createPlaceholderId()),
id: DirectoryId(`directory-${createPlaceholderId()}` as const),
title,
parentId,
permissions: assetPermissions,
Expand Down Expand Up @@ -1075,7 +1081,7 @@ export function createPlaceholderAssetId<Type extends AssetType>(
let result: AssetId
switch (assetType) {
case AssetType.directory: {
result = DirectoryId(id)
result = DirectoryId(`directory-${id}` as const)
break
}
case AssetType.project: {
Expand Down
9 changes: 7 additions & 2 deletions app/common/src/text/english.json
Original file line number Diff line number Diff line change
Expand Up @@ -495,13 +495,13 @@
"enableVersionCheckerDescription": "Show a dialog if the current version of the desktop app does not match the latest version.",
"disableAnimations": "Disable animations",
"disableAnimationsDescription": "Disable all animations in the app.",
"removeTheLocalDirectoryXFromFavorites": "remove the local folder '$0' from your favorites",
"removeTheLocalDirectoryXFromFavorites": "remove the local folder '$0' from your sidebar",
"changeLocalRootDirectoryInSettings": "Change the root folder",
"localStorage": "Local Storage",
"addLocalDirectory": "Add Folder",
"browseForNewLocalRootDirectory": "Browse for new Root Folder",
"resetLocalRootDirectory": "Reset Root Folder",
"removeDirectoryFromFavorites": "Remove folder from favorites",
"removeDirectoryFromFavorites": "Remove from Sidebar",
"organizationInviteTitle": "You have been invited!",
"organizationInvitePrefix": "The organization '",
"organizationInviteSuffix": "' is inviting you. Would you like to accept? All your assets will be moved with you to your personal space.",
Expand Down Expand Up @@ -692,6 +692,7 @@
"accessedDataColumnName": "Accessed data",
"docsColumnName": "Docs",
"rootFolderColumnName": "Root folder",
"pathColumnName": "Location",

"settingsShortcut": "Settings",
"closeTabShortcut": "Close Tab",
Expand Down Expand Up @@ -761,6 +762,10 @@
"accessedDataColumnHide": "Accessed Data",
"docsColumnShow": "Docs",
"docsColumnHide": "Docs",
"pathColumnShow": "Location",
"pathColumnHide": "Location",

"hideColumn": "Hide column",

"activityLog": "Activity Log",
"startDate": "Start Date",
Expand Down
63 changes: 63 additions & 0 deletions app/gui/integration-test/dashboard/actions/DrivePageActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,27 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
)
}

/**
* Expect the category to be selected.
*/
expectCategory(category: string) {
return this.step(`Expect category '${category}'`, (page) =>
expect(page.getByRole('button', { name: category })).toHaveAttribute('data-selected', 'true'),
)
}

/**
* Expect the category to be not selected.
*/
expectCategoryNotSelected(category: string) {
return this.step(`Expect category '${category}' not selected`, (page) =>
expect(page.getByRole('button', { name: category })).toHaveAttribute(
'data-selected',
'false',
),
)
}

/** Actions specific to the Drive table. */
get driveTable() {
// eslint-disable-next-line @typescript-eslint/no-this-alias
Expand All @@ -147,6 +168,11 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
.getByLabel(TEXT.sortByModificationDate)
.or(page.getByLabel(TEXT.sortByModificationDateDescending))
.or(page.getByLabel(TEXT.stopSortingByModificationDate))

const locatePathColumnHeading = (page: Page) => page.getByTestId('path-column-heading')
const locatePathColumnCell = (page: Page, title: string) =>
page.getByTestId(`path-column-cell-${title.toLowerCase().replace(/\s+/g, '-')}`)

return {
/** Click the column heading for the "name" column to change its sort order. */
clickNameColumnHeading() {
Expand All @@ -160,6 +186,16 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
callback(locateNameColumnHeading(page), context),
)
},
withPathColumnHeading(callback: LocatorCallback<Context>) {
return self.step('Interact with "path" column heading', (page, context) =>
callback(locatePathColumnHeading(page), context),
)
},
withPathColumnCell(title: string, callback: LocatorCallback<Context>) {
return self.step(`Interact with "path" column cell '${title}'`, (page, context) =>
callback(locatePathColumnCell(page, title), context),
)
},
/** Click the column heading for the "modified" column to change its sort order. */
clickModifiedColumnHeading() {
return self.step('Click "modified" column heading', (page) =>
Expand Down Expand Up @@ -208,6 +244,11 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
await callback(locateAssetRows(page), locateNonAssetRows(page), self.context, page)
})
},
withSelectedRows(callback: LocatorCallback<Context>) {
return self.step('Interact with selected drive table rows', async (page, context) => {
await callback(locateAssetRows(page).and(page.locator('[data-selected="true"]')), context)
})
},
/** Drag a row onto another row. */
dragRowToRow(from: number, to: number) {
return self.step(`Drag drive table row #${from} to row #${to}`, async (page) => {
Expand All @@ -230,6 +271,28 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
}),
)
},
expandDirectory(index: number) {
return self.step(`Expand drive table row #${index}`, async (page) => {
const expandButton = locateAssetRows(page)
.nth(index)
.getByTestId('directory-row-expand-button')

await expect(expandButton).toHaveAttribute('aria-label', TEXT.expand)

await expandButton.click()
})
},
collapseDirectory(index: number) {
return self.step(`Collapse drive table row #${index}`, async (page) => {
const collapseButton = locateAssetRows(page)
.nth(index)
.getByTestId('directory-row-expand-button')

await expect(collapseButton).toHaveAttribute('aria-label', TEXT.collapse)

return collapseButton.click()
})
},
/**
* A test assertion to confirm that there is only one row visible, and that row is the
* placeholder row displayed when there are no assets to show.
Expand Down
Loading
Loading