From 8cbf38db27e028adcf63326b1b99e6cf2db28802 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Tue, 10 Dec 2024 15:41:40 +0400 Subject: [PATCH 01/21] Sort assets by modified date --- app/gui/src/dashboard/hooks/backendHooks.tsx | 1 + app/gui/src/dashboard/services/RemoteBackend.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/app/gui/src/dashboard/hooks/backendHooks.tsx b/app/gui/src/dashboard/hooks/backendHooks.tsx index fbed7fb15f63..0b78913f4a80 100644 --- a/app/gui/src/dashboard/hooks/backendHooks.tsx +++ b/app/gui/src/dashboard/hooks/backendHooks.tsx @@ -210,6 +210,7 @@ const INVALIDATION_MAP: Partial< deleteAsset: ['listDirectory', 'listAssetVersions'], undoDeleteAsset: ['listDirectory'], updateAsset: ['listDirectory', 'listAssetVersions'], + openProject: ['listDirectory'], closeProject: ['listDirectory', 'listAssetVersions'], } diff --git a/app/gui/src/dashboard/services/RemoteBackend.ts b/app/gui/src/dashboard/services/RemoteBackend.ts index e503bc7559f4..2f9b14d931f0 100644 --- a/app/gui/src/dashboard/services/RemoteBackend.ts +++ b/app/gui/src/dashboard/services/RemoteBackend.ts @@ -521,6 +521,7 @@ export default class RemoteBackend extends Backend { }), ) .map((asset) => this.dynamicAssetUser(asset)) + .sort(backend.compareAssets) return ret } } From 47c53656a44075d16ec093e6da0ef9a78bda2859 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Wed, 11 Dec 2024 17:33:05 +0400 Subject: [PATCH 02/21] Add error state to Editable span --- app/common/src/text/english.json | 2 + .../components/AriaComponents/Form/Form.tsx | 4 + .../AriaComponents/Form/components/Field.tsx | 4 +- .../Form/components/FormError.tsx | 87 +---- .../Form/components/FormProvider.tsx | 3 +- .../AriaComponents/Form/components/Reset.tsx | 4 +- .../AriaComponents/Form/components/index.ts | 1 + .../AriaComponents/Form/components/types.ts | 4 + .../AriaComponents/Form/components/useForm.ts | 1 + .../Form/components/useFormError.ts | 77 ++++ .../AriaComponents/Inputs/Input/Input.tsx | 30 +- .../AriaComponents/Inputs/variants.ts | 2 +- .../components/AriaComponents/Text/Text.tsx | 8 +- .../components/AriaComponents/Underlay.tsx | 18 + .../src/dashboard/components/EditableSpan.tsx | 344 +++++++++++------- .../dashboard/DatalinkNameColumn.tsx | 10 +- .../dashboard/DirectoryNameColumn.tsx | 10 +- .../components/dashboard/FileNameColumn.tsx | 15 +- .../dashboard/ProjectNameColumn.tsx | 27 +- .../components/dashboard/SecretNameColumn.tsx | 2 +- .../dashboard/column/DocsColumn.tsx | 2 +- .../dashboard/column/LabelsColumn.tsx | 2 +- .../dashboard/column/ModifiedColumn.tsx | 6 +- .../dashboard/column/SharedWithColumn.tsx | 2 +- app/gui/src/dashboard/hooks/backendHooks.tsx | 1 + app/gui/src/dashboard/layouts/AssetsTable.tsx | 3 +- app/gui/src/dashboard/tailwind.css | 2 +- 27 files changed, 405 insertions(+), 266 deletions(-) create mode 100644 app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts create mode 100644 app/gui/src/dashboard/components/AriaComponents/Underlay.tsx diff --git a/app/common/src/text/english.json b/app/common/src/text/english.json index a89c0135bc24..0396ff9380f4 100644 --- a/app/common/src/text/english.json +++ b/app/common/src/text/english.json @@ -52,6 +52,8 @@ "otherUserIsUsingProjectError": "Someone else is using this project", "localBackendNotDetectedError": "Could not detect the local backend", + "invalidInput": "Invalid input", + "invalidEmailValidationError": "Please enter a valid email address", "projectHasNoSourceFilesPhrase": "project has no source files", diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/Form.tsx b/app/gui/src/dashboard/components/AriaComponents/Form/Form.tsx index 2651eb2f4601..968674b6609b 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/Form.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Form/Form.tsx @@ -130,6 +130,7 @@ export const Form = forwardRef(function Form< Field: typeof components.Field FormError: typeof components.FormError FieldValue: typeof components.FieldValue + Provider: typeof components.FormProvider useFormSchema: typeof components.useFormSchema Controller: typeof components.Controller FIELD_STYLES: typeof components.FIELD_STYLES @@ -138,6 +139,7 @@ export const Form = forwardRef(function Form< useWatch: typeof components.useWatch useFieldRegister: typeof components.useFieldRegister useFieldState: typeof components.useFieldState + useFormError: typeof components.useFormError /* eslint-enable @typescript-eslint/naming-convention */ } @@ -153,7 +155,9 @@ Form.useFormContext = components.useFormContext Form.useOptionalFormContext = components.useOptionalFormContext Form.Field = components.Field Form.Controller = components.Controller +Form.Provider = components.FormProvider Form.useWatch = components.useWatch Form.FIELD_STYLES = components.FIELD_STYLES Form.useFieldRegister = components.useFieldRegister Form.useFieldState = components.useFieldState +Form.useFormError = components.useFormError diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx b/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx index 950b32c89696..b04f2f2756bd 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx @@ -39,7 +39,7 @@ export interface FieldChildrenRenderProps { readonly isTouched: boolean readonly isValidating: boolean readonly hasError: boolean - readonly error?: string | undefined + readonly error?: string | null | undefined } export const FIELD_STYLES = tv({ @@ -88,7 +88,7 @@ export const Field = forwardRef(function Field( const classes = variants({ fullWidth, isInvalid: invalid, isHidden }) - const hasError = (error ?? fieldState.error) != null + const hasError = (error !== undefined ? error : fieldState.error) != null return (
{ - // We do not need to know the form fields. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - readonly form?: types.FormInstance -} +export interface FormErrorProps extends Omit, UseFormErrorProps {} /** Form error component. */ export function FormError(props: FormErrorProps) { const { size = 'large', variant = 'error', rounded = 'large', ...alertProps } = props - const form = formContext.useFormContext(props.form) - const { formState } = form - const { errors } = formState - const { getText } = textProvider.useText() + const errors = useFormError(props) - /** Get the error message. */ - const getSubmitError = (): string | null => { - const formErrors = errors.root + if (errors.length === 0) { + return null + } - if (formErrors) { - const submitError = formErrors.submit + return ( +
+ {errors.map((error) => { + const testId = `form-submit-${error.type}` + const finalVariant = error.type === 'offline' ? 'outline' : variant - if (submitError) { return ( - submitError.message ?? - getText('arbitraryErrorTitle') + '. ' + getText('arbitraryErrorSubtitle') + + + {error.message} + + ) - } else { - return null - } - } else { - return null - } - } - - const offlineMessage = errors.root?.offline?.message ?? null - const errorMessage = getSubmitError() - - const submitErrorAlert = - errorMessage != null ? - - - {errorMessage} - - - : null - - const offlineErrorAlert = - offlineMessage != null ? - - - {offlineMessage} - - - : null - - const hasSomethingToShow = submitErrorAlert || offlineErrorAlert - - return hasSomethingToShow ? -
- {submitErrorAlert} {offlineErrorAlert} -
- : null + })} +
+ ) } diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx b/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx index 6fe3e215464a..dc724d3e1eaf 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/FormProvider.tsx @@ -3,7 +3,6 @@ * * Context that injects form instance into the component tree. */ -import type { PropsWithChildren } from 'react' import { createContext, useContext } from 'react' import invariant from 'tiny-invariant' import type * as types from './types' @@ -20,7 +19,7 @@ const FormContext = createContext | null>(null) /** Provides the form instance to the component tree. */ export function FormProvider( - props: FormContextType & PropsWithChildren, + props: FormContextType & { children: React.ReactNode }, ) { const { children, form } = props diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/Reset.tsx b/app/gui/src/dashboard/components/AriaComponents/Form/components/Reset.tsx index aeec4fe2b3ef..51a071b3b2ef 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/Reset.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/Reset.tsx @@ -28,6 +28,7 @@ export function Reset(props: ResetProps): React.JSX.Element { size = 'medium', testId = 'form-reset-button', children = getText('reset'), + onPress, ...buttonProps } = props @@ -41,10 +42,11 @@ export function Reset(props: ResetProps): React.JSX.Element { isDisabled={formState.isSubmitting || !formState.isDirty} testId={testId} children={children} - onPress={() => { + onPress={(event) => { // `type="reset"` triggers native HTML reset, which does not work here as it clears inputs // rather than resetting them to default values. form.reset() + return onPress?.(event) }} /* This is safe because we are passing all props to the button */ /* eslint-disable-next-line @typescript-eslint/no-explicit-any,no-restricted-syntax */ diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/index.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/index.ts index 390c988d5df4..af02ec474398 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/index.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/index.ts @@ -16,4 +16,5 @@ export * from './useField' export * from './useFieldRegister' export * from './useFieldState' export * from './useForm' +export * from './useFormError' export * from './useFormSchema' diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts index a8660cc12e89..b1c1197966cf 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts @@ -121,6 +121,10 @@ export interface UseFormReturn readonly schema: Schema readonly setFormError: (error: string) => void readonly closeRef: React.MutableRefObject<() => void> + readonly formProps: { + readonly onSubmit: (event?: FormEvent | null) => Promise + readonly noValidate: boolean + } } /** diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts index a3dca199d5e4..f5197e1b7d91 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/useForm.ts @@ -234,6 +234,7 @@ export function useForm( setFormError, handleSubmit: formInstance.handleSubmit, closeRef, + formProps: { onSubmit: submit, noValidate: true }, } return form diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts new file mode 100644 index 000000000000..854157eb9eb0 --- /dev/null +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts @@ -0,0 +1,77 @@ +/** + * @file + * + * Hook to get the error message from the form. + */ +import { useText } from '#/providers/TextProvider' +import { useFormContext } from './FormProvider' +import type { FormInstance } from './types' + +/** + * + */ +export interface UseFormErrorProps { + // We do not need to know the form fields. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + readonly form?: FormInstance +} + +/** + * Error type. + */ +interface Error { + readonly type: 'error' | 'offline' + readonly message: string +} + +/** + * Hook to get the error message from the form. + */ +export function useFormError(props: UseFormErrorProps) { + const form = useFormContext(props.form) + + const { formState } = form + const { errors } = formState + const { getText } = useText() + + /** Get the error message. */ + const getSubmitError = (): string | null => { + const formErrors = errors.root + + if (formErrors) { + const submitError = formErrors.submit + + if (submitError) { + return ( + submitError.message ?? + getText('arbitraryErrorTitle') + '. ' + getText('arbitraryErrorSubtitle') + ) + } else { + return null + } + } else { + return null + } + } + + const offlineMessage = errors.root?.offline?.message ?? null + const errorMessage = getSubmitError() + + const result: Error[] = [] + + if (offlineMessage != null) { + result.push({ + type: 'offline', + message: offlineMessage, + }) + } + + if (errorMessage != null) { + result.push({ + type: 'error', + message: errorMessage, + }) + } + + return result +} diff --git a/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx b/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx index fa60bc34ee8b..053ee09efcb7 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx @@ -39,7 +39,7 @@ export interface InputProps, 'disabled' | 'invalid'>, TestIdProps { - readonly className?: string + // readonly className?: string readonly style?: CSSProperties readonly inputRef?: Ref readonly addonStart?: ReactNode @@ -71,6 +71,7 @@ export const Input = forwardRef(function Input< fieldVariants, form: formRaw, autoFocus = false, + className, ...inputProps } = props const form = Form.useFormContext(formRaw) @@ -99,15 +100,29 @@ export const Input = forwardRef(function Input< }, }) + const invalid = inputProps.isInvalid ?? fieldProps.isInvalid + const disabled = fieldProps.disabled || formInstance.formState.isSubmitting + const classes = variants({ variant, size, rounded, - invalid: fieldProps.isInvalid, + invalid, readOnly: inputProps.readOnly, - disabled: fieldProps.disabled || formInstance.formState.isSubmitting, + disabled, }) + const computedClassName = (states: aria.InputRenderProps) => { + if (typeof className === 'function') { + return className({ + ...states, + defaultClassName: classes.textArea(), + }) + } else { + return className + } + } + useAutoFocus({ ref: privateInputRef, disabled: !autoFocus }) return ( @@ -134,7 +149,14 @@ export const Input = forwardRef(function Input<
()( - { className: classes.textArea(), type, name }, + { + className: (classNameStates) => + classes.textArea({ className: computedClassName(classNameStates) }), + type, + name, + // eslint-disable-next-line @typescript-eslint/naming-convention + 'aria-invalid': invalid, + }, omit(inputProps, 'isInvalid', 'isRequired', 'isDisabled'), omit(fieldProps, 'isInvalid', 'isRequired', 'isDisabled', 'invalid'), )} diff --git a/app/gui/src/dashboard/components/AriaComponents/Inputs/variants.ts b/app/gui/src/dashboard/components/AriaComponents/Inputs/variants.ts index f8b6d0950777..908b5bdf988a 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Inputs/variants.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Inputs/variants.ts @@ -7,7 +7,7 @@ import { tv } from '#/utilities/tailwindVariants' import { TEXT_STYLE } from '../Text' export const INPUT_STYLES = tv({ - base: 'block w-full overflow-hidden bg-transparent transition-[border-color,outline] duration-200', + base: 'block w-full bg-transparent transition-[border-color,outline] duration-200', variants: { // All variants SHOULD use objects, otherwise extending from them with e.g. `{ base: '' }` // results in `readOnly: { true: 'cursor-default [object Object]' }` for example. diff --git a/app/gui/src/dashboard/components/AriaComponents/Text/Text.tsx b/app/gui/src/dashboard/components/AriaComponents/Text/Text.tsx index c18da69f59c6..927d8333bf9f 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Text/Text.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Text/Text.tsx @@ -9,13 +9,15 @@ import * as twv from '#/utilities/tailwindVariants' import { useEventCallback } from '#/hooks/eventCallbackHooks' import { forwardRef } from '#/utilities/react' import { memo } from 'react' +import type { TestIdProps } from '../types' import * as textProvider from './TextProvider' import * as visualTooltip from './useVisualTooltip' /** Props for the Text component */ export interface TextProps extends Omit, - twv.VariantProps { + twv.VariantProps, + TestIdProps { readonly elementType?: keyof HTMLElementTagNameMap readonly lineClamp?: number readonly tooltip?: React.ReactElement | string | false | null @@ -81,7 +83,7 @@ export const TEXT_STYLE = twv.tv({ }, truncate: { /* eslint-disable @typescript-eslint/naming-convention */ - '1': 'truncate ellipsis', + '1': 'block truncate ellipsis', '2': 'line-clamp-2 ellipsis', '3': 'line-clamp-3 ellipsis', '4': 'line-clamp-4 ellipsis', @@ -142,6 +144,7 @@ export const Text = memo( children, color, balance, + testId, elementType: ElementType = 'span', tooltip: tooltipElement = children, tooltipDisplay = 'whenOverflowing', @@ -209,6 +212,7 @@ export const Text = memo( // eslint-disable-next-line @typescript-eslint/no-unsafe-argument mergeRefs.mergeRefs(ref, textElementRef)(el) }} + data-testid={testId} className={textClasses} {...aria.mergeProps>()( ariaProps, diff --git a/app/gui/src/dashboard/components/AriaComponents/Underlay.tsx b/app/gui/src/dashboard/components/AriaComponents/Underlay.tsx new file mode 100644 index 000000000000..57e562f0bfdb --- /dev/null +++ b/app/gui/src/dashboard/components/AriaComponents/Underlay.tsx @@ -0,0 +1,18 @@ +/** @file An underlay that covers the entire screen. */ +import { DIALOG_BACKGROUND } from '#/components/AriaComponents/Dialog' +import type { VariantProps } from '#/utilities/tailwindVariants' +import type { HTMLAttributes } from 'react' + +/** + * Props for the {@link Underlay} component. + */ +export interface UnderlayProps + extends HTMLAttributes, + VariantProps {} + +/** An underlay that covers the entire screen. */ +export function Underlay(props: UnderlayProps) { + const { className, variants = DIALOG_BACKGROUND, variant, ...rest } = props + + return
+} diff --git a/app/gui/src/dashboard/components/EditableSpan.tsx b/app/gui/src/dashboard/components/EditableSpan.tsx index 39e486abbc39..52d79f663e7f 100644 --- a/app/gui/src/dashboard/components/EditableSpan.tsx +++ b/app/gui/src/dashboard/components/EditableSpan.tsx @@ -4,176 +4,258 @@ import * as React from 'react' import CrossIcon from '#/assets/cross.svg' import TickIcon from '#/assets/tick.svg' -import * as eventCallback from '#/hooks/eventCallbackHooks' - -import * as inputBindingsProvider from '#/providers/InputBindingsProvider' +import { Input, Text } from '#/components/AriaComponents' import * as textProvider from '#/providers/TextProvider' - -import * as aria from '#/components/aria' -import * as ariaComponents from '#/components/AriaComponents' - -import * as eventModule from '#/utilities/event' -import * as sanitizedEventTargets from '#/utilities/sanitizedEventTargets' import * as tailwindMerge from '#/utilities/tailwindMerge' +import { useInteractOutside } from '#/components/aria' +import { Form } from '#/components/AriaComponents' import { useAutoFocus } from '#/hooks/autoFocusHooks' -import { useSyncRef } from '#/hooks/syncRefHooks' - -// ================= -// === Constants === -// ================= +import { AnimatePresence, motion, type Variants } from 'framer-motion' +import { useLayoutEffect } from 'react' +import { useMeasure } from '../hooks/measureHooks' +import { Underlay } from './AriaComponents/Underlay' -const WIDTH_CLASS_NAME = 'max-w-60' - -// ==================== -// === EditableSpan === -// ==================== - -/** Props for an {@link EditableSpan}. */ +/** + * Props for {@link EditableSpan}. + */ export interface EditableSpanProps { readonly 'data-testid'?: string readonly className?: string readonly editable?: boolean readonly checkSubmittable?: (value: string) => boolean - readonly onSubmit: (value: string) => void + readonly onSubmit: (value: string) => Promise readonly onCancel: () => void - readonly inputPattern?: string - readonly inputTitle?: string readonly children: string } /** A `` that can turn into an ``. */ export default function EditableSpan(props: EditableSpanProps) { const { className = '', editable = false, children } = props - const { checkSubmittable, onSubmit, onCancel, inputPattern, inputTitle } = props + + if (editable) { + return + } + + return ( + + {children} + + ) +} + +/** + * Props for {@link EditForm}. + */ +interface EditFormProps extends EditableSpanProps {} + +const CONTAINER_VARIANTS: Variants = { + hidden: { + opacity: 0, + transition: { + staggerChildren: 1, + }, + }, + visible: { + opacity: 1, + transition: { + staggerChildren: 1, + }, + }, +} + +const CHILD_VARIANTS: Variants = { + hidden: { opacity: 0, x: 5 }, + visible: { opacity: 1, x: 0 }, +} + +// eslint-disable-next-line @typescript-eslint/no-magic-numbers +const TRANSITION_OPTIONS = { stiffness: 300, damping: 150, mass: 1 } + +/** + * Edit form for {@link EditableSpan}. + */ +function EditForm(props: EditFormProps) { + const { className = '', children, checkSubmittable, onSubmit, onCancel } = props + const { getText } = textProvider.useText() - const inputBindings = inputBindingsProvider.useInputBindings() - const [isSubmittable, setIsSubmittable] = React.useState(false) const formRef = React.useRef(null) const inputRef = React.useRef(null) - const cancelledRef = React.useRef(false) - const checkSubmittableRef = useSyncRef(checkSubmittable) + const form = Form.useForm({ + schema: (z) => + z.object({ + value: z + .string() + .min(1) + .trim() + .refine((value) => checkSubmittable?.(value)), + }), + defaultValues: { value: children }, + onSubmit: ({ value }) => onSubmit(value), + }) + + useInteractOutside({ ref: formRef, onInteractOutside: onCancel }) + useAutoFocus({ ref: inputRef }) - // Make sure that the event callback is stable to prevent the effect from re-running. - const onCancelEventCallback = eventCallback.useEventCallback(onCancel) + const { error } = Form.useFieldState({ name: 'value', form }) + const formErrors = Form.useFormError({ form }) - React.useEffect(() => { - if (editable) { - setIsSubmittable(checkSubmittableRef.current?.(inputRef.current?.value ?? '') ?? true) + const errorMessage = (() => { + if (error != null) { + return error } - }, [checkSubmittableRef, editable]) - - React.useEffect(() => { - if (editable) { - return inputBindings.attach(sanitizedEventTargets.document.body, 'keydown', { - cancelEditName: () => { - onCancelEventCallback() - cancelledRef.current = true - inputRef.current?.blur() - }, - }) - } else { - return + + if (formErrors.length > 0) { + return formErrors + .filter(({ type }) => type === 'error') + .map(({ message }) => message) + .join('\n') } - }, [editable, inputBindings, onCancelEventCallback]) - React.useEffect(() => { - cancelledRef.current = false - }, [editable]) + return null + })() - aria.useInteractOutside({ - ref: formRef, - isDisabled: !editable, - onInteractOutside: () => { - onCancel() - }, - }) - - useAutoFocus({ ref: inputRef, disabled: !editable }) + const hasError = errorMessage != null - if (editable) { - return ( -
{ - event.preventDefault() - if (inputRef.current != null) { - if (isSubmittable) { - onSubmit(inputRef.current.value) - } else { - onCancel() - } - } - }} - > -
- + +
+ { event.stopPropagation() }} onKeyDown={(event) => { - if (event.key !== 'Escape') { - event.stopPropagation() + if (event.key === 'Escape') { + event.preventDefault() + form.reset() + onCancel() } + + event.stopPropagation() }} - {...(inputPattern == null ? {} : { pattern: inputPattern })} - {...(inputTitle == null ? {} : { title: inputTitle })} - {...(checkSubmittable == null ? - {} - : { - onInput: (event) => { - setIsSubmittable(checkSubmittable(event.currentTarget.value)) - }, - })} /> - - {isSubmittable && ( - + + {hasError && } + + + {form.formState.isDirty && ( + + + + + + + + + )} - { - cancelledRef.current = true - onCancel() - window.setTimeout(() => { - cancelledRef.current = false - }) - }} - /> - + +
+ + + ) +} + +/** + * Props for {@link ErrorMessage}. + */ +interface ErrorMessageProps { + readonly message: string + readonly formRef: React.RefObject +} + +/** + * Error message for {@link EditableSpan}. + */ +function ErrorMessage(props: ErrorMessageProps) { + const { message, formRef } = props + + const [measureFormRef, formRect] = useMeasure({ useRAF: false }) + + const offset = 12 + const crossOffset = 36 + + // eslint-disable-next-line @typescript-eslint/no-magic-numbers + const outlineWidth = crossOffset + 10 + + useLayoutEffect(() => { + measureFormRef(formRef.current) + }, [measureFormRef, formRef]) + + return ( + <> + {formRect && ( +
+
- - ) - } else { - return ( - - {children} - - ) - } + + + {message} + + +
+ +
+ + ) } diff --git a/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx index 6da620af6452..822b59fff73b 100644 --- a/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx @@ -44,7 +44,7 @@ export default function DatalinkNameColumn(props: DatalinkNameColumnProps) { return (
{ @@ -64,12 +64,10 @@ export default function DatalinkNameColumn(props: DatalinkNameColumnProps) { { - setIsEditing(false) + onSubmit={async () => { + await doRename() - if (newTitle !== item.title) { - await doRename() - } + setIsEditing(false) }} onCancel={() => { setIsEditing(false) diff --git a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx index 97523e49a5c4..9b9d5828ed5e 100644 --- a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx @@ -61,18 +61,14 @@ export default function DirectoryNameColumn(props: DirectoryNameColumnProps) { } const doRename = async (newTitle: string) => { - if (isEditable) { - setIsEditing(false) - if (!string.isWhitespaceOnly(newTitle) && newTitle !== item.title) { - await updateDirectoryMutation.mutateAsync([item.id, { title: newTitle }, item.title]) - } - } + await updateDirectoryMutation.mutateAsync([item.id, { title: newTitle }, item.title]) + setIsEditing(false) } return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx index 249c7f971f02..a578fbd4306c 100644 --- a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx @@ -43,24 +43,15 @@ export default function FileNameColumn(props: FileNameColumnProps) { } } - // TODO[sb]: Wait for backend implementation. `editable` should also be re-enabled, and the - // context menu entry should be re-added. - // Backend implementation is tracked here: https://github.com/enso-org/cloud-v2/issues/505. const doRename = async (newTitle: string) => { - if (isEditable) { - setIsEditing(false) - if (string.isWhitespaceOnly(newTitle)) { - // Do nothing. - } else if (!isCloud && newTitle !== item.title) { - await updateFileMutation.mutateAsync([item.id, { title: newTitle }, item.title]) - } - } + await updateFileMutation.mutateAsync([item.id, { title: newTitle }, item.title]) + setIsEditing(false) } return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx index a6b73369556b..3b9ce3f6644f 100644 --- a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx @@ -18,9 +18,7 @@ import * as eventModule from '#/utilities/event' import * as indent from '#/utilities/indent' import * as object from '#/utilities/object' import * as permissions from '#/utilities/permissions' -import * as string from '#/utilities/string' import * as tailwindMerge from '#/utilities/tailwindMerge' -import * as validation from '#/utilities/validation' import { isOnMacOS } from 'enso-common/src/detect' // =================== @@ -53,7 +51,7 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { const { backend, nodeMap } = state const { user } = authProvider.useFullUserSession() - const { getText } = textProvider.useText() + const driveStore = useDriveStore() const doOpenProject = projectHooks.useOpenProject() const ownPermission = permissions.tryFindSelfPermission(user, item.permissions) @@ -80,23 +78,18 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { } const doRename = async (newTitle: string) => { + await updateProjectMutation.mutateAsync([ + item.id, + { ami: null, ideVersion: null, projectName: newTitle }, + item.title, + ]) setIsEditing(false) - - if (string.isWhitespaceOnly(newTitle)) { - // Do nothing. - } else if (newTitle !== item.title) { - await updateProjectMutation.mutateAsync([ - item.id, - { ami: null, ideVersion: null, projectName: newTitle }, - item.title, - ]) - } } return (
{ @@ -151,12 +144,6 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { onCancel={() => { setIsEditing(false) }} - {...(backend.type === backendModule.BackendType.local ? - { - inputPattern: validation.LOCAL_PROJECT_NAME_PATTERN, - inputTitle: getText('projectNameCannotBeEmpty'), - } - : {})} > {item.title} diff --git a/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx index 3f4dc2352fba..8374d1e03639 100644 --- a/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx @@ -52,7 +52,7 @@ export default function SecretNameColumn(props: SecretNameColumnProps) { return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/column/DocsColumn.tsx b/app/gui/src/dashboard/components/dashboard/column/DocsColumn.tsx index 5c1ca6de7b94..af4e5b85b7bc 100644 --- a/app/gui/src/dashboard/components/dashboard/column/DocsColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/column/DocsColumn.tsx @@ -8,7 +8,7 @@ export default function DocsColumn(props: column.AssetColumnProps) { const { item } = props return ( -
+
{item.description}
) diff --git a/app/gui/src/dashboard/components/dashboard/column/LabelsColumn.tsx b/app/gui/src/dashboard/components/dashboard/column/LabelsColumn.tsx index ff1c8bb4be0b..4f35aa27dc3d 100644 --- a/app/gui/src/dashboard/components/dashboard/column/LabelsColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/column/LabelsColumn.tsx @@ -45,7 +45,7 @@ export default function LabelsColumn(props: column.AssetColumnProps) { self?.permission === permissions.PermissionAction.admin) return ( -
+
{(item.labels ?? []) .filter((label) => labelsByName.has(label)) .map((label) => ( diff --git a/app/gui/src/dashboard/components/dashboard/column/ModifiedColumn.tsx b/app/gui/src/dashboard/components/dashboard/column/ModifiedColumn.tsx index 3a6b86a5a25c..c282e9ffd853 100644 --- a/app/gui/src/dashboard/components/dashboard/column/ModifiedColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/column/ModifiedColumn.tsx @@ -7,9 +7,5 @@ import { formatDateTime } from '#/utilities/dateTime' export default function ModifiedColumn(props: AssetColumnProps) { const { item } = props - return ( - - {formatDateTime(new Date(item.modifiedAt))} - - ) + return {formatDateTime(new Date(item.modifiedAt))} } diff --git a/app/gui/src/dashboard/components/dashboard/column/SharedWithColumn.tsx b/app/gui/src/dashboard/components/dashboard/column/SharedWithColumn.tsx index f04f2747e77a..d7611d32f747 100644 --- a/app/gui/src/dashboard/components/dashboard/column/SharedWithColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/column/SharedWithColumn.tsx @@ -48,7 +48,7 @@ export default function SharedWithColumn(props: SharedWithColumnPropsInternal) { (self?.permission === PermissionAction.own || self?.permission === PermissionAction.admin) return ( -
+
{(category.type === 'trash' ? assetPermissions.filter((permission) => permission.permission === PermissionAction.own) : assetPermissions diff --git a/app/gui/src/dashboard/hooks/backendHooks.tsx b/app/gui/src/dashboard/hooks/backendHooks.tsx index 0b78913f4a80..3b2cd55f775a 100644 --- a/app/gui/src/dashboard/hooks/backendHooks.tsx +++ b/app/gui/src/dashboard/hooks/backendHooks.tsx @@ -203,6 +203,7 @@ const INVALIDATION_MAP: Partial< createSecret: ['listDirectory'], updateSecret: ['listDirectory'], updateProject: ['listDirectory'], + updateFile: ['listDirectory'], updateDirectory: ['listDirectory'], createDatalink: ['listDirectory', 'getDatalink'], uploadFileEnd: ['listDirectory'], diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx index ffb6d2e0d592..6373170e5c21 100644 --- a/app/gui/src/dashboard/layouts/AssetsTable.tsx +++ b/app/gui/src/dashboard/layouts/AssetsTable.tsx @@ -1769,7 +1769,7 @@ function AssetsTable(props: AssetsTableProps) { const table = (
{ if (isAssetContextMenuVisible) { event.preventDefault() @@ -1809,6 +1809,7 @@ function AssetsTable(props: AssetsTableProps) { {headerRow} + {itemRows}
diff --git a/app/gui/src/dashboard/tailwind.css b/app/gui/src/dashboard/tailwind.css index 6c5fa304afad..ecc6078da9ee 100644 --- a/app/gui/src/dashboard/tailwind.css +++ b/app/gui/src/dashboard/tailwind.css @@ -32,7 +32,7 @@ --top-bar-height: 3rem; --row-height: 2rem; - --table-row-height: 2.3125rem; + --table-row-height: 2.25rem; --page-padding-x: 0.8125rem; /* The height of the main heading of a page. */ --heading-height: 2.375rem; From 2eaac134127ef07e320a6e1029384b3103516404 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Wed, 11 Dec 2024 18:38:42 +0400 Subject: [PATCH 03/21] Check for uniqness and display error accordingly --- app/common/src/services/Backend.ts | 30 +++++++++----- app/common/src/text/english.json | 3 +- .../AriaComponents/Form/components/schema.ts | 15 ++++--- .../AriaComponents/Form/components/types.ts | 14 ++++++- .../AriaComponents/Inputs/Input/Input.tsx | 3 +- .../components/AriaComponents/index.ts | 1 + .../src/dashboard/components/EditableSpan.tsx | 41 ++++++++++--------- .../dashboard/DatalinkNameColumn.tsx | 1 - .../dashboard/DirectoryNameColumn.tsx | 23 +++++++---- .../components/dashboard/FileNameColumn.tsx | 21 ++++++---- .../dashboard/ProjectNameColumn.tsx | 19 +++++---- 11 files changed, 108 insertions(+), 63 deletions(-) diff --git a/app/common/src/services/Backend.ts b/app/common/src/services/Backend.ts index ae80d71aa390..d360d08157f8 100644 --- a/app/common/src/services/Backend.ts +++ b/app/common/src/services/Backend.ts @@ -1540,18 +1540,28 @@ export function isNewTitleValid( item: AnyAsset, newTitle: string, siblings?: readonly AnyAsset[] | null, +) { + return newTitle !== '' && newTitle !== item.title && isNewTitleUnique(item, newTitle, siblings) +} + +/** + * Check whether a new title is unique among the siblings. + */ +export function isNewTitleUnique( + item: AnyAsset, + newTitle: string, + siblings?: readonly AnyAsset[] | null, ) { siblings ??= [] - return ( - newTitle !== '' && - newTitle !== item.title && - siblings.every(sibling => { - const isSelf = sibling.id === item.id - const hasSameType = sibling.type === item.type - const hasSameTitle = sibling.title === newTitle - return !(!isSelf && hasSameType && hasSameTitle) - }) - ) + + console.log('isNewTitleUnique()', { siblings, item, newTitle }) + + return siblings.every(sibling => { + const isSelf = sibling.id === item.id + const hasSameType = sibling.type === item.type + const hasSameTitle = sibling.title.toLowerCase() === newTitle.toLowerCase() + return !(!isSelf && hasSameType && hasSameTitle) + }) } /** Network error class. */ diff --git a/app/common/src/text/english.json b/app/common/src/text/english.json index 0396ff9380f4..cd6307e5271f 100644 --- a/app/common/src/text/english.json +++ b/app/common/src/text/english.json @@ -53,7 +53,8 @@ "localBackendNotDetectedError": "Could not detect the local backend", "invalidInput": "Invalid input", - + "nameShouldBeUnique": "Name must be unique", + "nameShouldNotContainInvalidCharacters": "Name should not contain invalid characters", "invalidEmailValidationError": "Please enter a valid email address", "projectHasNoSourceFilesPhrase": "project has no source files", diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/schema.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/schema.ts index 62fcb88c7a68..a3eb3ef6eb3d 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/schema.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/schema.ts @@ -1,7 +1,10 @@ -/** - * @file - * - * Create a schema for a form - */ +/** @file Create a schema for a form */ +import { z } from 'zod' +import type { SchemaCallback, TSchema } from './types' -export * as schema from 'zod' +/** Factory function to create a schema. */ +export function createSchema(callback: SchemaCallback) { + return callback(z) +} + +export { z as schema } from 'zod' diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts index b1c1197966cf..fc9fe23c35c0 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/types.ts @@ -30,6 +30,12 @@ export type TSchema = | z.ZodEffects | z.ZodEffects> +/** A callback that returns a schema. */ +export type SchemaCallback = (z: SchemaBuilder) => Schema + +/** The schema builder. */ +export type SchemaBuilder = typeof schemaModule.schema + /** OnSubmitCallbacks type. */ export interface OnSubmitCallbacks { readonly onSubmit?: @@ -70,7 +76,7 @@ export interface UseFormOptions 'handleSubmit' | 'resetOptions' | 'resolver' >, OnSubmitCallbacks { - readonly schema: Schema | ((schema: typeof schemaModule.schema) => Schema) + readonly schema: Schema | SchemaCallback /** * Whether the form can submit offline. * @default false @@ -170,6 +176,12 @@ export type FormInstanceValidated< // eslint-disable-next-line @typescript-eslint/no-explicit-any > = FormInstance | (any[] & NonNullable) +/** + * Form instance with unknown schema. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type AnyFormInstance = FormInstance + /** Props for the Field component. */ // Readonly omitted here to avoid type mismatch with native HTML attributes // eslint-disable-next-line no-restricted-syntax diff --git a/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx b/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx index 053ee09efcb7..57ed957788ed 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Inputs/Input/Input.tsx @@ -72,10 +72,11 @@ export const Input = forwardRef(function Input< form: formRaw, autoFocus = false, className, + testId: testIdRaw, ...inputProps } = props const form = Form.useFormContext(formRaw) - const testId = props.testId ?? props['data-testid'] + const testId = testIdRaw ?? props['data-testid'] const privateInputRef = useRef(null) const { fieldProps, formInstance } = Form.useFieldRegister< diff --git a/app/gui/src/dashboard/components/AriaComponents/index.ts b/app/gui/src/dashboard/components/AriaComponents/index.ts index cf9584182be2..4af82b44f4c9 100644 --- a/app/gui/src/dashboard/components/AriaComponents/index.ts +++ b/app/gui/src/dashboard/components/AriaComponents/index.ts @@ -15,4 +15,5 @@ export * from './Switch' export * from './Text' export * from './Tooltip' export * from './types' +export * from './Underlay' export * from './VisuallyHidden' diff --git a/app/gui/src/dashboard/components/EditableSpan.tsx b/app/gui/src/dashboard/components/EditableSpan.tsx index 52d79f663e7f..df41ebe7416a 100644 --- a/app/gui/src/dashboard/components/EditableSpan.tsx +++ b/app/gui/src/dashboard/components/EditableSpan.tsx @@ -9,12 +9,12 @@ import * as textProvider from '#/providers/TextProvider' import * as tailwindMerge from '#/utilities/tailwindMerge' import { useInteractOutside } from '#/components/aria' -import { Form } from '#/components/AriaComponents' +import { Form, Underlay } from '#/components/AriaComponents' import { useAutoFocus } from '#/hooks/autoFocusHooks' +import { useMeasure } from '#/hooks/measureHooks' import { AnimatePresence, motion, type Variants } from 'framer-motion' import { useLayoutEffect } from 'react' -import { useMeasure } from '../hooks/measureHooks' -import { Underlay } from './AriaComponents/Underlay' +import type { z } from 'zod' /** * Props for {@link EditableSpan}. @@ -23,10 +23,13 @@ export interface EditableSpanProps { readonly 'data-testid'?: string readonly className?: string readonly editable?: boolean - readonly checkSubmittable?: (value: string) => boolean readonly onSubmit: (value: string) => Promise readonly onCancel: () => void readonly children: string + /** + * Additional schema to validate the value. + */ + readonly schema?: (schema: typeof z) => z.ZodType } /** A `` that can turn into an ``. */ @@ -76,7 +79,7 @@ const TRANSITION_OPTIONS = { stiffness: 300, damping: 150, mass: 1 } * Edit form for {@link EditableSpan}. */ function EditForm(props: EditFormProps) { - const { className = '', children, checkSubmittable, onSubmit, onCancel } = props + const { className = '', children, onSubmit, onCancel, schema } = props const { getText } = textProvider.useText() @@ -84,14 +87,15 @@ function EditForm(props: EditFormProps) { const inputRef = React.useRef(null) const form = Form.useForm({ - schema: (z) => - z.object({ - value: z - .string() - .min(1) - .trim() - .refine((value) => checkSubmittable?.(value)), - }), + schema: (z) => { + const baseSchema = z.object({ value: z.string().min(1).trim() }) + + if (schema != null) { + return baseSchema.merge(z.object({ value: schema(z) })) + } + + return baseSchema + }, defaultValues: { value: children }, onSubmit: ({ value }) => onSubmit(value), }) @@ -124,28 +128,25 @@ function EditForm(props: EditFormProps) {
{ event.stopPropagation() }} onKeyDown={(event) => { if (event.key === 'Escape') { event.preventDefault() - form.reset() onCancel() } - event.stopPropagation() }} /> @@ -170,7 +171,7 @@ function EditForm(props: EditFormProps) { > { await doRename() - setIsEditing(false) }} onCancel={() => { diff --git a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx index 9b9d5828ed5e..5498a9dd2113 100644 --- a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx @@ -20,7 +20,6 @@ import { useStore } from '#/hooks/storeHooks' import * as eventModule from '#/utilities/event' import * as indent from '#/utilities/indent' import * as object from '#/utilities/object' -import * as string from '#/utilities/string' import * as tailwindMerge from '#/utilities/tailwindMerge' import * as validation from '#/utilities/validation' @@ -109,13 +108,21 @@ export default function DirectoryNameColumn(props: DirectoryNameColumnProps) { 'cursor-pointer bg-transparent font-naming', rowState.isEditingName ? 'cursor-text' : 'cursor-pointer', )} - checkSubmittable={(newTitle) => - validation.DIRECTORY_NAME_REGEX.test(newTitle) && - backendModule.isNewTitleValid( - item, - newTitle, - nodeMap.current.get(item.parentId)?.children?.map((child) => child.item), - ) + schema={(z) => + z + .string() + .regex(validation.DIRECTORY_NAME_REGEX, { + message: getText('nameShouldNotContainInvalidCharacters'), + }) + .refine( + (value) => + backendModule.isNewTitleUnique( + item, + value, + nodeMap.current.get(item.parentId)?.children?.map((child) => child.item), + ), + { message: getText('nameShouldBeUnique') }, + ) } onSubmit={doRename} onCancel={() => { diff --git a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx index a578fbd4306c..ab05fdb8c2d9 100644 --- a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx @@ -9,11 +9,11 @@ import SvgMask from '#/components/SvgMask' import * as backendModule from '#/services/Backend' +import { useText } from '#/providers/TextProvider' import * as eventModule from '#/utilities/event' import * as fileIcon from '#/utilities/fileIcon' import * as indent from '#/utilities/indent' import * as object from '#/utilities/object' -import * as string from '#/utilities/string' import * as tailwindMerge from '#/utilities/tailwindMerge' // ================ @@ -35,6 +35,7 @@ export default function FileNameColumn(props: FileNameColumnProps) { const { backend, nodeMap } = state const isCloud = backend.type === backendModule.BackendType.remote + const { getText } = useText() const updateFileMutation = useMutation(backendMutationOptions(backend, 'updateFile')) const setIsEditing = (isEditingName: boolean) => { @@ -72,17 +73,21 @@ export default function FileNameColumn(props: FileNameColumnProps) { data-testid="asset-row-name" editable={rowState.isEditingName} className="grow bg-transparent font-naming" - checkSubmittable={(newTitle) => - backendModule.isNewTitleValid( - item, - newTitle, - nodeMap.current.get(item.parentId)?.children?.map((child) => child.item), - ) - } onSubmit={doRename} onCancel={() => { setIsEditing(false) }} + schema={(z) => + z.string().refine( + (value) => + backendModule.isNewTitleUnique( + item, + value, + nodeMap.current.get(item.parentId)?.children?.map((child) => child.item), + ), + { message: getText('nameShouldBeUnique') }, + ) + } > {item.title} diff --git a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx index 3b9ce3f6644f..5f67b649c094 100644 --- a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx @@ -51,6 +51,7 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { const { backend, nodeMap } = state const { user } = authProvider.useFullUserSession() + const { getText } = textProvider.useText() const driveStore = useDriveStore() const doOpenProject = projectHooks.useOpenProject() @@ -133,17 +134,21 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { canExecute && !isOtherUserUsingProject && 'cursor-pointer', rowState.isEditingName && 'cursor-text', )} - checkSubmittable={(newTitle) => - backendModule.isNewTitleValid( - item, - newTitle, - nodeMap.current.get(item.parentId)?.children?.map((child) => child.item), - ) - } onSubmit={doRename} onCancel={() => { setIsEditing(false) }} + schema={(z) => + z.string().refine( + (value) => + backendModule.isNewTitleUnique( + item, + value, + nodeMap.current.get(item.parentId)?.children?.map((child) => child.item), + ), + { message: getText('nameShouldBeUnique') }, + ) + } > {item.title} From ab17135f00bab369c115cbc40e0b4759368e0e4a Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Wed, 11 Dec 2024 19:57:51 +0400 Subject: [PATCH 04/21] Fix tiny issues --- app/common/src/services/Backend.ts | 10 +++++----- app/gui/src/dashboard/components/EditableSpan.tsx | 15 ++++++++------- .../components/dashboard/DirectoryNameColumn.tsx | 3 +-- .../components/dashboard/FileNameColumn.tsx | 2 +- .../components/dashboard/ProjectNameColumn.tsx | 2 +- app/gui/src/dashboard/layouts/AssetsTable.tsx | 9 ++++++--- .../utilities/__tests__/validation.test.ts | 11 +++++------ app/gui/src/dashboard/utilities/validation.ts | 11 +++++++++++ 8 files changed, 38 insertions(+), 25 deletions(-) diff --git a/app/common/src/services/Backend.ts b/app/common/src/services/Backend.ts index d360d08157f8..7c6cecaf260a 100644 --- a/app/common/src/services/Backend.ts +++ b/app/common/src/services/Backend.ts @@ -1554,13 +1554,13 @@ export function isNewTitleUnique( ) { siblings ??= [] - console.log('isNewTitleUnique()', { siblings, item, newTitle }) - return siblings.every(sibling => { - const isSelf = sibling.id === item.id - const hasSameType = sibling.type === item.type + if (sibling.id === item.id) { + return true + } + const hasSameTitle = sibling.title.toLowerCase() === newTitle.toLowerCase() - return !(!isSelf && hasSameType && hasSameTitle) + return !hasSameTitle }) } diff --git a/app/gui/src/dashboard/components/EditableSpan.tsx b/app/gui/src/dashboard/components/EditableSpan.tsx index df41ebe7416a..ceb9d834e5a7 100644 --- a/app/gui/src/dashboard/components/EditableSpan.tsx +++ b/app/gui/src/dashboard/components/EditableSpan.tsx @@ -29,7 +29,7 @@ export interface EditableSpanProps { /** * Additional schema to validate the value. */ - readonly schema?: (schema: typeof z) => z.ZodType + readonly schema?: (schema: z.ZodType) => z.ZodType } /** A `` that can turn into an ``. */ @@ -88,10 +88,11 @@ function EditForm(props: EditFormProps) { const form = Form.useForm({ schema: (z) => { - const baseSchema = z.object({ value: z.string().min(1).trim() }) + const baseValueSchema = z.string().min(1).trim() + const baseSchema = z.object({ value: baseValueSchema }) if (schema != null) { - return baseSchema.merge(z.object({ value: schema(z) })) + return baseSchema.merge(z.object({ value: schema(baseValueSchema) })) } return baseSchema @@ -124,7 +125,7 @@ function EditForm(props: EditFormProps) { const hasError = errorMessage != null return ( -
+
{formRect && (
- + {message} -
+
diff --git a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx index 5498a9dd2113..a1cc6ff1e687 100644 --- a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx @@ -110,8 +110,7 @@ export default function DirectoryNameColumn(props: DirectoryNameColumnProps) { )} schema={(z) => z - .string() - .regex(validation.DIRECTORY_NAME_REGEX, { + .refine((value) => !validation.isDirectoryNameContainInvalidCharacters(value), { message: getText('nameShouldNotContainInvalidCharacters'), }) .refine( diff --git a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx index ab05fdb8c2d9..9d6ae991a923 100644 --- a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx @@ -78,7 +78,7 @@ export default function FileNameColumn(props: FileNameColumnProps) { setIsEditing(false) }} schema={(z) => - z.string().refine( + z.refine( (value) => backendModule.isNewTitleUnique( item, diff --git a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx index 5f67b649c094..4ec1ec770690 100644 --- a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx @@ -139,7 +139,7 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { setIsEditing(false) }} schema={(z) => - z.string().refine( + z.refine( (value) => backendModule.isNewTitleUnique( item, diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx index 6373170e5c21..0615751047df 100644 --- a/app/gui/src/dashboard/layouts/AssetsTable.tsx +++ b/app/gui/src/dashboard/layouts/AssetsTable.tsx @@ -1807,9 +1807,12 @@ function AssetsTable(props: AssetsTableProps) { }} > - {headerRow} - - + + {headerRow} + + + {/* Dummy row to increase the gap between the header and the first row */} + {itemRows} { - rootRef.current = element - - if (isKeyboardSelected && element?.contains(document.activeElement) === false) { - element.scrollIntoView({ block: 'nearest' }) - element.focus() - } - }} - className={tailwindMerge.twMerge( - 'h-table-row rounded-full transition-all ease-in-out rounded-rows-child', - visibility, - (isDraggedOver || selected) && 'selected', - )} - {...draggableProps} - onClick={(event) => { - unsetModal() - onClick(innerProps, event) - if ( - asset.type === backendModule.AssetType.directory && - eventModule.isDoubleClick(event) && - !rowState.isEditingName - ) { - // This must be processed on the next tick, otherwise it will be overridden - // by the default click handler. - window.setTimeout(() => { - setSelected(false) - }) - toggleDirectoryExpansion(asset.id) - } - }} - onContextMenu={(event) => { - if (allowContextMenu) { - event.preventDefault() - event.stopPropagation() - if (!selected) { - select(asset) - } - setModal( - , - ) - } - }} - onDragStart={(event) => { - if ( - rowState.isEditingName || - (projectState !== backendModule.ProjectState.closed && - projectState !== backendModule.ProjectState.created && - projectState != null) - ) { - event.preventDefault() - } else { - props.onDragStart?.(event, asset) - } - }} - onDragEnter={(event) => { - if (dragOverTimeoutHandle.current != null) { - window.clearTimeout(dragOverTimeoutHandle.current) - } - if (asset.type === backendModule.AssetType.directory) { - dragOverTimeoutHandle.current = window.setTimeout(() => { - toggleDirectoryExpansion(asset.id, true) - }, DRAG_EXPAND_DELAY_MS) - } - // Required because `dragover` does not fire on `mouseenter`. - props.onDragOver?.(event, asset) - onDragOver(event) - }} - onDragOver={(event) => { - if (state.category.type === 'trash') { - event.dataTransfer.dropEffect = 'none' - } - props.onDragOver?.(event, asset) - onDragOver(event) - }} - onDragEnd={(event) => { - clearDragState() - props.onDragEnd?.(event, asset) - }} - onDragLeave={(event) => { - if ( - dragOverTimeoutHandle.current != null && - (!(event.relatedTarget instanceof Node) || - !event.currentTarget.contains(event.relatedTarget)) - ) { - window.clearTimeout(dragOverTimeoutHandle.current) - } - if ( - event.relatedTarget instanceof Node && - !event.currentTarget.contains(event.relatedTarget) - ) { - clearDragState() - } - props.onDragLeave?.(event, asset) - }} - onDrop={(event) => { - if (state.category.type !== 'trash') { - props.onDrop?.(event, asset) - clearDragState() - const directoryId = - asset.type === backendModule.AssetType.directory ? asset.id : parentId - const payload = drag.ASSET_ROWS.lookup(event) - if ( - payload != null && - payload.every((innerItem) => innerItem.key !== directoryId) - ) { - event.preventDefault() - event.stopPropagation() - unsetModal() - toggleDirectoryExpansion(directoryId, true) - const ids = payload - .filter((payloadItem) => payloadItem.asset.parentId !== directoryId) - .map((dragItem) => dragItem.key) - cutAndPaste( - directoryId, - directoryId, - { backendType: backend.type, ids: new Set(ids), category }, - nodeMap.current, - ) - } else if (event.dataTransfer.types.includes('Files')) { - event.preventDefault() - event.stopPropagation() - toggleDirectoryExpansion(directoryId, true) - void uploadFiles(Array.from(event.dataTransfer.files), directoryId, null) + { + rootRef.current = element + + if (isKeyboardSelected && element?.contains(document.activeElement) === false) { + element.scrollIntoView({ block: 'nearest' }) + element.focus() + } + }} + className={tailwindMerge.twMerge( + 'h-table-row rounded-full transition-all ease-in-out rounded-rows-child', + visibility, + (isDraggedOver || selected) && 'selected', + )} + {...draggableProps} + onClick={(event) => { + unsetModal() + onClick(innerProps, event) + if ( + asset.type === backendModule.AssetType.directory && + eventModule.isDoubleClick(event) && + !rowState.isEditingName + ) { + // This must be processed on the next tick, otherwise it will be overridden + // by the default click handler. + window.setTimeout(() => { + setSelected(false) + }) + toggleDirectoryExpansion(asset.id) + } + }} + onContextMenu={(event) => { + if (allowContextMenu) { + event.preventDefault() + event.stopPropagation() + if (!selected) { + select(asset) + } + setModal( + - {columns.map((column) => { - const Render = columnModule.COLUMN_RENDERER[column] - return ( - + doCopy={doCopy} + doCut={doCut} + doPaste={doPaste} + doDelete={doDelete} + />, + ) + } + }} + onDragStart={(event) => { + if ( + rowState.isEditingName || + (projectState !== backendModule.ProjectState.closed && + projectState !== backendModule.ProjectState.created && + projectState != null) + ) { + event.preventDefault() + } else { + props.onDragStart?.(event, asset) + } + }} + onDragEnter={(event) => { + if (dragOverTimeoutHandle.current != null) { + window.clearTimeout(dragOverTimeoutHandle.current) + } + if (asset.type === backendModule.AssetType.directory) { + dragOverTimeoutHandle.current = window.setTimeout(() => { + toggleDirectoryExpansion(asset.id, true) + }, DRAG_EXPAND_DELAY_MS) + } + // Required because `dragover` does not fire on `mouseenter`. + props.onDragOver?.(event, asset) + onDragOver(event) + }} + onDragOver={(event) => { + if (state.category.type === 'trash') { + event.dataTransfer.dropEffect = 'none' + } + props.onDragOver?.(event, asset) + onDragOver(event) + }} + onDragEnd={(event) => { + clearDragState() + props.onDragEnd?.(event, asset) + }} + onDragLeave={(event) => { + if ( + dragOverTimeoutHandle.current != null && + (!(event.relatedTarget instanceof Node) || + !event.currentTarget.contains(event.relatedTarget)) + ) { + window.clearTimeout(dragOverTimeoutHandle.current) + } + if ( + event.relatedTarget instanceof Node && + !event.currentTarget.contains(event.relatedTarget) + ) { + clearDragState() + } + props.onDragLeave?.(event, asset) + }} + onDrop={(event) => { + if (state.category.type !== 'trash') { + props.onDrop?.(event, asset) + clearDragState() + const directoryId = + asset.type === backendModule.AssetType.directory ? asset.id : parentId + const payload = drag.ASSET_ROWS.lookup(event) + if ( + payload != null && + payload.every((innerItem) => innerItem.key !== directoryId) + ) { + event.preventDefault() + event.stopPropagation() + unsetModal() + toggleDirectoryExpansion(directoryId, true) + const ids = payload + .filter((payloadItem) => payloadItem.asset.parentId !== directoryId) + .map((dragItem) => dragItem.key) + cutAndPaste( + directoryId, + directoryId, + { backendType: backend.type, ids: new Set(ids), category }, + nodeMap.current, ) - })} - - - )} - {selected && allowContextMenu && !hidden && ( + } else if (event.dataTransfer.types.includes('Files')) { + event.preventDefault() + event.stopPropagation() + toggleDirectoryExpansion(directoryId, true) + void uploadFiles(Array.from(event.dataTransfer.files), directoryId, null) + } + } + }} + > + {columns.map((column) => { + const Render = columnModule.COLUMN_RENDERER[column] + return ( + + ) + })} + + + {selected && allowContextMenu && ( // This is a copy of the context menu, since the context menu registers keyboard // shortcut handlers. This is a bit of a hack, however it is preferable to duplicating // the entire context menu (once for the keyboard actions, once for the JSX). diff --git a/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx index 32718be507d8..6c97ad5900bf 100644 --- a/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/DatalinkNameColumn.tsx @@ -44,7 +44,7 @@ export default function DatalinkNameColumn(props: DatalinkNameColumnProps) { return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx index a1cc6ff1e687..5ca7b0fc97a3 100644 --- a/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/DirectoryNameColumn.tsx @@ -67,7 +67,7 @@ export default function DirectoryNameColumn(props: DirectoryNameColumnProps) { return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx index 9d6ae991a923..f0b5b7a0ff47 100644 --- a/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/FileNameColumn.tsx @@ -52,7 +52,7 @@ export default function FileNameColumn(props: FileNameColumnProps) { return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx index 4ec1ec770690..725589c36e21 100644 --- a/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/ProjectNameColumn.tsx @@ -90,7 +90,7 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) { return (
{ diff --git a/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx b/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx index 8374d1e03639..ba03a2c1b0e8 100644 --- a/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx +++ b/app/gui/src/dashboard/components/dashboard/SecretNameColumn.tsx @@ -52,7 +52,7 @@ export default function SecretNameColumn(props: SecretNameColumnProps) { return (
{ diff --git a/app/gui/src/dashboard/components/styled/FocusRing.tsx b/app/gui/src/dashboard/components/styled/FocusRing.tsx index 64449c506213..5913eafc69f8 100644 --- a/app/gui/src/dashboard/components/styled/FocusRing.tsx +++ b/app/gui/src/dashboard/components/styled/FocusRing.tsx @@ -1,6 +1,7 @@ /** @file A styled focus ring. */ import * as aria from '#/components/aria' +import { twMerge } from '#/utilities/tailwindMerge' // ================= // === FocusRing === diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx index b7033d770f70..48d2a26a8baf 100644 --- a/app/gui/src/dashboard/layouts/AssetsTable.tsx +++ b/app/gui/src/dashboard/layouts/AssetsTable.tsx @@ -1701,6 +1701,7 @@ function AssetsTable(props: AssetsTableProps) { {columns.map((column) => { // This is a React component, even though it does not contain JSX. const Heading = COLUMN_HEADING[column] + return (
diff --git a/app/gui/src/dashboard/utilities/__tests__/validation.test.ts b/app/gui/src/dashboard/utilities/__tests__/validation.test.ts index 6d970930447d..6c7ae7c7a844 100644 --- a/app/gui/src/dashboard/utilities/__tests__/validation.test.ts +++ b/app/gui/src/dashboard/utilities/__tests__/validation.test.ts @@ -61,10 +61,9 @@ v.test.each([ { name: '\\/', valid: false }, ])('directory name validation', (args) => { const { name, valid } = args - const regex = validation.DIRECTORY_NAME_REGEX - if (valid) { - v.expect(name, `'${name}' is a valid directory name`).toMatch(regex) - } else { - v.expect(name, `'${name}' is not a valid directory name`).not.toMatch(regex) - } + + v.expect( + !validation.isDirectoryNameContainInvalidCharacters(name), + `'${name}' is a valid directory name`, + ).toBe(valid) }) diff --git a/app/gui/src/dashboard/utilities/validation.ts b/app/gui/src/dashboard/utilities/validation.ts index 12134b1e2352..66bd5c413fd1 100644 --- a/app/gui/src/dashboard/utilities/validation.ts +++ b/app/gui/src/dashboard/utilities/validation.ts @@ -37,3 +37,14 @@ export const LOCAL_PROJECT_NAME_PATTERN = '.*\\S.*' * - `..` - parent directory */ export const DIRECTORY_NAME_REGEX = /^(?:[^/\\.]|[.](?=[^.]|$))+$/ + +/** + * Check if the directory name contains invalid characters. + */ +export function isDirectoryNameContainInvalidCharacters(name: string) { + if (name.includes('/') || name.includes('\\') || name.includes('..')) { + return true + } + + return false +} From cd30c89a10814232505a8d9bcdcef03bf510780f Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Thu, 12 Dec 2024 16:57:58 +0400 Subject: [PATCH 05/21] Fix `used by another dev behavior` --- app/common/src/text/index.ts | 52 +++++++++++ app/gui/env.d.ts | 8 ++ .../dashboard/actions/index.ts | 6 +- app/gui/integration-test/dashboard/api.ts | 19 ++++ .../dashboard/assetsTableFeatures.spec.ts | 57 +++++++++++- app/gui/src/dashboard/App.tsx | 67 +++++++------- .../components/Devtools/EnsoDevtools.tsx | 23 +++-- .../components/dashboard/ProjectIcon.tsx | 17 +++- app/gui/src/dashboard/hooks/projectHooks.ts | 4 +- .../providers/FeatureFlagsProvider.tsx | 91 +++++++++++++------ .../src/dashboard/providers/TextProvider.tsx | 20 ++-- eslint.config.mjs | 13 +++ 12 files changed, 283 insertions(+), 94 deletions(-) diff --git a/app/common/src/text/index.ts b/app/common/src/text/index.ts index 8c532137d50e..31a63e87bdbb 100644 --- a/app/common/src/text/index.ts +++ b/app/common/src/text/index.ts @@ -1,5 +1,6 @@ /** @file Functions related to displaying text. */ +import { unsafeKeys } from '../utilities/data/object' import ENGLISH from './english.json' with { type: 'json' } // ============= @@ -159,3 +160,54 @@ export interface Replacements export const TEXTS: Readonly> = { [Language.english]: ENGLISH, } +/** + * A function that gets localized text for a given key, with optional replacements. + * @param key - The key of the text to get. + * @param replacements - The replacements to insert into the text. + * If the text contains placeholders like `$0`, `$1`, etc., + * they will be replaced with the corresponding replacement. + */ +export type GetText = ( + dictionary: Texts, + key: K, + ...replacements: Replacements[K] +) => string + +/** + * Resolves the language texts based on the user's preferred language. + */ +export function resolveUserLanguage() { + const locale = navigator.language + const language = + unsafeKeys(LANGUAGE_TO_LOCALE).find(language => locale === LANGUAGE_TO_LOCALE[language]) ?? + Language.english + + return language +} + +/** + * Gets the dictionary for a given language. + * @param language - The language to get the dictionary for. + * @returns The dictionary for the given language. + */ +export function getDictionary(language: Language) { + return TEXTS[language] +} + +/** + * Gets the text for a given key, with optional replacements. + * @param dictionary - The dictionary to get the text from. + * @param key - The key of the text to get. + * @param replacements - The replacements to insert into the text. + * If the text contains placeholders like `$0`, `$1`, etc., + * they will be replaced with the corresponding replacement. + */ +export const getText: GetText = (dictionary, key, ...replacements) => { + const template = dictionary[key] + + return replacements.length === 0 ? + template + : template.replace(/[$]([$]|\d+)/g, (_match, placeholder: string) => + placeholder === '$' ? '$' : String(replacements[Number(placeholder)] ?? `$${placeholder}`), + ) +} diff --git a/app/gui/env.d.ts b/app/gui/env.d.ts index f7095e04d189..a7feefc836da 100644 --- a/app/gui/env.d.ts +++ b/app/gui/env.d.ts @@ -175,6 +175,7 @@ declare global { readonly projectManagementApi?: ProjectManagementApi readonly fileBrowserApi?: FileBrowserApi readonly versionInfo?: VersionInfo + toggleDevtools: () => void /** * If set to `true`, animations will be disabled. @@ -183,6 +184,13 @@ declare global { * ATM only affects the framer-motion animations. */ readonly DISABLE_ANIMATIONS?: boolean + readonly featureFlags: FeatureFlags + readonly setFeatureFlags: (flags: Partial) => void + /** + * Feature flags that override the default or stored feature flags. + * This is used by integration tests to set feature flags. + */ + readonly overrideFeatureFlags: Partial } namespace NodeJS { diff --git a/app/gui/integration-test/dashboard/actions/index.ts b/app/gui/integration-test/dashboard/actions/index.ts index 3c14d8c4f585..1ec1e538f5bf 100644 --- a/app/gui/integration-test/dashboard/actions/index.ts +++ b/app/gui/integration-test/dashboard/actions/index.ts @@ -1,7 +1,7 @@ /** @file Various actions, locators, and constants used in end-to-end tests. */ import * as test from '@playwright/test' -import { TEXTS } from 'enso-common/src/text' +import { TEXTS, getText as baseGetText, type Replacements, type TextId } from 'enso-common/src/text' import path from 'node:path' import * as apiModule from '../api' @@ -21,6 +21,10 @@ export const VALID_PASSWORD = 'Password0!' export const VALID_EMAIL = 'email@example.com' export const TEXT = TEXTS.english +export const getText = (key: TextId, ...replacements: Replacements[TextId]) => { + return baseGetText(TEXT, key, ...replacements) +} + // ================ // === Locators === // ================ diff --git a/app/gui/integration-test/dashboard/api.ts b/app/gui/integration-test/dashboard/api.ts index 496b3eebcbdf..a5c0ecd37787 100644 --- a/app/gui/integration-test/dashboard/api.ts +++ b/app/gui/integration-test/dashboard/api.ts @@ -15,6 +15,10 @@ import * as actions from './actions' import { readFileSync } from 'node:fs' import { dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' +import type { + FeatureFlags, + FeatureFlagsStore, +} from '../../src/dashboard/providers/FeatureFlagsProvider' import LATEST_GITHUB_RELEASES from './latestGithubReleases.json' with { type: 'json' } // ================= @@ -1140,6 +1144,21 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { deleteUser, addUserGroup, deleteUserGroup, + setFeatureFlags: (flags: Partial) => { + return page.addInitScript((flags: Partial) => { + const currentOverrideFeatureFlags = + 'overrideFeatureFlags' in window && typeof window.overrideFeatureFlags === 'object' ? + window.overrideFeatureFlags + : {} + + Object.defineProperty(window, 'overrideFeatureFlags', { + value: { ...currentOverrideFeatureFlags, ...flags }, + writable: false, + }) + + console.log('overrideFeatureFlags', window.overrideFeatureFlags) + }, flags) + }, // TODO: // addPermission, // deletePermission, diff --git a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts index 46ab233b5562..65a531fff55b 100644 --- a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts +++ b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts @@ -1,6 +1,6 @@ /** @file Test the drive view. */ +import * as backend from '#/services/Backend' import * as test from '@playwright/test' - import * as actions from './actions' const PASS_TIMEOUT = 5_000 @@ -98,3 +98,58 @@ test.test('can drop onto root directory dropzone', ({ page }) => test.expect(firstLeft, 'Siblings have same indentation').toEqual(secondLeft) }), ) + +test.test("can't run a project in browser by default", ({ page }) => + actions + .mockAllAndLogin({ + page, + setupAPI: async (api) => { + api.addProject({ title: 'a' }) + }, + }) + .driveTable.withRows(async (rows) => { + const row = rows.first() + + const startProjectButton = row.getByTestId('open-project') + await test.expect(startProjectButton).toBeDisabled() + }), +) + +test.test("can't start an already running by another user", ({ page }) => + actions + .mockAllAndLogin({ + page, + setupAPI: async (api) => { + await api.setFeatureFlags({ enableCloudExecution: true }) + + const userGroup = api.addUserGroup('Test Group') + + api.addUserGroupToUser(api.defaultUser.userId, userGroup.id) + + const peer = api.addUser('Test User', { + email: backend.EmailAddress('test@test.com'), + userGroups: [userGroup.id], + }) + + api.addProject({ + title: 'a', + projectState: { + type: backend.ProjectState.opened, + volumeId: '123', + openedBy: peer.email, + }, + }) + }, + }) + .driveTable.withRows(async (rows) => { + const row = rows.first() + const startProjectButton = row.getByTestId('open-project') + + await test.expect(row).toBeVisible() + await test.expect(row.getByTestId('switch-to-project')).not.toBeVisible() + await test.expect(startProjectButton).toBeDisabled() + await test + .expect(startProjectButton) + .toHaveAccessibleName(actions.getText('xIsUsingTheProject', 'Test User')) + }), +) diff --git a/app/gui/src/dashboard/App.tsx b/app/gui/src/dashboard/App.tsx index fe8d68ee04e0..5f208d1e93b3 100644 --- a/app/gui/src/dashboard/App.tsx +++ b/app/gui/src/dashboard/App.tsx @@ -88,7 +88,6 @@ import LocalBackend from '#/services/LocalBackend' import ProjectManager, * as projectManager from '#/services/ProjectManager' import RemoteBackend from '#/services/RemoteBackend' -import { FeatureFlagsProvider } from '#/providers/FeatureFlagsProvider' import * as appBaseUrl from '#/utilities/appBaseUrl' import * as eventModule from '#/utilities/event' import LocalStorage from '#/utilities/LocalStorage' @@ -519,40 +518,38 @@ function AppRouter(props: AppRouterProps) { ) return ( - - - - - - - {/* Ideally this would be in `Drive.tsx`, but it currently must be all the way out here - * due to modals being in `TheModal`. */} - - - - {routes} - - - - - - - - - - - - + + + + + + {/* Ideally this would be in `Drive.tsx`, but it currently must be all the way out here + * due to modals being in `TheModal`. */} + + + + {routes} + + + + + + + + + + + ) } diff --git a/app/gui/src/dashboard/components/Devtools/EnsoDevtools.tsx b/app/gui/src/dashboard/components/Devtools/EnsoDevtools.tsx index e41c813d5522..a1ac66cb7520 100644 --- a/app/gui/src/dashboard/components/Devtools/EnsoDevtools.tsx +++ b/app/gui/src/dashboard/components/Devtools/EnsoDevtools.tsx @@ -49,6 +49,7 @@ import { import { useLocalStorage } from '#/providers/LocalStorageProvider' import * as backend from '#/services/Backend' import LocalStorage, { type LocalStorageData } from '#/utilities/LocalStorage' +import { unsafeKeys } from '#/utilities/object' /** A component that provides a UI for toggling paywall features. */ export function EnsoDevtools() { @@ -142,7 +143,7 @@ export function EnsoDevtools() { - {/* eslint-disable-next-line no-restricted-syntax */} + {} @@ -198,12 +199,10 @@ export function EnsoDevtools() { gap="small" schema={FEATURE_FLAGS_SCHEMA} formOptions={{ mode: 'onChange' }} - defaultValues={{ - enableMultitabs: featureFlags.enableMultitabs, - enableAssetsTableBackgroundRefresh: featureFlags.enableAssetsTableBackgroundRefresh, - assetsTableBackgroundRefreshInterval: - featureFlags.assetsTableBackgroundRefreshInterval, - }} + defaultValues={Object.fromEntries( + // FEATURE_FLAGS_SCHEMA is statically known, so we can safely cast to keyof FeatureFlags. + unsafeKeys(FEATURE_FLAGS_SCHEMA.shape).map((key) => [key, featureFlags[key]]), + )} > {(form) => ( <> @@ -248,6 +247,16 @@ export function EnsoDevtools() { }} /> + + { + setFeatureFlags('enableCloudExecution', value) + }} + /> )} diff --git a/app/gui/src/dashboard/components/dashboard/ProjectIcon.tsx b/app/gui/src/dashboard/components/dashboard/ProjectIcon.tsx index dccfd3cdf7bc..8d580e96c13c 100644 --- a/app/gui/src/dashboard/components/dashboard/ProjectIcon.tsx +++ b/app/gui/src/dashboard/components/dashboard/ProjectIcon.tsx @@ -76,7 +76,9 @@ export interface ProjectIconProps { /** An interactive icon indicating the status of a project. */ export default function ProjectIcon(props: ProjectIconProps) { const { backend, item, isOpened, isDisabled: isDisabledRaw, isPlaceholder } = props + const isUnconditionallyDisabled = !projectHooks.useCanOpenProjects() + const isDisabled = isDisabledRaw || isUnconditionallyDisabled const openProject = projectHooks.useOpenProject() @@ -166,6 +168,9 @@ export default function ProjectIcon(props: ProjectIconProps) { openProjectTab(item.id) }) + const getTooltip = (defaultTooltip: string) => + disabledTooltip ?? userOpeningProjectTooltip ?? defaultTooltip + switch (state) { case backendModule.ProjectState.new: case backendModule.ProjectState.closing: @@ -176,11 +181,12 @@ export default function ProjectIcon(props: ProjectIconProps) { size="custom" variant="icon" icon={PlayIcon} - aria-label={disabledTooltip ?? getText('openInEditor')} + aria-label={getTooltip(getText('openInEditor'))} tooltipPlacement="left" extraClickZone="xsmall" isDisabled={isDisabled || projectState?.type === backendModule.ProjectState.closing} onPress={doOpenProject} + testId="open-project" /> ) case backendModule.ProjectState.openInProgress: @@ -195,11 +201,12 @@ export default function ProjectIcon(props: ProjectIconProps) { extraClickZone="xsmall" isDisabled={isDisabled || isOtherUserUsingProject} icon={StopIcon} - aria-label={userOpeningProjectTooltip ?? getText('stopExecution')} + aria-label={getTooltip(getText('stopExecution'))} tooltipPlacement="left" className={tailwindMerge.twJoin(isRunningInBackground && 'text-green')} {...(isOtherUserUsingProject ? { title: getText('otherUserIsUsingProjectError') } : {})} onPress={doCloseProject} + testId="stop-project" /> )} diff --git a/app/gui/src/dashboard/hooks/projectHooks.ts b/app/gui/src/dashboard/hooks/projectHooks.ts index 8448adf02037..1247022910c0 100644 --- a/app/gui/src/dashboard/hooks/projectHooks.ts +++ b/app/gui/src/dashboard/hooks/projectHooks.ts @@ -47,8 +47,8 @@ export interface CreateOpenedProjectQueryOptions { /** Whether the user can open projects. */ export function useCanOpenProjects() { - const localBackend = backendProvider.useLocalBackend() - return localBackend != null + const enableCloudExecution = useFeatureFlag('enableCloudExecution') + return enableCloudExecution } /** Return a function to update a project asset in the TanStack Query cache. */ diff --git a/app/gui/src/dashboard/providers/FeatureFlagsProvider.tsx b/app/gui/src/dashboard/providers/FeatureFlagsProvider.tsx index bf5001e5e4e6..0d277e0f2d9e 100644 --- a/app/gui/src/dashboard/providers/FeatureFlagsProvider.tsx +++ b/app/gui/src/dashboard/providers/FeatureFlagsProvider.tsx @@ -4,42 +4,33 @@ * Feature flags provider. * Feature flags are used to enable or disable certain features in the application. */ -import LocalStorage from '#/utilities/LocalStorage' -import type { ReactNode } from 'react' +import { IS_DEV_MODE, isOnElectron } from 'enso-common/src/detect' import { z } from 'zod' import { createStore, useStore } from 'zustand' import { persist } from 'zustand/middleware' - -declare module '#/utilities/LocalStorage' { - /** Local storage data structure. */ - interface LocalStorageData { - readonly featureFlags: z.infer - } -} - +import { unsafeEntries, unsafeFromEntries } from '../utilities/object' +import { unsafeWriteValue } from '../utilities/write' export const FEATURE_FLAGS_SCHEMA = z.object({ enableMultitabs: z.boolean(), enableAssetsTableBackgroundRefresh: z.boolean(), // eslint-disable-next-line @typescript-eslint/no-magic-numbers assetsTableBackgroundRefreshInterval: z.number().min(100), + enableCloudExecution: z.boolean(), }) -LocalStorage.registerKey('featureFlags', { schema: FEATURE_FLAGS_SCHEMA }) +/** Feature flags. */ +export type FeatureFlags = z.infer /** Feature flags store. */ -export interface FeatureFlags { - readonly featureFlags: { - readonly enableMultitabs: boolean - readonly enableAssetsTableBackgroundRefresh: boolean - readonly assetsTableBackgroundRefreshInterval: number - } - readonly setFeatureFlags: ( +export interface FeatureFlagsStore { + readonly featureFlags: FeatureFlags + readonly setFeatureFlags: ( key: Key, - value: FeatureFlags['featureFlags'][Key], + value: FeatureFlags[Key], ) => void } -const flagsStore = createStore()( +const flagsStore = createStore()( persist( (set) => ({ featureFlags: { @@ -47,12 +38,46 @@ const flagsStore = createStore()( enableAssetsTableBackgroundRefresh: true, // eslint-disable-next-line @typescript-eslint/no-magic-numbers assetsTableBackgroundRefreshInterval: 3_000, + enableCloudExecution: IS_DEV_MODE || isOnElectron(), }, setFeatureFlags: (key, value) => { set(({ featureFlags }) => ({ featureFlags: { ...featureFlags, [key]: value } })) }, }), - { name: 'featureFlags' }, + { + name: 'featureFlags', + version: 1, + merge: (persistedState, newState) => { + function unsafeMutateFeatureFlags(flags: Partial) { + unsafeWriteValue(newState, 'featureFlags', { + ...newState.featureFlags, + ...flags, + }) + } + + const parsedPersistedState = FEATURE_FLAGS_SCHEMA.safeParse(persistedState) + + if (parsedPersistedState.success) { + unsafeMutateFeatureFlags(parsedPersistedState.data) + } + + if (typeof window !== 'undefined') { + const predefinedFeatureFlags = FEATURE_FLAGS_SCHEMA.partial().safeParse( + window.overrideFeatureFlags, + ) + + if (predefinedFeatureFlags.success) { + const withOmittedUndefined = Object.fromEntries( + Object.entries(predefinedFeatureFlags.data).filter(([, value]) => value != null), + ) + // This is safe, because zod omits unset values. + unsafeMutateFeatureFlags(withOmittedUndefined) + } + } + + return newState + }, + }, ), ) @@ -62,9 +87,9 @@ export function useFeatureFlags() { } /** Hook to get a specific feature flag. */ -export function useFeatureFlag( +export function useFeatureFlag( key: Key, -): FeatureFlags['featureFlags'][Key] { +): FeatureFlagsStore['featureFlags'][Key] { return useStore(flagsStore, ({ featureFlags }) => featureFlags[key]) } @@ -73,11 +98,17 @@ export function useSetFeatureFlags() { return useStore(flagsStore, ({ setFeatureFlags }) => setFeatureFlags) } -/** - * Feature flags provider. - * Gets feature flags from local storage and sets them in the store. - * Also saves feature flags to local storage when they change. - */ -export function FeatureFlagsProvider({ children }: { children: ReactNode }) { - return <>{children} +// Define global API for managing feature flags +if (typeof window !== 'undefined') { + Object.defineProperty(window, 'featureFlags', { + value: flagsStore.getState().featureFlags, + configurable: false, + writable: false, + }) + + Object.defineProperty(window, 'setFeatureFlags', { + value: flagsStore.getState().setFeatureFlags, + configurable: false, + writable: false, + }) } diff --git a/app/gui/src/dashboard/providers/TextProvider.tsx b/app/gui/src/dashboard/providers/TextProvider.tsx index a8b1344c7027..5878ba829664 100644 --- a/app/gui/src/dashboard/providers/TextProvider.tsx +++ b/app/gui/src/dashboard/providers/TextProvider.tsx @@ -31,11 +31,9 @@ export type GetText = ( ...replacements: text.Replacements[K] ) => string -const DEFAULT_LANGUAGE = text.Language.english - const TextContext = React.createContext({ - language: DEFAULT_LANGUAGE, - locale: text.LANGUAGE_TO_LOCALE[DEFAULT_LANGUAGE], + language: text.resolveUserLanguage(), + locale: navigator.language, /** * Set `this.language`. It is NOT RECOMMENDED to use the default value, as this does not trigger * reactive updates. @@ -56,7 +54,7 @@ export type TextProviderProps = Readonly export default function TextProvider(props: TextProviderProps) { const { children } = props - const [language, setLanguage] = React.useState(text.Language.english) + const [language, setLanguage] = React.useState(() => text.resolveUserLanguage()) const locale = text.LANGUAGE_TO_LOCALE[language] const contextValue = React.useMemo( @@ -70,18 +68,12 @@ export default function TextProvider(props: TextProviderProps) { /** Exposes a property to get localized text, and get and set the current language. */ export function useText() { const { language, setLanguage, locale } = React.useContext(TextContext) - const localizedText = text.TEXTS[language] + + const localizedText = text.getDictionary(language) const getText = React.useCallback( (key, ...replacements) => { - const template = localizedText[key] - return replacements.length === 0 ? - template - : template.replace(/[$]([$]|\d+)/g, (_match, placeholder: string) => - placeholder === '$' ? '$' : ( - String(replacements[Number(placeholder)] ?? `$${placeholder}`) - ), - ) + return text.getText(localizedText, key, ...replacements) }, [localizedText], ) diff --git a/eslint.config.mjs b/eslint.config.mjs index fe6ba0167d58..8ae0480d1697 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -157,6 +157,7 @@ const RESTRICTED_SYNTAXES = [ message: 'Use `while (true)` instead of `for (;;)`', }, { + id: 'no-jsx-strings', selector: `:matches(\ JSXAttribute[name.name=/^(?:alt|error|label|placeholder|text|title|actionButtonLabel|actionText|aria-label)$/][value.raw=/^'|^"|^\`/], \ JSXText[value=/\\S/], \ @@ -569,6 +570,18 @@ export default [ '@typescript-eslint/naming-convention': 'off', }, }, + // === EnsoDevtools Rules === + // Allow JSX strings in EnsoDevtools.tsx. + { + files: ['app/gui/src/dashboard/**/EnsoDevtools.tsx'], + rules: { + 'no-restricted-syntax': [ + 'error', + ...RESTRICTED_SYNTAXES.filter(syntax => syntax.id !== 'no-jsx-strings'), + ], + }, + }, + // === React Compiler Rules === { files: ['app/gui/src/dashboard/**/*.ts', 'app/gui/src/dashboard/**/*.tsx'], ignores: ['**/*.d.ts', '**/*.spec.ts', '**/*.stories.tsx', '**/*.test.tsx', '**/*.test.ts'], From 7329bf78a4e348c40251f7942fa8f4e2b8969064 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Sat, 14 Dec 2024 01:24:05 +0400 Subject: [PATCH 06/21] FIxes --- app/common/src/services/Backend.ts | 8 +- .../dashboard/actions/DrivePageActions.ts | 4 +- .../integration-test/dashboard/actions/api.ts | 99 ++++- .../dashboard/actions/index.ts | 3 - .../dashboard/assetsTableFeatures.spec.ts | 87 +++-- .../integration-test/dashboard/copy.spec.ts | 97 +++-- .../dashboard/editAssetName.spec.ts | 208 ++++++++++- .../dashboard/pageSwitcher.spec.ts | 9 +- .../integration-test/dashboard/sort.spec.ts | 12 +- .../dashboard/startModal.spec.ts | 9 +- .../AriaComponents/Form/components/Field.tsx | 4 +- .../AriaComponents/Inputs/Input/Input.tsx | 16 +- .../src/dashboard/components/EditableSpan.tsx | 82 +++-- .../components/dashboard/AssetRow.tsx | 340 +++++++++--------- .../dashboard/DatalinkNameColumn.tsx | 2 +- .../dashboard/DirectoryNameColumn.tsx | 2 +- .../components/dashboard/FileNameColumn.tsx | 2 +- .../dashboard/ProjectNameColumn.tsx | 2 +- .../components/dashboard/SecretNameColumn.tsx | 2 +- .../dashboard/components/styled/FocusRing.tsx | 1 + app/gui/src/dashboard/layouts/AssetsTable.tsx | 9 +- 21 files changed, 660 insertions(+), 338 deletions(-) diff --git a/app/common/src/services/Backend.ts b/app/common/src/services/Backend.ts index 824771eaf241..d39984bae353 100644 --- a/app/common/src/services/Backend.ts +++ b/app/common/src/services/Backend.ts @@ -970,9 +970,7 @@ export function createSpecialLoadingAsset(directoryId: DirectoryId): SpecialLoad return { type: AssetType.specialLoading, title: '', - id: LoadingAssetId( - createPlaceholderId(`${AssetType.specialLoading}-${uniqueString.uniqueString()}`), - ), + id: LoadingAssetId(createPlaceholderId(`${AssetType.specialLoading}-${directoryId}`)), modifiedAt: dateTime.toRfc3339(new Date()), parentId: directoryId, permissions: [], @@ -998,7 +996,7 @@ export function createSpecialEmptyAsset(directoryId: DirectoryId): SpecialEmptyA return { type: AssetType.specialEmpty, title: '', - id: EmptyAssetId(`${AssetType.specialEmpty}-${uniqueString.uniqueString()}`), + id: EmptyAssetId(`${AssetType.specialEmpty}-${directoryId}`), modifiedAt: dateTime.toRfc3339(new Date()), parentId: directoryId, permissions: [], @@ -1024,7 +1022,7 @@ export function createSpecialErrorAsset(directoryId: DirectoryId): SpecialErrorA return { type: AssetType.specialError, title: '', - id: ErrorAssetId(`${AssetType.specialError}-${uniqueString.uniqueString()}`), + id: ErrorAssetId(`${AssetType.specialError}-${directoryId}`), modifiedAt: dateTime.toRfc3339(new Date()), parentId: directoryId, permissions: [], diff --git a/app/gui/integration-test/dashboard/actions/DrivePageActions.ts b/app/gui/integration-test/dashboard/actions/DrivePageActions.ts index 83122470e5a2..bea14460c749 100644 --- a/app/gui/integration-test/dashboard/actions/DrivePageActions.ts +++ b/app/gui/integration-test/dashboard/actions/DrivePageActions.ts @@ -41,7 +41,9 @@ function locateAssetRows(page: Page) { /** Find assets table placeholder rows. */ function locateNonAssetRows(page: Page) { - return locateAssetsTable(page).locator('tbody tr:not([data-testid="asset-row"])') + return locateAssetsTable(page).locator( + 'tbody tr:not([data-testid="asset-row"]):not([data-testid="dummy-row"])', + ) } /** Find a "new secret" icon. */ diff --git a/app/gui/integration-test/dashboard/actions/api.ts b/app/gui/integration-test/dashboard/actions/api.ts index 64aefbfb6fec..63da8ae53ac9 100644 --- a/app/gui/integration-test/dashboard/actions/api.ts +++ b/app/gui/integration-test/dashboard/actions/api.ts @@ -12,14 +12,10 @@ import * as uniqueString from 'enso-common/src/utilities/uniqueString' import * as actions from '.' +import type { FeatureFlags } from '#/providers/FeatureFlagsProvider' import { readFileSync } from 'node:fs' import { dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' -import type { - FeatureFlags, - FeatureFlagsStore, -} from '../../src/dashboard/providers/FeatureFlagsProvider' -import LATEST_GITHUB_RELEASES from './latestGithubReleases.json' with { type: 'json' } // ================= // === Constants === @@ -119,6 +115,7 @@ const INITIAL_CALLS_OBJECT = { createDirectory: array(), getProjectContent: array<{ projectId: backend.ProjectId }>(), getProjectAsset: array<{ projectId: backend.ProjectId }>(), + updateProject: array(), } const READONLY_INITIAL_CALLS_OBJECT: TrackedCallsInternal = INITIAL_CALLS_OBJECT @@ -193,7 +190,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { const assetMap = new Map() const deletedAssets = new Set() - const assets: backend.AnyAsset[] = [] + let assets: backend.AnyAsset[] = [] const labels: backend.Label[] = [] const labelsByValue = new Map() const labelMap = new Map() @@ -240,8 +237,8 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { } const addAsset = (asset: T) => { - assets.push(asset) assetMap.set(asset.id, asset) + assets = Array.from(assetMap.values()) return asset } @@ -258,6 +255,25 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { return wasDeleted } + const editAsset = (assetId: backend.AssetId, rest: Partial) => { + const asset = assetMap.get(assetId) + + if (asset == null) { + throw new Error(`Asset ${assetId} not found`) + } + + console.log('editAsset', { + asset, + rest, + }) + + const updated = object.merge(asset, rest) + + addAsset(updated) + + return updated + } + const createDirectory = (rest: Partial = {}): backend.DirectoryAsset => { const directoryTitles = new Set( assets @@ -371,19 +387,19 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { color, }) - const addDirectory = (rest: Partial) => { + const addDirectory = (rest: Partial = {}) => { return addAsset(createDirectory(rest)) } - const addProject = (rest: Partial) => { + const addProject = (rest: Partial = {}) => { return addAsset(createProject(rest)) } - const addFile = (rest: Partial) => { + const addFile = (rest: Partial = {}) => { return addAsset(createFile(rest)) } - const addSecret = (rest: Partial) => { + const addSecret = (rest: Partial = {}) => { return addAsset(createSecret(rest)) } @@ -881,15 +897,21 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { } }, ) - await patch(remoteBackendPaths.updateAssetPath(GLOB_ASSET_ID), (_route, request) => { + + await patch(remoteBackendPaths.updateAssetPath(GLOB_ASSET_ID), (route, request) => { + console.log('updateAssetPath', request.url()) const maybeId = request.url().match(/[/]assets[/]([^?]+)/)?.[1] - if (!maybeId) return + + if (!maybeId) throw new Error('updateAssetPath: Missing asset ID in path') // This could be an id for an arbitrary asset, but pretend it's a // `DirectoryId` to make TypeScript happy. const assetId = backend.DirectoryId(maybeId) const body: backend.UpdateAssetRequestBody = request.postDataJSON() + called('updateAsset', { ...body, assetId }) + const asset = assetMap.get(assetId) + if (asset != null) { if (body.description != null) { object.unsafeMutable(asset).description = body.description @@ -899,7 +921,10 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { object.unsafeMutable(asset).parentId = body.parentDirectoryId } } + + return route.fulfill({ json: asset }) }) + await patch(remoteBackendPaths.associateTagPath(GLOB_ASSET_ID), async (_route, request) => { const maybeId = request.url().match(/[/]assets[/]([^/?]+)/)?.[1] if (!maybeId) return @@ -925,6 +950,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { } return json }) + await put(remoteBackendPaths.updateDirectoryPath(GLOB_DIRECTORY_ID), async (route, request) => { const maybeId = request.url().match(/[/]directories[/]([^?]+)/)?.[1] if (!maybeId) return @@ -945,6 +971,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { }) } }) + await delete_(remoteBackendPaths.deleteAssetPath(GLOB_ASSET_ID), async (route, request) => { const maybeId = request.url().match(/[/]assets[/]([^?]+)/)?.[1] if (!maybeId) return @@ -955,6 +982,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { deleteAsset(assetId) await route.fulfill({ status: HTTP_STATUS_NO_CONTENT }) }) + await patch(remoteBackendPaths.UNDO_DELETE_ASSET_PATH, async (route, request) => { /** The type for the JSON request payload for this endpoint. */ interface Body { @@ -965,13 +993,51 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { undeleteAsset(body.assetId) await route.fulfill({ status: HTTP_STATUS_NO_CONTENT }) }) + + await put(remoteBackendPaths.projectUpdatePath(GLOB_PROJECT_ID), async (route, request) => { + const maybeId = request.url().match(/[/]projects[/]([^?/]+)/)?.[1] + + console.log('maybeId', maybeId) + + if (!maybeId) return route.fulfill({ status: HTTP_STATUS_NOT_FOUND }) + + const projectId = backend.ProjectId(maybeId) + + const body: backend.UpdateProjectRequestBody = await request.postDataJSON() + + called('updateProject', body) + + const newTitle = body.projectName + + console.log('newTitle', { + newTitle, + maybeId, + }) + + if (newTitle == null) { + return route.fulfill({ status: HTTP_STATUS_BAD_REQUEST }) + } + + console.log('editAsset', { + projectId, + newTitle, + }) + + return route.fulfill({ + json: editAsset(projectId, { title: newTitle }), + }) + }) + await post(remoteBackendPaths.CREATE_USER_PATH + '*', async (_route, request) => { const body: backend.CreateUserRequestBody = await request.postDataJSON() + const organizationId = body.organizationId ?? defaultUser.organizationId const rootDirectoryId = backend.DirectoryId( organizationId.replace(/^organization-/, 'directory-'), ) + called('createUser', body) + currentUser = { email: body.userEmail, name: body.userName, @@ -984,12 +1050,14 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { } return currentUser }) + await post(remoteBackendPaths.CREATE_USER_GROUP_PATH + '*', async (_route, request) => { const body: backend.CreateUserGroupRequestBody = await request.postDataJSON() called('createUserGroup', body) const userGroup = addUserGroup(body.name) return userGroup }) + await put( remoteBackendPaths.changeUserGroupPath(GLOB_USER_ID) + '*', async (route, request) => { @@ -1094,7 +1162,9 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { await post(remoteBackendPaths.CREATE_DIRECTORY_PATH + '*', (_route, request) => { const body: backend.CreateDirectoryRequestBody = request.postDataJSON() + called('createDirectory', body) + const id = backend.DirectoryId(`directory-${uniqueString.uniqueString()}`) const parentId = body.parentId ?? defaultDirectoryId @@ -1198,6 +1268,7 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { currentOrganizationProfilePicture: () => currentOrganizationProfilePicture, addAsset, deleteAsset, + editAsset, undeleteAsset, createDirectory, createProject, @@ -1226,8 +1297,6 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { value: { ...currentOverrideFeatureFlags, ...flags }, writable: false, }) - - console.log('overrideFeatureFlags', window.overrideFeatureFlags) }, flags) }, // TODO: diff --git a/app/gui/integration-test/dashboard/actions/index.ts b/app/gui/integration-test/dashboard/actions/index.ts index c446ec9dc8e1..d83238589606 100644 --- a/app/gui/integration-test/dashboard/actions/index.ts +++ b/app/gui/integration-test/dashboard/actions/index.ts @@ -1,5 +1,4 @@ /** @file Various actions, locators, and constants used in end-to-end tests. */ -import * as test from '@playwright/test' import { TEXTS, getText as baseGetText, type Replacements, type TextId } from 'enso-common/src/text' @@ -7,8 +6,6 @@ import path from 'node:path' import { expect, test, type Page } from '@playwright/test' -import { TEXTS } from 'enso-common/src/text' - import { INITIAL_CALLS_OBJECT, mockApi, diff --git a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts index 8436e973fd2d..92a1823b37c1 100644 --- a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts +++ b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts @@ -1,7 +1,8 @@ /** @file Test the drive view. */ import { expect, test, type Locator, type Page } from '@playwright/test' -import { mockAllAndLogin, TEXT } from './actions' +import { EmailAddress, ProjectState } from '#/services/Backend' +import { getText, mockAllAndLogin, TEXT } from './actions' /** Find an extra columns button panel. */ function locateExtraColumns(page: Page) { @@ -117,54 +118,50 @@ test('can drop onto root directory dropzone', ({ page }) => })) test("can't run a project in browser by default", ({ page }) => - actions - .mockAllAndLogin({ - page, - setupAPI: async (api) => { - api.addProject({ title: 'a' }) - }, - }) - .driveTable.withRows(async (rows) => { - const row = rows.first() + mockAllAndLogin({ + page, + setupAPI: async (api) => { + api.addProject({ title: 'a' }) + }, + }).driveTable.withRows(async (rows) => { + const row = rows.first() - const startProjectButton = row.getByTestId('open-project') - await expect(startProjectButton).toBeDisabled() - })) + const startProjectButton = row.getByTestId('open-project') + await expect(startProjectButton).toBeDisabled() + })) test("can't start an already running by another user", ({ page }) => - actions - .mockAllAndLogin({ - page, - setupAPI: async (api) => { - await api.setFeatureFlags({ enableCloudExecution: true }) + mockAllAndLogin({ + page, + setupAPI: async (api) => { + await api.setFeatureFlags({ enableCloudExecution: true }) - const userGroup = api.addUserGroup('Test Group') + const userGroup = api.addUserGroup('Test Group') - api.addUserGroupToUser(api.defaultUser.userId, userGroup.id) + api.addUserGroupToUser(api.defaultUser.userId, userGroup.id) - const peer = api.addUser('Test User', { - email: backend.EmailAddress('test@test.com'), - userGroups: [userGroup.id], - }) + const peer = api.addUser('Test User', { + email: EmailAddress('test@test.com'), + userGroups: [userGroup.id], + }) - api.addProject({ - title: 'a', - projectState: { - type: backend.ProjectState.opened, - volumeId: '123', - openedBy: peer.email, - }, - }) - }, - }) - .driveTable.withRows(async (rows) => { - const row = rows.first() - const startProjectButton = row.getByTestId('open-project') - - await expect(row).toBeVisible() - await expect(row.getByTestId('switch-to-project')).not.toBeVisible() - await expect(startProjectButton).toBeDisabled() - await expect(startProjectButton).toHaveAccessibleName( - actions.getText('xIsUsingTheProject', 'Test User'), - ) - })) + api.addProject({ + title: 'a', + projectState: { + type: ProjectState.opened, + volumeId: '123', + openedBy: peer.email, + }, + }) + }, + }).driveTable.withRows(async (rows) => { + const row = rows.first() + const startProjectButton = row.getByTestId('open-project') + + await expect(row).toBeVisible() + await expect(row.getByTestId('switch-to-project')).not.toBeVisible() + await expect(startProjectButton).toBeDisabled() + await expect(startProjectButton).toHaveAccessibleName( + getText('xIsUsingTheProject', 'Test User'), + ) + })) diff --git a/app/gui/integration-test/dashboard/copy.spec.ts b/app/gui/integration-test/dashboard/copy.spec.ts index b12431b9e76e..7d489deefa60 100644 --- a/app/gui/integration-test/dashboard/copy.spec.ts +++ b/app/gui/integration-test/dashboard/copy.spec.ts @@ -29,18 +29,24 @@ test('copy', ({ page }) => .createFolder() // Assets: [0: Folder 2, 1: Folder 1] .createFolder() - .driveTable.rightClickRow(0) + .driveTable.rightClickRow(1) // Assets: [0: Folder 2 , 1: Folder 1] .contextMenu.copy() - .driveTable.rightClickRow(1) + .driveTable.rightClickRow(0) // Assets: [0: Folder 2, 1: Folder 1, 2: Folder 2 (copy) ] .contextMenu.paste() .driveTable.withRows(async (rows) => { await expect(rows).toHaveCount(3) - await expect(rows.nth(2)).toBeVisible() - await expect(rows.nth(2)).toHaveText(/^New Folder 1 [(]copy[)]*/) - const parentLeft = await getAssetRowLeftPx(rows.nth(1)) - const childLeft = await getAssetRowLeftPx(rows.nth(2)) + + const child = rows.nth(1) + const parent = rows.nth(0) + + await expect(child).toBeVisible() + await expect(child).toHaveText(/^New Folder 1 [(]copy[)]*/) + + const parentLeft = await getAssetRowLeftPx(parent) + const childLeft = await getAssetRowLeftPx(child) + expect(childLeft, 'child is indented further than parent').toBeGreaterThan(parentLeft) })) @@ -50,18 +56,24 @@ test('copy (keyboard)', ({ page }) => .createFolder() // Assets: [0: Folder 2, 1: Folder 1] .createFolder() - .driveTable.clickRow(0) + .driveTable.clickRow(1) // Assets: [0: Folder 2 , 1: Folder 1] .press('Mod+C') - .driveTable.clickRow(1) + .driveTable.clickRow(0) // Assets: [0: Folder 2, 1: Folder 1, 2: Folder 2 (copy) ] .press('Mod+V') .driveTable.withRows(async (rows) => { await expect(rows).toHaveCount(3) - await expect(rows.nth(2)).toBeVisible() - await expect(rows.nth(2)).toHaveText(/^New Folder 1 [(]copy[)]*/) - const parentLeft = await getAssetRowLeftPx(rows.nth(1)) - const childLeft = await getAssetRowLeftPx(rows.nth(2)) + + const child = rows.nth(1) + const parent = rows.nth(0) + + await expect(child).toBeVisible() + await expect(child).toHaveText(/^New Folder 1 [(]copy[)]*/) + + const parentLeft = await getAssetRowLeftPx(parent) + const childLeft = await getAssetRowLeftPx(child) + expect(childLeft, 'child is indented further than parent').toBeGreaterThan(parentLeft) })) @@ -79,39 +91,58 @@ test('move', ({ page }) => .contextMenu.paste() .driveTable.withRows(async (rows) => { await expect(rows).toHaveCount(2) - await expect(rows.nth(1)).toBeVisible() - await expect(rows.nth(1)).toHaveText(/^New Folder 1/) - const parentLeft = await getAssetRowLeftPx(rows.nth(0)) - const childLeft = await getAssetRowLeftPx(rows.nth(1)) + + const child = rows.nth(1) + const parent = rows.nth(0) + + await expect(child).toBeVisible() + await expect(child).toHaveText(/^New Folder 2/) + + const parentLeft = await getAssetRowLeftPx(parent) + const childLeft = await getAssetRowLeftPx(child) + expect(childLeft, 'child is indented further than parent').toBeGreaterThan(parentLeft) })) test('move (drag)', ({ page }) => - mockAllAndLogin({ page }) - // Assets: [0: Folder 1] - .createFolder() - // Assets: [0: Folder 2, 1: Folder 1] - .createFolder() - // Assets: [0: Folder 1, 1: Folder 2 ] + mockAllAndLogin({ + page, + setupAPI: (api) => { + api.addDirectory({ + title: 'New Folder 1', + }) + api.addDirectory({ + title: 'New Folder 2', + }) + }, + }) .driveTable.dragRowToRow(0, 1) .driveTable.withRows(async (rows) => { await expect(rows).toHaveCount(2) - await expect(rows.nth(1)).toBeVisible() - await expect(rows.nth(1)).toHaveText(/^New Folder 1/) - const parentLeft = await getAssetRowLeftPx(rows.nth(0)) - const childLeft = await getAssetRowLeftPx(rows.nth(1)) + + const child = rows.nth(1) + const parent = rows.nth(0) + + await expect(child).toBeVisible() + await expect(child).toHaveText(/^New Folder 1/) + + const parentLeft = await getAssetRowLeftPx(parent) + const childLeft = await getAssetRowLeftPx(child) + expect(childLeft, 'child is indented further than parent').toBeGreaterThan(parentLeft) })) test('move to trash', ({ page }) => - mockAllAndLogin({ page }) - // Assets: [0: Folder 1] - .createFolder() - // Assets: [0: Folder 2, 1: Folder 1] - .createFolder() + mockAllAndLogin({ + page, + setupAPI: (api) => { + api.addDirectory() + api.addDirectory() + }, + }) // NOTE: For some reason, `react-aria-components` causes drag-n-drop to break if `Mod` is still // held. - .withModPressed((modActions) => modActions.driveTable.clickRow(0).driveTable.clickRow(1)) + .withModPressed((modActions) => modActions.driveTable.clickRow(1).driveTable.clickRow(0)) .driveTable.dragRow(0, locateTrashCategory(page)) .driveTable.expectPlaceholderRow() .goToCategory.trash() @@ -134,7 +165,7 @@ test('move (keyboard)', ({ page }) => .driveTable.withRows(async (rows) => { await expect(rows).toHaveCount(2) await expect(rows.nth(1)).toBeVisible() - await expect(rows.nth(1)).toHaveText(/^New Folder 1/) + await expect(rows.nth(1)).toHaveText(/^New Folder 2/) const parentLeft = await getAssetRowLeftPx(rows.nth(0)) const childLeft = await getAssetRowLeftPx(rows.nth(1)) expect(childLeft, 'child is indented further than parent').toBeGreaterThan(parentLeft) diff --git a/app/gui/integration-test/dashboard/editAssetName.spec.ts b/app/gui/integration-test/dashboard/editAssetName.spec.ts index bca1059965fa..6e952642f361 100644 --- a/app/gui/integration-test/dashboard/editAssetName.spec.ts +++ b/app/gui/integration-test/dashboard/editAssetName.spec.ts @@ -1,7 +1,7 @@ /** @file Test copying, moving, cutting and pasting. */ import { expect, test, type Locator, type Page } from '@playwright/test' -import { TEXT, mockAllAndLogin } from './actions' +import { TEXT, getText, mockAllAndLogin } from './actions' const NEW_NAME = 'foo bar baz' const NEW_NAME_2 = 'foo bar baz quux' @@ -17,6 +17,10 @@ function locateAssetRowName(locator: Locator) { return locator.getByTestId('asset-row-name') } +function locateInput(nameLocator: Locator) { + return nameLocator.getByRole('textbox') +} + /** Find a tick button. */ function locateEditingTick(page: Locator) { return page.getByLabel(TEXT.confirmEdit) @@ -35,7 +39,7 @@ test('edit name (double click)', ({ page }) => const nameEl = locateAssetRowName(row) await nameEl.click() await nameEl.click() - await nameEl.fill(NEW_NAME) + await locateInput(nameEl).fill(NEW_NAME) const calls = api.trackCalls() await locateEditingTick(row).click() await expect(row).toHaveText(new RegExp('^' + NEW_NAME)) @@ -52,10 +56,10 @@ test('edit name (context menu)', ({ page }) => .getByText(/Rename/) .click() const nameEl = locateAssetRowName(row) - await expect(nameEl).toBeVisible() - await expect(nameEl).toBeFocused() - await nameEl.fill(NEW_NAME) - await expect(nameEl).toHaveValue(NEW_NAME) + await expect(locateInput(nameEl)).toBeVisible() + await expect(locateInput(nameEl)).toBeFocused() + await locateInput(nameEl).fill(NEW_NAME) + await expect(locateInput(nameEl)).toHaveValue(NEW_NAME) const calls = api.trackCalls() await nameEl.press('Enter') await expect(row).toHaveText(new RegExp('^' + NEW_NAME)) @@ -72,7 +76,7 @@ test('edit name (keyboard)', ({ page }) => .driveTable.withRows(async (rows, _, { api }) => { const row = rows.nth(0) const nameEl = locateAssetRowName(row) - await nameEl.fill(NEW_NAME_2) + await locateInput(nameEl).fill(NEW_NAME_2) const calls = api.trackCalls() await nameEl.press('Enter') await expect(row).toHaveText(new RegExp('^' + NEW_NAME_2)) @@ -88,7 +92,7 @@ test('cancel editing name (double click)', ({ page }) => const oldName = (await nameEl.textContent()) ?? '' await nameEl.click() await nameEl.click() - await nameEl.fill(NEW_NAME) + await nameEl.getByTestId('input').fill(NEW_NAME) const calls = api.trackCalls() await locateEditingCross(row).click() await expect(row).toHaveText(new RegExp('^' + oldName)) @@ -107,7 +111,7 @@ test('cancel editing name (keyboard)', ({ page }) => { const row = rows.nth(0) const nameEl = locateAssetRowName(row) oldName = (await nameEl.textContent()) ?? '' - await nameEl.fill(NEW_NAME_2) + await nameEl.getByTestId('input').fill(NEW_NAME_2) const calls = api.trackCalls() await nameEl.press('Escape') await expect(row).toHaveText(new RegExp('^' + oldName)) @@ -124,8 +128,8 @@ test('change to blank name (double click)', ({ page }) => const oldName = (await nameEl.textContent()) ?? '' await nameEl.click() await nameEl.click() - await nameEl.fill('') - await expect(locateEditingTick(row)).not.toBeVisible() + await nameEl.getByTestId('input').fill('') + await expect(locateEditingTick(row)).toBeVisible() const calls = api.trackCalls() await locateEditingCross(row).click() await expect(row).toHaveText(new RegExp('^' + oldName)) @@ -143,9 +147,189 @@ test('change to blank name (keyboard)', ({ page }) => const row = rows.nth(0) const nameEl = locateAssetRowName(row) const oldName = (await nameEl.textContent()) ?? '' - await nameEl.fill('') + await nameEl.getByTestId('input').fill('') const calls = api.trackCalls() await nameEl.press('Enter') await expect(row).toHaveText(new RegExp('^' + oldName)) expect(calls.updateDirectory).toMatchObject([]) })) + +test('edit name, error message is visible', ({ page }) => { + return mockAllAndLogin({ + page, + setupAPI: (api) => { + for (let i = 0; i < 100; i++) { + api.addProject({ title: 'Some Project ' + i }) + } + }, + }).driveTable.withRows(async (rows) => { + const row = rows.nth(0) + await locateAssetRowName(row).click() + + const nameEl = locateAssetRowName(row) + await nameEl.click() + await nameEl.click() + + const inputEl = locateInput(nameEl) + + await inputEl.fill('') + + await locateEditingTick(row).click() + + const formElement = row.getByTestId('editable-span-form') + const errorOutline = formElement.getByTestId('error-message-outline') + const errorText = formElement.getByTestId('error-message-text') + + await expect(errorOutline).toBeVisible() + // Clicking the element to be sure it's not overlapped by another element. + await errorText.click() + + await inputEl.fill('Another Project') + await locateEditingTick(row).click() + + await expect(errorOutline).not.toBeAttached() + await expect(errorText).not.toBeAttached() + }) +}) + +test('edit name (empty name)', ({ page }) => + mockAllAndLogin({ + page, + setupAPI: (api) => { + api.addProject({ title: 'Some Project' }) + api.addProject({ title: 'Other Project' }) + api.addProject({ title: 'Yet Another Project' }) + }, + }).driveTable.withRows(async (rows) => { + const row = rows.nth(0) + await locateAssetRowName(row).click() + + const nameEl = locateAssetRowName(row) + await nameEl.click() + await nameEl.click() + + const inputEl = locateInput(nameEl) + + await inputEl.fill('') + + await locateEditingTick(row).click() + + const formElement = row.getByTestId('editable-span-form') + const errorOutline = formElement.getByTestId('error-message-outline') + const errorContainer = formElement.getByTestId('error-message-container') + const errorText = formElement.getByTestId('error-message-text') + + await expect(errorOutline).toBeVisible() + await expect(errorContainer).toBeVisible() + await expect(errorText).toHaveText(getText('arbitraryFieldRequired')) + + await inputEl.fill('Another Project') + await locateEditingTick(row).click() + + await expect(row).toHaveText(/^Another Project/) + await expect(errorOutline).not.toBeAttached() + await expect(errorContainer).not.toBeAttached() + })) + +test('edit name (invalid name)', ({ page }) => + mockAllAndLogin({ + page, + setupAPI: (api) => { + api.addDirectory({ title: 'Some Directory' }) + }, + }).driveTable.withRows(async (rows) => { + const row = rows.nth(0) + await locateAssetRowName(row).click() + + const nameEl = locateAssetRowName(row) + await nameEl.click() + await nameEl.click() + + const inputEl = locateInput(nameEl) + + await inputEl.fill('../') + + await locateEditingTick(row).click() + + const formElement = row.getByTestId('editable-span-form') + const errorOutline = formElement.getByTestId('error-message-outline') + const errorContainer = formElement.getByTestId('error-message-container') + const errorText = formElement.getByTestId('error-message-text') + + await expect(errorOutline).toBeVisible() + await expect(errorContainer).toBeVisible({ + visible: true, + }) + await expect(errorText).toHaveText(getText('nameShouldNotContainInvalidCharacters')) + + await inputEl.fill('Other Directory') + await locateEditingTick(row).click() + + await expect(row).toHaveText(/^Other Directory/) + })) + +test('edit name (duplicate name)', ({ page }) => + mockAllAndLogin({ + page, + setupAPI: (api) => { + api.addDirectory({ title: 'Some Directory' }) + api.addProject({ title: 'Some Project' }) + }, + }).driveTable.withRows(async (rows) => { + const row = rows.nth(0) + await locateAssetRowName(row).click() + + const nameEl = locateAssetRowName(row) + await nameEl.click() + await nameEl.click() + + const inputEl = locateInput(nameEl) + + await inputEl.fill('Some Project') + await locateEditingTick(row).click() + + const formElement = row.getByTestId('editable-span-form') + const errorText = formElement.getByTestId('error-message-text') + + await expect(errorText).toHaveText(getText('nameShouldBeUnique')) + + await inputEl.fill('Other Directory') + await locateEditingTick(row).click() + + await expect(row).toHaveText(/^Other Directory/) + })) + +test('error should not overlay the table header', ({ page }) => + mockAllAndLogin({ + page, + setupAPI: (api) => { + for (let i = 0; i < 100; i++) { + api.addProject({ title: 'Some Project ' + i }) + } + }, + }).driveTable.withRows(async (rows) => { + const row = rows.nth(1) + await locateAssetRowName(row).click() + + const nameEl = locateAssetRowName(row) + await nameEl.click() + await nameEl.click() + + const inputEl = locateInput(nameEl) + + await inputEl.fill('Some Project 0') + await locateEditingTick(row).click() + + const formElement = row.getByTestId('editable-span-form') + const errorOutline = formElement.getByTestId('error-message-outline') + const errorContainer = formElement.getByTestId('error-message-container') + const errorText = formElement.getByTestId('error-message-text') + + await expect(errorText).toHaveText(getText('nameShouldBeUnique')) + + await rows.nth(51).scrollIntoViewIfNeeded() + + await expect(errorOutline).not.toBeInViewport() + await expect(errorContainer).not.toBeInViewport() + await expect(errorText).not.toBeInViewport() + })) diff --git a/app/gui/integration-test/dashboard/pageSwitcher.spec.ts b/app/gui/integration-test/dashboard/pageSwitcher.spec.ts index f59e9cf3da20..de26fe3bded7 100644 --- a/app/gui/integration-test/dashboard/pageSwitcher.spec.ts +++ b/app/gui/integration-test/dashboard/pageSwitcher.spec.ts @@ -15,10 +15,11 @@ function locateDriveView(page: Page) { return page.getByTestId('drive-view') } -// FIXME[sb]: https://github.com/enso-org/cloud-v2/issues/1615 -// Unskip once cloud execution in the browser is re-enabled. -test.skip('page switcher', ({ page }) => - mockAllAndLogin({ page }) +test('page switcher', ({ page }) => + mockAllAndLogin({ + page, + setupAPI: (api) => api.setFeatureFlags({ enableCloudExecution: true }), + }) // Create a new project so that the editor page can be switched to. .newEmptyProjectTest() .do(async (thePage) => { diff --git a/app/gui/integration-test/dashboard/sort.spec.ts b/app/gui/integration-test/dashboard/sort.spec.ts index ebcfdbdac550..16483dcb6672 100644 --- a/app/gui/integration-test/dashboard/sort.spec.ts +++ b/app/gui/integration-test/dashboard/sort.spec.ts @@ -80,14 +80,14 @@ test('sort', ({ page }) => // By default, assets should be grouped by type. // Assets in each group are ordered by insertion order. await expect(rows).toHaveText([ - /^a directory/, /^G directory/, + /^a directory/, /^C project/, /^b project/, /^d file/, /^e file/, - /^H secret/, /^f secret/, + /^H secret/, ]) }) // Sort by name ascending. @@ -135,14 +135,14 @@ test('sort', ({ page }) => }) .driveTable.withRows(async (rows) => { await expect(rows).toHaveText([ - /^a directory/, /^G directory/, + /^a directory/, /^C project/, /^b project/, /^d file/, /^e file/, - /^H secret/, /^f secret/, + /^H secret/, ]) }) // Sort by date ascending. @@ -190,13 +190,13 @@ test('sort', ({ page }) => }) .driveTable.withRows(async (rows) => { await expect(rows).toHaveText([ - /^a directory/, /^G directory/, + /^a directory/, /^C project/, /^b project/, /^d file/, /^e file/, - /^H secret/, /^f secret/, + /^H secret/, ]) })) diff --git a/app/gui/integration-test/dashboard/startModal.spec.ts b/app/gui/integration-test/dashboard/startModal.spec.ts index 411825adb944..3c1ef1b6b8cb 100644 --- a/app/gui/integration-test/dashboard/startModal.spec.ts +++ b/app/gui/integration-test/dashboard/startModal.spec.ts @@ -21,12 +21,9 @@ function locateSamples(page: Page) { return locateSamplesList(page).getByRole('button') } -// FIXME[sb]: https://github.com/enso-org/cloud-v2/issues/1615 -// Unskip once cloud execution in the browser is re-enabled. - -test.skip('create project from template', ({ page }) => - mockAllAndLogin({ page }) - .expectStartModal() +test('create project from template', ({ page }) => + mockAllAndLogin({ page, setupAPI: (api) => api.setFeatureFlags({ enableCloudExecution: true }) }) + .openStartModal() .createProjectFromTemplate(0) .do(async (thePage) => { await expect(locateEditor(thePage)).toBeAttached() diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx b/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx index b04f2f2756bd..eccad3d50ef5 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/Field.tsx @@ -109,7 +109,7 @@ export const Field = forwardRef(function Field( {label} {isRequired && ( -
- -
+ +
{ if (isAssetContextMenuVisible) { event.preventDefault() @@ -1806,13 +1807,13 @@ function AssetsTable(props: AssetsTableProps) { } }} > - +
{headerRow} {/* Dummy row to increase the gap between the header and the first row */} - + {itemRows} - {/* Dummy row to increase the gap between the header and the first row */} - {itemRows} : displayItems.map((item) => { + const isOpenedByYou = openedProjects.some(({ id }) => item.item.id === id) + const isOpenedOnTheBackend = + item.item.projectState?.type != null ? + IS_OPENING_OR_OPENED[item.item.projectState.type] + : false return ( item.item.id === id)} + isOpened={isOpenedByYou || isOpenedOnTheBackend} visibility={visibilities.get(item.key)} columns={columns} id={item.item.id} diff --git a/app/gui/src/dashboard/layouts/Drive/assetsTableItemsHooks.tsx b/app/gui/src/dashboard/layouts/Drive/assetsTableItemsHooks.tsx index 3f5cafcb119d..18b7792824dc 100644 --- a/app/gui/src/dashboard/layouts/Drive/assetsTableItemsHooks.tsx +++ b/app/gui/src/dashboard/layouts/Drive/assetsTableItemsHooks.tsx @@ -43,9 +43,7 @@ export function useAsset(id: AssetId) { return useStore( ASSET_ITEMS_STORE, (store) => store.items.find((item) => item.id === id) ?? null, - { - unsafeEnableTransition: true, - }, + { unsafeEnableTransition: true }, ) } From a0333b0925bfb51d379937f8f656c851562488a8 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Thu, 19 Dec 2024 18:28:38 +0400 Subject: [PATCH 18/21] Fix tests --- app/gui/integration-test/dashboard/actions/index.ts | 3 +-- .../dashboard/assetsTableFeatures.spec.ts | 8 ++++---- .../src/dashboard/pages/authentication/LoadingScreen.tsx | 5 ++++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/gui/integration-test/dashboard/actions/index.ts b/app/gui/integration-test/dashboard/actions/index.ts index d83238589606..a83e3e37fad2 100644 --- a/app/gui/integration-test/dashboard/actions/index.ts +++ b/app/gui/integration-test/dashboard/actions/index.ts @@ -72,8 +72,7 @@ async function login({ page }: MockParams, email = 'email@example.com', password async function waitForLoaded(page: Page) { await page.waitForLoadState() - await expect(page.getByTestId('spinner')).toHaveCount(0) - await expect(page.getByTestId('loading-app-message')).not.toBeVisible({ timeout: 30_000 }) + await expect(page.getByTestId('loading-screen')).toHaveCount(0, { timeout: 30_000 }) } /** Wait for the dashboard to load. */ diff --git a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts index 86e68a69bdea..3be7e5461dbc 100644 --- a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts +++ b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts @@ -157,11 +157,11 @@ test("can't start an already running by another user", ({ page }) => }).driveTable.withRows(async (rows) => { const row = rows.first() const startProjectButton = row.getByTestId('open-project') + const stopProjectButton = row.getByTestId('stop-project') await expect(row).toBeVisible() await expect(row.getByTestId('switch-to-project')).not.toBeVisible() - await expect(startProjectButton).toBeDisabled() - await expect(startProjectButton).toHaveAccessibleName( - getText('xIsUsingTheProject', 'Test User'), - ) + await expect(startProjectButton).not.toBeVisible() + await expect(stopProjectButton).toBeDisabled() + await expect(stopProjectButton).toHaveAccessibleName(getText('xIsUsingTheProject', 'Test User')) })) diff --git a/app/gui/src/dashboard/pages/authentication/LoadingScreen.tsx b/app/gui/src/dashboard/pages/authentication/LoadingScreen.tsx index 4f445fd4f44a..9b172463f1aa 100644 --- a/app/gui/src/dashboard/pages/authentication/LoadingScreen.tsx +++ b/app/gui/src/dashboard/pages/authentication/LoadingScreen.tsx @@ -19,7 +19,10 @@ export default function LoadingScreen() { const { getText } = useText() return ( -
+
From e7840ca919943986e55a6942e7a86f0667b4a0ae Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 19 Dec 2024 12:59:35 +0100 Subject: [PATCH 19/21] Promote broken values instead of ignoring them (#11777) Partially fixes #5430 by propagating `DataflowError`s found during statement execution out of the method. # Important Notes This change [may affect behavior](https://github.com/enso-org/enso/pull/11673/files#r1871128327) of existing methods that ignore `DataflowError` as [discussed here](https://github.com/enso-org/enso/pull/11673/files#r1871128327). --- CHANGELOG.md | 2 + .../0.0.0-dev/src/Data/Index_Sub_Range.enso | 10 +- .../Base/0.0.0-dev/src/Data/Range.enso | 5 +- .../0.0.0-dev/src/Data/Time/Date_Range.enso | 5 +- .../src/Internal/Array_Like_Helpers.enso | 2 +- .../Test/0.0.0-dev/src/Extensions.enso | 105 ++++++++------ .../0.0.0-dev/src/Extensions_Helpers.enso | 14 ++ docs/types/errors.md | 134 ++++++++++++------ .../DataflowErrorPropagationTest.java | 86 +++++++++++ .../node/callable/function/BlockNode.java | 19 ++- test/Base_Tests/src/Data/Decimal_Spec.enso | 4 +- test/Base_Tests/src/Data/Function_Spec.enso | 1 - test/Base_Tests/src/Data/Range_Spec.enso | 32 +++-- .../src/Data/Time/Date_Range_Spec.enso | 8 +- test/Base_Tests/src/Random_Spec.enso | 1 - test/Base_Tests/src/System/File_Spec.enso | 2 +- .../Cross_Tab_Spec.enso | 1 - .../Common_Table_Operations/Nothing_Spec.enso | 3 +- .../Table_Tests/src/Database/SQLite_Spec.enso | 27 ++-- .../Table_Tests/src/Database/Upload_Spec.enso | 2 +- test/Table_Tests/src/IO/Fetch_Spec.enso | 16 ++- .../src/In_Memory/Split_Tokenize_Spec.enso | 11 +- test/Table_Tests/src/Util.enso | 18 ++- test/Table_Tests/src/Util_Spec.enso | 66 ++++++++- test/Test_Tests/package.yaml | 7 + test/Test_Tests/src/Extensions_Spec.enso | 86 +++++++++++ test/Test_Tests/src/Helpers.enso | 17 +++ test/Test_Tests/src/Main.enso | 13 ++ 28 files changed, 540 insertions(+), 157 deletions(-) create mode 100644 distribution/lib/Standard/Test/0.0.0-dev/src/Extensions_Helpers.enso create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/semantic/DataflowErrorPropagationTest.java create mode 100644 test/Test_Tests/package.yaml create mode 100644 test/Test_Tests/src/Extensions_Spec.enso create mode 100644 test/Test_Tests/src/Helpers.enso create mode 100644 test/Test_Tests/src/Main.enso diff --git a/CHANGELOG.md b/CHANGELOG.md index b1e53b92c712..1d4db090dd76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,13 @@ #### Enso Language & Runtime +- [Promote broken values instead of ignoring them][11777]. - [Intersection types & type checks][11600] - A constructor or type definition with a single inline argument definition was previously allowed to use spaces in the argument definition without parentheses. [This is now a syntax error.][11856] +[11777]: https://github.com/enso-org/enso/pull/11777 [11600]: https://github.com/enso-org/enso/pull/11600 [11856]: https://github.com/enso-org/enso/pull/11856 diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso index 95dc0af8ac17..360497b52657 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso @@ -204,9 +204,8 @@ take_helper length at single_slice slice_ranges range:(Index_Sub_Range | Range | Index_Sub_Range.First count -> single_slice 0 (length.min count) Index_Sub_Range.Last count -> single_slice length-count length Index_Sub_Range.While predicate -> - end = 0.up_to length . find i-> (predicate (at i)).not - true_end = if end.is_nothing then length else end - single_slice 0 true_end + end = 0.up_to length . find (i-> (predicate (at i)).not) if_missing=length + single_slice 0 end Index_Sub_Range.By_Index one_or_many_descriptors -> Panic.recover [Index_Out_Of_Bounds, Illegal_Argument] <| indices = case one_or_many_descriptors of _ : Vector -> one_or_many_descriptors @@ -255,9 +254,8 @@ drop_helper length at single_slice slice_ranges range:(Index_Sub_Range | Range | Index_Sub_Range.First count -> single_slice count length Index_Sub_Range.Last count -> single_slice 0 length-count Index_Sub_Range.While predicate -> - end = 0.up_to length . find i-> (predicate (at i)).not - true_end = if end.is_nothing then length else end - single_slice true_end length + end = 0.up_to length . find (i-> (predicate (at i)).not) if_missing=length + single_slice end length Index_Sub_Range.By_Index one_or_many_descriptors -> Panic.recover [Index_Out_Of_Bounds, Illegal_Argument] <| indices = case one_or_many_descriptors of _ : Vector -> one_or_many_descriptors diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Range.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Range.enso index 8c588cb434cd..2064d7927fdb 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Range.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Range.enso @@ -7,6 +7,7 @@ import project.Data.Text.Text import project.Data.Vector.Vector import project.Error.Error import project.Errors.Common.Index_Out_Of_Bounds +import project.Errors.Common.Not_Found import project.Errors.Empty_Error.Empty_Error import project.Errors.Illegal_Argument.Illegal_Argument import project.Errors.Illegal_State.Illegal_State @@ -397,7 +398,7 @@ type Range @condition range_default_filter_condition_widget any : (Filter_Condition | (Integer -> Boolean)) -> Boolean any self (condition : Filter_Condition | (Integer -> Boolean)) = - self.find condition . is_nothing . not + self.find condition if_missing=Nothing . is_nothing . not ## GROUP Selections ICON find @@ -422,7 +423,7 @@ type Range 1.up_to 100 . find (..Greater than=10) @condition range_default_filter_condition_widget find : (Filter_Condition | (Integer -> Boolean)) -> Integer -> Any -> Any - find self (condition : Filter_Condition | (Integer -> Boolean)) (start : Integer = 0) ~if_missing=Nothing = + find self (condition : Filter_Condition | (Integer -> Boolean)) (start : Integer = 0) ~if_missing=(Error.throw Not_Found) = predicate = unify_condition_or_predicate condition check_start_valid start self used_start-> result = find_internal self used_start predicate diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Range.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Range.enso index e51d9f12062e..3f0d9077dfba 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Range.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Range.enso @@ -11,6 +11,7 @@ import project.Data.Time.Period.Period import project.Data.Vector.Vector import project.Error.Error import project.Errors.Common.Index_Out_Of_Bounds +import project.Errors.Common.Not_Found import project.Errors.Empty_Error.Empty_Error import project.Errors.Illegal_Argument.Illegal_Argument import project.Function.Function @@ -418,7 +419,7 @@ type Date_Range @condition date_range_default_filter_condition_widget any : (Filter_Condition | (Date -> Boolean)) -> Boolean any self (condition : Filter_Condition | (Date -> Boolean)) = - self.find condition . is_nothing . not + self.find condition if_missing=Nothing . is_nothing . not ## GROUP Selections ICON find @@ -438,7 +439,7 @@ type Date_Range (Date.new 2020 10 01).up_to (Date.new 2020 10 31) . find (d-> d.day_of_week == Day_Of_Week.Monday) @condition date_range_default_filter_condition_widget find : (Filter_Condition | (Date -> Boolean)) -> Integer -> Any -> Any - find self (condition : Filter_Condition | (Date -> Boolean)) (start : Integer = 0) ~if_missing=Nothing = + find self (condition : Filter_Condition | (Date -> Boolean)) (start : Integer = 0) ~if_missing=(Error.throw Not_Found) = predicate = unify_condition_or_predicate condition index = self.index_of predicate start case index of diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Array_Like_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Array_Like_Helpers.enso index 131d1de9fd9f..b2351bcde3b7 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Array_Like_Helpers.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Array_Like_Helpers.enso @@ -216,7 +216,7 @@ find vector condition start ~if_missing = predicate = unify_condition_or_predicate condition self_len = vector.length check_start_valid start self_len used_start-> - found = used_start.up_to self_len . find (idx -> (predicate (vector.at idx))) + found = used_start.up_to self_len . find (idx -> (predicate (vector.at idx))) if_missing=Nothing if found.is_nothing then if_missing else vector.at found transpose vec_of_vecs = diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso index 46741a56a5c0..ff852368a507 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso @@ -1,9 +1,11 @@ from Standard.Base import all +import Standard.Base.Errors.Common.Incomparable_Values import Standard.Base.Errors.Common.No_Such_Method import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import project.Spec_Result.Spec_Result import project.Test.Test +from project.Extensions_Helpers import rhs_error_check ## Expect a function to fail with the provided dataflow error. @@ -70,23 +72,25 @@ Error.should_fail_with self matcher frames_to_skip=0 unwrap_errors=True = example_should_equal = Examples.add_1_to 1 . should_equal 2 Any.should_equal : Any -> Integer -> Spec_Result -Any.should_equal self that frames_to_skip=0 = case self == that of - True -> Spec_Result.Success - False -> - loc = Meta.get_source_location 2+frames_to_skip - additional_comment = case self of - _ : Vector -> case that of - _ : Vector -> - case self.length == that.length of - True -> - diff = self.zip that . index_of p-> - p.first != p.second - "; first difference at index " + diff.to_text + " " - False -> "; lengths differ (" + self.length.to_text + " != " + that.length.to_text + ") " +Any.should_equal self that frames_to_skip=0 = + rhs_error_check that + loc = Meta.get_source_location 1+frames_to_skip + case self == that of + True -> Spec_Result.Success + False -> + additional_comment = case self of + _ : Vector -> case that of + _ : Vector -> + case self.length == that.length of + True -> + diff = self.zip that . index_of p-> + p.first != p.second + "; first difference at index " + diff.to_text + " " + False -> "; lengths differ (" + self.length.to_text + " != " + that.length.to_text + ") " + _ -> "" _ -> "" - _ -> "" - msg = self.pretty + " did not equal " + that.pretty + additional_comment + " (at " + loc + ")." - Test.fail msg + msg = self.pretty + " did not equal " + that.pretty + additional_comment + " (at " + loc + ")." + Test.fail msg ## Asserts that `self` value is equal to the expected type value. @@ -130,12 +134,13 @@ Error.should_equal_type self that frames_to_skip=0 = example_should_not_equal = Examples.add_1_to 1 . should_not_equal 2 Any.should_not_equal : Any -> Integer -> Spec_Result -Any.should_not_equal self that frames_to_skip=0 = case self != that of - True -> Spec_Result.Success - False -> - loc = Meta.get_source_location 2+frames_to_skip - msg = self.to_text + " did equal " + that.to_text + " (at " + loc + ")." - Test.fail msg +Any.should_not_equal self that frames_to_skip=0 = if that.is_error then (Panic.throw (Illegal_Argument.Error "Expected value provided as `that` for `should_not_equal` cannot be an error, but got: "+that.to_display_text)) else + loc = Meta.get_source_location 2+frames_to_skip + case self != that of + True -> Spec_Result.Success + False -> + msg = self.to_text + " did equal " + that.to_text + " (at " + loc + ")." + Test.fail msg ## Added so that dataflow errors are not silently lost. Error.should_not_equal self that frames_to_skip=0 = @@ -183,15 +188,16 @@ Error.should_not_equal_type self that frames_to_skip=0 = example_should_start_with = "Hello World!" . should_start_with "Hello" Any.should_start_with : Text -> Integer -> Spec_Result -Any.should_start_with self that frames_to_skip=0 = case self of - _ : Text -> if self.starts_with that then Spec_Result.Success else - loc = Meta.get_source_location 3+frames_to_skip - msg = self.to_text + " does not start with " + that.to_text + " (at " + loc + ")." - Test.fail msg - _ -> - loc = Meta.get_source_location 2+frames_to_skip - msg = self.to_text + " is not a `Text` value (at " + loc + ")." - Test.fail msg +Any.should_start_with self that frames_to_skip=0 = + rhs_error_check that + loc = Meta.get_source_location 1+frames_to_skip + case self of + _ : Text -> if self.starts_with that then Spec_Result.Success else + msg = self.to_text + " does not start with " + that.to_text + " (at " + loc + ")." + Test.fail msg + _ -> + msg = self.to_text + " is not a `Text` value (at " + loc + ")." + Test.fail msg ## Asserts that `self` value is a Text value and ends with `that`. @@ -207,15 +213,16 @@ Any.should_start_with self that frames_to_skip=0 = case self of example_should_end_with = "Hello World!" . should_end_with "ld!" Any.should_end_with : Text -> Integer -> Spec_Result -Any.should_end_with self that frames_to_skip=0 = case self of - _ : Text -> if self.ends_with that then Spec_Result.Success else - loc = Meta.get_source_location 3+frames_to_skip - msg = self.to_text + " does not end with " + that.to_text + " (at " + loc + ")." - Test.fail msg - _ -> - loc = Meta.get_source_location 2+frames_to_skip - msg = self.to_text + " is not a `Text` value (at " + loc + ")." - Test.fail msg +Any.should_end_with self that frames_to_skip=0 = + rhs_error_check that + loc = Meta.get_source_location 1+frames_to_skip + case self of + _ : Text -> if self.ends_with that then Spec_Result.Success else + msg = self.to_text + " does not end with " + that.to_text + " (at " + loc + ")." + Test.fail msg + _ -> + msg = self.to_text + " is not a `Text` value (at " + loc + ")." + Test.fail msg ## Asserts that `self` value is a Text value and starts with `that`. @@ -267,7 +274,7 @@ Error.should_end_with self that frames_to_skip=0 = example_should_equal = Examples.add_1_to 1 . should_equal 2 Error.should_equal : Any -> Integer -> Spec_Result Error.should_equal self that frames_to_skip=0 = - _ = [that] + rhs_error_check that Test.fail_match_on_unexpected_error self 1+frames_to_skip ## Asserts that `self` is within `epsilon` from `that`. @@ -294,13 +301,18 @@ Error.should_equal self that frames_to_skip=0 = 1.00000001 . should_equal 1.00000002 epsilon=0.0001 Number.should_equal : Float -> Float -> Integer -> Spec_Result Number.should_equal self that epsilon=0 frames_to_skip=0 = + rhs_error_check that + loc = Meta.get_source_location 1+frames_to_skip matches = case that of - n : Number -> self.equals n epsilon + n : Number -> self.equals n epsilon . catch Incomparable_Values _-> + ## Incomparable_Values is thrown if one of the values is NaN. + We fallback to is_same_object_as, + because in tests we actually NaN.should_equal NaN to succeed. + self.is_same_object_as n _ -> self==that case matches of True -> Spec_Result.Success False -> - loc = Meta.get_source_location 2+frames_to_skip msg = self.to_text + " did not equal " + that.to_text + " (at " + loc + ")." Test.fail msg @@ -313,6 +325,7 @@ Number.should_equal self that epsilon=0 frames_to_skip=0 = displayed as the source of this error. Decimal.should_equal : Number -> Float-> Float -> Integer -> Spec_Result Decimal.should_equal self that epsilon=0 frames_to_skip=0 = + rhs_error_check that self.to_float . should_equal that.to_float epsilon frames_to_skip+1 ## Asserts that `self` value is not an error. @@ -423,6 +436,7 @@ Error.should_be_false self = Test.fail_match_on_unexpected_error self 1 example_should_be_a = 1.should_be_a Boolean Any.should_be_a : Any -> Spec_Result Any.should_be_a self typ = + rhs_error_check typ loc = Meta.get_source_location 1 fail_on_wrong_arg_type = Panic.throw <| @@ -490,6 +504,8 @@ Any.should_be_a self typ = Any.should_equal_ignoring_order : Any -> Integer -> Spec_Result Any.should_equal_ignoring_order self that frames_to_skip=0 = loc = Meta.get_source_location 1+frames_to_skip + if that.is_a Vector . not then + Panic.throw (Illegal_Argument.Error "Expected a Vector, but got a "+that.to_display_text+" (at "+loc+").") that.each element-> if self.contains element . not then msg = "The collection (" + self.to_text + ") did not contain " + element.to_text + " (at " + loc + ")." @@ -556,6 +572,7 @@ Error.should_equal_ignoring_order self that frames_to_skip=0 = example_should_equal = [1, 2] . should_only_contain_elements_in [1, 2, 3, 4] Any.should_only_contain_elements_in : Any -> Integer -> Spec_Result Any.should_only_contain_elements_in self that frames_to_skip=0 = + rhs_error_check that loc = Meta.get_source_location 1+frames_to_skip self.each element-> if that.contains element . not then @@ -609,6 +626,7 @@ Error.should_only_contain_elements_in self that frames_to_skip=0 = example_should_equal = "foobar".should_contain "foo" Any.should_contain : Any -> Integer -> Spec_Result Any.should_contain self element frames_to_skip=0 = + rhs_error_check element loc = Meta.get_source_location 1+frames_to_skip contains_result = Panic.catch No_Such_Method (self.contains element) caught_panic-> if caught_panic.payload.method_name != "contains" then Panic.throw caught_panic else @@ -652,6 +670,7 @@ Error.should_contain self element frames_to_skip=0 = implementing a method `contains : a -> Boolean`. Any.should_not_contain : Any -> Integer -> Spec_Result Any.should_not_contain self element frames_to_skip=0 = + rhs_error_check element loc = Meta.get_source_location 1+frames_to_skip contains_result = Panic.catch No_Such_Method (self.contains element) caught_panic-> if caught_panic.payload.method_name != "contains" then Panic.throw caught_panic else diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions_Helpers.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions_Helpers.enso new file mode 100644 index 000000000000..73f4437ef8bf --- /dev/null +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions_Helpers.enso @@ -0,0 +1,14 @@ +private + +from Standard.Base import all +import Standard.Base.Errors.Illegal_Argument.Illegal_Argument + +## PRIVATE + A helper that ensures that the expected value provided in some of the Test + operations is not an error. + The left-hand side may be an error and that will cause a test failure. + But the right-hand side being an error is bad test design and should be fixed. +rhs_error_check that = + if that.is_error then + msg = "Dataflow error ("+that.to_display_text+") provided as expected value. Use `should_fail_with` or change the test."+ ' Error stack trace was:\n'+that.get_stack_trace_text + Panic.throw (Illegal_Argument.Error msg) diff --git a/docs/types/errors.md b/docs/types/errors.md index 653645734301..8d0aeb4f3187 100644 --- a/docs/types/errors.md +++ b/docs/types/errors.md @@ -1,15 +1,17 @@ --- layout: developer-doc -title: Errors +title: Errors & Panics category: types tags: [types, errors] order: 12 --- -# Errors +# Errors & Panics -Enso supports two notions of errors. One is the standard exceptions model, while -the other is a theory of 'broken values' that propagate through computations. +Enso supports two notions of errors. One is the standard exceptions model (built +around `Panic.throw` and related methods), while the other is a theory of +_broken values_ that propagate through computations (represented by `Error` and +created by `Error.throw` method). > [!WARNING] The actionables for this section are: > @@ -19,65 +21,109 @@ the other is a theory of 'broken values' that propagate through computations. -- [Async Exceptions](#async-exceptions) +- [Exceptions/Panics](#errors--panics) - [Broken Values](#broken-values) -## Async Exceptions +## Exceptions/Panics > [!WARNING] The actionables for this section are: > -> - why is this called _"asynchronous"_ when the `Panic` is raised -> synchronously? -> - Formalise the model of async exceptions as implemented. +> - Formalise the model of `Panic.throw` as implemented. ## Broken Values -In Enso we have the notion of a 'broken' value: one which is in an invalid state -but not an error. While these may initially seem a touch useless, they are -actually key for the display of errors in the GUI. +In Enso we have the notion of a _broken value_: one which is in an invalid +state. Such values are very useful for displaying errors in the GUI. -Broken values can be thought of like checked monadic exceptions in Haskell, but -with an automatic propagation mechanism: +Broken values are fast to allocate and pass around the program. They record line +of their own creation - e.g. where `Error.throw` has happened. Shall that not be +enough, one can run with `-ea` flag, like: -- Broken values that aren't handled explicitly are automatically promoted - through the parent scope. This is trivial inference as no evidence discharge - will have occurred on the value. +```bash +enso$ JAVA_OPTS=-ea ./built-distribution/enso-engine-*/enso-*/bin/enso --run x.enso +``` - ```ruby - open : String -> String in IO ! IO.Exception - open = ... +to get full stack where the _broken value_ has been created. Collecting such +full stack trace however prevents the execution to run at _full speed_. - test = - print 'Opening the gates!' - txt = open 'gates.txt' - print 'Gates were opened!' - 7 - ``` +### Promotion of Broken Values - In the above example, the type of test is inferred to - `test : Int in IO ! IO.Exception`, because no evidence discharge has taken - place as the potential broken value hasn't been handled. +Broken values that aren't handled explicitly are automatically promoted through +the parent scope. Let's assume an `open` function that can yield a `Text` or +_broken value_ representing a `File_Error`: -- This allows for very natural error handling in the GUI. +```ruby +open file_name:Text -> Text ! File_Error = ... +``` -> [!WARNING] The actionables for this section are: -> -> - Determine what kinds of APIs we want to use async exceptions for, and which -> broken values are more suited for. -> - Ensure that we are okay with initially designing everything around async -> exceptions as broken values are very hard to support without a type checker. -> - Initially not supported for APIs. +Then imagine following `test` function trying to open a non-existing file +`gates.txt` + +```ruby +test = + IO.println 'Opening the gates!' + open 'gates.txt' + IO.println 'Gates were opened!' +``` + +Execution of such function will: + +- print `Opening the gates!` text +- finish with `File_Error` _broken value_ +- **not print** `Gates were opened!` + +E.g. the execution of a function body ends after first _uhandled broken value_. + +### Propagation of Broken Values -Broken values (implemented as `DataflowError` class in the interpreter) are fast -to allocate and pass around the program. They record line of their own -creation - e.g. where `Error.throw` has happened. Shall that not be enough, one -can run with `-ea` flag, like: +Let's modify the previous example a bit. Let's assign the read text (or _broken +value_) to a variable and return it from the `test` function: + +```ruby +test = + IO.println 'Opening the gates!' + content = open 'gates.txt' + IO.println 'Gates were opened!' + content +``` + +If the `gates.txt` file exists, its content is returned from the `test` +function. If a `File_Error` _broken value_ is returned from the `open` function, +then the variable `content` will contain such a _broken value_ and as `content` +is the return value from the `test` function, the `File_Error` will be returned +from the `test` function and propagated further as a _broken value_. + +In both situations (if the file exists or not) both `IO.println` statements are +executed and the execution of `test` function thus prints both +`Opening the gates!` as well as `Gates were opened!`. + +### Detection of Unused Broken Values + +Should the last statement (e.g. `content`) of the `test` function defined in +previous section be missing, then the _broken value_ assigned to `content` +variable might _"disappear"_ unnoticed. However in such a situation the Enso +compiler emits a _compile time warning_: ```bash -enso$ JAVA_OPTS=-ea ./built-distribution/enso-engine-*/enso-*/bin/enso --run x.enso +test.enso:3:3: warning: Unused variable content. + 3 | content = open 'gates.txt' + | ^~~~~~~ ``` -to get full stack where the _broken value_ has been created. Collecting such -full stack trace however prevents the execution to run at _full speed_. +The combination of _detection_, _propagation_ and _promotion_ of _broken values_ +ensures `File_Error` and other _broken values_ are **never lost** +(unintentionally). Should _loosing a broken value_ be a goal, one can change the +line in question to: + +```ruby + _ = open 'gates.txt' +``` + +e.g. assign it to anonymous variable. That signals to the system one doesn't +care about the result of the `open` function call. No _compiler warning_ is thus +reported and the _broken value_ gets lost during execution. + +To handle _broken values_ properly and recover from such an errorneous state, +use methods offered by `Standard.Base.Error` type like `catch`. diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/semantic/DataflowErrorPropagationTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/semantic/DataflowErrorPropagationTest.java new file mode 100644 index 000000000000..f5facc9d72d1 --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/semantic/DataflowErrorPropagationTest.java @@ -0,0 +1,86 @@ +package org.enso.interpreter.test.semantic; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.enso.common.MethodNames; +import org.enso.test.utils.ContextUtils; +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.PolyglotException; +import org.graalvm.polyglot.Value; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +public class DataflowErrorPropagationTest { + private static Context ctx; + private static Value suppressError; + private static Value suppressErrorWithAssign; + + @BeforeClass + public static void prepareCtx() { + ctx = ContextUtils.createDefaultContext(); + var code = + """ + from Standard.Base import all + + private yield_error yes:Boolean -> Text = + if yes then Error.throw "Yielding an error" else + "OK" + + suppress_error yes:Boolean value = + yield_error yes + value + + suppress_error_with_assign yes:Boolean value = + _ = yield_error yes + value + """; + suppressError = + ctx.eval("enso", code).invokeMember(MethodNames.Module.EVAL_EXPRESSION, "suppress_error"); + suppressErrorWithAssign = + ctx.eval("enso", code) + .invokeMember(MethodNames.Module.EVAL_EXPRESSION, "suppress_error_with_assign"); + } + + @AfterClass + public static void disposeCtx() { + ctx.close(); + ctx = null; + } + + @Test + public void noErrorReturnValue() { + var value = suppressError.execute(false, 42); + assertTrue("It is a number", value.isNumber()); + assertEquals(42, value.asInt()); + } + + @Test + public void propagateErrorImmediatelly() { + var value = suppressError.execute(true, 42); + assertFalse("It is not a number", value.isNumber()); + assertTrue("It is an error", value.isException()); + try { + throw value.throwException(); + } catch (PolyglotException ex) { + assertEquals("Yielding an error", ex.getMessage()); + } + } + + @Test + public void noErrorReturnValueWithAssign() { + var value = suppressErrorWithAssign.execute(false, 42); + assertTrue("It is a number", value.isNumber()); + assertEquals(42, value.asInt()); + } + + @Test + public void errorIsAssignedAndThatIsEnoughReturnValue() { + var value = suppressErrorWithAssign.execute(true, 42); + assertTrue("It is a number", value.isNumber()); + assertFalse("Not an error", value.isException()); + assertEquals(42, value.asInt()); + } +} diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java index 994b68b34149..41d51a54a9ee 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java @@ -6,9 +6,12 @@ import com.oracle.truffle.api.instrumentation.Tag; import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.NodeInfo; +import com.oracle.truffle.api.profiles.BranchProfile; import com.oracle.truffle.api.source.SourceSection; import java.util.Set; import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.error.DataflowError; /** * This node defines the body of a function for execution, as well as the protocol for executing the @@ -16,11 +19,17 @@ */ @NodeInfo(shortName = "Block") public class BlockNode extends ExpressionNode { + private final BranchProfile unexpectedReturnValue; @Children private final ExpressionNode[] statements; @Child private ExpressionNode returnExpr; private BlockNode(ExpressionNode[] expressions, ExpressionNode returnExpr) { this.statements = expressions; + if (expressions.length > 0) { + this.unexpectedReturnValue = BranchProfile.create(); + } else { + this.unexpectedReturnValue = BranchProfile.getUncached(); + } this.returnExpr = returnExpr; } @@ -55,8 +64,16 @@ public static BlockNode buildSilent(ExpressionNode[] expressions, ExpressionNode @Override @ExplodeLoop public Object executeGeneric(VirtualFrame frame) { + var ctx = EnsoContext.get(this); + var nothing = ctx.getBuiltins().nothing(); for (ExpressionNode statement : statements) { - statement.executeGeneric(frame); + var result = statement.executeGeneric(frame); + if (result != nothing) { + unexpectedReturnValue.enter(); + if (result instanceof DataflowError err) { + return err; + } + } } return returnExpr.executeGeneric(frame); } diff --git a/test/Base_Tests/src/Data/Decimal_Spec.enso b/test/Base_Tests/src/Data/Decimal_Spec.enso index 3465ce9cd85a..98a9d096b215 100644 --- a/test/Base_Tests/src/Data/Decimal_Spec.enso +++ b/test/Base_Tests/src/Data/Decimal_Spec.enso @@ -484,10 +484,10 @@ add_specs suite_builder = (Decimal.from_integer -29388920982834 . subtract (Decimal.from_integer 842820) (Math_Context.new 7)) . should_equal (Decimal.from_integer -29388920000000) (Decimal.new "-8273762787.3535345" . subtract (Decimal.new "76287273.23434535") (Math_Context.new 10)) . should_equal (Decimal.new "-8350050061") - (Decimal.from_integer 7297927982888383 . multiply (Decimal.from_integer 828737) (Math_Context.new 6)) . should_equal (Decimal.from_integer 6048060000000000000000 ) + (Decimal.from_integer 7297927982888383 . multiply (Decimal.from_integer 828737) (Math_Context.new 6)) . should_equal (Decimal.from_integer 6048060000000000000000) (Decimal.new "893872388.3535345" . multiply (Decimal.new "72374727737.23434535") (Math_Context.new 14)) . should_equal (Decimal.new "64693770738918000000") - (Decimal.new "909678645268840" . divide (Decimal.new "28029830") (Math_Context.new 6)) . should_equal (Decimal.new "32453900 ") + (Decimal.new "909678645268840" . divide (Decimal.new "28029830") (Math_Context.new 6)) . should_equal (Decimal.new "32453900") (Decimal.new "384456406.7860325392609633764" . divide (Decimal.new "24556.125563546") (Math_Context.new 7)) . should_equal (Decimal.new "15656.23") (Decimal.from_integer 3948539458034580838458034803485 . add (Decimal.from_integer 237957498573948579387495837459837) (Math_Context.new 20)) . should_equal (Decimal.from_integer 241906038031983160230000000000000) diff --git a/test/Base_Tests/src/Data/Function_Spec.enso b/test/Base_Tests/src/Data/Function_Spec.enso index b0cd9099e11a..c808ce77d9c8 100644 --- a/test/Base_Tests/src/Data/Function_Spec.enso +++ b/test/Base_Tests/src/Data/Function_Spec.enso @@ -90,4 +90,3 @@ main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder suite.run_with_filter filter - diff --git a/test/Base_Tests/src/Data/Range_Spec.enso b/test/Base_Tests/src/Data/Range_Spec.enso index a80a5cd52a0f..5bb3ecc8446f 100644 --- a/test/Base_Tests/src/Data/Range_Spec.enso +++ b/test/Base_Tests/src/Data/Range_Spec.enso @@ -3,6 +3,7 @@ import Standard.Base.Data.Vector.Builder import Standard.Base.Errors.Empty_Error.Empty_Error import Standard.Base.Errors.Common.Index_Out_Of_Bounds import Standard.Base.Errors.Common.No_Such_Method +import Standard.Base.Errors.Common.Not_Found import Standard.Base.Errors.Common.Type_Error import Standard.Base.Errors.Common.Unsupported_Argument_Types import Standard.Base.Errors.Illegal_Argument.Illegal_Argument @@ -250,9 +251,10 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> group_builder.specify "should find elements" <| 1.up_to 10 . find (> 5) . should_equal 6 1.up_to 10 . find (..Greater 5) . should_equal 6 - 1.up_to 10 . find (> 10) . should_be_a Nothing + 1.up_to 10 . find (> 10) . should_fail_with Not_Found 1.up_to 10 . find (v-> v%4 == 0) start=6 . should_equal 8 - 1.up_to 10 . find (< 5) start=6 . should_be_a Nothing + 1.up_to 10 . find (< 5) start=6 if_missing=Nothing . should_be_a Nothing + 1.up_to 10 . find (< 5) start=6 . should_fail_with Not_Found 1.up_to 10 . find (< 5) start=10 . should_fail_with Index_Out_Of_Bounds 1.up_to 10 . find (< 5) start=10 . catch . should_equal (Index_Out_Of_Bounds.Error 10 10) Test.expect_panic_with (1.up_to 10 . find "invalid arg") Type_Error @@ -343,7 +345,8 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r.fold 0 (+) . should_equal 0 r.any _->True . should_equal False r.all _->False . should_equal True - r.find _->True . should_equal Nothing + r.find _->True if_missing=Nothing . should_equal Nothing + r.find _->True . should_fail_with Not_Found verify_contains r [] [-1, 0, 1, 2, 10] check_empty_range (0.up_to 0) @@ -370,7 +373,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r1.all (_ == 10) . should_equal True r1.all (_ == 11) . should_equal False r1.find (x-> x*x == 100) . should_equal 10 - r1.find (x-> x*x == 25) . should_equal Nothing + r1.find (x-> x*x == 25) if_missing=Nothing . should_equal Nothing verify_contains r1 [10] [-1, 0, 1, 2, 9, 11, 12] group_builder.specify "should behave correctly with step greater than 1" <| @@ -387,7 +390,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r1.all (x-> x % 2 == 0) . should_equal True r1.all (_ == 2) . should_equal False r1.find (x-> x*x == 16) . should_equal 4 - r1.find (x-> x*x == 25) . should_equal Nothing + r1.find (x-> x*x == 25) if_missing=Nothing . should_equal Nothing verify_contains r1 [0, 2, 4, 6, 8] [-3, -2, -1, 1, 3, 5, 7, 11, 12, 13, 14] r2 = Range.Between 0 3 2 @@ -402,7 +405,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r2.any (_ == 3) . should_equal False r2.all (x-> x % 2 == 0) . should_equal True r2.all (_ == 2) . should_equal False - r2.find (x-> x*x == 16) . should_equal Nothing + r2.find (x-> x*x == 16) . should_fail_with Not_Found r2.find (x-> x*x == 4) . should_equal 2 verify_contains r2 [0, 2] [-3, -2, -1, 1, 3, 4, 5] @@ -418,7 +421,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r3.any (_ == 3) . should_equal False r3.all (_ == 5) . should_equal True r3.all (_ == 3) . should_equal False - r3.find (x-> x*x == 16) . should_equal Nothing + r3.find (x-> x*x == 16) . should_fail_with Not_Found r3.find (x-> x*x == 25) . should_equal 5 verify_contains r3 [5] [0, 1, 4, 6, 7, 10] @@ -435,7 +438,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r4.all (x-> x % 2 == 1) . should_equal True r4.all (_ == 5) . should_equal False r4.find (x-> x*x == 25) . should_equal 5 - r4.find (x-> x*x == 4) . should_equal Nothing + r4.find (x-> x*x == 4) if_missing=Nothing . should_equal Nothing verify_contains r4 [5, 7] [0, 1, 4, 6, 8, 10] r5 = Range.Between 5 7 2 @@ -451,7 +454,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r5.all (x-> x % 2 == 1) . should_equal True r5.all (_ == 5) . should_equal True r5.find (x-> x*x == 25) . should_equal 5 - r5.find (x-> x*x == 4) . should_equal Nothing + r5.find (x-> x*x == 4) if_missing=Nothing . should_equal Nothing verify_contains r5 [5] [0, 1, 4, 6, 7, 10] r6 = Range.Between 0 10 3 @@ -467,7 +470,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r6.all (x-> x % 2 == 0) . should_equal False r6.all (x-> x % 3 == 0) . should_equal True r6.find (x-> x*x == 9) . should_equal 3 - r6.find (x-> x*x == 25) . should_equal Nothing + r6.find (x-> x*x == 25) if_missing=Nothing . should_equal Nothing r6.filter (_ < 4) . should_equal [0, 3] verify_contains r6 [0, 3, 6, 9] [-3, -2, -1, 1, 2, 4, 5, 7, 8, 10, 11] @@ -485,7 +488,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r1.all (x-> x % 2 == 0) . should_equal False r1.all (_ > 0) . should_equal True r1.find (x-> x*x == 16) . should_equal 4 - r1.find (x-> x*x == 0) . should_equal Nothing + r1.find (x-> x*x == 0) if_missing=Nothing . should_equal Nothing verify_contains r1 [4, 3, 2, 1] [-2, -1, 0, 5, 6, 7, 10] r2 = Range.Between 4 0 -2 @@ -501,7 +504,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r2.all (x-> x % 2 == 0) . should_equal True r2.all (_ > 2) . should_equal False r2.find (x-> x*x == 16) . should_equal 4 - r2.find (x-> x*x == 0) . should_equal Nothing + r2.find (x-> x*x == 0) . should_fail_with Not_Found verify_contains r2 [4, 2] [-2, -1, 0, 1, 3, 5, 6, 7, 10] r3 = Range.Between 4 0 -10 @@ -517,7 +520,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r3.all (x-> x % 2 == 0) . should_equal True r3.all (_ > 4) . should_equal False r3.find (x-> x*x == 16) . should_equal 4 - r3.find (x-> x*x == 0) . should_equal Nothing + r3.find (x-> x*x == 0) . should_fail_with Not_Found verify_contains r3 [4] [-2, -1, 0, 1, 2, 3, 5, 6, 7, 10] r4 = Range.Between 3 0 -3 @@ -533,7 +536,7 @@ add_specs suite_builder = suite_builder.group "Range" group_builder-> r4.all (x-> x % 2 == 0) . should_equal False r4.all (_ > 0) . should_equal True r4.find (x-> x*x == 9) . should_equal 3 - r4.find (x-> x*x == 0) . should_equal Nothing + r4.find (x-> x*x == 0) . should_fail_with Not_Found verify_contains r4 [3] [-3, -2, -1, 0, 1, 2, 4, 5, 6, 7, 10] group_builder.specify "should report errors if trying to set step to 0" <| @@ -582,4 +585,3 @@ main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder suite.run_with_filter filter - diff --git a/test/Base_Tests/src/Data/Time/Date_Range_Spec.enso b/test/Base_Tests/src/Data/Time/Date_Range_Spec.enso index b2b469ae1c61..9d74f3604c7b 100644 --- a/test/Base_Tests/src/Data/Time/Date_Range_Spec.enso +++ b/test/Base_Tests/src/Data/Time/Date_Range_Spec.enso @@ -1,4 +1,5 @@ from Standard.Base import all +import Standard.Base.Errors.Common.Not_Found import Standard.Base.Errors.Common.Type_Error import Standard.Base.Errors.Empty_Error.Empty_Error import Standard.Base.Errors.Illegal_Argument.Illegal_Argument @@ -157,7 +158,7 @@ add_specs suite_builder = r.partition p . should_equal (r.to_vector.partition p) r.all p . should_equal (r.to_vector.all p) r.any p . should_equal (r.to_vector.any p) - r.find p . should_equal (r.to_vector.find p) + r.find p if_missing="not found" . should_equal (r.to_vector.find p if_missing="not found") r.index_of p . should_equal (r.to_vector.index_of p) r.last_index_of p . should_equal (r.to_vector.last_index_of p) count_mondays acc date = @@ -170,7 +171,7 @@ add_specs suite_builder = r.partition fc . should_equal (r.to_vector.partition fc) r.all fc . should_equal (r.to_vector.all fc) r.any fc . should_equal (r.to_vector.any fc) - r.find fc . should_equal (r.to_vector.find fc) + r.find fc if_missing="not found" . should_equal (r.to_vector.find fc if_missing="not found") r.index_of fc . should_equal (r.to_vector.index_of fc) r.last_index_of fc . should_equal (r.to_vector.last_index_of fc) @@ -182,6 +183,9 @@ add_specs suite_builder = Test.expect_panic_with (r.index_of invalid_arg) Type_Error Test.expect_panic_with (r.last_index_of invalid_arg) Type_Error + # If `if_missing` is not provided, it defaults to `Not_Found` dataflow error + r.find (== 123) . should_fail_with Not_Found + reducer x y = if x > y then x else y case r.length of diff --git a/test/Base_Tests/src/Random_Spec.enso b/test/Base_Tests/src/Random_Spec.enso index ecb2b6ac07cb..980dc33707a2 100644 --- a/test/Base_Tests/src/Random_Spec.enso +++ b/test/Base_Tests/src/Random_Spec.enso @@ -157,4 +157,3 @@ main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder suite.run_with_filter filter - diff --git a/test/Base_Tests/src/System/File_Spec.enso b/test/Base_Tests/src/System/File_Spec.enso index 4f56688efa4f..826771ed3de6 100644 --- a/test/Base_Tests/src/System/File_Spec.enso +++ b/test/Base_Tests/src/System/File_Spec.enso @@ -391,7 +391,7 @@ add_specs suite_builder = subdir.should_succeed cleanup = Enso_User.flush_caches - subdir.delete + subdir.delete recursive=True Panic.with_finalizer cleanup <| Test_Environment.unsafe_with_environment_override "ENSO_CLOUD_PROJECT_DIRECTORY_PATH" subdir.path <| # Flush caches to ensure fresh dir is used diff --git a/test/Table_Tests/src/Common_Table_Operations/Cross_Tab_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Cross_Tab_Spec.enso index c0d17b70a245..ad79702d5c8b 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Cross_Tab_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Cross_Tab_Spec.enso @@ -252,7 +252,6 @@ add_specs suite_builder setup = r1.catch.to_display_text . should_contain "cannot contain the NUL character" r2 = data.table2.cross_tab [] "Key" values=[Aggregate_Column.Average "Value" as='x\0'] - r2.print r2.should_fail_with Invalid_Column_Names r2.catch.to_display_text . should_contain "cannot contain the NUL character" diff --git a/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso index 43596f79bbd8..e929623dc265 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Nothing_Spec.enso @@ -1,4 +1,5 @@ from Standard.Base import all +import Standard.Base.Errors.Common.Incomparable_Values from Standard.Table import all @@ -257,7 +258,7 @@ add_nothing_specs suite_builder setup = other_value = triple.at 1 value_type = triple.at 2 - is_comparable = case Ordering.compare value other_value of + is_comparable = case Ordering.compare value other_value . catch Incomparable_Values of _:Ordering -> True _ -> False diff --git a/test/Table_Tests/src/Database/SQLite_Spec.enso b/test/Table_Tests/src/Database/SQLite_Spec.enso index 1c10b7d3b1ea..5ea4e8ce960b 100644 --- a/test/Table_Tests/src/Database/SQLite_Spec.enso +++ b/test/Table_Tests/src/Database/SQLite_Spec.enso @@ -3,6 +3,7 @@ import Standard.Base.Runtime.Ref.Ref from Standard.Base.Runtime import assert import Standard.Base.Errors.File_Error.File_Error import Standard.Base.Errors.Illegal_Argument.Illegal_Argument +import Standard.Base.Runtime.Context from Standard.Table import Table, Value_Type, Bits from Standard.Table.Errors import Invalid_Column_Names, Duplicate_Output_Column_Names @@ -54,8 +55,8 @@ type Metadata_Data [connection, tinfo, t] teardown self = - self.connection.drop_table self.t.name - self.connection.drop_table self.tinfo + self.connection.drop_table self.t.name if_exists=True + self.connection.drop_table self.tinfo if_exists=True self.connection.close @@ -81,9 +82,9 @@ type Tables_And_Table_Types_Data [connection, tinfo, vinfo, temporary_table] teardown self = - self.connection.drop_table self.tinfo - self.connection.drop_table self.vinfo - self.connection.drop_table self.temporary_table + self.connection.drop_table self.tinfo if_exists=True + self.connection.execute_update "DROP VIEW IF EXISTS '"+self.vinfo+"'" + self.connection.drop_table self.temporary_table if_exists=True self.connection.close @@ -377,9 +378,7 @@ type Database_File Value ~file create = Database_File.Value <| - transient_dir = enso_project.data / "transient" - assert transient_dir.exists ("The directory " + transient_dir.path + " should exist (ensured by containing a `.gitignore` file).") - f = transient_dir / "sqlite_test.db" + f = File.create_temporary_file "sqlite-test" ".db" f.delete_if_exists f @@ -388,7 +387,10 @@ create_inmem_connection = create_file_connection file = connection = Database.connect (SQLite.From_File file) - connection.execute_update 'CREATE TABLE "Dummy" ("strs" VARCHAR, "ints" INTEGER, "bools" BOOLEAN, "reals" REAL)' + ## We need to re-enable the context because this initializer may be executed + lazily in a context where Output was disabled (e.g. Upload_Spec). + Context.Output.with_enabled <| + connection.execute_update 'CREATE TABLE IF NOT EXISTS "Dummy" ("strs" VARCHAR, "ints" INTEGER, "bools" BOOLEAN, "reals" REAL)' connection type File_Connection @@ -401,10 +403,6 @@ type File_Connection assert tmp_file.exists tmp_file - teardown self = - assert self.file.exists - self.file.delete - add_specs suite_builder = in_file_prefix = "[SQLite File] " @@ -425,9 +423,6 @@ add_specs suite_builder = suite_builder.group "SQLite_Format should allow connecting to SQLite files" group_builder-> data = File_Connection.setup database_file - group_builder.teardown <| - data.teardown - group_builder.specify "should recognise a SQLite database file" <| Auto_Detect.get_reading_format data.file . should_be_a SQLite_Format diff --git a/test/Table_Tests/src/Database/Upload_Spec.enso b/test/Table_Tests/src/Database/Upload_Spec.enso index ffaa385e9e3c..dce15a1d1fbb 100644 --- a/test/Table_Tests/src/Database/Upload_Spec.enso +++ b/test/Table_Tests/src/Database/Upload_Spec.enso @@ -498,7 +498,7 @@ add_specs suite_builder setup make_new_connection persistent_connector=True = copied_table = db_table.select_into_database_table tmp_connection (Name_Generator.random_name "copied-table") temporary=False copied_table.is_trivial_query . should_be_true name = copied_table.name - Panic.with_finalizer (data.connection.drop_table name) <| + Panic.with_finalizer (data.connection.drop_table name if_exists=True) <| setup.expect_integer_type <| copied_table.at "X" copied_table.at "Y" . value_type . is_text . should_be_true copied_table.at "Z" . value_type . is_floating_point . should_be_true diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 0e27747890c4..c4b11c1a27c6 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -4,8 +4,10 @@ import Standard.Base.Errors.Common.Response_Too_Large import Standard.Base.Errors.File_Error.File_Error import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Base.Network.HTTP.Cache_Policy.Cache_Policy +import Standard.Base.Network.HTTP.HTTP_Error.HTTP_Error import Standard.Base.Network.HTTP.Request.Request import Standard.Base.Network.HTTP.Request_Body.Request_Body +import Standard.Base.Network.HTTP.Request_Error import Standard.Base.Network.HTTP.Response.Response import Standard.Base.Runtime.Context import Standard.Base.Runtime.Ref.Ref @@ -412,11 +414,11 @@ add_specs suite_builder = group_builder.specify "Should not cache if the request fails" pending=pending_has_url <| Test.with_retries <| with_default_cache <| - HTTP.fetch url0 . decode_as_text + HTTP.fetch url0 . decode_as_text . should_succeed get_num_response_cache_entries . should_equal 1 - HTTP.fetch base_url_with_slash+'crash' . decode_as_text + HTTP.fetch base_url_with_slash+'crash' . decode_as_text . should_fail_with Request_Error get_num_response_cache_entries . should_equal 1 - HTTP.fetch base_url_with_slash+'nonexistent_endpoint' . decode_as_text + HTTP.fetch base_url_with_slash+'nonexistent_endpoint' . decode_as_text . should_fail_with HTTP_Error get_num_response_cache_entries . should_equal 1 cloud_setup = Cloud_Tests_Setup.prepare @@ -437,9 +439,9 @@ add_specs suite_builder = . add_query_argument "arg1" secret2 . add_query_argument "arg2" "plain value" - HTTP.fetch url1 . decode_as_text + HTTP.fetch url1 . decode_as_text . should_succeed get_num_response_cache_entries . should_equal 1 - HTTP.fetch uri2 . decode_as_text + HTTP.fetch uri2 . decode_as_text . should_succeed get_num_response_cache_entries . should_equal 2 group_builder.specify "Should work with secrets in the headers" pending=pending_has_url <| Test.with_retries <| @@ -455,9 +457,9 @@ add_specs suite_builder = headers1 = [Header.new "A-Header" secret1] headers2 = [Header.new "A-Header" secret2] - HTTP.fetch headers=headers1 uri . decode_as_text + HTTP.fetch headers=headers1 uri . decode_as_text . should_succeed get_num_response_cache_entries . should_equal 1 - HTTP.fetch headers=headers2 uri . decode_as_text + HTTP.fetch headers=headers2 uri . decode_as_text . should_succeed get_num_response_cache_entries . should_equal 2 group_builder.specify "Does not attempt to make room for the maximum file size when that is larger than the total cache size" pending=pending_has_url <| Test.with_retries <| diff --git a/test/Table_Tests/src/In_Memory/Split_Tokenize_Spec.enso b/test/Table_Tests/src/In_Memory/Split_Tokenize_Spec.enso index 2f365d7ce65c..bfdb332f3199 100644 --- a/test/Table_Tests/src/In_Memory/Split_Tokenize_Spec.enso +++ b/test/Table_Tests/src/In_Memory/Split_Tokenize_Spec.enso @@ -2,7 +2,7 @@ from Standard.Base import all import Standard.Test.Extensions -from Standard.Table import Table +from Standard.Table import Table, Value_Type from Standard.Table.Columns_To_Add import Columns_To_Add from Standard.Table.Errors import Invalid_Value_Type, Column_Count_Exceeded, Duplicate_Output_Column_Names, No_Such_Column from Standard.Test import all @@ -204,7 +204,7 @@ add_specs suite_builder = cols = [["foo", [0, 1, 2]], ["bar", ["abc", "cbdbef", "ghbijbu"]]] t = Table.new cols expected_rows = [[0, "a", "c", Nothing, Nothing], [1, "c", "d", "ef", Nothing], [2, "gh", "ij", "u", Nothing]] - expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 3"] expected_rows + expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 4"] expected_rows . cast "bar 4" Value_Type.Char t2 = t.split_to_columns "bar" "b" column_count=(Columns_To_Add.First 4) t2.should_equal expected t2.at "bar 3" . value_type . is_text . should_be_true @@ -213,7 +213,7 @@ add_specs suite_builder = cols = [["foo", [0, 1, 2]], ["bar", ["abc", "cbdbef", "ghbijbu"]]] t = Table.new cols expected_rows = [[0, "a", "c", Nothing, Nothing], [1, "c", "d", "ef", Nothing], [2, "gh", "ij", "u", Nothing]] - expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 3"] expected_rows + expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 4"] expected_rows . cast "bar 4" Value_Type.Char t2 = t.split_to_columns "bar" "b" column_count=4 t2.should_equal expected t2.at "bar 3" . value_type . is_text . should_be_true @@ -262,7 +262,7 @@ add_specs suite_builder = cols = [["foo", [0, 1, 2]], ["bar", ["ghbijbu", "cbdbef", "abc"]]] t = Table.new cols expected_rows = [[0, "gh", "ij", "u", Nothing], [1, "c", "d", "ef", Nothing], [2, "a", "c", Nothing, Nothing]] - expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 3"] expected_rows + expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 4"] expected_rows . cast "bar 4" Value_Type.Char t2 = t.split_to_columns "bar" "b" column_count=(Columns_To_Add.First 4) t2.should_equal expected t2.at "bar 3" . value_type . is_text . should_be_true @@ -271,7 +271,7 @@ add_specs suite_builder = cols = [["foo", [0, 1, 2]], ["bar", ["ghbijbu", "cbdbef", "abc"]]] t = Table.new cols expected_rows = [[0, "gh", "ij", "u", Nothing], [1, "c", "d", "ef", Nothing], [2, "a", "c", Nothing, Nothing]] - expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 3"] expected_rows + expected = Table.from_rows ["foo", "bar 1", "bar 2", "bar 3", "bar 4"] expected_rows . cast "bar 4" Value_Type.Char t2 = t.split_to_columns "bar" "b" column_count=4 t2.should_equal expected t2.at "bar 3" . value_type . is_text . should_be_true @@ -433,4 +433,3 @@ main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder suite.run_with_filter filter - diff --git a/test/Table_Tests/src/Util.enso b/test/Table_Tests/src/Util.enso index b956fe84dd66..2976f150d6f9 100644 --- a/test/Table_Tests/src/Util.enso +++ b/test/Table_Tests/src/Util.enso @@ -1,4 +1,5 @@ from Standard.Base import all +import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Database.DB_Table.DB_Table import Standard.Database.DB_Column.DB_Column @@ -11,23 +12,27 @@ polyglot java import org.enso.base_test_helpers.FileSystemHelper Table.should_equal : Any -> Integer -> Any Table.should_equal self expected frames_to_skip=0 = loc = Meta.get_source_location 1+frames_to_skip + rhs_error_check expected Panic.catch Test_Failure_Error (table_should_equal_impl self expected loc) error-> - Test.fail error.payload.message + Test.fail error.payload.message Column.should_equal : Any -> Integer -> Any Column.should_equal self expected frames_to_skip=0 = loc = Meta.get_source_location 1+frames_to_skip + rhs_error_check expected Panic.catch Test_Failure_Error (column_should_equal_impl self expected loc) error-> Test.fail error.payload.message DB_Table.should_equal : DB_Table -> Integer -> Any DB_Table.should_equal self expected frames_to_skip=0 = + rhs_error_check expected t0 = self.read t1 = expected.read t0 . should_equal t1 frames_to_skip+1 DB_Column.should_equal : DB_Column -> Integer -> Any DB_Column.should_equal self expected frames_to_skip=0 = + rhs_error_check expected t0 = self.read t1 = expected.read t0 . should_equal t1 frames_to_skip+1 @@ -142,3 +147,14 @@ Error.should_have_relative_ordering self example = loc = Meta.get_source_location 1 _ = example Test.fail "Expected a vector but got a dataflow error "+self.catch.to_display_text+" (at "+loc+")." + + +## PRIVATE + A helper that ensures that the expected value provided in some of the Test + operations is not an error. + The left-hand side may be an error and that will cause a test failure. + But the right-hand side being an error is bad test design and should be fixed. +rhs_error_check that = + if that.is_error then + msg = "Dataflow error ("+that.to_display_text+") provided as expected value. Use `should_fail_with` or change the test."+ ' Error stack trace was:\n'+that.get_stack_trace_text + Panic.throw (Illegal_Argument.Error msg) diff --git a/test/Table_Tests/src/Util_Spec.enso b/test/Table_Tests/src/Util_Spec.enso index f8f02507d2a0..d51f3a6c8d9c 100644 --- a/test/Table_Tests/src/Util_Spec.enso +++ b/test/Table_Tests/src/Util_Spec.enso @@ -1,10 +1,59 @@ from Standard.Base import all + from Standard.Table import Column, Table -from project.Util import all + +from Standard.Database import all + from Standard.Test import all +from enso_dev.Test_Tests.Helpers import expect_test_failure + +from project.Util import all + add_specs suite_builder = - suite_builder.group "Column should_equal" group_builder-> + suite_builder.group "Table/Column.should_equal helpers" group_builder-> + group_builder.specify "should report correct location for Table" <| + r1 = expect_test_failure <| + (Table.new [["X", [1]]]) . should_equal (Table.new [["X", [2]]]) + r1.message.should_contain "Util_Spec.enso:17" + + r2 = expect_test_failure <| + (Table.new [["X", [1]]]) . should_equal (Table.new [["A", [1]]]) + r2.message.should_contain "Util_Spec.enso:21" + + group_builder.specify "should report correct location for Column" <| + r1 = expect_test_failure <| + Column.from_vector "X" [1] . should_equal (Column.from_vector "X" [2]) + r1.message.should_contain "Util_Spec.enso:26" + + r2 = expect_test_failure <| + Column.from_vector "X" [1] . should_equal (Column.from_vector "A" [1]) + r2.message.should_contain "Util_Spec.enso:30" + + group_builder.specify "should report correct location for DB_Table" <| + tables = DB_Tables.make + r1 = expect_test_failure <| + tables.t1 . should_equal tables.t2 + r1.message.should_contain "Util_Spec.enso:36" + + r2 = expect_test_failure <| + tables.t1 . should_equal tables.tA + r2.message.should_contain "Util_Spec.enso:40" + + group_builder.specify "should report correct location for DB_Column" <| + tables = DB_Tables.make + c1 = tables.t1.at "X" + c2 = tables.t2.at "X" + cA = tables.tA.at "A" + + r1 = expect_test_failure <| + c1 . should_equal c2 + r1.message.should_contain "Util_Spec.enso:50" + + r2 = expect_test_failure <| + c1 . should_equal cA + r2.message.should_contain "Util_Spec.enso:54" + group_builder.specify "Two Columns Are Equal" <| expected_column = Column.from_vector "Col" ["Quis", "custodiet", "ipsos", "custodes?"] actual_column = Column.from_vector "Col" ["Quis", "custodiet", "ipsos", "custodes?"] @@ -44,7 +93,7 @@ add_specs suite_builder = expected_column = Column.from_vector "Col" [1.0, 2.0, Number.nan] actual_column = Column.from_vector "Col" [1.0, 2.0, Number.nan] actual_column.should_equal expected_column - suite_builder.group "Table should_equal" group_builder-> + group_builder.specify "Two Tables Are Equal" <| expected_table = Table.new [Column.from_vector "Col1" ["Quis", "custodiet", "ipsos", "custodes?"], Column.from_vector "Col2" ["Who", "guards", "the", "guards?"]] actual_table = Table.new [Column.from_vector "Col1" ["Quis", "custodiet", "ipsos", "custodes?"], Column.from_vector "Col2" ["Who", "guards", "the", "guards?"]] @@ -75,6 +124,17 @@ add_specs suite_builder = res = Panic.recover Test_Failure_Error (table_should_equal_impl actual_table expected_table "LOCATION_PATH") res.catch.message.should_equal "Got a Table, but expected a 42 (at LOCATION_PATH)." +type DB_Tables + Value t1 t2 tA + + make = + connection = Database.connect ..In_Memory + t1 = (Table.new [["X", [1]]]).select_into_database_table connection "t1" + t2 = (Table.new [["X", [2]]]).select_into_database_table connection "t2" + tA = (Table.new [["A", [1]]]).select_into_database_table connection "tA" + DB_Tables.Value t1 t2 tA + + main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder diff --git a/test/Test_Tests/package.yaml b/test/Test_Tests/package.yaml new file mode 100644 index 000000000000..2d35ea3d05a9 --- /dev/null +++ b/test/Test_Tests/package.yaml @@ -0,0 +1,7 @@ +name: Test_Tests +namespace: enso_dev +version: 0.0.1 +license: MIT +author: enso-dev@enso.org +maintainer: enso-dev@enso.org +prefer-local-libraries: true diff --git a/test/Test_Tests/src/Extensions_Spec.enso b/test/Test_Tests/src/Extensions_Spec.enso new file mode 100644 index 000000000000..afee91d94597 --- /dev/null +++ b/test/Test_Tests/src/Extensions_Spec.enso @@ -0,0 +1,86 @@ +from Standard.Base import all +import Standard.Base.Errors.Illegal_Argument.Illegal_Argument + +from Standard.Test import all + +from project.Helpers import expect_test_failure + +main filter=Nothing = + suite = Test.build suite_builder-> + add_specs suite_builder + suite.run_with_filter filter + +add_specs suite_builder = + suite_builder.group "should_equal extension method" group_builder-> + group_builder.specify "should report correct location for Text" <| + r1 = expect_test_failure <| + "a".should_equal "b" + r1.message.should_contain "Extensions_Spec.enso:17" + + group_builder.specify "should report correct location for numbers" <| + r1 = expect_test_failure <| + 1.should_equal 2 + r1.message.should_contain "Extensions_Spec.enso:22" + + r2 = expect_test_failure <| + 1.0 . should_equal 2 + r2.message.should_contain "Extensions_Spec.enso:26" + + r3 = expect_test_failure <| + 1.to_decimal . should_equal 2 + r3.message.should_contain "Extensions_Spec.enso:30" + + r4 = expect_test_failure <| + Number.nan.should_equal 2 + r4.message.should_contain "Extensions_Spec.enso:34" + + group_builder.specify "should report correct location for errors" <| + error = Error.throw (Illegal_Argument.Error "foo") + r1 = expect_test_failure <| + error.should_equal 10 + r1.message.should_contain "Extensions_Spec.enso:40" + + group_builder.specify "should panic if error is expected" <| + error = Error.throw (Illegal_Argument.Error "foo") + Test.expect_panic Illegal_Argument <| + 10.should_equal error + + suite_builder.group "should_not_equal extension method" group_builder-> + group_builder.specify "should report correct location" <| + r1 = expect_test_failure <| + 1.should_not_equal 1 + r1.message.should_contain "Extensions_Spec.enso:51" + + group_builder.specify "should report correct location for errors" <| + error = Error.throw (Illegal_Argument.Error "foo") + r1 = expect_test_failure <| + error.should_not_equal 1 + r1.message.should_contain "Extensions_Spec.enso:57" + + suite_builder.group "should_contain extension method" group_builder-> + group_builder.specify "should report correct location" <| + r1 = expect_test_failure <| + [1, 2].should_contain 3 + r1.message.should_contain "Extensions_Spec.enso:63" + + r2 = expect_test_failure <| + "abc".should_contain "d" + r2.message.should_contain "Extensions_Spec.enso:67" + + suite_builder.group "should_not_contain extension method" group_builder-> + group_builder.specify "should report correct location" <| + r1 = expect_test_failure <| + [1, 2].should_not_contain 2 + r1.message.should_contain "Extensions_Spec.enso:73" + + suite_builder.group "should_start_with extension method" group_builder-> + group_builder.specify "should report correct location" <| + r1 = expect_test_failure <| + "abc".should_start_with "d" + r1.message.should_contain "Extensions_Spec.enso:79" + + suite_builder.group "should_end_with extension method" group_builder-> + group_builder.specify "should report correct location" <| + r1 = expect_test_failure <| + "abc".should_end_with "d" + r1.message.should_contain "Extensions_Spec.enso:85" diff --git a/test/Test_Tests/src/Helpers.enso b/test/Test_Tests/src/Helpers.enso new file mode 100644 index 000000000000..beaa75f34fcb --- /dev/null +++ b/test/Test_Tests/src/Helpers.enso @@ -0,0 +1,17 @@ +from Standard.Base import all + +import Standard.Test.Spec_Result.Spec_Result +from Standard.Test import Test + +## Expects the inner action to report a test failure exception and returns its payload. +expect_test_failure ~action -> Spec_Result = + loc = Meta.get_source_location 1 + handle_panic caught_panic = + result = caught_panic.payload + case result of + Spec_Result.Failure _ _ -> result + _ -> Test.fail "Expected test failure, but "+result.to_text+" was raised as error." + + Panic.catch Spec_Result handler=handle_panic <| + action + Test.fail "Expected the inner action to fail, but there was no failure (at "+loc+")." diff --git a/test/Test_Tests/src/Main.enso b/test/Test_Tests/src/Main.enso new file mode 100644 index 000000000000..a4542f7d522b --- /dev/null +++ b/test/Test_Tests/src/Main.enso @@ -0,0 +1,13 @@ +from Standard.Base import all + +from Standard.Test import Test + +import project.Extensions_Spec + +add_specs suite_builder = + Extensions_Spec.add_specs suite_builder + +main filter=Nothing = + suite = Test.build suite_builder-> + add_specs suite_builder + suite.run_with_filter filter From 4653ecaaa8501b061f488b2a380367faad5df1c0 Mon Sep 17 00:00:00 2001 From: James Dunkerley Date: Thu, 19 Dec 2024 13:07:56 +0000 Subject: [PATCH 20/21] Missed one :( (#11918) --- app/ide-desktop/client/tasks/signArchivesMacOs.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/ide-desktop/client/tasks/signArchivesMacOs.ts b/app/ide-desktop/client/tasks/signArchivesMacOs.ts index 64310fad8816..979fa51c65bc 100644 --- a/app/ide-desktop/client/tasks/signArchivesMacOs.ts +++ b/app/ide-desktop/client/tasks/signArchivesMacOs.ts @@ -106,6 +106,7 @@ async function ensoPackageSignables(resourcesDir: string): Promise { [ 'org/jline/nativ/Mac/arm64/libjlinenative.jnilib', 'org/jline/nativ/Mac/x86_64/libjlinenative.jnilib', + 'org/jline/nativ/Mac/x86/libjlinenative.jnilib', ], ], [ From b50ae41563f19e2a9df7f55cedf0bc66a9546b7e Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Thu, 19 Dec 2024 18:37:07 +0400 Subject: [PATCH 21/21] remove console logs --- .../integration-test/dashboard/actions/api.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/app/gui/integration-test/dashboard/actions/api.ts b/app/gui/integration-test/dashboard/actions/api.ts index 63da8ae53ac9..458b13141770 100644 --- a/app/gui/integration-test/dashboard/actions/api.ts +++ b/app/gui/integration-test/dashboard/actions/api.ts @@ -262,11 +262,6 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { throw new Error(`Asset ${assetId} not found`) } - console.log('editAsset', { - asset, - rest, - }) - const updated = object.merge(asset, rest) addAsset(updated) @@ -899,7 +894,6 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { ) await patch(remoteBackendPaths.updateAssetPath(GLOB_ASSET_ID), (route, request) => { - console.log('updateAssetPath', request.url()) const maybeId = request.url().match(/[/]assets[/]([^?]+)/)?.[1] if (!maybeId) throw new Error('updateAssetPath: Missing asset ID in path') @@ -997,8 +991,6 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { await put(remoteBackendPaths.projectUpdatePath(GLOB_PROJECT_ID), async (route, request) => { const maybeId = request.url().match(/[/]projects[/]([^?/]+)/)?.[1] - console.log('maybeId', maybeId) - if (!maybeId) return route.fulfill({ status: HTTP_STATUS_NOT_FOUND }) const projectId = backend.ProjectId(maybeId) @@ -1009,20 +1001,10 @@ async function mockApiInternal({ page, setupAPI }: MockParams) { const newTitle = body.projectName - console.log('newTitle', { - newTitle, - maybeId, - }) - if (newTitle == null) { return route.fulfill({ status: HTTP_STATUS_BAD_REQUEST }) } - console.log('editAsset', { - projectId, - newTitle, - }) - return route.fulfill({ json: editAsset(projectId, { title: newTitle }), })
@@ -1969,7 +1970,7 @@ function AssetsTable(props: AssetsTableProps) { /> )}
-
{table}
+
{table}
From ee1c3821a047d3e541ec507cde070884d68e0031 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Sat, 14 Dec 2024 01:25:47 +0400 Subject: [PATCH 07/21] Remove unused import --- app/gui/src/dashboard/components/styled/FocusRing.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/gui/src/dashboard/components/styled/FocusRing.tsx b/app/gui/src/dashboard/components/styled/FocusRing.tsx index 5913eafc69f8..64449c506213 100644 --- a/app/gui/src/dashboard/components/styled/FocusRing.tsx +++ b/app/gui/src/dashboard/components/styled/FocusRing.tsx @@ -1,7 +1,6 @@ /** @file A styled focus ring. */ import * as aria from '#/components/aria' -import { twMerge } from '#/utilities/tailwindMerge' // ================= // === FocusRing === From 96fb603c88b5787e5a74ece180d3cf546b73ca94 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Sat, 14 Dec 2024 01:41:14 +0400 Subject: [PATCH 08/21] FIxes x2 --- .../components/AriaComponents/Form/components/useFormError.ts | 4 +++- app/gui/src/dashboard/layouts/AssetsTable.tsx | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts b/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts index 854157eb9eb0..756c8da11541 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Form/components/useFormError.ts @@ -8,7 +8,7 @@ import { useFormContext } from './FormProvider' import type { FormInstance } from './types' /** - * + * Props for {@link useFormError}. */ export interface UseFormErrorProps { // We do not need to know the form fields. @@ -20,7 +20,9 @@ export interface UseFormErrorProps { * Error type. */ interface Error { + /** The type of the error, either caused by a form field or by an offline error. */ readonly type: 'error' | 'offline' + /** The error message. */ readonly message: string } diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx index 48d2a26a8baf..a5293d7c984b 100644 --- a/app/gui/src/dashboard/layouts/AssetsTable.tsx +++ b/app/gui/src/dashboard/layouts/AssetsTable.tsx @@ -1812,8 +1812,6 @@ function AssetsTable(props: AssetsTableProps) { {headerRow}
@@ -1970,7 +1968,7 @@ function AssetsTable(props: AssetsTableProps) { /> )}
-
{table}
+
{table}
From c77b38e743c78851dcd42a5199f13a66ce4406c5 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Sat, 14 Dec 2024 01:48:59 +0400 Subject: [PATCH 09/21] Add border --- .../src/dashboard/components/dashboard/column/columnUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts b/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts index 7c78549b3261..6e5cbe3ea15e 100644 --- a/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts +++ b/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts @@ -69,7 +69,7 @@ const NORMAL_COLUMN_CSS_CLASSES = `px-cell-x py ${COLUMN_CSS_CLASSES}` /** CSS classes for every column. */ export const COLUMN_CSS_CLASS: Readonly> = { - [Column.name]: `z-10 sticky left-0 bg-dashboard rounded-rows-skip-level min-w-drive-name-column h-full p-0 border-l-0 ${COLUMN_CSS_CLASSES}`, + [Column.name]: `z-10 sticky left-0 bg-dashboard rounded-rows-skip-level min-w-drive-name-column h-full p-0 border-l-0 after:absolute after:right-0 after:top-0 after:bottom-0 after:border-r-[1.5px] after:border-primary/5 ${COLUMN_CSS_CLASSES}`, [Column.modified]: `min-w-drive-modified-column rounded-rows-have-level ${NORMAL_COLUMN_CSS_CLASSES}`, [Column.sharedWith]: `min-w-drive-shared-with-column rounded-rows-have-level ${NORMAL_COLUMN_CSS_CLASSES}`, [Column.labels]: `min-w-drive-labels-column rounded-rows-have-level ${NORMAL_COLUMN_CSS_CLASSES}`, From d8a6bee017d0ce60f3fde128705a45959b741cd1 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Sat, 14 Dec 2024 02:02:45 +0400 Subject: [PATCH 10/21] Improve animations --- .../dashboard/assetsTableFeatures.spec.ts | 2 +- .../src/dashboard/components/EditableSpan.tsx | 77 ++++++++++--------- app/gui/src/dashboard/layouts/AssetsTable.tsx | 2 +- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts index 92a1823b37c1..86e68a69bdea 100644 --- a/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts +++ b/app/gui/integration-test/dashboard/assetsTableFeatures.spec.ts @@ -54,7 +54,7 @@ test('extra columns should stick to right side of assets table', ({ page }) => const assetsTableRight = await assetsTable.evaluate( (element) => element.getBoundingClientRect().right, ) - expect(extraColumnsRight).toEqual(assetsTableRight - 12) + expect(extraColumnsRight).toEqual(assetsTableRight - 8) }).toPass({ timeout: PASS_TIMEOUT }) })) diff --git a/app/gui/src/dashboard/components/EditableSpan.tsx b/app/gui/src/dashboard/components/EditableSpan.tsx index 15aa30110786..48a03fff6f17 100644 --- a/app/gui/src/dashboard/components/EditableSpan.tsx +++ b/app/gui/src/dashboard/components/EditableSpan.tsx @@ -4,12 +4,11 @@ import * as React from 'react' import CrossIcon from '#/assets/cross.svg' import TickIcon from '#/assets/tick.svg' -import { Input, Text } from '#/components/AriaComponents' +import { Button, Form, Input, Text, Underlay } from '#/components/AriaComponents' import * as textProvider from '#/providers/TextProvider' import * as tailwindMerge from '#/utilities/tailwindMerge' import { useInteractOutside } from '#/components/aria' -import { Form, Underlay } from '#/components/AriaComponents' import { useAutoFocus } from '#/hooks/autoFocusHooks' import { useMeasure } from '#/hooks/measureHooks' import { AnimatePresence, motion, type Variants } from 'framer-motion' @@ -38,14 +37,22 @@ export interface EditableSpanProps { export default function EditableSpan(props: EditableSpanProps) { const { className = '', editable = false, children } = props - if (editable) { - return - } - return ( - - {children} - + + {editable && } + + {!editable && ( + + + {children} + + + )} + ) } @@ -164,14 +171,14 @@ function EditForm(props: EditFormProps) { - {form.formState.isDirty && ( - + + {form.formState.isDirty && ( + )} - - - + +