search

jhu-bids / TermHub

Web app and CLI tools for working with biomedical terminologies. https://github.com/orgs/jhu-bids/projects/9/views/7
https://bit.ly/termhub
GNU General Public License v3.0
8 stars 10 forks source link

Bug: Concept paths sometimes at in wrong position #707

Open joeflack4 opened 4 months ago

joeflack4 commented 4 months ago

(this is closely related to #547)

Overview

In the comparison page, Siggie recently changed the UI to where, rather than nested levels of concepts, you can see a single row, where instead the concept name displays a path. However, there is a bug sometimes where, upon expanding a node, I believe, the position of a concept path could be in the wrong location (and incorrect level of indent, I think). For example, it might show indented at the level of its parent, perhaps as the first row at that level, rather than beneath and indented to the right of its parent.

add to gh issue

For example, in this screenshot, the item is in the correct location. When this issue manifests, it would be as if this item were on the same level as "gilmeperide", perhaps at the top, right above "tolazimide"

Additional info

Siggie thinks it tends to happen if path is more than 2 levels deep. This probably happens during addNodeToVisible when processingshowThroughCollapsed.

Related

Sigfried commented 3 months ago

I think the problem in the image above is fixed. But still having showThoughCollapsed in wrong place:

Image

... have now fixed that.

Sigfried commented 3 months ago

Have another problem, with http://localhost:3000/cset-comparison?codeset_ids=718894835&codeset_ids=1000017855&comparison_rpt=718894835-1000017855, 761797 is a non-standard concept that appears twice and has 20 records. When I hide either zeroRecord concepts, it still appears as it should, or if I hide allButFirstOccurrence, it appropriately hides the second occurrence; but if I hide both of those, it disappears entirely.

I subclassed Set and Map to convert Set values and Map keys to strings, which is probably good for this application where concept_ids show up as integers in some places and strings in others, but it didn't fix the problem.

Sigfried commented 3 months ago

@joeflack4, once I do get this fixed, it would be great if you could create tests to make sure that the problem remains fixed -- and also test other combinations of hiding and setting things in stats and options work, because they are very delicate and break easily. Image

joeflack4 commented 3 months ago

That sounds good. Playwright tests, yes?

Sigfried commented 3 months ago

I was thinking that would be the only way to do it, and some end-to-end tests for this would be good. But it would be a big pain I think to do all the combinations and stuff, so maybe do Jest tests...

I think to set it up, you'd need to run the app and capture data from the chrome debugger for the tests. Then something like this:

  1. import graphReducer from "./??/state/GraphState"
  2. gcDispatch({type: "CREATE", payload: {...graphData, concepts, specialConcepts, csmi}}); That's line 128 of CsetComparisonPage; so you'll need to have captured data for those four variables.
  3. ...

Hmmm. This is going to be hard. I just had a chat with chatgpt and I think you'll get a lot out of reading the whole thing -- not just for this testing problem but for testing react apps in general: https://chat.openai.com/share/a322d4a6-cc64-4d7d-8e7f-e3a213e7bb71

Sigfried commented 3 months ago

BTW, this is closely related to #547, which I've just done a bunch of updating of.