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

accurate-relationship-forming #454

Merged
merged 12 commits into from
Apr 8, 2022
Merged

Conversation

alishaevn
Copy link
Contributor

@alishaevn alishaevn commented Apr 5, 2022

use case

I imported the same collection on 3 different importers while upgrading bl. the first time the collection wasn't created. so on the 3rd importer where the collection had been created, it was still finding the entry from the first importer. and therefore entry.factory.find was nil when it shouldn't have been.

expected behavior

  • get the entry from the current importer and not the first entry, if applicable
  • return the entry and fedora record from find_record
  • update code around importing FileSet rows

demo

works file set
image image

@cla-bot cla-bot bot added the cla-signed label Apr 5, 2022
@alishaevn alishaevn added the enhancement for release notes (New feature or request) label Apr 6, 2022
Copy link
Contributor

@sephirothkod sephirothkod left a comment

Choose a reason for hiding this comment

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

good catch on this, we do need to specify the entry for the specific importer!

@@ -152,7 +153,7 @@ def update_collection(attrs)

# This method is heavily inspired by Hyrax's AttachFilesToWorkJob
def create_file_set(attrs)
work = find_record(attributes[related_parents_parsed_mapping].first)
_, work = find_record(attributes[related_parents_parsed_mapping].first, importer_run_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

find_record now returns two values, but we don't need the entry here

@@ -37,11 +37,11 @@ def perform(parent_identifier:, importer_run_id:)
end.sort_by(&:order)

@importer_run_id = importer_run_id
@parent_record = find_record(parent_identifier)
@parent_entry, @parent_record = find_record(parent_identifier, importer_run_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

example use case that made this necessary:
a collection was imported in importer 1, and in importer 2 we are importing a work to add to that collection. in that case, line 56 below would have failed because this importer_run doesn't have the collection.

@@ -63,7 +66,8 @@ def check_parent_is_a_work!(parent_identifier)
end

def find_parent_record(parent_identifier)
@parent_record ||= find_record(parent_identifier)
@parent_record ||= find_record(parent_identifier, importer_run_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried calling .last on this method but it didn't work.

@alishaevn alishaevn changed the title accurate-relationship-forming draft: accurate-relationship-forming Apr 8, 2022
@alishaevn alishaevn changed the title draft: accurate-relationship-forming accurate-relationship-forming Apr 8, 2022
@alishaevn alishaevn merged commit 8d0c632 into main Apr 8, 2022
@alishaevn alishaevn deleted the accurate-relationship-forming branch August 5, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed enhancement for release notes (New feature or request)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants