OnroerendErfgoed / atramhasis

An online SKOS editor
http://atramhasis.readthedocs.io/
GNU General Public License v3.0
53 stars 11 forks source link

Tree keeps on loading with non-numeric id #816

Closed koenedaele closed 1 year ago

koenedaele commented 1 year ago

Using the latest development branch with the BLUEBIRDS conceptscheme. When I open the full scheme, the tree works. When I open a concept in the scheme, the spinner keeps spinning. Looking at the console I see the following error:

norwegianblue?view_tree:454 Uncaught TypeError: Cannot read properties of undefined (reading '_children')
    at expandPath (norwegianblue?view_tree:454:40)
    at norwegianblue?view_tree:438:15
    at Array.forEach (<anonymous>)
    at expandToCurrent (norwegianblue?view_tree:436:26)
    at norwegianblue?view_tree:442:15
    at Array.forEach (<anonymous>)
    at expandToCurrent (norwegianblue?view_tree:436:26)
    at norwegianblue?view_tree:665:9
    at Array.forEach (<anonymous>)
    at norwegianblue?view_tree:664:21

The tree that gets returned by the server (http://localhost:6543/conceptschemes/BLUEBIRDS/tree?language=en) looks ok.

koenedaele commented 1 year ago

Also reported by @mielvds

cedrikv commented 1 year ago

We use a dot notation with the id's to maintain a hierarchy: image

The bluebirds scheme has an url as id which contains dots; this breaks the logic

Easy solution: add disclaimer that custom id's can't have dots? The whole tree logic is build around this system, so it will be a big refactor otherwise :/

koenedaele commented 1 year ago

URI's are valid id's (although clearly not ideal), so it should work. Saying that custom id's can't have a dot would mean that URI's can no longer be an id. Then we are back to requiring that every importable SKOS file has a local identifier. Only solution around that I can see is having some sort of function on import that generates local identifiers from URI's.

Can we replace the dot with a pipe and urlencode the id:

mielvds commented 1 year ago

Can't you do an API call to get the skos:narrower/skos:broader relations?

cedrikv commented 1 year ago

The child id's are created by the api in the tree call (http://local.onroerenderfgoed.be:6543/conceptschemes/GEOGRAPHY/tree?language=en): image

cedrikv commented 1 year ago

to clarify: the code traverses this chain of id's to open up the correct nodes when viewing a concept. it performs a 'split' on the id field to know the path to follow.

cedrikv commented 1 year ago

I can catch this error, but the the tree wont open properly by default: image This way you can still use it to navigate the scheme though.

koenedaele commented 1 year ago

Can't you do an API call to get the skos:narrower/skos:broader relations?

Due to collections being in between concepts, it's a bit more complicated than that. Skosprovider does have API methods https://thesaurus.onroerenderfgoed.be/conceptschemes/ERFGOEDTYPES/displaytop that show all top concepts and collections and https://thesaurus.onroerenderfgoed.be/conceptschemes/ERFGOEDTYPES/c/1071/displaychildren that show narrower collections and concepts of a certain concept that could be used to build the entire tree on level at a time.

But at the time, we ended up generating the entire tree in one go through https://thesaurus.onroerenderfgoed.be/conceptschemes/ERFGOEDTYPES/tree?language=en (use application/json Accept header) that also encodes the full path from a concept to the root node so it can be opened in the right place. Not sure why we didn't use the recursive methods, possibly it was too slow or the developer at the time didn't get it working with D3.

koenedaele commented 1 year ago

to clarify: the code traverses this chain of id's to open up the correct nodes when viewing a concept. it performs a 'split' on the id field to know the path to follow.

Yes, but if we encode the chain differently on the server like I suggested, wouldn't it work? I don't think it can work the way the server render the tree right now.

Other option is a full rewrite of the tree component, having it call those two API methods I mentioned. Which would allow Atramhasis to handle larger thesauri, but it would be slower for smaller thesauri. Doens't seem like something we could add in this version.

cedrikv commented 1 year ago

I did some local testing, and the urlencode + '|' change seems to work. urlencode is also good for things line this: http://local.onroerenderfgoed.be:6543/conceptschemes/BLUEBIRDS/c/http%3A%2F%2Fid.bluebirds.org%2Fbird?view_tree

cedrikv commented 1 year ago

fixed with https://github.com/OnroerendErfgoed/atramhasis/commit/591d1d267a92df26b456cc32b66f21b58d9d222c