diff --git a/CHANGELOG-HMP-515.md b/CHANGELOG-HMP-515.md new file mode 100644 index 0000000000..e86b67e016 --- /dev/null +++ b/CHANGELOG-HMP-515.md @@ -0,0 +1 @@ +- Allow reopening closed workspace tabs without needing to relaunch the entire session. diff --git a/CHANGELOG-workspace-launch-form-fix.md b/CHANGELOG-workspace-launch-form-fix.md new file mode 100644 index 0000000000..b79cdcd981 --- /dev/null +++ b/CHANGELOG-workspace-launch-form-fix.md @@ -0,0 +1,2 @@ +- Fix bug with workspace creation locking after unsuccessful submit. +- Fix errors launching workspace when another workspace is running. diff --git a/context/app/static/js/components/workspaces/NewWorkspaceDialog/useCreateWorkspaceForm.ts b/context/app/static/js/components/workspaces/NewWorkspaceDialog/useCreateWorkspaceForm.ts index 43fa7bcdef..5b4b9366ea 100644 --- a/context/app/static/js/components/workspaces/NewWorkspaceDialog/useCreateWorkspaceForm.ts +++ b/context/app/static/js/components/workspaces/NewWorkspaceDialog/useCreateWorkspaceForm.ts @@ -51,7 +51,7 @@ function useCreateWorkspaceForm({ defaultName }: UseCreateWorkspaceTypes) { handleSubmit, control, reset, - formState: { errors, isSubmitting, isSubmitted }, + formState: { errors, isSubmitting, isSubmitSuccessful }, } = useForm({ defaultValues: { 'workspace-name': defaultName ?? '', @@ -68,7 +68,7 @@ function useCreateWorkspaceForm({ defaultName }: UseCreateWorkspaceTypes) { } async function onSubmit({ templateKeys, uuids, workspaceName }: CreateTemplateNotebooksTypes) { - if (isSubmitting || isSubmitted) return; + if (isSubmitting || isSubmitSuccessful) return; await createTemplateNotebooks({ templateKeys, uuids, workspaceName }); reset(); handleClose(); @@ -82,7 +82,7 @@ function useCreateWorkspaceForm({ defaultName }: UseCreateWorkspaceTypes) { control, errors, onSubmit, - isSubmitting: isSubmitting || isSubmitted, + isSubmitting: isSubmitting || isSubmitSuccessful, }; } diff --git a/context/app/static/js/components/workspaces/WorkspaceLaunchStopButton/index.ts b/context/app/static/js/components/workspaces/WorkspaceLaunchStopButton/index.ts deleted file mode 100644 index 22743a1c26..0000000000 --- a/context/app/static/js/components/workspaces/WorkspaceLaunchStopButton/index.ts +++ /dev/null @@ -1,3 +0,0 @@ -import WorkspaceLaunchStopButton from './WorkspaceLaunchStopButton'; - -export default WorkspaceLaunchStopButton; diff --git a/context/app/static/js/components/workspaces/WorkspaceLaunchStopButton/WorkspaceLaunchStopButton.tsx b/context/app/static/js/components/workspaces/WorkspaceLaunchStopButtons/WorkspaceLaunchStopButtons.tsx similarity index 59% rename from context/app/static/js/components/workspaces/WorkspaceLaunchStopButton/WorkspaceLaunchStopButton.tsx rename to context/app/static/js/components/workspaces/WorkspaceLaunchStopButtons/WorkspaceLaunchStopButtons.tsx index f94f8021b2..0e86a338a4 100644 --- a/context/app/static/js/components/workspaces/WorkspaceLaunchStopButton/WorkspaceLaunchStopButton.tsx +++ b/context/app/static/js/components/workspaces/WorkspaceLaunchStopButtons/WorkspaceLaunchStopButtons.tsx @@ -1,5 +1,6 @@ import React, { ElementType } from 'react'; import { ButtonProps } from '@mui/material/Button'; +import Stack from '@mui/material/Stack'; import { useSnackbarActions } from 'js/shared-styles/snackbars'; import { useLaunchWorkspace } from 'js/components/workspaces/hooks'; @@ -14,12 +15,36 @@ interface WorkspaceButtonProps { button: ElementType; } -function WorkspaceLaunchStopButton({ +function StopWorkspaceButton({ workspace, handleStopWorkspace, - isStoppingWorkspace, button: Button, + isStoppingWorkspace, }: WorkspaceButtonProps) { + const { toastError } = useSnackbarActions(); + const currentWorkspaceIsRunning = isRunningWorkspace(workspace); + if (!currentWorkspaceIsRunning) { + return null; + } + return ( + + ); +} + +function WorkspaceLaunchStopButtons(props: WorkspaceButtonProps) { + const { workspace, button: Button } = props; const { handleLaunchWorkspace } = useLaunchWorkspace(workspace); const { toastError } = useSnackbarActions(); if (workspace.status === 'deleting') { @@ -29,39 +54,25 @@ function WorkspaceLaunchStopButton({ ); } - const currentWorkspaceIsRunning = isRunningWorkspace(workspace); - if (currentWorkspaceIsRunning) { - return ( + return ( + + - ); - } - return ( - + ); } -export default WorkspaceLaunchStopButton; +export default WorkspaceLaunchStopButtons; diff --git a/context/app/static/js/components/workspaces/WorkspaceLaunchStopButtons/index.ts b/context/app/static/js/components/workspaces/WorkspaceLaunchStopButtons/index.ts new file mode 100644 index 0000000000..432b6f2dbb --- /dev/null +++ b/context/app/static/js/components/workspaces/WorkspaceLaunchStopButtons/index.ts @@ -0,0 +1,3 @@ +import WorkspaceLaunchStopButtons from './WorkspaceLaunchStopButtons'; + +export default WorkspaceLaunchStopButtons; diff --git a/context/app/static/js/components/workspaces/WorkspaceListItem.tsx b/context/app/static/js/components/workspaces/WorkspaceListItem.tsx index fcfa63d145..f95c33ddf4 100644 --- a/context/app/static/js/components/workspaces/WorkspaceListItem.tsx +++ b/context/app/static/js/components/workspaces/WorkspaceListItem.tsx @@ -7,7 +7,7 @@ import Button, { ButtonProps } from '@mui/material/Button'; import { PanelWrapper } from 'js/shared-styles/panels'; import WorkspaceDetails from 'js/components/workspaces/WorkspaceDetails'; import { SecondaryBackgroundTooltip } from 'js/shared-styles/tooltips'; -import WorkspaceLaunchStopButton from './WorkspaceLaunchStopButton'; +import WorkspaceLaunchStopButtons from './WorkspaceLaunchStopButtons'; import { MergedWorkspace } from './types'; import { jobStatuses } from './statusCodes'; import { useWorkspacesList } from './hooks'; @@ -56,7 +56,7 @@ function WorkspaceListItem({ workspace, toggleItem, selected }: WorkspaceListIte - { @@ -16,7 +28,7 @@ describe('mergeJobsIntoWorkspaces', () => { id: 1, other_ws_info: true, status: 'active', - path: 'workspace.ipynb', + path: '/workspace.ipynb', jobs, workspace_details, }, @@ -33,8 +45,8 @@ describe('mergeJobsIntoWorkspaces', () => { const jobs = []; const mergedWorkspaces = mergeJobsIntoWorkspaces(jobs, workspaces); expect(mergedWorkspaces).toEqual([ - { id: 1, status: 'active', jobs: [], path: 'workspace.ipynb', workspace_details }, - { id: 2, status: 'idle', jobs: [], path: 'workspace.ipynb', workspace_details }, + { id: 1, status: 'active', jobs: [], path: '/workspace.ipynb', workspace_details }, + { id: 2, status: 'idle', jobs: [], path: '/workspace.ipynb', workspace_details }, ]); }); @@ -51,6 +63,7 @@ describe('mergeJobsIntoWorkspaces', () => { status: 'active', workspace_details: { current_workspace_details: {}, // Response currently not guaranteed to have "files" key. + request_workspace_details: {}, }, }, { @@ -60,6 +73,9 @@ describe('mergeJobsIntoWorkspaces', () => { current_workspace_details: { files: [], // too few }, + request_workspace_details: { + ...workspace_details.request_workspace_details, + }, }, }, { @@ -69,6 +85,9 @@ describe('mergeJobsIntoWorkspaces', () => { current_workspace_details: { files: [{ name: 'workspace1.ipynb' }, { name: 'workspace2.ipynb' }], // too many... take first }, + request_workspace_details: { + ...workspace_details.request_workspace_details, + }, }, }, { @@ -76,10 +95,21 @@ describe('mergeJobsIntoWorkspaces', () => { status: 'active', workspace_details, // just right! }, + { + id: 3, + status: 'active', + workspace_details: workspace_details_request, // works when `request_workspace_details` has entry and `current_workspace_details` does not + }, ]; const jobs = []; const mergedWorkspaces = mergeJobsIntoWorkspaces(jobs, workspaces); - expect(mergedWorkspaces.map((ws) => ws.path)).toEqual(['', '', 'workspace1.ipynb', 'workspace.ipynb']); + expect(mergedWorkspaces.map((ws) => ws.path)).toEqual([ + '', + '', + '/workspace1.ipynb', + '/workspace.ipynb', + '/workspace.ipynb', + ]); }); }); diff --git a/context/app/static/js/components/workspaces/utils.ts b/context/app/static/js/components/workspaces/utils.ts index 88feb43893..f238718243 100644 --- a/context/app/static/js/components/workspaces/utils.ts +++ b/context/app/static/js/components/workspaces/utils.ts @@ -57,14 +57,33 @@ function getWorkspaceFileName(file: WorkspaceFile) { return file.name; } +/** + * Prepends a slash to a file name if it doesn't already have one + * @param fileName The file name to format + */ +function formatFileName(fileName: string) { + if (fileName.startsWith('/')) { + return fileName; + } + return `/${fileName}`; +} + /** * Gets the path of the notebook for a workspace * @param workspace The workspace to get the notebook path for * @returns The path of the notebook for the workspace */ function getNotebookPath(workspace: Workspace) { - const { files } = workspace.workspace_details.current_workspace_details; - return (files || []).reduce((acc, file) => { + const { files = [] } = workspace.workspace_details.current_workspace_details; + const { files: requestFiles = [] } = workspace.workspace_details.request_workspace_details; + const combinedFiles = files.concat(requestFiles).map((file) => { + const fileName = getWorkspaceFileName(file); + if (typeof file === 'string') { + return formatFileName(fileName); + } + return { ...file, name: formatFileName(fileName) }; + }); + return combinedFiles.reduce((acc, file) => { const path = getWorkspaceFileName(file); if (path.endsWith('.ipynb') && acc.length === 0) { return path; @@ -228,7 +247,8 @@ async function locationIfJobRunning({ } function getWorkspaceStartLink(workspace: Workspace, templatePath?: string) { - return `/workspaces/start/${workspace.id}?notebook_path=${encodeURIComponent(templatePath ?? workspace.path)}`; + const path = getNotebookPath(workspace); + return `/workspaces/start/${workspace.id}?notebook_path=${encodeURIComponent(templatePath ?? path)}`; } function getWorkspaceLink(workspace: Workspace) { diff --git a/context/app/static/js/pages/Workspace/Workspace.tsx b/context/app/static/js/pages/Workspace/Workspace.tsx index c75a75be01..c9496520c8 100644 --- a/context/app/static/js/pages/Workspace/Workspace.tsx +++ b/context/app/static/js/pages/Workspace/Workspace.tsx @@ -14,7 +14,7 @@ import LabelledSectionText from 'js/shared-styles/sections/LabelledSectionText'; import { condenseJobs, getWorkspaceFileName } from 'js/components/workspaces/utils'; import JobStatus from 'js/components/workspaces/JobStatus'; import { useWorkspaceDetail } from 'js/components/workspaces/hooks'; -import WorkspaceLaunchStopButton from 'js/components/workspaces/WorkspaceLaunchStopButton'; +import WorkspaceLaunchStopButtons from 'js/components/workspaces/WorkspaceLaunchStopButtons'; import Typography from '@mui/material/Typography'; import { SpacedSectionButtonRow } from 'js/shared-styles/sections/SectionButtonRow'; import SectionHeader from 'js/shared-styles/sections/SectionHeader'; @@ -51,7 +51,8 @@ function useMatchingWorkspaceTemplates(workspace: MergedWorkspace) { } function LaunchStopButton(props: ButtonProps) { - return