Skip to content
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

Enable nested to-many in Workbench #6216

Open
wants to merge 9 commits into
base: issue-6127
Choose a base branch
from
Open

Enable nested to-many in Workbench #6216

wants to merge 9 commits into from

Conversation

sharadsw
Copy link
Contributor

@sharadsw sharadsw commented Feb 10, 2025

Fixes #2331

Warning

This PR is based on #5417. Use either a db that was used to test #5417 or create a new db

This PR enables nested to many in the data mapper and fixes some automated tests. The underlying backend support for nested to many is in #5417.

For reference:
Nested to-manys in the workbench are mapping lines that involve multiple to-many relationships. For example: Collection Object -> Determination (to many) -> DeterminationCitation (to-many) -> ...

image

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)
  • Add a reverse migration if a migration is present in the PR

Testing instructions

  • Create a Workbench dataset
  • Map columns that include nested to-manys
    • Examples:
      • CO -> Determination -> DeterminationCitation (see above)
      • CO -> Preparation -> PreparationProperty
      • CO -> Preparation -> PreparationAttribute
  • Enter data
  • Test upload
  • Verify data was uploaded correctly (you can verify through queries)
  • Test rollback

@sharadsw sharadsw mentioned this pull request Feb 14, 2025
12 tasks
@sharadsw sharadsw changed the base branch from issue-5413 to issue-6127 February 26, 2025 18:42
@sharadsw sharadsw requested review from a team March 3, 2025 20:30
@sharadsw sharadsw marked this pull request as ready for review March 3, 2025 21:09
Copy link
Collaborator

@lexiclevenger lexiclevenger left a comment

Choose a reason for hiding this comment

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

  • Test upload
  • Verify data was uploaded correctly (you can verify through queries)
  • Test rollback

Looks good! I tested CO -> Determination -> DeterminationCitation and CO -> Preparation -> PreparationAttribute

@lexiclevenger lexiclevenger requested a review from a team March 5, 2025 18:42
Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • Test upload
  • Verify data was uploaded correctly (you can verify through queries)
  • Test rollback

Looks good, it seems like the RS owner issue was reintroduced in this PR though.

Issue branch:

03-05_14.14.mp4

Production:

03-05_14.20.mp4

@sharadsw sharadsw requested a review from emenslin March 6, 2025 21:32
@sharadsw
Copy link
Contributor Author

sharadsw commented Mar 6, 2025

@emenslin Should be fixed now. The branch didn't have the latest production changes

Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • Test upload
  • Verify data was uploaded correctly (you can verify through queries)
  • Test rollback

Looks good now!

@emenslin emenslin requested a review from a team March 10, 2025 17:47
@sharadsw sharadsw modified the milestones: 7.10.2, 7.10.3 Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Dev Attention Needed
Development

Successfully merging this pull request may close these issues.

4 participants