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

update-collection-handling #451

Merged
merged 5 commits into from
Apr 7, 2022
Merged

update-collection-handling #451

merged 5 commits into from
Apr 7, 2022

Conversation

alishaevn
Copy link
Contributor

@alishaevn alishaevn commented Mar 31, 2022

TODO

discuss with braydon

expected behavior

  • determine possible collection ids by source_identifier and not title

@cla-bot cla-bot bot added the cla-signed label Mar 31, 2022
@@ -41,6 +41,7 @@ def build_metadata
self.parsed_metadata = {}
add_identifier
add_ingested_metadata
add_collections
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sephirothkod I don't think this is a good choice for bulkrax because this line seems to have been removed intentionally for some reason. however, without it self.collection_ids is always empty so collections_created? will return false and throw a CollectionCreatedError. even when the collection is in fact created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sephirothkod @bkiahstroud actually...I now needed to bring this code into LV and BL through overrides. may it be better to put this code into bulkrax proper, but with a note to reevaluate removing add_collections from here in the future once the updated logic is worked out?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure lets leave it for now and then I'll have a look when i have time as to how to remove the collections stuff entirely and only reference collections via the new parents code.

Comment on lines +245 to +250
parent_field_mapping = self.class.parent_field(parser)
return [] unless parent_field_mapping.present? && record[parent_field_mapping].present?

identifiers = []
split_titles = record[collection_field_mapping].split(/\s*[;|]\s*/)
split_titles.each do |c_title|
matching_collection_entries = importerexporter.entries.select { |e| e.raw_metadata['title'] == c_title }
split_references = record[parent_field_mapping].split(/\s*[;|]\s*/)
split_references.each do |c_reference|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

more semantic names.

matching_collection_entries = importerexporter.entries.select { |e| e.raw_metadata['title'] == c_title }
split_references = record[parent_field_mapping].split(/\s*[;|]\s*/)
split_references.each do |c_reference|
matching_collection_entries = importerexporter.entries.select { |e| e.raw_metadata['source_identifier'] == c_reference && e.is_a?(CsvCollectionEntry) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we're no longer using titles to track collections. we also are making sure that we only return collection parents and not work parents so that collections_created? isn't incorrectly throwing false.

@alishaevn alishaevn changed the title draft: update-collection-handling update-collection-handling Apr 6, 2022
@alishaevn alishaevn merged commit 1e699fe into main Apr 7, 2022
@alishaevn alishaevn deleted the update-collection-handling branch August 5, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants