From 8889496a34543589b9427a5f2ef45d92625a7b1a Mon Sep 17 00:00:00 2001 From: Xavier Bustamante Talavera Date: Mon, 15 Oct 2018 11:21:21 +0200 Subject: [PATCH] Fix Price not always using Decimal --- ereuse_devicehub/resources/event/models.py | 37 +++++++++++++++------ ereuse_devicehub/resources/event/models.pyi | 6 ++++ ereuse_devicehub/resources/event/schemas.py | 6 ++-- tests/test_event.py | 12 ++++++- tests/test_rate.py | 17 +++++----- 5 files changed, 55 insertions(+), 23 deletions(-) diff --git a/ereuse_devicehub/resources/event/models.py b/ereuse_devicehub/resources/event/models.py index ea6604bf..4c3e218c 100644 --- a/ereuse_devicehub/resources/event/models.py +++ b/ereuse_devicehub/resources/event/models.py @@ -1,5 +1,6 @@ from collections import Iterable from datetime import datetime, timedelta +from decimal import Decimal, ROUND_HALF_EVEN, ROUND_UP from distutils.version import StrictVersion from typing import Set, Union from uuid import uuid4 @@ -488,9 +489,11 @@ class AggregateRate(Rate): class Price(JoinedWithOneDeviceMixin, EventWithOneDevice): + SCALE = 4 + ROUND = ROUND_HALF_EVEN currency = Column(DBEnum(Currency), nullable=False) currency.comment = """The currency of this price as for ISO 4217.""" - price = Column(Numeric(precision=19, scale=4), check_range('price', 0), nullable=False) + price = Column(Numeric(precision=19, scale=SCALE), check_range('price', 0), nullable=False) price.comment = """The value.""" software = Column(DBEnum(PriceSoftware)) software.comment = """The software used to compute this price, @@ -510,8 +513,17 @@ class Price(JoinedWithOneDeviceMixin, EventWithOneDevice): primaryjoin=AggregateRate.id == rating_id) def __init__(self, **kwargs) -> None: - super().__init__(**kwargs) - self.currency = self.currency or app.config['PRICE_CURRENCY'] + if 'price' in kwargs: + assert isinstance(kwargs['price'], Decimal), 'Price must be a Decimal' + super().__init__(currency=kwargs.pop('currency', app.config['PRICE_CURRENCY']), **kwargs) + + @classmethod + def to_price(cls, value: Union[Decimal, float], rounding=ROUND) -> Decimal: + """Returns a Decimal value with the correct scale for Price.price.""" + if isinstance(value, float): + value = Decimal(value) + # equation from marshmallow.fields.Decimal + return value.quantize(Decimal((0, (1,), -cls.SCALE)), rounding=rounding) class EreusePrice(Price): @@ -522,9 +534,10 @@ class EreusePrice(Price): } class Type: - def __init__(self, percentage, price) -> None: + def __init__(self, percentage: float, price: Decimal) -> None: # see https://stackoverflow.com/a/29651462 for the - 0.005 - self.amount = round(price * percentage - 0.005, 2) + self.amount = EreusePrice.to_price(price * Decimal(percentage)) + self.percentage = EreusePrice.to_price(price * Decimal(percentage)) self.percentage = round(percentage - 0.005, 2) class Service: @@ -560,7 +573,7 @@ class EreusePrice(Price): } SCHEMA[Server] = SCHEMA[Desktop] - def __init__(self, device, rating_range, role, price) -> None: + def __init__(self, device, rating_range, role, price: Decimal) -> None: cls = device.__class__ if device.__class__ != Server else Desktop rate = self.SCHEMA[cls][rating_range] self.standard = EreusePrice.Type(rate[self.STANDARD][role], price) @@ -570,11 +583,15 @@ class EreusePrice(Price): def __init__(self, rating: AggregateRate, **kwargs) -> None: if rating.rating_range == RatingRange.VERY_LOW: raise ValueError('Cannot compute price for Range.VERY_LOW') - self.price = round(rating.rating * self.MULTIPLIER[rating.device.__class__], 2) - super().__init__(rating=rating, device=rating.device, **kwargs) + # We pass ROUND_UP strategy so price is always greater than what refurbisher... amounts + price = self.to_price(rating.rating * self.MULTIPLIER[rating.device.__class__], ROUND_UP) + super().__init__(rating=rating, + device=rating.device, + price=price, + software=kwargs.pop('software', app.config['PRICE_SOFTWARE']), + version=kwargs.pop('version', app.config['PRICE_VERSION']), + **kwargs) self._compute() - self.software = self.software or app.config['PRICE_SOFTWARE'] - self.version = self.version or app.config['PRICE_VERSION'] @orm.reconstructor def _compute(self): diff --git a/ereuse_devicehub/resources/event/models.pyi b/ereuse_devicehub/resources/event/models.pyi index a146c908..fbac19e4 100644 --- a/ereuse_devicehub/resources/event/models.pyi +++ b/ereuse_devicehub/resources/event/models.pyi @@ -242,6 +242,8 @@ class WorkbenchRate(ManualRate): class Price(EventWithOneDevice): + SCALE = ... + ROUND = ... currency = ... # type: Column price = ... # type: Column software = ... # type: Column @@ -257,6 +259,10 @@ class Price(EventWithOneDevice): self.version = ... # type: StrictVersion self.rating = ... # type: AggregateRate + @classmethod + def to_price(cls, value: Union[Decimal, float], rounding=ROUND) -> Decimal: + pass + class EreusePrice(Price): MULTIPLIER = ... # type: Dict diff --git a/ereuse_devicehub/resources/event/schemas.py b/ereuse_devicehub/resources/event/schemas.py index 1ba41030..242c8886 100644 --- a/ereuse_devicehub/resources/event/schemas.py +++ b/ereuse_devicehub/resources/event/schemas.py @@ -1,5 +1,3 @@ -import decimal - from flask import current_app as app from marshmallow import Schema as MarshmallowSchema, ValidationError, validates_schema from marshmallow.fields import Boolean, DateTime, Decimal, Float, Integer, List, Nested, String, \ @@ -156,8 +154,8 @@ class AggregateRate(Rate): class Price(EventWithOneDevice): currency = EnumField(Currency, required=True, description=m.Price.currency.comment) - price = Decimal(places=4, - ounding=decimal.ROUND_HALF_EVEN, + price = Decimal(places=m.Price.SCALE, + rounding=m.Price.ROUND, required=True, description=m.Price.price.comment) software = EnumField(PriceSoftware, dump_only=True, description=m.Price.software.comment) diff --git a/tests/test_event.py b/tests/test_event.py index 342e0356..cd4f150d 100644 --- a/tests/test_event.py +++ b/tests/test_event.py @@ -1,5 +1,6 @@ import ipaddress from datetime import timedelta +from decimal import Decimal from typing import Tuple import pytest @@ -265,7 +266,7 @@ def test_migrate(): def test_price_custom(): computer = Desktop(serial_number='sn1', model='ml1', manufacturer='mr1', chassis=ComputerChassis.Docking) - price = models.Price(price=25.25, currency=Currency.EUR) + price = models.Price(price=Decimal(25.25), currency=Currency.EUR) price.device = computer assert computer.price == price db.session.add(computer) @@ -280,3 +281,12 @@ def test_price_custom(): c, _ = client.get(res=Device, item=computer.id) assert c['price']['id'] == p['id'] + + +@pytest.mark.xfail(reson='Develop test') +def test_ereuse_price(): + """Tests the several ways of creating eReuse Price, emulating + from an AggregateRate and ensuring that the different Range + return correct results.""" + # important to check Range.low no returning warranty2 + # Range.verylow not returning nothing diff --git a/tests/test_rate.py b/tests/test_rate.py index bf1cee90..99a9420b 100644 --- a/tests/test_rate.py +++ b/tests/test_rate.py @@ -1,3 +1,4 @@ +from decimal import Decimal from distutils.version import StrictVersion import pytest @@ -64,17 +65,17 @@ def test_rate(): rate.device = pc events = main.main(rate, RatingSoftware.ECost, StrictVersion('1.0')) price = next(e for e in events if isinstance(e, EreusePrice)) - assert price.price == 92.2 - assert price.retailer.standard.amount == 40.97 - assert price.platform.standard.amount == 18.84 - assert price.refurbisher.standard.amount == 32.38 + assert price.price == Decimal('92.2001') + assert price.retailer.standard.amount == Decimal('40.9714') + assert price.platform.standard.amount == Decimal('18.8434') + assert price.refurbisher.standard.amount == Decimal('32.3853') assert price.price >= price.retailer.standard.amount \ + price.platform.standard.amount \ + price.refurbisher.standard.amount - assert price.retailer.warranty2.amount == 55.30 - assert price.platform.warranty2.amount == 25.43 - assert price.refurbisher.warranty2.amount == 43.72 - assert price.warranty2 == 124.45 + assert price.retailer.warranty2.amount == Decimal('55.3085') + assert price.platform.warranty2.amount == Decimal('25.4357') + assert price.refurbisher.warranty2.amount == Decimal('43.7259') + assert price.warranty2 == Decimal('124.47') # Checks relationships workbench_rate = next(e for e in events if isinstance(e, WorkbenchRate)) aggregate_rate = next(e for e in events if isinstance(e, AggregateRate))