diff --git a/HISTORY.rst b/HISTORY.rst index 7a6b43fe2..14a2c709b 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -38,8 +38,13 @@ Unreleased Changes ------------------ * Workbench - * Several small updates to the model input form UI to improve usability and visual consistency (https://github.com/natcap/invest/issues/912) - * Fixed a bug that was allowing readonly workspace directories on Windows (https://github.com/natcap/invest/issues/1599) + * Several small updates to the model input form UI to improve usability + and visual consistency (https://github.com/natcap/invest/issues/912) + * Fixed a bug that caused the application to crash when attempting to + open a workspace without a valid logfile + (https://github.com/natcap/invest/issues/1598) + * Fixed a bug that was allowing readonly workspace directories on Windows + (https://github.com/natcap/invest/issues/1599) 3.14.2 (2024-05-29) ------------------- diff --git a/workbench/src/main/ipcMainChannels.js b/workbench/src/main/ipcMainChannels.js index de7ca13c1..f908d7c33 100644 --- a/workbench/src/main/ipcMainChannels.js +++ b/workbench/src/main/ipcMainChannels.js @@ -14,6 +14,7 @@ export const ipcMainChannels = { IS_FIRST_RUN: 'is-first-run', LOGGER: 'logger', OPEN_EXTERNAL_URL: 'open-external-url', + OPEN_PATH: 'open-path', OPEN_LOCAL_HTML: 'open-local-html', SET_SETTING: 'set-setting', SHOW_ITEM_IN_FOLDER: 'show-item-in-folder', diff --git a/workbench/src/main/setupDialogs.js b/workbench/src/main/setupDialogs.js index cecc003a9..3e79df45b 100644 --- a/workbench/src/main/setupDialogs.js +++ b/workbench/src/main/setupDialogs.js @@ -26,4 +26,10 @@ export default function setupDialogs() { shell.showItemInFolder(filepath); } ); + + ipcMain.handle( + ipcMainChannels.OPEN_PATH, async (event, dirpath) => { + return await shell.openPath(dirpath); + } + ); } diff --git a/workbench/src/renderer/components/InvestTab/index.jsx b/workbench/src/renderer/components/InvestTab/index.jsx index 0c250a4e3..7e26369c8 100644 --- a/workbench/src/renderer/components/InvestTab/index.jsx +++ b/workbench/src/renderer/components/InvestTab/index.jsx @@ -7,6 +7,8 @@ import TabContainer from 'react-bootstrap/TabContainer'; import Nav from 'react-bootstrap/Nav'; import Row from 'react-bootstrap/Row'; import Col from 'react-bootstrap/Col'; +import Modal from 'react-bootstrap/Modal'; +import Button from 'react-bootstrap/Button'; import { MdKeyboardArrowRight, } from 'react-icons/md'; @@ -45,10 +47,6 @@ async function investGetSpec(modelName) { return undefined; } -function handleOpenWorkspace(logfile) { - ipcRenderer.send(ipcMainChannels.SHOW_ITEM_IN_FOLDER, logfile); -} - /** * Render an invest model setup form, log display, etc. * Manage launching of an invest model in a child process. @@ -64,6 +62,7 @@ class InvestTab extends React.Component { uiSpec: null, userTerminated: false, executeClicked: false, + showErrorModal: false, }; this.investExecute = this.investExecute.bind(this); @@ -71,6 +70,8 @@ class InvestTab extends React.Component { this.terminateInvestProcess = this.terminateInvestProcess.bind(this); this.investLogfileCallback = this.investLogfileCallback.bind(this); this.investExitCallback = this.investExitCallback.bind(this); + this.handleOpenWorkspace = this.handleOpenWorkspace.bind(this); + this.showErrorModal = this.showErrorModal.bind(this); } async componentDidMount() { @@ -154,6 +155,7 @@ class InvestTab extends React.Component { updateJobProperties(tabID, { argsValues: args, status: undefined, // in case of re-run, clear an old status + logfile: undefined, // in case of re-run where logfile may no longer exist, clear old logfile path }); ipcRenderer.send( @@ -186,6 +188,22 @@ class InvestTab extends React.Component { ); } + async handleOpenWorkspace(workspace_dir) { + if (workspace_dir) { + const error = await ipcRenderer.invoke(ipcMainChannels.OPEN_PATH, workspace_dir); + if (error) { + logger.error(`Error opening workspace (${workspace_dir}). ${error}`); + this.showErrorModal(true); + } + } + } + + showErrorModal(shouldShow) { + this.setState({ + showErrorModal: shouldShow, + }); + } + render() { const { activeTab, @@ -193,6 +211,7 @@ class InvestTab extends React.Component { argsSpec, uiSpec, executeClicked, + showErrorModal, } = this.state; const { status, @@ -213,88 +232,99 @@ class InvestTab extends React.Component { const sidebarFooterElementId = `sidebar-footer-${tabID}`; return ( - - - - -
-
- -
-
+ + + - { - status - ? ( - handleOpenWorkspace(logfile)} - terminateInvestProcess={this.terminateInvestProcess} - /> - ) - : null - } -
- - - - - + {t('Setup')} + + + + {t('Log')} + + + +
+
+ - - +
- - - - - - + { + status + ? ( + this.handleOpenWorkspace(argsValues?.workspace_dir)} + terminateInvestProcess={this.terminateInvestProcess} + /> + ) + : null + } +
+ + + + + + + + + + + + + + this.showErrorModal(false)} aria-labelledby="error-modal-title"> + + {t('Error opening workspace')} + + {t('Failed to open workspace directory. Make sure the directory exists and that you have write access to it.')} + + + + + ); } } diff --git a/workbench/tests/main/main.test.js b/workbench/tests/main/main.test.js index a65e806ff..2b479b688 100644 --- a/workbench/tests/main/main.test.js +++ b/workbench/tests/main/main.test.js @@ -187,6 +187,7 @@ describe('createWindow', () => { ipcMainChannels.GET_N_CPUS, ipcMainChannels.INVEST_VERSION, ipcMainChannels.IS_FIRST_RUN, + ipcMainChannels.OPEN_PATH, ipcMainChannels.SHOW_OPEN_DIALOG, ipcMainChannels.SHOW_SAVE_DIALOG, ]; diff --git a/workbench/tests/renderer/investtab.test.js b/workbench/tests/renderer/investtab.test.js index 74e9c9aa8..2a125fbdd 100644 --- a/workbench/tests/renderer/investtab.test.js +++ b/workbench/tests/renderer/investtab.test.js @@ -118,9 +118,71 @@ describe('Run status Alert renders with status from a recent run', () => { }); const { findByRole } = renderInvestTab(job); - const openWorkspace = await findByRole('button', { name: 'Open Workspace' }) - openWorkspace.click(); - expect(shell.showItemInFolder).toHaveBeenCalledTimes(1); + const openWorkspaceBtn = await findByRole('button', { name: 'Open Workspace' }); + expect(openWorkspaceBtn).toBeTruthy(); + }); +}); + +describe('Open Workspace button', () => { + const spec = { + args: {}, + }; + + const baseJob = { + ...DEFAULT_JOB, + status: 'success', + }; + + beforeEach(() => { + getSpec.mockResolvedValue(spec); + fetchValidation.mockResolvedValue([]); + uiConfig.UI_SPEC = mockUISpec(spec); + setupDialogs(); + }); + + afterEach(() => { + removeIpcMainListeners(); + }); + + test('should open workspace', async () => { + const job = { + ...baseJob, + argsValues: { + workspace_dir: '/workspace', + }, + }; + + jest.spyOn(ipcRenderer, 'invoke'); + + const { findByRole } = renderInvestTab(job); + const openWorkspaceBtn = await findByRole('button', { name: 'Open Workspace' }) + openWorkspaceBtn.click(); + + expect(ipcRenderer.invoke).toHaveBeenCalledTimes(1); + expect(ipcRenderer.invoke).toHaveBeenCalledWith(ipcMainChannels.OPEN_PATH, job.argsValues.workspace_dir); + }); + + test('should present an error message to the user if workspace cannot be opened (e.g., if it does not exist)', async () => { + const job = { + ...baseJob, + status: 'error', + argsValues: { + workspace_dir: '/nonexistent-workspace', + }, + }; + + jest.spyOn(ipcRenderer, 'invoke'); + ipcRenderer.invoke.mockResolvedValue('Error opening workspace'); + + const { findByRole } = renderInvestTab(job); + const openWorkspaceBtn = await findByRole('button', { name: 'Open Workspace' }); + openWorkspaceBtn.click(); + + expect(ipcRenderer.invoke).toHaveBeenCalledTimes(1); + expect(ipcRenderer.invoke).toHaveBeenCalledWith(ipcMainChannels.OPEN_PATH, job.argsValues.workspace_dir); + + const errorModal = await findByRole('dialog', { name: 'Error opening workspace'}); + expect(errorModal).toBeTruthy(); }); });