Skip to content

Allow to upload COG in WB#6073

Merged
CarolineDenis merged 23 commits into
productionfrom
issue-5418
Jan 22, 2025
Merged

Allow to upload COG in WB#6073
CarolineDenis merged 23 commits into
productionfrom
issue-5418

Conversation

@CarolineDenis

@CarolineDenis CarolineDenis commented Jan 14, 2025

Copy link
Copy Markdown
Contributor

Fixes #5418
Fixes #1835

Written with @melton-jason

Sets the following required relationships as optional for WorkBench mappings:

  • CollectionObjectGroupType -> type
  • CollectionObjectGroupJoin -> precedence
  • CollectionObjectGroupJoin -> isSubstrate
  • (CollectionObjectGroupJoin -> isPrimary is already hidden)

Adds a businessrule to automatically set the precedence field of CollectionObjectGroupJoin records if not set on the backend

Allows uploading through 'remote' to-one relationships (where the Foreign Key is on the other side of a relationship) through the WorkBench.

cog_cojo_wb.mov
cog-cojo-rollback.mov

Data and upload plan as shown in the above videos:

upload_plan.json
COG Data Set.csv

Checklist

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

Testing instructions

  • Create a WorkBench data set with a base table of CollectionObjectGroup (COG)

    • Map at least one field through cojo -> parentCog
      • This means that when data is present for a parentCog, you will be uploading a COG which is already a child of another COG
    • Map at least one CollectionObject through the children -> childCo relationship
    • Map at least one COG through the children -> childCog relationship
    • Map any other desired fields in the Data Mapper
    • Have at least one row in the Data Set where there are one or more COGs in the database which would match the data provided for cojo -> parentCog
    • Have at least one row in the Data Set where there are no matching COGs for the data provided for the cojo -> parentCog relationship
    • Have at least one row in the Data Set where there are one or more COs which would match the data provided in a children -> childCo
    • Have at least one row in the Data Set where there are no matching COs for the data provided in a children -> childCo
    • More generally, for any related record from the base table, the WorkBench will try and match related records first: if no potential matches are found, then the WorkBench will create a new record
    • Validate the WorkBench data set
      • Ensure you can disambiguate related records when there is more than one potential match
      • Ensure that matched records do not count as "New Cells"
    • Upload the WorkBench data set
      • Create a record set with the uploaded results, and ensure that all data is uploaded correctly
    • RollBack the WorkBench data set
      • Ensure that all created records are deleted and all independent relationships (for example, CO -> cojo if assigning an existing CO to a new/existing COG) are reverted to their state prior to the upload
  • Create a WorkBench data set with a base table of CollectionObject (CO)

    • Ensure CollectionObjects can be added to existing COGs via the WorkBench
    • Ensure that COGs can be created as part of the upload process for a CO and that COs can be associated with
  • When uploading any CollectionObjectGroupJoin (COJO) via the WorkBench

    • Ensure a COJO can not be created with both a childCo and childCog
    • Ensure a COJO can not be created without a childCo and childCog

@CarolineDenis CarolineDenis added this to the 7.9.11 milestone Jan 14, 2025
Comment thread specifyweb/workbench/upload/upload_table.py Outdated
@CarolineDenis
CarolineDenis requested a review from a team January 15, 2025 16:06
Comment thread specifyweb/businessrules/rules/cojo_rules.py

@emenslin emenslin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • Ensure you can disambiguate related records when there is more than one potential match
    • Ensure that matched records do not count as "New Cells"
    • Create a record set with the uploaded results, and ensure that all data is uploaded correctly
    • Ensure that all created records are deleted and all independent relationships (for example, CO -> cojo if assigning an existing CO to a new/existing COG) are reverted to their state prior to the upload
  • Ensure CollectionObjects can be added to existing COGs via the WorkBench
  • Ensure that COGs can be created as part of the upload process for a CO and that COs can be associated with
  • Ensure a COJO can not be created with both a childCo and childCog
  • Ensure a COJO can not be created without a childCo and childCog
  • Ensure the precedence of the COJO records are automatically set based on the order they are added

Cannot disambiguate

01-16-2025_12-58.mp4

Can upload a COJO without any children and can upload a COJO with both a childCO and childCOG

01-16-2025_13-28.mp4

@melton-jason

melton-jason commented Jan 21, 2025

Copy link
Copy Markdown
Contributor

Hey guys!

Cannot disambiguate

01-16-2025_12-58.mp4

Can upload a COJO without any children and can upload a COJO with both a childCO and childCOG

01-16-2025_13-28.mp4

From #6073 (review)

These Issues have been fixed. You should now be able to properly disambiguate. The businessrules to prevent saving when a COJO had had neither a childCO or childCOG or both a childCO and childCOG were disabled in fc85009, but have been re-enabled!

The actual content of the error message can likely be modified/improved in whichever PR tackles #6073 (comment).


Now, remote one-to-one relationships are instead parsed as a toMany relationship. What this means is that CO -> cojo and COG -> cojo will be treated the same way we treat Locality -> localityDetails and Locality -> geoCoordDetails in the Workbench.
From the perspective of a user of the Web-Application, this means that an additional click is needed to map the "toOne" as a singular "toMany" :

Before

as_toOne.mov

Now

as_toMany.mov

This decision is the result of the discussion in #6073 (comment) (thank you for the insightful replies, realVinayak). This approach leverages functionality which already exists in Specify: no change to the internal logic of disambiguating, uploading, and rollback was needed. Because of this, the changes in this branch will hopefully have a minimal impact for batch edit (#4929 and #5417).

Also, I've included a fix for #1835. You should now longer longer receive the error message for a successful validation, upload, or rollback.

@melton-jason
melton-jason requested review from a team January 21, 2025 06:58

@emenslin emenslin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • Ensure you can disambiguate related records when there is more than one potential match
    • Ensure that matched records do not count as "New Cells"
    • Create a record set with the uploaded results, and ensure that all data is uploaded correctly
    • Ensure that all created records are deleted and all independent relationships (for example, CO -> cojo if assigning an existing CO to a new/existing COG) are reverted to their state prior to the upload
  • Ensure CollectionObjects can be added to existing COGs via the WorkBench
  • Ensure that COGs can be created as part of the upload process for a CO and that COs can be associated with
  • Ensure a COJO can not be created with both a childCo and childCog
  • Ensure a COJO can not be created without a childCo and childCog
  • Ensure the precedence of the COJO records are automatically set based on the order they are added

Can successfully disambiguate now and errors are caught with COJO!
Screenshot 2025-01-21 101108

I know this is a little different so maybe this won't be fixed here but sometimes I get a validation error (that you physically have to press close on) but then it says it successfully validates, this could be just a data set error but I'm not sure.

chrome_01-21_10.11.mp4

Not sure if precedence is being set correctly, when querying it shows mutliple children under the same COG with a precedence of zero but I'm not sure if this has to do with it being an existing COG or not.

image

@melton-jason

Copy link
Copy Markdown
Contributor

I know this is a little different so maybe this won't be fixed here but sometimes I get a validation error (that you physically have to press close on) but then it says it successfully validates, this could be just a data set error but I'm not sure.

chrome_01-21_10.11.mp4

Were the mappings for this data set from before the recent changes? You will get the error message shown in the video when the upload plan contains either CO -> cojo or COG -> cojo as a toOne relationship.

Is this the New Data Set Thu Jan 16 2025 (2) in the Fossil Invertebrates collection for user spadmin?
I'm not able to recreate the issue, and the upload_plan.json is correct for that data set: you theoretically should never encounter the error with the data set given the current upload plan.

For testing the changes after #6073 (comment), I would recommend creating a new Data Set, or if working on pre-existing data sets: unmap and remap any COG -> cojo and CO -> cojo mappings.

I've updated the example upload_plan.json in the testing instructions.

If you do encounter the same problem again, can you include the upload plan of the Data Set in the message? (You can copy the contents shown in Tools -> Upload Plan from the Data Set view).


Not sure if precedence is being set correctly, when querying it shows mutliple children under the same COG with a precedence of zero but I'm not sure if this has to do with it being an existing COG or not.

image

Do you know if these children were associated with the parent CollectionObjectGroup while on this branch?

Precedence should be set for any newly created or saved CollectionObjectGroupJoin records on this branch. The change is not applied retroactively which means that any existing CollectionObjectGroupJoin records will not have their precedence set until they are saved.

Additionally, the precedence is automatically only set if a precedence is not explicitly set for the CollectionObjectGroupJoin record being saved: this means that if the user manually specifies each CollectionObjectGroupJoin's precedence (e.g., setting them all to 0), then it's possible for there to be multiple CollectionObjectGroupJoin records with the same precedence in the same parent.
Do you think we should be strict about ensuring a unique precedence for a CollectionObjectGroupJoin record?

@melton-jason
melton-jason requested review from a team and emenslin January 21, 2025 19:46
@emenslin

Copy link
Copy Markdown
Collaborator

I cannot recreate the issue, but I know I had updated the mappings before testing that data set so I'm not sure what happened. I did notice though that the current workbench naming convention makes uploading COGs pretty confusing. This is probably out of the scope of this pr but is there anything we can do to fix this? I just feel like this isn't super clear
image

Upload plan:
New Data Set Tue Jan 21 2025 (2).json

I also noticed that querying on the COG table then going to collection object group join -> parent -> name, the heading is 'Arent' instead of 'Parent'

image

It does seem like precedence is being correctly set for newly created COGs so maybe someone else edited some of the ones with multiple zeros that I was seeing. Based on how it's been described to me, I think it would make more sense to have unique precedence for each CollectionObjectGroupJoin record, however, I can see letting users decide what they want the precedence to be, including letting multiple have the same number. @grantfitzsimmons what do you think? Will users want to have the same precedence for multiple CollectionObjectGroupJoin records or will they want a unique one for each?

@emenslin emenslin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See above comment

@CarolineDenis

Copy link
Copy Markdown
Contributor Author

@emenslin,

I also noticed that querying on the COG table then going to collection object group join -> parent -> name, the heading is 'Arent' instead of 'Parent'

this is fixed in prod

@grantfitzsimmons

Copy link
Copy Markdown
Member

@grantfitzsimmons what do you think? Will users want to have the same precedence for multiple CollectionObjectGroupJoin records or will they want a unique one for each?

They will likely want something unique, but I don't think we should rule out the possibility of having the same precedence.

@melton-jason

Copy link
Copy Markdown
Contributor

Thanks for your contributions to the PR everyone! We will merge this PR for now, so here's a comment to wrap things up:

I also noticed that querying on the COG table then going to collection object group join -> parent -> name, the heading is 'Arent' instead of 'Parent'

From #6073 (comment)

I think the Issue you found probably has the same cause as #6099, so I have re-opened the Issue and left your findings as a comment: #6099 (comment)

I did notice though that the current workbench naming convention makes uploading COGs pretty confusing. This is probably out of the scope of this pr but is there anything we can do to fix this? I just feel like this isn't super clear

Could you expand more on what you mean by this? Do you think the actual labels for the fields/relationships should be changed, the auto-generated column names for some mapping path should be changed, or something else entirely? Regardless, it will need to be a new Bug/Feature Request: anything to formalize/specify the requirements of the expected behavior is beneficial!

@melton-jason
melton-jason dismissed emenslin’s stale review January 22, 2025 17:50

Issues in the review have been addressed, see #6073 (comment)

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.

Issues with uploading COGs through workbench WorkBench upload status error for success

7 participants