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

Stop using Python money #2145

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ module = [
"jwcrypto",
"kombu.*",
"lxml.*",
"money",
"multipledispatch",
"nameparser",
"onelogin.saml2.*",
Expand Down Expand Up @@ -236,7 +235,6 @@ jsonschema = "^4.22.0"
jwcrypto = "^1.4.2"
levenshtein = "^0.26"
lxml = {extras = ["html-clean"], version = "^5.2.1"}
money = "1.3.0"
multipledispatch = "^1.0"
nameparser = "^1.1" # nameparser is for author name manipulations
opensearch-dsl = "~1.0"
Expand Down
12 changes: 2 additions & 10 deletions src/palace/manager/api/authentication/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from abc import ABC, abstractmethod
from typing import TypeVar

from money import Money
from sqlalchemy.orm import Session
from werkzeug.datastructures import Authorization

Expand Down Expand Up @@ -222,10 +221,8 @@ def __init__(
:param external_type: A string classifying the patron
according to some library-specific scheme.

:param fines: A Money object representing the amount the
patron owes in fines. Note that only the value portion of the
Money object will be stored in the database; the currency portion
will be ignored. (e.g. "20 USD" will become 20)
:param fines: A Decimal object representing the amount the
patron owes in fines.

:param block_reason: A string indicating why the patron is
blocked from borrowing items. (Even if this is set to None, it
Expand Down Expand Up @@ -324,11 +321,6 @@ def fines(self):

@fines.setter
def fines(self, value):
"""When setting patron fines, only store the numeric portion of
a Money object.
"""
if isinstance(value, Money):
value = value.amount
self._fines = value

def apply(self, patron: Patron):
Expand Down
2 changes: 1 addition & 1 deletion src/palace/manager/api/circulation_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def __init__(
parsed_fines = None
if fines:
try:
parsed_fines = MoneyUtility.parse(fines).amount
parsed_fines = MoneyUtility.parse(fines)
except ValueError:
# If the fines are not in a valid format, we'll just leave them as None.
...
Expand Down
8 changes: 4 additions & 4 deletions src/palace/manager/api/config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from collections.abc import Iterable
from decimal import Decimal

from Crypto.Cipher import PKCS1_OAEP
from Crypto.Cipher.PKCS1_OAEP import PKCS1OAEP_Cipher
from Crypto.PublicKey import RSA
from flask_babel import lazy_gettext as _
from money import Money

from palace.manager.core.config import CannotLoadConfiguration # noqa: autoflake
from palace.manager.core.config import IntegrationException # noqa: autoflake
Expand Down Expand Up @@ -93,10 +93,10 @@ def tiny_collection_languages(cls, library: Library) -> list[str]:
return library.settings.tiny_collection_languages

@classmethod
def max_outstanding_fines(cls, library: Library) -> Money | None:
if library.settings.max_outstanding_fines is None:
def max_outstanding_fines(cls, library: Library) -> Decimal | None:
if (max_fines := library.settings.max_outstanding_fines) is None:
return None
return MoneyUtility.parse(library.settings.max_outstanding_fines)
return MoneyUtility.parse(max_fines)

@classmethod
def estimate_language_collections_for_library(cls, library: Library) -> None:
Expand Down
5 changes: 2 additions & 3 deletions src/palace/manager/api/millenium_patron.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import dateutil
from flask_babel import lazy_gettext as _
from lxml import etree
from money import Money
from pydantic import field_validator

from palace.manager.api.authentication.base import PatronData
Expand Down Expand Up @@ -475,9 +474,9 @@ def patron_dump_to_patrondata(self, current_identifier, content):
fines = MoneyUtility.parse(v)
except ValueError:
self.log.warning(
'Malformed fine amount for patron: "%s". Treating as no fines.'
f"Malformed fine amount ({v}) for patron. Treating as no fines."
)
fines = Money("0", "USD")
fines = MoneyUtility.parse(0)
elif k == self.BLOCK_FIELD:
block_reason = self._patron_block_reason(self.block_types, v)
elif k == self.EXPIRATION_FIELD:
Expand Down
2 changes: 1 addition & 1 deletion src/palace/manager/api/sip/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ def info_to_patrondata_block_reason(
# patron has excessive fines, we can use that as the reason
# they're blocked.
if "fee_limit" in info:
fee_limit = MoneyUtility.parse(info["fee_limit"]).amount
fee_limit = MoneyUtility.parse(info["fee_limit"])
if fee_limit and patrondata.fines > fee_limit:
block_reason = PatronData.EXCESSIVE_FINES

Expand Down
9 changes: 2 additions & 7 deletions src/palace/manager/api/util/patron.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import datetime

import dateutil
from money import Money

from palace.manager.api.circulation_exceptions import (
AuthorizationBlocked,
Expand Down Expand Up @@ -102,13 +101,9 @@ def has_excess_fines(cls, patron):
if not patron.fines:
return False

if isinstance(patron.fines, Money):
patron_fines = patron.fines
else:
patron_fines = MoneyUtility.parse(patron.fines)
actual_fines = patron_fines.amount
actual_fines = MoneyUtility.parse(patron.fines)
max_fines = Configuration.max_outstanding_fines(patron.library)
if max_fines is not None and actual_fines > max_fines.amount:
if max_fines is not None and actual_fines > max_fines:
return True
return False

Expand Down
25 changes: 13 additions & 12 deletions src/palace/manager/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
import string
from collections import Counter
from collections.abc import Generator, Iterable, Sequence
from decimal import Decimal, InvalidOperation
from typing import Any, TypeVar

import sqlalchemy
from money import Money

import palace.manager.sqlalchemy.flask_sqlalchemy_session

Expand Down Expand Up @@ -462,25 +462,26 @@ def process_data(cls, data, bigrams):


class MoneyUtility:
DEFAULT_CURRENCY = "USD"

@classmethod
def parse(cls, amount: str | float | int | None) -> Money:
"""Attempt to turn a string into a Money object."""
currency = cls.DEFAULT_CURRENCY
def parse(cls, amount: str | float | int | None) -> Decimal:
"""Attempt to turn a string into a Decimal object representing money."""
if not amount:
amount = "0"
amount = str(amount)

if amount[0] == "$":
currency = "USD"
amount = amount[1:]

# `Money` does not properly handle commas in the amount, so we strip
# them out of US dollar amounts, since it won't change the "value."
if currency == "USD":
amount = "".join(amount.split(","))
return Money(amount, currency)
# We assume we are working with currency amounts where ',' is used as a possible
# thousands seperator, so we can safely strip them.
amount = "".join(amount.split(","))
try:
return Decimal(amount).quantize(Decimal("1.00"))
except InvalidOperation:
raise ValueError(
"amount value could not be converted to "
"Decimal(): '{}'".format(amount)
) from None


def is_session(value: object) -> bool:
Expand Down
10 changes: 5 additions & 5 deletions tests/manager/api/test_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from _pytest._code import ExceptionInfo
from flask import url_for
from freezegun import freeze_time
from money import Money
from sqlalchemy.orm import Session
from werkzeug.datastructures import Authorization

Expand Down Expand Up @@ -72,6 +71,7 @@
)
from palace.manager.sqlalchemy.model.library import Library, LibraryLogo
from palace.manager.sqlalchemy.model.patron import Patron
from palace.manager.util import MoneyUtility
from palace.manager.util.authentication_for_opds import AuthenticationForOPDSDocument
from palace.manager.util.datetime_helpers import utc_now
from palace.manager.util.http import RemoteIntegrationException
Expand Down Expand Up @@ -173,7 +173,7 @@ def patron_data() -> PatronData:
personal_name="4",
email_address="5",
authorization_expires=utc_now(),
fines=Money(6, "USD"),
fines=MoneyUtility.parse(6),
block_reason=PatronData.NO_VALUE,
)

Expand Down Expand Up @@ -225,7 +225,7 @@ def test_to_dict(self, patron_data: PatronData):
authorization_expires=patron_data.authorization_expires.strftime(
"%Y-%m-%d"
),
fines="6",
fines="6.00",
block_reason=None,
)
assert data == expect
Expand Down Expand Up @@ -388,7 +388,7 @@ def test_apply_on_incomplete_information(self, db: DatabaseTransactionFixture):
patron.authorization_identifier = "1234"
patron.username = "user"
patron.last_external_sync = now
patron.fines = Money(10, "USD")
patron.fines = MoneyUtility.parse(10)
authenticated_by_username = PatronData(
authorization_identifier="user", complete=False
)
Expand Down Expand Up @@ -2434,7 +2434,7 @@ def test_authentication_creates_missing_patron(
patrondata = PatronData(
permanent_id=db.fresh_str(),
authorization_identifier=db.fresh_str(),
fines=Money(1, "USD"),
fines=MoneyUtility.parse(1),
)

provider = mock_basic(
Expand Down
4 changes: 2 additions & 2 deletions tests/manager/api/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ def test_max_outstanding_fines(
settings.max_outstanding_fines = 0
max_fines = m(library)
assert max_fines is not None
assert 0 == max_fines.amount
assert 0 == max_fines

# A more lenient approach.
settings.max_outstanding_fines = 100.0
max_fines = m(library)
assert max_fines is not None
assert 100 == max_fines.amount
assert 100 == max_fines
1 change: 1 addition & 0 deletions tests/manager/api/test_patron_utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ def test_has_excess_fines(
# If you accrue excessive fines you lose borrowing privileges.
# Verify that all these tests work no matter what data type has been stored in
# patron.fines.
patron_fines: str | int | float | Decimal | None
for patron_fines in ("1", "0.75", 1, 1.0, Decimal(1), MoneyUtility.parse("1")):
patron.fines = patron_fines

Expand Down
32 changes: 14 additions & 18 deletions tests/manager/util/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from decimal import Decimal

import pytest
from money import Money

from palace.manager.util import (
Bigrams,
Expand Down Expand Up @@ -375,32 +374,29 @@ def test_median(self):

class TestMoneyUtility:
@pytest.mark.parametrize(
"expected_amount, input_amount, money_amount, money_currency",
"expected_amount, expected_string, input_amount",
[
[Decimal("0"), None, "0", "USD"],
[Decimal("4.00"), "4", "4.00", "USD"],
[Decimal("-4.00"), "-4", "-4.00", "USD"],
[Decimal("4.40"), "4.40", "4.40", "USD"],
[Decimal("4.40"), "$4.40", "4.40", "USD"],
[Decimal("4.4"), 4.4, "4.40", "USD"],
[Decimal("4"), 4, "4", "USD"],
[Decimal("0.4"), 0.4, ".4", "USD"],
[Decimal("0.4"), ".4", ".4", "USD"],
[Decimal("4444.40"), "4,444.40", "4444.40", "USD"],
[Decimal("0"), "0.00", None],
[Decimal("4.00"), "4.00", "4"],
[Decimal("-4.00"), "-4.00", "-4"],
[Decimal("4.40"), "4.40", "4.40"],
[Decimal("4.40"), "4.40", "$4.40"],
[Decimal("4.4"), "4.40", 4.4],
[Decimal("4"), "4.00", 4],
[Decimal("0.4"), "0.40", 0.4],
[Decimal("0.4"), "0.40", ".4"],
[Decimal("4444.40"), "4444.40", "4,444.40"],
],
)
def test_parse(
self,
expected_amount: Decimal,
expected_string: str,
input_amount: str | float | int | None,
money_amount: str,
money_currency: str,
):
parsed = MoneyUtility.parse(input_amount)
money = Money(money_amount, money_currency)

assert money == parsed
assert expected_amount == parsed.amount
assert expected_amount == parsed
assert expected_string == str(parsed)

@pytest.mark.parametrize(
"bad_value",
Expand Down