Repository: django-money/django-money

Issue:
djmoney alters existing objects
We have an application that uses `moneyed` and (sub-classes of) its `Money` type in a non-Django environment. We recently started integrating this into a Django app that uses djmoney and started getting weird errors in seemingly unrelated cases  because instances of `djmoney.money.Money` started showing up there.

We tracked this down to a behavior of the djmoney package in `djmoney.models.fields`:
https://github.com/django-money/django-money/blob/bb222ed619f72a0112aef8c0fd9a4823b1e6e388/djmoney/models/fields.py#L38
https://github.com/django-money/django-money/blob/bb222ed619f72a0112aef8c0fd9a4823b1e6e388/djmoney/models/fields.py#L209

The two places alter **the type of an existing object**. Can we change this to copying the objects before altering them?

Discussion:
benjaoming: Hi @moser - you should be able to get the tests passing by merging in the latest master branch :+1: 
Stranger6667: I absolutely agree with these changes - it was a wrong decision to mutate the input. It will be nice to have a regression test + a changelog entry
Stranger6667: I will take care of these :)

Patch:
diff --git a/djmoney/models/fields.py b/djmoney/models/fields.py
index 8b812be..e24af66 100644
--- a/djmoney/models/fields.py
+++ b/djmoney/models/fields.py
@@ -35,7 +35,7 @@ def get_value(obj, expr):
     else:
         expr = expr.value
     if isinstance(expr, OldMoney):
-        expr.__class__ = Money
+        expr = Money(expr.amount, expr.currency)
     return expr
 
 
@@ -206,7 +206,7 @@ class MoneyField(models.DecimalField):
         elif isinstance(default, (float, Decimal, int)):
             default = Money(default, default_currency)
         elif isinstance(default, OldMoney):
-            default.__class__ = Money
+            default = Money(default.amount, default.currency)
         if default is not None and default is not NOT_PROVIDED and not isinstance(default, Money):
             raise ValueError("default value must be an instance of Money, is: %s" % default)
         return default
diff --git a/docs/changes.rst b/docs/changes.rst
index 1622376..49d3aa1 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -4,6 +4,10 @@ Changelog
 `Unreleased`_ - TBD
 -------------------
 
+**Fixed**
+
+- Do not mutate the input ``moneyed.Money`` class to ``djmoney.money.Money`` in ``MoneyField.default`` and F-expressions. `#603`_ (`moser`_)
+
 `1.3`_ - 2021-01-10
 -------------------
 
@@ -700,6 +704,7 @@ wrapping with ``money_manager``.
 .. _0.3: https://github.com/django-money/django-money/compare/0.2...0.3
 .. _0.2: https://github.com/django-money/django-money/compare/0.2...a6d90348085332a393abb40b86b5dd9505489b04
 
+.. _#603: https://github.com/django-money/django-money/issues/603
 .. _#595: https://github.com/django-money/django-money/issues/595
 .. _#593: https://github.com/django-money/django-money/issues/593
 .. _#586: https://github.com/django-money/django-money/issues/586
@@ -852,6 +857,7 @@ wrapping with ``money_manager``.
 .. _lobziik: https://github.com/lobziik
 .. _mattions: https://github.com/mattions
 .. _mithrilstar: https://github.com/mithrilstar
+.. _moser: https://github.com/moser
 .. _MrFus10n: https://github.com/MrFus10n
 .. _msgre: https://github.com/msgre
 .. _mstarostik: https://github.com/mstarostik


Test patch:
diff --git a/tests/test_models.py b/tests/test_models.py
index d13ec03..3641356 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -83,6 +83,31 @@ class TestVanillaMoneyField:
         retrieved = model_class.objects.get(pk=instance.pk)
         assert retrieved.money == expected
 
+    def test_old_money_not_mutated_default(self):
+        # See GH-603
+        money = OldMoney(1, "EUR")
+
+        # When `moneyed.Money` is passed as a default value to a model
+        class Model(models.Model):
+            price = MoneyField(default=money, max_digits=10, decimal_places=2)
+
+            class Meta:
+                abstract = True
+
+        # Its class should remain the same
+        assert type(money) is OldMoney
+
+    def test_old_money_not_mutated_f_object(self):
+        # See GH-603
+        money = OldMoney(1, "EUR")
+
+        instance = ModelWithVanillaMoneyField.objects.create(money=Money(5, "EUR"), integer=2)
+        # When `moneyed.Money` is a part of an F-expression
+        instance.money = F("money") + money
+
+        # Its class should remain the same
+        assert type(money) is OldMoney
+
     def test_old_money_defaults(self):
         instance = ModelWithDefaultAsOldMoney.objects.create()
         assert instance.money == Money(".01", "RUB")