Opened 10 years ago

Closed 10 years ago

#22555 closed Bug (invalid)

Migration doesn't work when fields are added using contribute_to_class

Reported by: Matt3o12 Owned by: nobody
Component: Migrations Version: 1.7-beta-2
Severity: Release blocker Keywords: migration, fake, __fake__, contribute_to_class
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When you create a custom Field that implements the contribute_to_class and add another Field that way, it won't be possible to use migration.

The code I used:

class MyField(models.CharField):
    def contribute_to_class(self, cls, name):
        if not cls._meta.abstract:
            field = CharField(max_length = 20)
            cls.add_to_class("%s_mycontribute" % name, field)
        
        
        super(MyField, self).contribute_to_class(cls, name)

What happens if the original model is used (app.models.MyModel):

  1. contribute_to_class is called and adds additional fields (in my example, MyField).
  2. contribute_to_class calls it super function and adds the original field (CharField).

Everything works. The model has 2 fields: MyField, and CharField.

---

When python tries to create a migration (now the model __fake__.MyModel is used).

  1. contribute_to_class is called. The model already has the contributed CharField (with the name %s_mycontribute) (why does the model already have it?).
  2. MyField with the exactly name is added again (and no exception is thrown?!).
  3. The original field (MyField) is added.

The view now has 3 fields: MyField, CharField, and CharField again (both CharFields have the same name).

The next step django does is trying to migrate my model but it fails with the exception: "django.db.utils.OperationalError: duplicate column name: charFile_mycontribute" because the field was created twice.

My guess:
The original model is somewhere copied and the fields in _meta are not cleaned correctly.

Change History (1)

comment:1 by Andrew Godwin, 10 years ago

Resolution: invalid
Status: newclosed

This isn't a Django bug (we've never documented or encouraged adding dynamic fields this way so we're not going to keep it backwards compatible), you need to work with the migration serialiser instead.

The key thing is that Django will preserve fields as it sees them on the model at the time of the migration being created - which means that models that dynamically create fields and fields that dynamically create fields simply need to check and make sure the field they're trying to add isn't there already. It should be a one-line addition.

In a more general sense, I would encourage extra fields like this to be explicitly declared on the model and passed in to the other field if it needs them - like the width and height fields for images. That's how Django expects fields to work and how migrations will work best.

Note: See TracTickets for help on using tickets.
Back to Top