diff --git a/HISTORY.rst b/HISTORY.rst index 772b0de21..316436b57 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -59,6 +59,9 @@ Unreleased Changes * Closing the main window will now close any user's guide windows that are open. Fixed a bug where the app could not be reopened after closing. (`#1258 `_) + * Fixed a bug where invalid metadata for a recent run would result + in an uncaught exception. + (`#1286 `_) * Middle clicking an InVEST model tab was opening a blank window. Now middle clicking will close that tab as expected. (`#1261 `_) diff --git a/workbench/src/renderer/InvestJob.js b/workbench/src/renderer/InvestJob.js index 5f5ab66e9..59a1fc13f 100644 --- a/workbench/src/renderer/InvestJob.js +++ b/workbench/src/renderer/InvestJob.js @@ -58,8 +58,8 @@ export default class InvestJob { const lastKey = sortedJobHashes.pop(); investJobStore.removeItem(lastKey); } - await investJobStore.setItem(HASH_ARRAY_KEY, sortedJobHashes); await investJobStore.setItem(job.hash, job); + await investJobStore.setItem(HASH_ARRAY_KEY, sortedJobHashes); return InvestJob.getJobStore(); } diff --git a/workbench/src/renderer/app.jsx b/workbench/src/renderer/app.jsx index b0c5602c0..bc05cb38e 100644 --- a/workbench/src/renderer/app.jsx +++ b/workbench/src/renderer/app.jsx @@ -57,7 +57,12 @@ export default class App extends React.Component { const recentJobs = await InvestJob.getJobStore(); this.setState({ investList: investList, - recentJobs: recentJobs, + // filter out models that do not exist in current version of invest + recentJobs: recentJobs.filter((job) => ( + Object.values(investList) + .map((m) => m.model_name) + .includes(job.modelRunName) + )), showDownloadModal: this.props.isFirstRun, }); await i18n.changeLanguage(window.Workbench.LANGUAGE); diff --git a/workbench/src/renderer/components/HomeTab/index.jsx b/workbench/src/renderer/components/HomeTab/index.jsx index 32b6d5f74..ec12bfc60 100644 --- a/workbench/src/renderer/components/HomeTab/index.jsx +++ b/workbench/src/renderer/components/HomeTab/index.jsx @@ -11,6 +11,8 @@ import { useTranslation } from 'react-i18next'; import OpenButton from '../OpenButton'; import InvestJob from '../../InvestJob'; +const { logger } = window.Workbench; + /** * Renders a table of buttons for each invest model and * a list of cards for each cached invest job. @@ -110,46 +112,51 @@ HomeTab.propTypes = { */ function RecentInvestJobs(props) { const { recentJobs, openInvestModel } = props; - const handleClick = (jobMetadata) => { - openInvestModel(new InvestJob(jobMetadata)); - } const { t, i18n } = useTranslation(); - // Buttons to load each recently saved state + const handleClick = (jobMetadata) => { + try { + openInvestModel(new InvestJob(jobMetadata)); + } catch (error) { + logger.debug(error); + } + }; + const recentButtons = []; recentJobs.forEach((job) => { - if (!job.argsValues) { return; } - recentButtons.push( - handleClick(job)} - > - - - {job.modelHumanName} - - - {'Workspace: '} - {job.argsValues.workspace_dir} - - - {'Suffix: '} - {job.argsValues.results_suffix} - - - {job.humanTime} - - {(job.status === 'success' - ? {t('Model Complete')} - : {job.status} - )} - - - - - ); + if (job && job.argsValues && job.modelHumanName) { + recentButtons.push( + handleClick(job)} + > + + + {job.modelHumanName} + + + {'Workspace: '} + {job.argsValues.workspace_dir} + + + {'Suffix: '} + {job.argsValues.results_suffix} + + + {job.humanTime} + + {(job.status === 'success' + ? {t('Model Complete')} + : {job.status} + )} + + + + + ); + } }); return ( diff --git a/workbench/src/renderer/components/OpenButton/index.jsx b/workbench/src/renderer/components/OpenButton/index.jsx index 154736a2c..ed5945d52 100644 --- a/workbench/src/renderer/components/OpenButton/index.jsx +++ b/workbench/src/renderer/components/OpenButton/index.jsx @@ -11,6 +11,7 @@ import { fetchDatastackFromFile } from '../../server_requests'; import { ipcMainChannels } from '../../../main/ipcMainChannels'; const { ipcRenderer } = window.Workbench.electron; +const { logger } = window.Workbench; /** * Render a button that loads args from a datastack, parameterset, or logfile. @@ -23,9 +24,22 @@ class OpenButton extends React.Component { } async browseFile() { + const { t } = this.props; const data = await ipcRenderer.invoke(ipcMainChannels.SHOW_OPEN_DIALOG); if (!data.canceled) { - const datastack = await fetchDatastackFromFile(data.filePaths[0]); + let datastack; + try { + datastack = await fetchDatastackFromFile(data.filePaths[0]); + } catch (error) { + logger.error(error); + alert( + t( + 'No InVEST model data can be parsed from the file:\n {{filepath}}', + { filepath: data.filePaths[0] } + ) + ); + return; + } const job = new InvestJob({ modelRunName: datastack.model_run_name, modelHumanName: datastack.model_human_name, diff --git a/workbench/src/renderer/components/SetupTab/index.jsx b/workbench/src/renderer/components/SetupTab/index.jsx index 4cabaa575..c96dc6dd0 100644 --- a/workbench/src/renderer/components/SetupTab/index.jsx +++ b/workbench/src/renderer/components/SetupTab/index.jsx @@ -26,6 +26,7 @@ import { argsDictFromObject } from '../../utils'; import { ipcMainChannels } from '../../../main/ipcMainChannels'; const { ipcRenderer } = window.Workbench.electron; +const { logger } = window.Workbench; /** Initialize values of InVEST args based on the model's UI Spec. * @@ -54,7 +55,7 @@ function initializeArgValues(argsSpec, uiSpec, argsDict) { if (argsSpec[argkey].type === 'boolean') { value = argsDict[argkey] || false; } else if (argsSpec[argkey].type === 'option_string') { - if (argsDict[argkey]) { + if (argsDict[argkey]) { value = argsDict[argkey]; } else { // default to first if (Array.isArray(argsSpec[argkey].options)) { @@ -274,8 +275,20 @@ class SetupTab extends React.Component { } async loadParametersFromFile(filepath) { - const datastack = await fetchDatastackFromFile(filepath); const { pyModuleName, switchTabs, t } = this.props; + let datastack; + try { + datastack = await fetchDatastackFromFile(filepath); + } catch (error) { + logger.error(error); + alert( // eslint-disable-line no-alert + t( + 'No InVEST model data can be parsed from the file:\n {{filepath}}', + { filepath: filepath } + ) + ); + return; + } if (datastack.module_name === pyModuleName) { this.batchUpdateArgs(datastack.args); switchTabs('setup'); diff --git a/workbench/src/renderer/server_requests.js b/workbench/src/renderer/server_requests.js index cf5ab74f4..286bcd690 100644 --- a/workbench/src/renderer/server_requests.js +++ b/workbench/src/renderer/server_requests.js @@ -82,7 +82,6 @@ export function fetchDatastackFromFile(payload) { headers: { 'Content-Type': 'application/json' }, }) .then((response) => response.json()) - .catch((error) => logger.error(error.stack)) ); } diff --git a/workbench/tests/renderer/app.test.js b/workbench/tests/renderer/app.test.js index 9ece74d04..60e5f6c89 100644 --- a/workbench/tests/renderer/app.test.js +++ b/workbench/tests/renderer/app.test.js @@ -268,7 +268,7 @@ describe('Various ways to open and close InVEST models', () => { describe('Display recently executed InVEST jobs on Home tab', () => { beforeEach(() => { - getInvestModelNames.mockResolvedValue({}); + getInvestModelNames.mockResolvedValue(MOCK_INVEST_LIST); }); afterEach(async () => { @@ -277,7 +277,7 @@ describe('Display recently executed InVEST jobs on Home tab', () => { test('Recent Jobs: each has a button', async () => { const job1 = new InvestJob({ - modelRunName: 'carbon', + modelRunName: MOCK_MODEL_RUN_NAME, modelHumanName: 'Carbon Sequestration', argsValues: { workspace_dir: 'work1', @@ -286,7 +286,7 @@ describe('Display recently executed InVEST jobs on Home tab', () => { }); await InvestJob.saveJob(job1); const job2 = new InvestJob({ - modelRunName: 'sdr', + modelRunName: MOCK_MODEL_RUN_NAME, modelHumanName: 'Sediment Ratio Delivery', argsValues: { workspace_dir: 'work2', @@ -328,7 +328,7 @@ describe('Display recently executed InVEST jobs on Home tab', () => { test('Recent Jobs: a job with incomplete data is skipped', async () => { const job1 = new InvestJob({ - modelRunName: 'carbon', + modelRunName: MOCK_MODEL_RUN_NAME, modelHumanName: 'invest A', argsValues: { workspace_dir: 'dir', @@ -337,7 +337,7 @@ describe('Display recently executed InVEST jobs on Home tab', () => { }); const job2 = new InvestJob({ // argsValues is missing - modelRunName: 'sdr', + modelRunName: MOCK_MODEL_RUN_NAME, modelHumanName: 'invest B', status: 'success', }); @@ -350,6 +350,23 @@ describe('Display recently executed InVEST jobs on Home tab', () => { expect(queryByText(job2.modelHumanName)).toBeNull(); }); + test('Recent Jobs: a job from a deprecated model is not displayed', async () => { + const job1 = new InvestJob({ + modelRunName: 'does not exist', + modelHumanName: 'invest A', + argsValues: { + workspace_dir: 'dir', + }, + status: 'success', + }); + await InvestJob.saveJob(job1); + const { findByText, queryByText } = render(); + + expect(queryByText(job1.modelHumanName)).toBeNull(); + expect(await findByText(/Set up a model from a sample datastack file/)) + .toBeInTheDocument(); + }); + test('Recent Jobs: placeholder if there are no recent jobs', async () => { const { findByText } = render( @@ -361,7 +378,7 @@ describe('Display recently executed InVEST jobs on Home tab', () => { test('Recent Jobs: cleared by button', async () => { const job1 = new InvestJob({ - modelRunName: 'carbon', + modelRunName: MOCK_MODEL_RUN_NAME, modelHumanName: 'Carbon Sequestration', argsValues: { workspace_dir: 'work1', diff --git a/workbench/tests/renderer/setuptab.test.js b/workbench/tests/renderer/setuptab.test.js index babdcbac3..843aba394 100644 --- a/workbench/tests/renderer/setuptab.test.js +++ b/workbench/tests/renderer/setuptab.test.js @@ -163,6 +163,21 @@ describe('Arguments form input types', () => { expect(input).toHaveValue('a'); expect(input).not.toHaveValue('b'); }); + + test('initial arg values can contain extra args', async () => { + const spec = baseArgsSpec('number'); + const displayedValue = '1'; + const missingValue = '0'; + const initArgs = { + [Object.keys(spec.args)[0]]: displayedValue, + paramZ: missingValue, // paramZ is not in the ARGS_SPEC or UI_SPEC + }; + + const { findByLabelText, queryByText } = renderSetupFromSpec(spec, UI_SPEC, initArgs); + const input = await findByLabelText(`${spec.args.arg.name} (${spec.args.arg.units})`); + await waitFor(() => expect(input).toHaveValue(displayedValue)); + expect(queryByText(missingValue)).toBeNull(); + }); }); describe('Arguments form interactions', () => {