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

django rest framework get_value with amount=None and currency is not None throws an InvalidOperation exception #458

Closed
carvincarl opened this issue Jan 9, 2019 · 1 comment

Comments

@carvincarl
Copy link
Contributor

carvincarl commented Jan 9, 2019

In the DRF code, when deserializing a JSON object with a value for currency and None for the amount, there is an InvalidOperation exception instantiating the Money object.

Example stack trace:

.tox/django111-py36/lib/python3.6/site-packages/rest_framework/serializers.py:236: in is_valid
    self._validated_data = self.run_validation(self.initial_data)
.tox/django111-py36/lib/python3.6/site-packages/rest_framework/serializers.py:434: in run_validation
    value = self.to_internal_value(data)
.tox/django111-py36/lib/python3.6/site-packages/rest_framework/serializers.py:486: in to_internal_value
    primitive_value = field.get_value(data)
djmoney/contrib/django_rest_framework/fields.py:34: in get_value
    return Money(amount, currency)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <[AttributeError("'Money' object has no attribute 'amount'") raised in repr()] Money object at 0x10f4cb470>, amount = None
currency = 'USD'

    def __init__(self, amount=Decimal('0.0'), currency=DEFAULT_CURRENCY_CODE):
        if not isinstance(amount, Decimal):
>           amount = Decimal(str(amount))
E           decimal.InvalidOperation: [<class 'decimal.ConversionSyntax'>]

.tox/django111-py36/lib/python3.6/site-packages/moneyed/classes.py:83: InvalidOperation

Here is the method with the problem:

    def get_value(self, data):
        amount = super(MoneyField, self).get_value(data)
        currency = data.get(get_currency_field_name(self.field_name), None)
        if currency:  # Missing a check for amount is None.
            return Money(amount, currency)
        return amount

The fix is to also check that the amount is not None.

        if currency and amount is not None:
            return Money(amount, currency)

Here is a test to expose the error:

    @pytest.mark.parametrize(
        'body, expected', (
            ({'field': '10', 'field_currency': 'EUR'}, Money(10, 'EUR')),
            ({'field': '12.20', 'field_currency': 'GBP'}, Money(12.20, 'GBP')),
            ({'field': '15.15', 'field_currency': 'USD'}, Money(15.15, 'USD')),
            ({'field': None, 'field_currency': 'USD'}, None),  # Add this line for amount is None and currency is not None.
        ),
    )
    def test_post_put_values(self, body, expected):
        serializer = self.get_serializer(NullMoneyFieldModel, data=body)
        serializer.is_valid()
        assert serializer.validated_data['field'] == expected

I will submit a pull request with the test and fix.

Thanks

@carvincarl carvincarl changed the title django rest framework to_internal_value with amount=None and currency is not None throws an InvalidOperation exception django rest framework get_value with amount=None and currency is not None throws an InvalidOperation exception Jan 9, 2019
carvincarl added a commit to carvincarl/django-money that referenced this issue Jan 10, 2019
…rib.django_rest_framework.fields.MoneyField.get_value when amount is None and currency is not None.
@ahurracode
Copy link

Phew glad to see this..

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

No branches or pull requests

3 participants