Arvados » Arvados Workbench 2

Supporting this story, I've created the branch 14990-support-email-exported-config to add Mail.SupportEmailAddress config to controller's exported config endpoint.
Test run: developer-run-tests: #1758

• Although the ticket didn’t explained it, I think we need to mimic workbench1’s behavior as much as possible, so I think it would be best to show the “not found” component as a default route instead of doing the redirect, that way the URL isn’t changed.
• Also, in the spirit of copying Workbench1’s style, when requesting for a known object type, it should show a message like: The <object_type> with UUID zzzzz-yyyyyy-xxxxxxxxxxxxxxxx was not found. Perhaps you'd like to browse all <object_type>s? (with a link)
• When requesting a trashed project/group or collection, instead of showing a “not found” message, workbench1 shows a message saying it is trashed, the date when the object will be definitely deleted and a button allowing the user to recover it from the trash. See “untrash” method on https://doc.arvados.org/v2.0/api/methods/groups.html as an example.
  • In the particular case of collections, when accessing to /collections/some-valid-uuid, where some-valid-uuid has the correct format (for example: x2jbo-4zz18-xr1ir7qowrj2y2k) but it doesn’t exist or is trashed, an error toast is being displayed with a 404 message and the UI is blank or partially filled if we’re requesting a previously non-trashed collection. Instead, workbench2 should show a message like the one described for the projects/groups.
  • Also, when accessing to /collections/not-a-valid-uuid, Workbench2 does a content address search, although workbench1 doesn’t offer the possibility to “untrash" trashed collections by PDH, Workbench2 should have this feature.
• On every “not found” page, workbench1 also provides the user the ability to send a message to the admins via a support form or email. The support form is backed by workbench1’s rails application, and we don’t have that capability on workbench2, OTOH we could provide an email link that pre-populates the email with API version number, url being accessed, etc. The admin email should be accessible via the public config endpoint (for example: https://ce8i5.arvadosapi.com/arvados/v1/config) at the Mail.SupportEmailAddress key. This config key will be available when the branch 14990-support-email-exported-config is merged on arvados’ repo.

New version: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/dd4004015ae711939d2474dd8c6e031b6be2d0ff
Test run: developer-tests-workbench2: #53

• Added 404 page with wildcard route

Reviewing arvados-workbench2|dd40040 - branch 14990-attempting-to-navigate-to-non-existent-path-not-handled

The default routing looks good. What's missing is a link/button that allows the user to send a pre-populated email to the site admin to report an issue, like on Workbench1. You can get the admin's email address from the exported config's Mail.SupportEmailAddress key available at state.auth.config.clusterConfig

New version: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/51041db20a296235ca14c6cc73f1ff7f1db2c0b7
Version with testfix: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/625e72407ad730e05c1551df3e2919a1a9ffd3e4
Test run: developer-tests-workbench2: #56

• collection uuid / object hash check and 404 additional notification
• unit tests added

Both test runs failed, have you already tried to run them locally and they're only failing on Jekins?

New version (mostly master merge): https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/6cab974c8255a413d0714d53dcdf125d869bfc62

Version with most code changes: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/51041db20a296235ca14c6cc73f1ff7f1db2c0b7

Test run: developer-tests-workbench2: #60

collection uuid / object hash check and 404 additional notification
unit tests added

Reviewing arvados-workbench2|70ec540

• File src/views/not-found-panel/not-found-panel-root.tsx - Line 66: repeated ‘us’ word on ‘…email us us if you suspect…’ error message. Also, on its test case.
• When the support email address isn’t configured, the link will have an undefined mailto: href. Do you think it would be better to display another message? Like, “The page you requested was not found, contact the site admin if you suspect this is a bug.” this way we save the user the frustration of emailing an inexistent address. (Please add a test for this, too.)
• The email link could include additional information so that the user doesn’t need to add it manually. Example: “mailto:admin@example.com?subject=…&body=…” (Check RFC 2368 to know more). It would be nice to add the following information:
  • User’s uuid trying to access the object
  • Object being tried to access (PDH/UUID that produced the error)
  • URL being accessed
  • Maybe a section header so that the user can add their own notes/comments.
• The UUID incorrect format message only works for collections, but we use UUIDs for all types of objects: projects/groups, containers, container requests (aka: processes on wb2), users, etc.
• When requesting a collection with a valid but unreachable uuid, the page is blank and a snackbar with a 404 error briefly appears. As described on the ticket’s description, it should show a page with the message "The <object_type> with UUID zzzzz-yyyyyy-xxxxxxxxxxxxxxxx was not found."
• When requesting a project with a valid but unreachable uuid, it seems to display an empty project, and a error snackbar storm starts, we should handle any ‘valid but not found’ uuid object request like the previous point.
• Also, make sure no snackbar error message appears when the "not found" cases are handled.
• The added unit tests check that the “not found” component renders the correct information. I think a check that the component is used on correct cases is missing. My first thought would be to make an e2e test, but maybe you know a better (faster) way to test that.
• Requesting trashed objects doesn’t inform the user that the object being displayed is trashed. I don’t remember if we talked about this on our last call with Peter.

New version: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/90460e90dfb2ac4863b133bc477098252f677fea
Test run: developer-tests-workbench2: #64

added e2e tests

Reviewing arvados-workbench2|90460e9

• The email link is being rendered like this:
  

  <a class="Component-active-499" href="mailto:mailto:arvados@example.com?body=Problem while viewing page /collections/x2jbo-4zz18-yvzizixe23360u;subject=Workbench problem report">email us</a>

  The problems I see on it are:
  • Double mailto: scheme.
  • No url-encoded params' values. Example from wb1:
    

    <a href="mailto:arvados@example.com?body=Problem%20while%20viewing%20page%20https%3A%2F%2Fworkbench.ce8i5.arvadosapi.com%2Fcollections%2Fce8i5-4zz18-8gkju5htq6mziu&subject=Workbench%20problem%20report">email us</a>
    

• There’s commented out code at file src/views-components/not-found-dialog/not-found-dialog.tsx line 52.
• If the Mail.SupportEmailAddress cluster config value is not set (ie: having an empty string as its value), I think it’s not a good UX to offer the user a mailto: link that won’t work, do you agree? Please add a test checking this.

New version: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/4badc373d653f27010b01ddb12fb5275e706b666
Test run: developer-tests-workbench2: #65

Fixed double mailto, removed commented code, added tests and email check

Updates at arvados-workbench2|4badc37 LGTM, thanks!