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

don't return null when no preview #771

Merged
merged 1 commit into from
Aug 4, 2021
Merged

don't return null when no preview #771

merged 1 commit into from
Aug 4, 2021

Conversation

jacobthill
Copy link
Contributor

Why was this change made?

at least one AUB record did not have a thumbnail and was failing type validation

How was this change tested?

local transform

Which documentation and/or configurations were updated?

n/a

Copy link
Member

@jmartin-sul jmartin-sul left a comment

Choose a reason for hiding this comment

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

one question since i haven't seen this exact construct before, but 👍 since it's tested and i think i understand what's going on 🙂

'wr_edm_rights' => [literal('CC BY-ND: https://creativecommons.org/licenses/by-nd/4.0/')],
'wr_id' => [extract_poha('/*/dc:thumbnail')]
)
end
Copy link
Member

Choose a reason for hiding this comment

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

mostly asking for my own traject-related edification: is it correct to say that the old version would apply the transform_values call to every record, pushing the result onto accumulator, and that the new version examines every record, applying the transform_values call to the record and pushing the result onto accumulator, but skipping the record entirely if a thumbnail xpath` isn't present?

wondering because this is the first time i've seen the each_record call in the middle of a bunch of to_field configuration calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is correct. It will leave agg_preview out of the IR if a thumbnail xpath is not found.

@jmartin-sul
Copy link
Member

on second thought, refraining from merging till @aaron-collier can take a look. i got hesitant since i don't totally grok the to_field in an each_record loop thing.

@aaron-collier aaron-collier merged commit 6a2b906 into main Aug 4, 2021
@aaron-collier aaron-collier deleted the aub-thumb branch August 4, 2021 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants