Skip to content

Commit

Permalink
Merge branch 'develop' into wip/sergeigarin/fix-concurrent-access
Browse files Browse the repository at this point in the history
# Conflicts:
#	app/gui/src/dashboard/components/AriaComponents/Form/components/FormError.tsx
  • Loading branch information
MrFlashAccount committed Dec 22, 2024
2 parents 59f5dd2 + 3932679 commit 58b753f
Show file tree
Hide file tree
Showing 29 changed files with 561 additions and 336 deletions.
1 change: 1 addition & 0 deletions app/common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"./src/backendQuery": "./src/backendQuery.ts",
"./src/queryClient": "./src/queryClient.ts",
"./src/utilities/data/array": "./src/utilities/data/array.ts",
"./src/utilities/errors": "./src/utilities/errors.ts",
"./src/utilities/data/dateTime": "./src/utilities/data/dateTime.ts",
"./src/utilities/data/newtype": "./src/utilities/data/newtype.ts",
"./src/utilities/data/object": "./src/utilities/data/object.ts",
Expand Down
5 changes: 4 additions & 1 deletion app/common/src/queryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export function createQueryClient<TStorageValue = string>(
storage: persisterStorage,
// Prefer online first and don't rely on the local cache if user is online
// fallback to the local cache only if the user is offline
maxAge: queryCore.onlineManager.isOnline() ? -1 : DEFAULT_QUERY_PERSIST_TIME_MS,
maxAge: DEFAULT_QUERY_PERSIST_TIME_MS,
buster: DEFAULT_BUSTER,
filters: { predicate: query => query.meta?.persist !== false },
prefix: 'enso:query-persist:',
Expand Down Expand Up @@ -130,6 +130,9 @@ export function createQueryClient<TStorageValue = string>(
defaultOptions: {
queries: {
...(persister != null ? { persister } : {}),
// Default set to 'always' to don't pause ongoing queries
// and make them fail.
networkMode: 'always',
refetchOnReconnect: 'always',
staleTime: DEFAULT_QUERY_STALE_TIME_MS,
retry: (failureCount, error: unknown) => {
Expand Down
14 changes: 11 additions & 3 deletions app/common/src/text/english.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"submit": "Submit",
"retry": "Retry",

"arbitraryFetchError": "An error occurred while fetching data",
"arbitraryFetchImageError": "An error occurred while fetching an image",

"createFolderError": "Could not create new folder",
"createProjectError": "Could not create new project",
"createDatalinkError": "Could not create new Datalink",
Expand All @@ -17,6 +21,9 @@
"deleteAssetError": "Could not delete '$0'",
"restoreAssetError": "Could not restore '$0'",

"refetchQueriesPending": "Getting latest updates...",
"refetchQueriesError": "Could not get latest updates. Some information may be outdated",

"localBackendDatalinkError": "Cannot create Datalinks on the local drive",
"localBackendSecretError": "Cannot create secrets on the local drive",
"offlineUploadFilesError": "Cannot upload files when offline",
Expand Down Expand Up @@ -177,6 +184,7 @@
"getCustomerPortalUrlBackendError": "Could not get customer portal URL",
"duplicateLabelError": "This label already exists.",
"emptyStringError": "This value must not be empty.",
"resolveProjectAssetPathBackendError": "Could not get asset",

"directoryAssetType": "folder",
"directoryDoesNotExistError": "Unable to find directory. Does it exist?",
Expand Down Expand Up @@ -480,9 +488,9 @@
"hidePassword": "Hide password",
"showPassword": "Show password",
"copiedToClipboard": "Copied to clipboard",
"noResultsFound": "No results found.",
"youAreOffline": "You are offline.",
"cannotCreateAssetsHere": "You do not have the permissions to create assets here.",
"noResultsFound": "No results found",
"youAreOffline": "You are offline",
"cannotCreateAssetsHere": "You do not have the permissions to create assets here",
"enableVersionChecker": "Enable Version Checker",
"enableVersionCheckerDescription": "Show a dialog if the current version of the desktop app does not match the latest version.",
"disableAnimations": "Disable animations",
Expand Down
52 changes: 52 additions & 0 deletions app/common/src/utilities/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* An error that occurs when a network request fails.
*
* This error is used to indicate that a network request failed due to a network error,
* such as a timeout or a connection error.
*/
export class NetworkError extends Error {
/**
* Create a new {@link NetworkError} with the specified message.
* @param message - The message to display when the error is thrown.
*/
constructor(message: string, options?: ErrorOptions) {
super(message, options)
this.name = 'NetworkError'
}
}

/**
* An error that occurs when the user is offline.
*
* This error is used to indicate that the user is offline, such as when they are
* not connected to the internet or when they are on an airplane.
*/
export class OfflineError extends Error {
/**
* Create a new {@link OfflineError} with the specified message.
* @param message - The message to display when the error is thrown.
*/
constructor(message: string = 'User is offline', options?: ErrorOptions) {
super(message, options)
this.name = 'OfflineError'
}
}

/**
* An error with a display message.
*
* This message can be shown to a user.
*/
export class ErrorWithDisplayMessage extends Error {
readonly displayMessage: string
/**
* Create a new {@link ErrorWithDisplayMessage} with the specified message and display message.
* @param message - The message to display when the error is thrown.
* @param options - The options to pass to the error.
*/
constructor(message: string, options: ErrorOptions & { displayMessage: string }) {
super(message, options)
this.name = 'ErrorWithDisplayMessage'
this.displayMessage = options.displayMessage
}
}
29 changes: 18 additions & 11 deletions app/gui/integration-test/dashboard/actions/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { FeatureFlags } from '#/providers/FeatureFlagsProvider'
import { readFileSync } from 'node:fs'
import { dirname, join } from 'node:path'
import { fileURLToPath } from 'node:url'
import invariant from 'tiny-invariant'

// =================
// === Constants ===
Expand Down Expand Up @@ -1190,17 +1191,23 @@ async function mockApiInternal({ page, setupAPI }: MockParams) {
})
})

await get(remoteBackendPaths.getProjectAssetPath(GLOB_PROJECT_ID, '*'), (route, request) => {
const maybeId = request.url().match(/[/]projects[/]([^?/]+)/)?.[1]
if (!maybeId) return
const projectId = backend.ProjectId(maybeId)
called('getProjectAsset', { projectId })
return route.fulfill({
// This is a mock SVG image. Just a square with a black background.
body: '/mock/svg.svg',
contentType: 'text/plain',
})
})
await get(
remoteBackendPaths.getProjectAssetPath(GLOB_PROJECT_ID, '*'),
async (route, request) => {
const maybeId = request.url().match(/[/]projects[/]([^?/]+)/)?.[1]

invariant(maybeId, 'Unable to parse the ID provided')

const projectId = backend.ProjectId(maybeId)

called('getProjectAsset', { projectId })

return route.fulfill({
// This is a mock SVG image. Just a square with a black background.
path: join(__dirname, '../mock/example.png'),
})
},
)

await page.route('mock/svg.svg', (route) => {
return route.fulfill({ body: MOCK_SVG, contentType: 'image/svg+xml' })
Expand Down
23 changes: 22 additions & 1 deletion app/gui/integration-test/dashboard/assetPanel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { EmailAddress, UserId } from '#/services/Backend'

import { PermissionAction } from '#/utilities/permissions'

import { mockAllAndLogin } from './actions'
import { mockAllAndLogin, TEXT } from './actions'

/** Find an asset panel. */
function locateAssetPanel(page: Page) {
Expand Down Expand Up @@ -87,4 +87,25 @@ test('Asset Panel documentation view', ({ page }) =>
await expect(assetPanel.getByTestId('asset-panel-tab-panel-docs')).toBeVisible()
await expect(assetPanel.getByTestId('asset-docs-content')).toBeVisible()
await expect(assetPanel.getByTestId('asset-docs-content')).toHaveText(/Project Goal/)
await expect(assetPanel.getByText(TEXT.arbitraryFetchImageError)).not.toBeVisible()
}))

test('Assets Panel docs images', ({ page }) => {
return mockAllAndLogin({
page,
setupAPI: (api) => {
api.addProject({})
},
})
.do(() => {})
.driveTable.clickRow(0)
.toggleDocsAssetPanel()
.withAssetPanel(async (assetPanel) => {
await expect(assetPanel.getByTestId('asset-docs-content')).toBeVisible()

for (const image of await assetPanel.getByRole('img').all()) {
await expect(image).toBeVisible()
await expect(image).toHaveJSProperty('complete', true)
}
})
})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
23 changes: 23 additions & 0 deletions app/gui/src/dashboard/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ import { STATIC_QUERY_OPTIONS } from '#/utilities/reactQuery'

import { useInitAuthService } from '#/authentication/service'
import { InvitedToOrganizationModal } from '#/modals/InvitedToOrganizationModal'
import { useMutation } from '@tanstack/react-query'
import { useOffline } from './hooks/offlineHooks'

// ============================
// === Global configuration ===
Expand Down Expand Up @@ -214,6 +216,9 @@ export default function App(props: AppProps) {
},
})

const { isOffline } = useOffline()
const { getText } = textProvider.useText()

const queryClient = props.queryClient

// Force all queries to be stale
Expand All @@ -235,6 +240,24 @@ export default function App(props: AppProps) {
refetchInterval: 2 * 60 * 1000,
})

const { mutate: executeBackgroundUpdate } = useMutation({
mutationKey: ['refetch-queries', { isOffline }],
scope: { id: 'refetch-queries' },
mutationFn: () => queryClient.refetchQueries({ type: 'all' }),
networkMode: 'online',
onError: () => {
toastify.toast.error(getText('refetchQueriesError'), {
position: 'bottom-right',
})
},
})

React.useEffect(() => {
if (!isOffline) {
executeBackgroundUpdate()
}
}, [executeBackgroundUpdate, isOffline])

// Both `BackendProvider` and `InputBindingsProvider` depend on `LocalStorageProvider`.
// Note that the `Router` must be the parent of the `AuthProvider`, because the `AuthProvider`
// will redirect the user between the login/register pages and the dashboard.
Expand Down
5 changes: 5 additions & 0 deletions app/gui/src/dashboard/assets/offline_filled.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions app/gui/src/dashboard/assets/offline_outline.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*
* Form error component.
*/
import Offline from '#/assets/offline_filled.svg'
import { Alert, Text, type AlertProps } from '#/components/AriaComponents'
import { useFormError, type UseFormErrorProps } from './useFormError'

Expand All @@ -11,7 +12,7 @@ export interface FormErrorProps extends Omit<AlertProps, 'children'>, UseFormErr

/** Form error component. */
export function FormError(props: FormErrorProps) {
const { size = 'large', variant = 'error', rounded = 'large', ...alertProps } = props
const { size = 'large', variant = 'error', rounded = 'xxlarge', ...alertProps } = props

const errors = useFormError(props)

Expand All @@ -24,9 +25,10 @@ export function FormError(props: FormErrorProps) {
{errors.map((error) => {
const testId = `form-submit-${error.type}`
const finalVariant = error.type === 'offline' ? 'outline' : variant
const icon = error.type === 'offline' ? Offline : null

return (
<Alert size={size} variant={finalVariant} rounded={rounded} {...alertProps}>
<Alert size={size} variant={finalVariant} rounded={rounded} icon={icon} {...alertProps}>
<Text variant="body" truncate="3" color="primary" testId={testId}>
{error.message}
</Text>
Expand Down
Loading

0 comments on commit 58b753f

Please sign in to comment.