-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add new feature to change taxon tree type in tree viewer #5036
Conversation
specifyweb/frontend/js_src/lib/components/InitialContext/treeRanks.ts
Outdated
Show resolved
Hide resolved
Here is a summary of the major changes I did BackendI have gone ahead and implemented the endpoint I suggested in a520069#commitcomment-143518914. The endpoint returns all of the tree-structure related information for every tree the user has read access to. The frontend previously was doing a serious of requests to create an object similar to what this endpoint now returns. specify7/specifyweb/frontend/js_src/lib/components/InitialContext/treeRanks.ts Lines 58 to 104 in b893ef7
Feel free to share what you think: specify7/specifyweb/specify/tree_views.py Lines 420 to 444 in c64b8b6
Also did some other small refactoring where I saw fit: primarily improving typing. FrontendThe frontend work consisted of using the newly defined endpoint, getting the cache for the definition names up and running, and changing the pre-existing code to be compatible with all of the changes we have done to Instead of having a single object in the cache which keeps track of each Tree's definition the user was last using, now we are creating a separate cache entry for each tree. This should make it simpler and easier to both access and set. 💡 Although I would like to use the definition ids rather than the name internally to keep track of the 'current' tree. If the names of the trees change, this would automatically handle the case. The code is completely functional right now: if the user has more than one Tree of a specific type created, they can switch which tree they are using. in the TreeView. Other places which used the |
@melton-jason,
Yes, I think that would be good
We'll have to define where we set this default |
@emenslin , or I guess @specify/ux-testing, clear your cache out before testing |
actually, this'll be in production too..... |
I did clear my cache and it's still happening |
Sorry, I didn't get to test it locally before pushing. Resolved now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing instructions
==> you need to add a new taxon tree in your db to test this PR
Tree Viewer
- Open the taxon tree viewer
- If the db as only one taxon tree, verify that the header is a H2
- If the db as several taxon trees, verify that the header is a drop down
- Verify that the header drop down contain all the trees of the db
- Change the tree type
- Verify the tree has changed
- click on a node, add a child
- verify that the ranks in the drop down are from the correct tree
QB
- Open the a new Taxon query
- Verify it is the same case when doing: New CO query > Determination > Taxon
- Query on any rank from any tree
- Query on a rank present in 2 or more trees and verify all nodes are doing return
Everything looks good from what I can tell, did not run into any of the previous errors this time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing instructions
==> you need to add a new taxon tree in your db to test this PR
Tree Viewer
- Open the taxon tree viewer
- If the db as only one taxon tree, verify that the header is a H2
- If the db as several taxon trees, verify that the header is a drop down
- Verify that the header drop down contain all the trees of the db
- Change the tree type
- Verify the tree has changed
- click on a node, add a child
- verify that the ranks in the drop down are from the correct tree
QB
- Open the a new Taxon query
- Verify it is the same case when doing: New CO query > Determination > Taxon
- Query on any rank from any tree
- Query on a rank present in 2 or more trees and verify all nodes are doing return
Tree viewer and QB looks good across multiple different DBs that I tested with. Didn't come across any major issues 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing instructions
==> you need to add a new taxon tree in your db to test this PR
Tree Viewer
- Open the taxon tree viewer
- If the db as only one taxon tree, verify that the header is a H2
- If the db as several taxon trees, verify that the header is a drop down
- Verify that the header drop down contain all the trees of the db
- Change the tree type
- Verify the tree has changed
- click on a node, add a child
- verify that the ranks in the drop down are from the correct tree
QB
- Open the a new Taxon query
- Verify it is the same case when doing: New CO query > Determination > Taxon
- Query on any rank from any tree
- Query on a rank present in 2 or more trees and verify all nodes are doing return
Looks good, everything seems to be working as expected. 👍
reviewed
@acwhite211 could have at least tried to add unit tests 🤦 |
This pull request has been mentioned on Specify Community Forum. There might be relevant details there: https://discourse.specifysoftware.org/t/specify-7-9-7-release-announcement/1979/1 |
Fixes #4829
Fixes #4978
Fixes #4979
Checklist
and self-explanatory (or properly documented)
Testing instructions
==> you need to add a new taxon tree in your db to test this PR
Tree Viewer
QB