'MoneyField' object has no attribute '_currency_field' when running migration

[richygit]

Hi there,

I have a migration to add the following field:

from djmoney.models.fields import MoneyField

class Invoice(Model):
    my_amount = MoneyField(max_digits=14, decimal_places=2, default="0 USD")

I added some code to update this field in a view and that all appears to be working fine.

Now I want to add a data migration like below:

from djmoney.money import Money

from django.db import migrations

def migrate_invoices(app, _):
    Invoice = app.get_model("users", "Invoice")
    for invoice in Invoice.objects.all():
        invoice.my_amount = Money(10, 'AUD')
        invoice.save()
 

class Migration(migrations.Migration):

    operations = [
        migrations.RunPython(migrate_invoices, migrations.RunPython.noop)
    ]

However this is producing the following error:

  File "/app/myapp/users/migrations/0159_auto_20230929_0113.py", line 9, in migrate_invoices
    for invoice in Invoice.objects.all():
  File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 280, in __iter__
    self._fetch_all()
  File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 69, in __iter__
    obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
  File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py", line 515, in from_db
    new = cls(*values)
  File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py", line 437, in __init__
    _setattr(self, field.attname, val)
  File "/usr/local/lib/python3.9/site-packages/djmoney/models/fields.py", line 111, in __set__
    and self.field._currency_field.null
AttributeError: 'MoneyField' object has no attribute '_currency_field'

Not sure if I'm missing something?

Project uses:

• Django 3.2.19
• Python 3.9
• Django-money 3.3

[vanschelven]

Likely caused by #726 (i.e. caused by me) although I don't have the time to research this further at the moment

[richygit]

Thanks for the update.

Workaround for me is to use django-money v3.2

[vanschelven]

Workaround for me is to use django-money v3.2

I'm sure downgrading to 3.2 works because it's indeed the new code that causes this.

[vanschelven]

Given that I caused this my 2 cents before I finaly remove myself from this issue for lack of time 😄

The bug is that, in the recent PR, it was made so that in the "migration world" _currency_field is never set on MoneyField instances.
This was done in the assumption that the only reason "fake realities" are created in the migration world is to detect what kind of migration work needs to be done.

The current issue proves that assumption incorrect: when RunPython migrations are run, the "fake realities" are needed to create actual object-instances.
Because these object-instances live in the fake migration world, they now lack their _currency_field.
Then, when the following is reached (setting object values), it crashes:

  File "/usr/local/lib/python3.9/site-packages/djmoney/models/fields.py", line 111, in __set__
    and self.field._currency_field.null

The simplest (but ugly, and potentially incomplete) solution would be to simply a guard like hasattr(field, "_currency_field") in this location. It seems that there are actually only very limited usages of ._currency_field so this might just work.But it still feels super-hacky.

A more proper solution would be to link MoneyField and CurrencyField together using the _currency_field attribute even when the CurrencyField was not actually created as part of contribute_to_class.
I managed to get this to work if the order of field-creation is currency-before-money, but not for migrations where it is the other way around. Something like this:

    def contribute_to_class(self, cls, name):
        cls._meta.has_money_field = True

        if not hasattr(self, "_currency_field"):
            if not cls.__module__ == "__fake__":
                self.add_currency_field(cls, name)
            else:
                field_name = get_currency_field_name(name, self)
                if hasattr(cls, field_name):
                    self._currency_field = getattr(cls, field_name)

        super().contribute_to_class(cls, name)

        setattr(cls, self.name, self.money_descriptor_class(self))

[ignat14]

Any updates on this? I'm having the same issue when adding a data migration.

[majdalsado]

Had a similar error creating a migration that didn't involve any MoneyFields but was interacting with a model which contains a MoneyField. Very strange but downgrading to v3.2

[grimwm]

Thanks for leaving it broken instead of reverting your PR. It's awesome, because I was on 3.4 and now have to move to 3.2.

[washeck]

Hi, I don't agree with grimwm's wording. I'd like to thank the authors for the work they put into maintenance of this library.

On the other hand, we also had to downgrade to 3.2 and it seems reverting the PR might a good thing.

@benjaoming What do you think?

[Stranger6667]

Thanks for leaving it broken instead of reverting your PR. It's awesome, because I was on 3.4 and now have to move to 3.2.

While I think that the problem is reasonably frustrating, I don't think blaming the original author is constructive in any way. Anybody with sufficient bandwidth and the desire to fix it could have opened a PR reverting that change (as per your suggestion). Or, better, adding some test cases, even failing ones (under xfail) would be a good step to prevent such issues in the future.

Other than that I do think we need to have stricter rules for incoming PRs - changes like that should always have test cases that are failing on the main branch and don't on the PR's one.

I don't have much capacity for diving deeper into the details at the moment but would be happy to collaborate on reviewing PRs to fix it later on.

[washeck]

The thing is that, as a random contributor, it is difficult to guess it the maintainers prefer to revert the PR or fix it.

So is sending a PR with revert of #726 and tests proving that the revert fixed the current issue a good way forward?

[Stranger6667]

Another thing I'd like to add to this area is a set of tests with Hypothesis, specifically stateful tests. Years ago I had some prototypes but didn't finish them. The idea is to define a state machine where transitions are adding / changing / removing MoneyField, manipulating other fields, and adding data migrations. Then, in the final state, it will check that all the changes are applied after running the generated migration + no internal errors occur.

[Stranger6667]

@washeck I think that reverting is strictly better than having an internal error, as the previous state presumably has a workaround (manually writing undetected migrations) and a smaller impact. Then we can work on a proper fix for the original problem that PR aimed to solve.

[ChrisFrankoPhD]

is there a work around for this that doesn't involve downgrading? Having the same issue still on 3.4 when querying a model with MoneyField in a custom migration