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

713 empty renewal count causes loan not to be migrated #824

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ealexch
Copy link
Collaborator

@ealexch ealexch commented Jan 14, 2025

Purpose

This PR addresses the issue where loan records with empty or missing renewal_count fields were not being migrated properly. Fixes #713. The solution ensures that missing renewal_count values are set to 0, preventing migration errors.

Changes Made in this PR

  • Added logic to check for empty or missing renewal_count fields in loan records.
  • Set renewal_count to 0 if it is absent.
  • Ensured compatibility with existing functionality for valid renewal_count values.

Code Review Specifics

  • Read the code and provide feedback if necessary.
  • Fire it up and verify the correct handling of empty renewal_count fields.

Task Checklist

  • Ran nox -rs safety.
  • Ran pre-commit run --all-files.
  • Tests cover new or modified code.
  • Ran test suite: nox -rs tests.
  • Code runs and outputs default usage info: cd src; poetry run python3 -m folio_migration_tools -h.
  • Documentation updated.

Warning Checklist

  • New dependencies added.
  • Includes breaking changes.

How to Verify

  1. Run the loan migrator with sample data containing empty or missing renewal_count values.
  2. Verify that such records are migrated with renewal_count set to 0.

Open Questions

None at the moment.


Learn Anything Cool?

I learned about how folio_migration_tools handles optional fields like renewal_count during the migration process.

Copy link
Collaborator

@bltravis bltravis left a comment

Choose a reason for hiding this comment

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

Is there a particular reason this PR includes the changes from #771, as well?

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.

Loans migrator -- empty renewal count causes loan not to be migrated
2 participants