OpenConceptLab / ocl_issues

Issues for all OCL repos. NOTE: Install ZenHub Browser Extension and request access to the OCL Roadmap board to view all issues and to contribute
4 stars 1 forks source link

Simple error message fix for adding references to a collection #1825

Closed jamlung-ri closed 3 months ago

jamlung-ri commented 5 months ago

Users are finding OCL's error messages unhelpful when adding references to a collection. I think a better approach to that overall is needed for v3, but I do have an idea for a (hopefully) quick change that could have high impact for v2 users.

OCL's results popup when adding references to a collection should do the following things:

  1. Separate errors out into their own sections/categories that show whether the error is due to a Validation Schema conflict vs. a Duplicate Reference conflict vs. something else
  2. Collapse all sections in the error message, particularly if the expanded error list would cause the user to have to scroll to see the errors. Currently, successes are shown at the top of the results and errors at the bottom, so it's easy to miss the errors.
  3. (Ideal but don't do this yet if it's not a small lift) When the error is due to a conflicting name in the target collection, show the name of the concept(s) that are conflicting in the error message.

Recording where I talk through this more: https://iu.mediaspace.kaltura.com/media/t/1_4uddukb5

Example error message where this would be effective: image cc: @bmamlin

jamlung-ri commented 5 months ago

@paulsonder Adding you to this ticket too - do you feel like this needs any design work? Or is it simple enough to go ahead with development?

bmamlin commented 5 months ago

Instead of

/orgs/CIEL/sources/CIEL/concepts/703/:Concept or Mapping reference name must be unique in a collection.

it would be more helpful to see something more like:

Concept "Positive" (703) could not be added because the collection already contains a reference that includes "Positive" (703).

It would also be helpful if we had links that would take the user to a specific concept or reference of a collection in the Term Browser.

paynejd commented 5 months ago

I would assume that the API response stays the same and that the TB replaces any relative concept/mapping/repo URLs with a chip that provides additional info and adds links. Does that seem right?

On Thu, Apr 25, 2024 at 4:23 PM Burke Mamlin @.***> wrote:

Instead of

/orgs/CIEL/sources/CIEL/concepts/703/:Concept or Mapping reference name must be unique in a collection.

it would be more helpful to see something more like:

Concept "Positive" (703) https://app.staging.openconceptlab.org/#/orgs/CIEL/sources/CIEL/concepts/703/ could not be added because the collection already contains a reference https://api.staging.openconceptlab.org/users/burke/collections/test/references/3385135/ that includes "Positive" (703).

It would also be helpful if we had links that would take the user to a specific concept or reference of a collection in the Term Browser.

— Reply to this email directly, view it on GitHub https://github.com/OpenConceptLab/ocl_issues/issues/1825#issuecomment-2078111153, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJCOOPRV23CE2KLESYMVT3Y7FQ5XAVCNFSM6AAAAABGPX2OP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZYGEYTCMJVGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

snyaggarwal commented 5 months ago

@paynejd some of the API error responses need to change as well. API needs to throw more information. Right now everything that you see in the errors on UI is from API only.

snyaggarwal commented 5 months ago

@paynejd @jamlung-ri Can this be picked next?

jamlung-ri commented 5 months ago

@snyaggarwal I think so, but one warning first: It's possible that we'll provide further collection management requirements in the near future, which might affect this work. Do you think it's okay to start on the work in this ticket for now, but also maybe revisit it or further improve it in the coming weeks/months?

snyaggarwal commented 5 months ago

@jamlung-ri Ya that should be ok. The only thing we need to confirm on the breaking change on API errors structure.

paynejd commented 5 months ago

If we can identify one or 2 iterative updates that will specifically help Grace with collection management, then this is probably worth it. Let's try not to take on the updates that require major refactoring at this point--it would be better to do that as part of "v3 repo management".

On Mon, Apr 29, 2024 at 9:15 AM Sunny Aggarwal @.***> wrote:

@jamlung-ri https://github.com/jamlung-ri Ya that should be ok. The only thing we need to confirm on the breaking change on API errors structure.

— Reply to this email directly, view it on GitHub https://github.com/OpenConceptLab/ocl_issues/issues/1825#issuecomment-2082717029, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJCOONOD7SSWZGNZY2RIKDY7ZBYHAVCNFSM6AAAAABGPX2OP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBSG4YTOMBSHE . You are receiving this because you were mentioned.Message ID: @.***>

snyaggarwal commented 5 months ago

@jamlung-ri @bmamlin This is deployed on QA. Please try it out

jamlung-ri commented 5 months ago

@snyaggarwal I'm having some successes and some issues when doing this. I'll list them below.

Success:

  1. When trying to add a reference to a collection where the reference already exists in some form, I do see that the reference wasn't added and can click on the conflicting reference to see more details.
  2. The OpenMRS Cascade is still working great - it made it possible to update my collection with new names from my source!

Issues

  1. When there's a validation schema conflict, I can see in the API response that there was an error, but the TermBrowser isn't displaying it correctly. It just displays a blank error message next to the reference.
  2. The successes show above the errors in the Add to Collection results screen, so the errors are hidden. If it's not a small lift to collapse those sections in the results screen, can the errors at least be placed above the successes in that results screen?

Success 1: image

Issue 1: image

Issue 2: image

jamlung-ri commented 5 months ago

Video where I walk through some of these issues, if helpful: https://iu.zoom.us/rec/share/no5ezOX6LQNTfVMUY7ZtbOctzthbFwNttl-rWUTQ6Gmv77Urc3BpWwVfjrIjb6z8.8vXbW8JVIwbG6POA

snyaggarwal commented 5 months ago

@jamlung-ri The fixes are deployed on QA

jamlung-ri commented 5 months ago

Awesome, making good progress! I had a couple more good successes, and just one more thing that I'm noticing to fix:

Successes:

  1. The results screen now uses collapsed sections, and the errors are at the top!
  2. The validation schema error for "fully specified name must be unique..." is showing the basic info needed for diagnosing! See example:

    /orgs/CIEL/sources/CIEL/concepts/703/: Concept fully specified name must be unique for same collection and locale.Conflicting with Concept sdf:Positive name Positive from existing reference.

Issue:

  1. When I add one single reference using the OpenMRS Cascade (with "Extensional" box unchecked), the results screen shows that the reference couldn't be added, but it doesn't show why. See screenshots below for more info.
    1. Note: The API response does provide the information, but there are often going to be multiple errors, so I'm not entirely sure how we should display these. See this file for what the error response contains. OCL-add-to-collection-results-with-multiple-errors.json

Issue screenshots:

How I'm adding the concept to the collection: image

Results screen: image

snyaggarwal commented 5 months ago

@jamlung-ri Ya multiple errors are something that we need to handle with better design. Right now I am just showing the first one. Can you give it one more try on this.

jamlung-ri commented 5 months ago

Great! It worked this time! This is all my test cases, so I think this is good to go.

Intensional Reference with error: image

Extensional references with different types of errors: image image