PUBLIC-1009 add better error handling when a staging area directory...

[briehl]

...doesn't exist.
Previously, it was a big red box that was only removable by refreshing the page.

Now, it should look something like this:

[briehl]

This means a user can now navigate back to a directory that does exist.

[coveralls]

Coverage increased (+0.06%) to 17.354% when pulling 7bd42f9 on briehl:PUBLIC-1009-staging-subdir into b6f1cdf on kbase:develop.

[briehl]

Detailed notes on changes.

• Previously, when an error occurred during lookup, the whole table and control area was replaced with a big red error. Now it renders the controls, and shows a table with either a more sensible "no data" message, or the error message.
• Wrapped the call in a Bluebird promise, since that's used everywhere else, and it was affecting testing (by making it impossible).

[eapearson]

Code changes look great.
I've set this up locally and replicated the issue. I can see the fix in effect.
For the issue specifically raised in PUBLIC-1009, I can see the change from an error message which replaces the content below the drag-and-drop upload area to retaining the file list table and controls with a message like "path eapearson/test3 does not exist".
This is perhaps too subtle?
The message could be a red error message to get the user's attention, and to signify that it is indeed an error. I didn't see the error message initially.
Also, since there are indeed no files to show, I think it would actually be appropriate to replace the table view with an error message (but retaining the controls above it), and either show controls or do something to help the user out of the situation. E.g. navigate to the next existing parent directory.
Also the error message could be more useful. E.g. instead of "path eapearson/test3 does not exist", perhaps "directory 'test3' cannot be displayed because it has been deleted", since we know it did exist earlier and must have been deleted.

[eapearson]

Another issue was raised when uncovering a (current, in CI) bug listing any newly-added files. Easy to replicate, just upload a file, the import table will immediately show an error message. Since the error goes away if the file is removed (e.g. via globus), it is reasonable to assume it is in the server code which processes the file listing.

The Narrative issue is that it shows an error message when the error is first triggered which is not displayed after the narrative is reloaded and the import tab revisited.

I noticed, too, that the "500 Internal Server Error" error message displayed is not styled with red error colors (background or lettering).

Also, if there is an error fetching the entries for a directory, it is debatable that the table, or at least the table paraphernalia ("Showing ...", previous/next buttons, search) controls should be displayed since they imply that there is something to be displayed or searched over, when in the case of a directory listing throwing an error, there is nothing to be done there (and if it is at the top level directory, nothing the user can do at all!)

[briehl]

The message could be a red error message to get the user's attention, and to signify that it is indeed an error. I didn't see the error message initially.

Yup, agreed.

Also, since there are indeed no files to show, I think it would actually be appropriate to replace the table view with an error message (but retaining the controls above it), and either show controls or do something to help the user out of the situation. E.g. navigate to the next existing parent directory.

Also agreed. That control is actually rendered by embedding it in the datatables call. I was avoiding removing that and fighting against the datatables rendering pipeline. But we can probably just fork how that render logic gets run. I was just trying to avoid the extra refactor. :)

But you're right. It's worth the effort.

Also the error message could be more useful. E.g. instead of "path eapearson/test3 does not exist", perhaps "directory 'test3' cannot be displayed because it has been deleted", since we know it did exist earlier and must have been deleted.

Good idea. It should be possible to parse the errors into something better than just a giant 500 Internal Server Error, which is just big and imposing.

[briehl]

Should be some better error messaging here now, in line with our conversation.