Bug #14990

[Navigation] Attempting to navigate to non-existent path not handled

Added by Peter Amstutz over 1 year ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
Start date:
02/28/2020
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

Attempting to navigate to an unknown path should produce an error message. Examples where there is not currently the case include:

https://workbench2.c97qk.arvadosapi.com/project/c97qk-j7d0g-4xans0bhlqiyrkv - This spins forever ("project" misspelled)
https://workbench2.c97qk.arvadosapi.com/projects/c97qk-j7d0g-4xans0bhlqiyrk - also spins forever (last character of uuid is missing)
https://workbench2.c97qk.arvadosapi.com/projects/c97qk-j7d0g-4xans0bhlqiyrkk - loads an empty project page instead of returning an error saying non-existent project

Possibly relevant - https://stackoverflow.com/questions/32128978/react-router-no-not-found-route

This should 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.

  • 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.
  • 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 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.

Subtasks

Arvados - Task #16207: Review 14990-attempting-to-navigate-to-non-existent-path-not-handledResolvedDaniel Kutyła

Associated revisions

Revision 894d7e9c
Added by Lucas Di Pentima 6 months ago

Merge branch '14990-support-email-exported-config'

Refs #14990

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <>

Revision 011705a4
Added by Daniel Kutyła about 1 month ago

Merge branch '14990-attempting-to-navigate-to-non-existent-path-not-handled'
Closes #14990

Arvados-DCO-1.1-Signed-off-by: Daniel Kutyła <>

History

#1 Updated by Peter Amstutz over 1 year ago

  • Status changed from New to In Progress

#2 Updated by Peter Amstutz over 1 year ago

  • Subject changed from Attempting to navigate to non-existent path spins forever to Attempting to navigate to non-existent path not handled
  • Description updated (diff)

#3 Updated by Peter Amstutz over 1 year ago

  • Status changed from In Progress to New

#4 Updated by Chrystian Klingenberg over 1 year ago

  • Target version set to Backlog Q1, Q2

#5 Updated by Chrystian Klingenberg about 1 year ago

  • Target version changed from Backlog Q1, Q2 to Workbench2 Q3, Q4

#6 Updated by Chrystian Klingenberg about 1 year ago

  • Subject changed from Attempting to navigate to non-existent path not handled to [Navigation] Attempting to navigate to non-existent path not handled

#7 Updated by Tom Morris 12 months ago

  • Description updated (diff)

#8 Updated by Peter Amstutz 8 months ago

  • Release set to 20
  • Target version deleted (Workbench2 Q3, Q4)

#9 Updated by Peter Amstutz 6 months ago

  • Target version set to 2020-03-11 Sprint

#10 Updated by Peter Amstutz 6 months ago

  • Release deleted (20)
  • Assigned To set to Piotr Mrzygłowski
  • Category set to Workbench2

#11 Updated by Lucas Di Pentima 6 months ago

  • Status changed from New to In Progress

#12 Updated by Lucas Di Pentima 6 months ago

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: https://ci.arvados.org/job/developer-run-tests/1758/

#13 Updated by Lucas Di Pentima 6 months ago

  • 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.

#14 Updated by Peter Amstutz 5 months ago

  • Target version changed from 2020-03-11 Sprint to 2020-03-25 Sprint

#15 Updated by Peter Amstutz 5 months ago

  • Assigned To deleted (Piotr Mrzygłowski)

#16 Updated by Peter Amstutz 5 months ago

  • Release set to 31
  • Target version deleted (2020-03-25 Sprint)

#17 Updated by Peter Amstutz 2 months ago

  • Target version set to 2020-06-17 Sprint
  • Assigned To set to Daniel Kutyła

#18 Updated by Peter Amstutz 2 months ago

  • Description updated (diff)

#19 Updated by Peter Amstutz about 2 months ago

  • Target version changed from 2020-06-17 Sprint to 2020-07-01 Sprint

#21 Updated by Lucas Di Pentima about 2 months ago

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

#23 Updated by Lucas Di Pentima about 2 months ago

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

#24 Updated by Peter Amstutz about 1 month ago

  • Target version changed from 2020-07-01 Sprint to 2020-07-15

#26 Updated by Lucas Di Pentima about 1 month ago

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:?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.

#28 Updated by Lucas Di Pentima about 1 month ago

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.

#30 Updated by Lucas Di Pentima about 1 month ago

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

#31 Updated by Daniel Kutyła about 1 month ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved

Also available in: Atom PDF