Feedback on live system

colin-nolan commented 6 years ago

UI

Route List

[x] It is not clear what search does (feedforward is too general - what is being searched?). No feedback is given if no routes (appears to do nothing).
[x] The user is likely to want to the route (in a nicely copiable form), opposed to just the token (which the user probably doesn't care about.
[x] Cursor does not give correct feedforward when hovering over list items (suggests text edit opposed to a link).
[x] It would be good to see the statistics here too

Route Add

[x] Errors when adding existing routes do not disappear.
[x] It is unclear what "Cancel" means when adding an existing route (not in middle of transaction).

Route Details

[ ] ~Route location on expected to be a link.~
[x] Give information in "Remove from my routes" confirmation that the action is not going to remove the route and that the route can be re-added later.
[x] Route details page does not know if the user have the route in their inventory - "Remove from my routes" should change to "Add to my routes" (or similar) if not in routes.
[x] Unsure if statistics are working?
[ ] It would be good if changes to the route on the details page were saved automatically (with feedback that save has taken place).

Routes

[ ] Request to a route that does not exist gives a 500 (expecting 404).

Configuration server

[x] Swagger UI on /ui is broken.

ThomasHickman commented 6 years ago

From c2f00dfec704e50905a97ed08b4aa13b96ebe251, all of the ticked features have been added:

It is not clear what search does (feedforward is too general - what is being searched?). No feedback is given if no routes (appears to do nothing).

"Search" has been changed to "Search Routes" and "No search results appears if no search results are found.

The user is likely to want to the route (in a nicely copiable form), opposed to just the token (which the user probably doesn't care about.

This is implemented in the route details page:

It would be good to see the statistics here too

It is unclear what "Cancel" means when adding an existing route (not in middle of transaction).

Changed to "Back"

All the other ticked features have been implemented as proposed

Errors when adding existing routes do not disappear.

I haven't been able to replicate this error

Route location on expected to be a link.

There doesn't seem to be any use in providing a link, as the user is not supposed to visit that page in their web browsers, if they do, it will just generate an error, because of a incorrect GET request.

Request to a route that does not exist gives a 500 (expecting 404).

I haven't been able to replicate this

colin-nolan commented 6 years ago

Errors when adding existing routes do not disappear.

I haven't been able to replicate this error

On the "Add an existing route" page, enter an invalid UUID, repeatidly press "Add route".

Route location on expected to be a link.

There doesn't seem to be any use in providing a link, as the user is not supposed to visit that page in their web browsers, if they do, it will just generate an error, because of a incorrect GET request.

You're right; expecting it to be copied opposed to navigated to is more reasonable.

Request to a route that does not exist gives a 500 (expecting 404).

I haven't been able to replicate this

http://webhook-router.hgi.sanger.ac.uk:8082/kittens

colin-nolan commented 6 years ago

Further feedback:

[x] It would be better to redirect the user to the route details page after it is created, opposed to the homepage. The justification is that it is very likely that the user wants to use the route after creation.
[x] The cursor for the link location does not give the correct feedforward (it suggests a portion of the text can be highlighted, which is not true as the click release leads to it all being highlighted). A pointer cursor would make more sense.

I looked for XSS issues - found none :).

wtsi-hgi / webhook-router