Workbench: validate metadata that is used to open invest models #1370
Conversation
davemfish
changed the title
| @@ -82,7 +82,6 @@ export function fetchDatastackFromFile(payload) { | |||
| headers: { 'Content-Type': 'application/json' }, | |||
| }) | |||
| .then((response) => response.json()) | |||
| .catch((error) => logger.error(error.stack)) | |||
There was a problem hiding this comment.
Just making sure I understand - this error would now be caught farther up?
There was a problem hiding this comment.
That's right. It felt better to add the user-facing alert message to UI component, so that's where the error-handling is now.
| logger.error(error); | ||
| alert( // eslint-disable-line no-alert | ||
| t( | ||
| 'No InVEST model data can be parsed from the file:\n {{filepath}}', |
There was a problem hiding this comment.
I wonder if this message could provide any more information to help the user solve the problem. Maybe something like
File could not be opened as an InVEST model run. Are you sure this is a valid datastack for a current InVEST model?
There was a problem hiding this comment.
I agree it's not perfect but I'm kind of at a loss for how to improve it. This is from the "Load Parameters" button on a model's sidebar, so we're not really opening a "model run" here. If "datastack" had a clear meaning to users then I'd agree we could say, "this is not a valid datastack", but I'm afraid that term is mostly insider-jargon.
My hope is that anyone who sees this message will just realize they clicked on the wrong file or something. If they really don't know what file they should be looking for, then that's a larger design flaw and we should try to add some guidance before the user reaches this alert. We have a tooltip that tries to do that now.
There was a problem hiding this comment.
Oh I see what you mean. Yeah, this should be fine for now.
There was a problem hiding this comment.
@davemfish this all makes sense! Just one suggestion about the alert message.
thanks for reviewing @emlys ! I replied to your comments; back to you. |
This PR adds some validation before rendering data from the recent jobs store, to avoid rendering a card for a model that no longer exists in the current version of invest, and to skip over jobs that are somehow missing critical pieces of data.
It also adds error handling for trying to load a datastack from a deprecated model.
Fixes #1275 and fixes #1286
Checklist