Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Not found message by context #228

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

syphax-bouazzouni
Copy link
Contributor

@syphax-bouazzouni syphax-bouazzouni commented Nov 17, 2022

What

This PR make error messages more descripitf, for the case of Onotology not found , Class not found and Onotology no roots found errors

Before

Missing roots error

image

Onotology not found error

image

Class not found

image

After

Missing roots error

image

Onotology not found error

image

Class not found

image

@syphax-bouazzouni syphax-bouazzouni changed the title Not found message by context Feature: Not found message by context Nov 17, 2022
@jonquet
Copy link

jonquet commented Nov 17, 2022

This is a first patch for a deeper refactoring of the exceptions handling on the front end. Ultimately the idea is to avoid the "Something went wrong" screen for things that are trivial to inform the user.

@jvendetti
Copy link
Member

Hi Syphax & Clement. If you guys are looking at refactoring the error pages and descriptions, I have some comments for your consideration.

I think it would be nice to uniformly use "not found" for 404 errors. In other words, instead of "missing" for classes and "not found" for ontologies, it seems cleaner to use "not found" for both.

Also, using the full class ID in the class not found error doesn't look that user-friendly to me. I think it would be nicer to use the preferred label. You could add the class ID below - perhaps using a smaller text in a greyed out color.

I'm imagining something similar to Google's 404 pages:

404. Class not found error.
The requested class 'accident reporting' was not found in AGROVOC.

Class ID: http://aims.fao.org/aos/agrovoc/c_331462

I understand the above is problematic if there's no preferred label, but I'm just thinking out loud here.

Possible equivalent for ontologies:

404. Ontology not found error.
The requested ontology 'Animal Trait Ontology for Livestock' was not found in AgroPortal.

Ontology ID: ATOL

With regard to the "missing roots" error - this feels like a developer-centric error message that I would expect to see in a log file as the underlying cause of why the Rails application was unable to display a class tree. It's immediately evident to me what it means, but I don't think it means anything to an end user. I'm having a hard time thinking of a good alternative mesage - perhaps something like "Unable to load the class tree for OMO".

It's kind of unfortunate that missing roots results in different presentations of an error depending on what URL pattern is used. This URL (https://bioportal.bioontology.org/ontologies/OMO/?p=classes) results in the main 404 page:

Screen Shot 2022-11-18 at 5 07 33 PM

... and this URL (https://bioportal.bioontology.org/ontologies/OMO/?p=classes&conceptid=root) results in the Classes tab with an error description:

Screen Shot 2022-11-18 at 5 08 13 PM

@jonquet
Copy link

jonquet commented Dec 21, 2022

@jvendetti Thanks for remarks. We have not discuss this again with @syphax-bouazzouni but will do. Bye.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants