Rothamsted / knetminer

KnetMiner - webapp to search and visualize genome-scale knowledge graphs
https://knetminer.com
MIT License
25 stars 16 forks source link

Searching with a gene list yeilding no results loads infinitely #779

Closed Arnedeklerk closed 12 months ago

Arnedeklerk commented 1 year ago

As in title, there is a bug where searching with a gene in the gene list box, such as: TraesCS3D01G513000. Loads forever. Try typing randomwordsgenelist, too, and it won't load.

It might be related to: Uncaught TypeError: Cannot read properties of null (reading 'map') at replaceOndexId (data-utils.js:1010:38) at genomicViewContent (data-utils.js:220:20) at Object. (data-utils.js:179:7) at l (jquery-bstrap.min.js:32:23304) at Object.fireWith [as resolveWith] (jquery-bstrap.min.js:32:24066) at C (jquery-bstrap.min.js:32:82191) at XMLHttpRequest.n (jquery-bstrap.min.js:32:86481)

marco-brandizi commented 1 year ago

We discussed this ondexId issue and I thought we agreed that we don't need to hide 'ondexId'. If that's still the case, it's better that we first remove the code doing it.

In the specific case, the problem is that this search returns an empty result and replaceOndexId() receives an empty tableData as parameter.

Which means code is to be improved, you can't ignore such obvious edge cases, at least in the most common situations, you need to validate the parameters at some point (eg, in the API response) and control the flow or correct the output accordingly (eg, show an error, stop processing the results, turn nulls into empty values, ensure the downstream code process empty values correctly).

It also means we need to do more testing, chasing these errors every other day isn't very good. Try to use the manual testing repo more often, like every 7-10 days of significant development, propose additions and changes to them, I didn't write them for myself only.

marco-brandizi commented 1 year ago

Also, use the project:* labels in the GH issue descriptions.

Arnedeklerk commented 1 year ago

I also thought the header changing code was removed, but I've downloaded a table and see indeed Ondexid is being changed to NodeID. @lawal-olaotan, please can you start by purging that code, then we test again to start.

@marco-brandizi more testing, indeed. Just don't have much time to work on it...

Arnedeklerk commented 1 year ago

@lawal-olaotan Sorry :/ but this doesn't appear to be complete. There are issues remaining here:

marco-brandizi commented 1 year ago

If we agreed on it, remove this replaceOndexId() and check edge cases better in the remaining code.

lawal-olaotan commented 1 year ago

I reverted the nodeId to ondexId.

Image

I would like to know if this error message is acceptable.

Arnedeklerk commented 1 year ago

Thanks @lawal-olaotan, I'll update the error, no worries. Thanks for reverting.

Arnedeklerk commented 1 year ago

One thing, @lawal-olaotan, is that this text is black where it's usually red. I was going to just fix it in-line but it would be better done the same way as the rest, if you get time...

Arnedeklerk commented 12 months ago

Done and working - thanks.