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

IN 1127 - Handle missing fund_distribution entries with empty strings #273

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

ghukill
Copy link
Contributor

@ghukill ghukill commented Dec 16, 2024

Purpose and background context

PO lines were pulled from Alma where the fund_distribution node in the object contained a fund with empty strings for values like amount.sum. This threw an error when attemping to convert to a Decimal.

Example from PO line retrieved from Alma:

"fund_distribution": [
  {
    "amount": {
      "sum": "",
      "currency": {}
    },
    "fund_code": {
      "value": "",
      "desc": ""
    }
  }
],

Default behavior in other situations is to return $0.00 when a price cannot be determined from the PO line, which will extend to this scenario.

How this addresses that need:

  • get_total_price_from_fund_distribution() is updated to handle a fund entry with empty strings, and treat them as $0.00

How can a reviewer manually see the effects of these changes?

1- Set Dev1 admin credentials

2- Set production credentials for ALMA_API_URL and ALMA_API_READ_KEY in shell (these were retrieved from SSM paramters and Secrets)

3- The following invocation will send a ccslips email to [email protected] (currently the only configured email address in Dev1 this will successfully send to):

pipenv run ccslips --verbose -s [email protected] -r [email protected] -d 2024-12-13

Formerly this was failing, because one of these entries would throw an exception when fund_distribution contained funds with empty strings.

Includes new or updated dependencies?

NO

Changes expectations for external applications?

YES: Emails will successfully generate with '$0.00' reported when the fund distribution information is empty strings

What are the relevant tickets?

Developer

  • All new ENV is documented in README
  • All new ENV has been added to staging and production environments
  • All related Jira tickets are linked in commit message(s)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer(s)

  • The commit message is clear and follows our guidelines (not just this PR message)
  • There are appropriate tests covering any new functionality
  • The provided documentation is sufficient for understanding any new functionality introduced
  • Any manual tests have been performed and verified
  • New dependencies are appropriate or there were no changes

Why these changes are being introduced:

PO lines were pulled from Alma where the fund_distribution node in the
object contained a fund with empty strings for values like 'amount.sum'.
This threw an error when attemping to convert to a Decimal.

Default behavior is to return '$0.00' when a price cannot be determined
from the PO line, which will extend to this scenario.

How this addresses that need:
* get_total_price_from_fund_distribution is updated to handle a fund entry
with empty strings, and treat them as '$0.00'

Side effects of this change:
* Emails will successfully generate with '$0.00' reported when the fund
distribution information is empty strings

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/IN-1127
@ghukill ghukill requested a review from ehanson8 December 16, 2024 19:38
@ghukill ghukill merged commit 2cd7b12 into main Dec 16, 2024
3 checks passed
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.

2 participants