Skip to content

Commit

Permalink
Remove listGroups while building categories (#11988)
Browse files Browse the repository at this point in the history
This PR adds new field called `groups` on the User object to avoid fetching the usergroups list to get the names of the groups.

Also this PR removes the `<ManagePermisssionsModal />`
  • Loading branch information
MrFlashAccount authored Jan 15, 2025
1 parent 6dc3e33 commit b1cc4f1
Show file tree
Hide file tree
Showing 25 changed files with 429 additions and 718 deletions.
1 change: 0 additions & 1 deletion .github/workflows/gui-changed-files.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ jobs:
.github/workflows/gui*
.github/workflows/storybook.yml
files_ignore: |
app/ide-desktop/**
app/gui/scripts/**
app/gui/.gitignore
.git-*
Expand Down
24 changes: 20 additions & 4 deletions app/common/src/services/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,25 @@ export interface User extends UserInfo {
readonly isOrganizationAdmin: boolean
readonly rootDirectoryId: DirectoryId
readonly profilePicture?: HttpsUrl
/**
* Contains the IDs of the user groups that the user is a member of.
* @deprecated Use `groups` instead.
*/
readonly userGroups: readonly UserGroupId[] | null
readonly removeAt?: dateTime.Rfc3339DateTime | null
readonly plan?: Plan | undefined
/**
* Contains the user groups that the user is a member of.
* Has enriched metadata, like the name of the group and the home directory ID.
*/
readonly groups?: readonly UserGroup[]
}

/** A user related to the current user. */
export interface UserGroup {
readonly id: UserGroupId
readonly name: string
readonly homeDirectoryId: DirectoryId
}

/** A `Directory` returned by `createDirectory`. */
Expand Down Expand Up @@ -488,7 +504,7 @@ export interface UserPermission {

/** User permission for a specific user group. */
export interface UserGroupPermission {
readonly userGroup: UserGroupInfo
readonly userGroup: UserGroup
readonly permission: permissions.PermissionAction
}

Expand Down Expand Up @@ -544,7 +560,7 @@ export function isUserGroupPermissionAnd(predicate: (permission: UserGroupPermis

/** Get the property representing the name on an arbitrary variant of {@link UserPermission}. */
export function getAssetPermissionName(permission: AssetPermission) {
return isUserPermission(permission) ? permission.user.name : permission.userGroup.groupName
return isUserPermission(permission) ? permission.user.name : permission.userGroup.name
}

/** Get the property representing the id on an arbitrary variant of {@link UserPermission}. */
Expand Down Expand Up @@ -1170,8 +1186,8 @@ export function compareAssetPermissions(a: AssetPermission, b: AssetPermission)
} else {
// NOTE [NP]: Although `userId` is unique, and therefore sufficient to sort permissions, sort
// name first, so that it's easier to find a permission in a long list (i.e., for readability).
const aName = 'user' in a ? a.user.name : a.userGroup.groupName
const bName = 'user' in b ? b.user.name : b.userGroup.groupName
const aName = 'user' in a ? a.user.name : a.userGroup.name
const bName = 'user' in b ? b.user.name : b.userGroup.name
const aUserId = 'user' in a ? a.user.userId : a.userGroup.id
const bUserId = 'user' in b ? b.user.userId : b.userGroup.id
return (
Expand Down
7 changes: 5 additions & 2 deletions app/gui/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import url from 'node:url'

const DEBUG = process.env.DEBUG_TEST === 'true'
const isCI = process.env.CI === 'true'

const isProd = process.env.PROD === 'true'
const TIMEOUT_MS = DEBUG ? 100_000_000 : 25_000

// We tend to use less CPU on CI to reduce the number of failures due to timeouts.
Expand Down Expand Up @@ -162,7 +162,10 @@ export default defineConfig({
reuseExistingServer: false,
},
{
command: `corepack pnpm exec vite -c vite.test.config.ts build && vite -c vite.test.config.ts preview --port ${ports.dashboard} --strictPort`,
command:
isCI || isProd ?
`corepack pnpm exec vite -c vite.test.config.ts build && vite -c vite.test.config.ts preview --port ${ports.dashboard} --strictPort`
: `NODE_ENV=test corepack pnpm exec vite -c vite.test.config.ts --port ${ports.dashboard}`,
timeout: 240 * 1000,
port: ports.dashboard,
reuseExistingServer: false,
Expand Down
6 changes: 4 additions & 2 deletions app/gui/src/dashboard/authentication/cognito.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export class Cognito {
localStorage.setItem(MOCK_EMAIL_KEY, username)
const result = await results.Result.wrapAsync(async () => {
listen.authEventListener?.(listen.AuthEvent.signIn)
await Promise.resolve()
return Promise.resolve(await this.userSession())
})
return result
.mapErr(original.intoAmplifyErrorOrThrow)
Expand All @@ -238,8 +238,10 @@ export class Cognito {

/** Sign out the current user. */
async signOut() {
listen.authEventListener?.(listen.AuthEvent.signOut)
this.isSignedIn = false
listen.authEventListener?.(listen.AuthEvent.signOut)
localStorage.removeItem(MOCK_EMAIL_KEY)
localStorage.removeItem(MOCK_ORGANIZATION_ID_KEY)
return Promise.resolve(null)
}

Expand Down
125 changes: 125 additions & 0 deletions app/gui/src/dashboard/components/Activity.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/**
* @file Activity component
*
* This component is used to suspend the rendering of a subtree until a promise is resolved.
*/
import { unsafeWriteValue } from '#/utilities/write'
import { Suspense, useEffect, useLayoutEffect, useRef, useState } from 'react'
import { useAwait } from './Await'

/**
* Props for {@link Activity}
*/
export interface ActivityProps {
/**
* The mode of the activity.
* - `active`: The subtree is active (default).
* - `inactive`: The activity of the subtree is paused.
* - `inactive-hidden`: The activity of the subtree is paused, and the subtree is hidden.
* @default 'active'
*/
readonly mode: 'active' | 'inactive-hidden' | 'inactive'
readonly children: React.ReactNode
}

/**
* A component that pauses all activity inside it's subtree.
*
* ---
* ## The component is EXPERIMENTAL, please use with caution.
* ---
*/
export function Activity(props: ActivityProps) {
const { mode, children } = props

const contentRef = useRef<HTMLDivElement>(null)

const [promise, setPromise] = useState<Promise<void> | null>(null)

const isActive = mode === 'active'
const fallback =
mode === 'inactive-hidden' ? null : <UnhideSuspendedTree contentRef={contentRef} />

useEffect(() => {
if (isActive) {
return
}

let resolve = () => {}

setPromise(
new Promise((res) => {
resolve = res
}),
)

return () => {
resolve()
setPromise(null)
}
}, [isActive])

return (
<div ref={contentRef} className="contents">
<Suspense fallback={fallback}>
<ActivityInner promise={promise}>{children}</ActivityInner>
</Suspense>
</div>
)
}

/**
* Props for {@link ActivityInner}
*/
interface ActivityInnerProps {
readonly children: React.ReactNode
readonly promise?: Promise<unknown> | null | undefined
}

/**
* A component that suspends the tree using promises.
* @param props - The props of the component.
* @returns The children of the component.
*/
function ActivityInner(props: ActivityInnerProps) {
const { promise, children } = props

// Suspend the subtree
useAwait(promise)

return children
}

/**
* Props for {@link UnhideSuspendedTree}
*/
interface UnhideSuspendedTreeProps {
readonly contentRef: React.RefObject<HTMLDivElement>
}

/**
* Hack, that unhides the suspended tree.
*/
function UnhideSuspendedTree(props: UnhideSuspendedTreeProps) {
const { contentRef } = props

useLayoutEffect(() => {
const element: HTMLDivElement | null = contentRef.current

if (element == null) {
return
}

const chidlren = element.childNodes

for (let i = 0; i < chidlren.length; i++) {
const child = chidlren[i]

if (child instanceof HTMLElement) {
unsafeWriteValue(child.style, 'display', '')
}
}
}, [contentRef])

return null
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ export function FormError(props: FormErrorProps) {
const icon = error.type === 'offline' ? Offline : null

return (
<Alert size={size} variant={finalVariant} rounded={rounded} icon={icon} {...alertProps}>
<Alert
key={error.message}
size={size}
variant={finalVariant}
rounded={rounded}
icon={icon}
{...alertProps}
>
<Text variant="body" truncate="3" color="primary" testId={testId}>
{error.message}
</Text>
Expand Down
27 changes: 25 additions & 2 deletions app/gui/src/dashboard/components/Await.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,34 @@ const PRIVATE_AWAIT_PROMISE_STATE = Symbol('PRIVATE_AWAIT_PROMISE_STATE_REF')
* This component throws the promise and trigger the Suspense boundary
* inside the {@link Await} component.
* @throws {Promise} - The promise that is being awaited by Suspense.
* @throws {unknown} - The error that is being thrown by the promise. Triggers error boundary inside the {@link Await} component.
*/
function AwaitInternal<PromiseType>(props: AwaitProps<PromiseType>) {
const { promise, children } = props

const data = useAwait(promise)

return typeof children === 'function' ? children(data) : children
}

export function useAwait(promise?: null): void
export function useAwait<PromiseType>(promise: Promise<PromiseType>): PromiseType
export function useAwait<PromiseType>(
promise?: Promise<PromiseType> | null,
): PromiseType | undefined

/**
* A hook that accepts a promise and triggers the Suspense boundary until the promise is resolved.
* @param promise - The promise to await.
* @throws {Promise} - The promise that is being awaited by Suspense
* @returns The data of the promise.
*/
export function useAwait<PromiseType>(
promise?: Promise<PromiseType> | null,
): PromiseType | undefined {
if (promise == null) {
return
}

/**
* Define the promise state on the promise.
*/
Expand Down Expand Up @@ -154,5 +177,5 @@ function AwaitInternal<PromiseType>(props: AwaitProps<PromiseType>) {
throw promiseState.error
}

return typeof children === 'function' ? children(promiseState.data) : children
return promiseState.data
}
95 changes: 95 additions & 0 deletions app/gui/src/dashboard/components/__tests__/Activity.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { render, screen } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { useEffect, useState } from 'react'
import { describe } from 'vitest'
import { Activity } from '../Activity'

describe('Activity', (it) => {
it('should render the children', ({ expect }) => {
render(
<Activity mode="active">
<div>Hello</div>
</Activity>,
)

expect(screen.getByText('Hello')).toBeInTheDocument()
})

it('should render children when inactive', ({ expect }) => {
render(
<Activity mode="inactive">
<div>Hello</div>
</Activity>,
)

expect(screen.getByText('Hello')).toBeInTheDocument()
})

it('should render children when inactive-hidden', ({ expect }) => {
render(
<Activity mode="inactive-hidden">
<div>Hello</div>
</Activity>,
)

expect(screen.getByText('Hello')).toBeInTheDocument()
})

it('should display children when inactive', ({ expect }) => {
render(
<Activity mode="inactive">
<div>Hello</div>
</Activity>,
)

expect(screen.getByText('Hello')).toBeVisible()
})

it('should not unmount children when inactive', async ({ expect }) => {
let count = 0

const Component = () => {
useEffect(
() => () => {
count++
},
[],
)

return <div>{count}</div>
}

function Container() {
const [mode, setMode] = useState<'active' | 'inactive'>('active')

return (
<div>
<button
onClick={() => {
setMode('inactive')
}}
>
Inactive
</button>
<button
onClick={() => {
setMode('active')
}}
>
Active
</button>
<Activity mode={mode}>
<Component />
</Activity>
</div>
)
}

render(<Container />)

await userEvent.click(screen.getByText('Inactive'))
await userEvent.click(screen.getByText('Active'))

expect(count).toBe(0)
})
})
Loading

0 comments on commit b1cc4f1

Please sign in to comment.