Skip to content

Conversation

@CarolineDenis
Copy link
Contributor

@CarolineDenis CarolineDenis commented Jul 10, 2024

Fixes #4980

Checklist

  • Self-review the PR after opening it to make sure the changes look good
    and self-explanatory (or properly documented)
  • Add relevant issue to release milestone

Testing instructions

I created a database named 'sp7demofish_mots.sql' on the test panel, this has some sample Taxon trees created to use in this PR's test. Use the screenshots for extra testing details.

Here is a test panel instance to use: https://sp7demofishmots-issue-4980.test.specifysystems.org

  • Setup the WB test by adding a new taxon tree and ranks. Also, create a new CollectionObjectType.
  • Add new CollectionObjects with a Determination and CollectionObjectType. Do a workbench upload with CollectionObject as the base table. Create workbench columns like "Det->Det 1->TestTree->TestKingdom->Name" (Edit: ignore this step, COType is now hidden in v7.9.7)
  • Add new CollectionObjects with Determination, this time without specifying a CollectionObjectType.
  • Add new Taxons to Ranks of different Taxon trees. Do a workbench upload with Taxon as the base table. Create workbench columns like "Taxon->TestTree->TestKingdom->Name".
  • Add new CollectionObjects from Determinations. Do a workbench upload with Determination as the base table. Create workbench columns like "Determination->Taxon->TestTree->TestKingdom->Name".
  • Check that the Taxon Trees have been updated in the Tree Viewer.

image
image
image
image
image
image
image
image

@CarolineDenis CarolineDenis changed the title Issue 4980 Add a feature to choose type of taxon tree in WB Jul 10, 2024
@CarolineDenis
Copy link
Contributor Author

TODO:

  • fix upload plan in WB

(@acwhite211 )

@CarolineDenis CarolineDenis added this to the 7.9.7 milestone Jul 16, 2024
@CarolineDenis CarolineDenis changed the base branch from production to issue-4829 July 16, 2024 15:44
@realVinayak
Copy link
Contributor

realVinayak commented Jul 30, 2024

IIRC, we were going to make it so that users select treedef when they open CO form. It'd absolutely simplify things here too...

Also, this is simpler and more straightforward on the backend than the query builder...provided upload plan identifies the tree for us

@CarolineDenis
Copy link
Contributor Author

NOTES:

  • need backend fixes

@acwhite211

@CarolineDenis
Copy link
Contributor Author

IIRC, we were going to make it so that users select treedef when they open CO form. It'd absolutely simplify things here too...

Also, this is simpler and more straightforward on the backend than the query builder...provided upload plan identifies the tree for us

This was a discussion we had during a meeting but we decided not to go with this solution

@CarolineDenis
Copy link
Contributor Author

NOTES:

  • when selecting Taxon as base table in the WB, the user is now presented a list of available tree to upload to
  • the tree def id that as been chosen as a base is now added to the workbench upload plan

MISSING 🚨 🚨:

  • the backend needs to handle the new attribute tree def id to determine which tree has been selected
  • on the frontend, when selecting a table that has a relationship to Taxon, we also need to implement a feature that allows the user to decide which tree they want to upload in or choose the collection default
  • same in query field line

@CarolineDenis
Copy link
Contributor Author

NOTES:

Need to remove: when selecting Taxon as base table in the WB, the user is now presented a list of available tree to upload to

Implement a first column in the query mapper with the trees and only when selected the ranks, same in the query lines

Base automatically changed from issue-4829 to production August 1, 2024 17:15
@CarolineDenis
Copy link
Contributor Author

CarolineDenis commented Sep 6, 2024

NOTES:

  • We have been testing on several dbs adding new det to taxon table with a root node and with no root node, it creates as expected the 'Uploaded' root node for the ones that did not have a root.
    i.e:
    tree 1 Kingdom Name (root node present in the tree)
    tree 1 Genus Name (root node present in the tree)
    tree 2 Kingdom Name (root node NOT present in the tree)
    tree 2 Phylum Name (root node NOT present in the tree)

  • the code for the 'no root' warning has been removed and the behavior will be the same as before (no root node, a new 'Uploaded' node is created)

  • there is some kind of issue with the herps trees in the db @combs-a

  • add warning in the case the user tries to add multiple Ranks In one Row from different tree (

    elif len(targeted_treedefids) > 1:
    logger.warning(f"Multiple treedefs found in row: {targeted_treedefids}")
    return self, WorkBenchParseFailure('multipleRanksInRow', {}, list(ranks_columns_in_row_not_null)[0])
    )

Copy link
Contributor

@realVinayak realVinayak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing out the root issue!!

Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice feature!

most of the code looks good.
have questions about the newly added mapping path symbols

@acwhite211 acwhite211 dismissed stale reviews from maxpatiiuk, realVinayak, and combs-a September 11, 2024 20:37

more in 7.9.8

@acwhite211 acwhite211 merged commit dfa30a6 into production Sep 11, 2024
@acwhite211 acwhite211 deleted the issue-4980 branch September 11, 2024 20:38
@specifysoftware
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add support in the WorkBench for specifying which tree you want to upload to