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

bugfix for DEV-562: make SharedPrint::Finder phase queries more specific #295

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

mwarin
Copy link
Contributor

@mwarin mwarin commented Oct 25, 2023

Further fix for: https://hathitrust.atlassian.net/browse/DEV-562

SharedPrint::Finder.new(phase: [x]).commitments should only yield commitments with phase: x, but the released code was missing a condition and was erroneously yielding all commitments in a cluster if any one of the commitments in the cluster had phase: x.

This PR adds that missing condition and adds the tests to prove it's doing the right thing.

@mwarin mwarin changed the title bugfix for DEV-562: make phase query more specific bugfix for DEV-562: make SharedPrint::Finder phase queries more specific Oct 25, 2023
@coveralls
Copy link

coveralls commented Oct 25, 2023

Coverage Status

coverage: 95.034% (+0.02%) from 95.012% when pulling 37fb01d on finder-phase-fix into cc0e52c on main.

@mwarin mwarin marked this pull request as ready for review October 25, 2023 20:48
@mwarin mwarin requested review from aelkiss and moseshll October 25, 2023 20:49
Copy link

@moseshll moseshll left a comment

Choose a reason for hiding this comment

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

Alas, I was not able to run this locally because Mongo was acting up. Just the one suggestion.

@mwarin mwarin merged commit 8398573 into main Oct 26, 2023
1 check passed
@aelkiss aelkiss deleted the finder-phase-fix branch January 7, 2025 20:39
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