#34716 closed Bug (fixed)

Class methods from nested classes cannot be used as Field.default.

Reported by: Nicolò Intrieri Owned by: Nicolò Intrieri
Component: Migrations Version: 4.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mariusz Felisiak)

Given the following model:

  
class Profile(models.Model):

    class Capability(models.TextChoices):
        BASIC = ("BASIC", "Basic")
        PROFESSIONAL = ("PROFESSIONAL", "Professional")
        
        @classmethod
        def default(cls) -> list[str]:
            return [cls.BASIC]

    capabilities = ArrayField(
        models.CharField(choices=Capability.choices, max_length=30, blank=True),
        null=True,
        default=Capability.default
    )


The resulting migration contained the following:

   # ...
     migrations.AddField(
           model_name='profile',
           name='capabilities',
           field=django.contrib.postgres.fields.ArrayField(base_field=models.CharField(blank=True, choices=[('BASIC', 'Basic'), ('PROFESSIONAL', 'Professional')], max_length=30), default=appname.models.Capability.default, null=True, size=None),
       ),
   # ...

As you can see, migrations.AddField is passed as argument "default" a wrong value "appname.models.Capability.default", which leads to an error when trying to migrate. The right value should be "appname.models.Profile.Capability.default".

Change History (9)

comment:1 by Mariusz Felisiak, 16 months ago

Description: modified (diff)
Summary: Using subclass method as a callable for a field's default value results in wrong reference in the corresponding migrationClass methods from subclasses cannot be used as Field.default.
Triage Stage: UnreviewedAccepted

Thanks for the report. It seems that FunctionTypeSerializer should use __qualname__ instead of __name__:

  • django/db/migrations/serializer.py

    diff --git a/django/db/migrations/serializer.py b/django/db/migrations/serializer.py
    index d88cda6e20..06657ebaab 100644
    a b class FunctionTypeSerializer(BaseSerializer):  
    168168        ):
    169169            klass = self.value.__self__
    170170            module = klass.__module__
    171             return "%s.%s.%s" % (module, klass.__name__, self.value.__name__), {
     171            return "%s.%s.%s" % (module, klass.__qualname__, self.value.__name__), {
    172172                "import %s" % module
    173173            }
    174174        # Further error checking

Would you like to prepare a patch? (regression test is required)

comment:2 by David Sanders, 16 months ago

Also to nitpick the terminology: Capability is a nested class, not a subclass. (fyi for anyone preparing tests/commit message)

comment:3 by David Sanders, 16 months ago

Summary: Class methods from subclasses cannot be used as Field.default.Class methods from nested classes cannot be used as Field.default.

in reply to:  2 comment:4 by Nicolò Intrieri, 16 months ago

Replying to David Sanders:

Also to nitpick the terminology: Capability is a nested class, not a subclass. (fyi for anyone preparing tests/commit message)

You're right, that was inaccurate. Thanks for having fixed the title

in reply to:  1 ; comment:5 by Nicolò Intrieri, 16 months ago

Replying to Mariusz Felisiak:

Thanks for the report. It seems that FunctionTypeSerializer should use __qualname__ instead of __name__:

  • django/db/migrations/serializer.py

    diff --git a/django/db/migrations/serializer.py b/django/db/migrations/serializer.py
    index d88cda6e20..06657ebaab 100644
    a b class FunctionTypeSerializer(BaseSerializer):  
    168168        ):
    169169            klass = self.value.__self__
    170170            module = klass.__module__
    171             return "%s.%s.%s" % (module, klass.__name__, self.value.__name__), {
     171            return "%s.%s.%s" % (module, klass.__qualname__, self.value.__name__), {
    172172                "import %s" % module
    173173            }
    174174        # Further error checking

Would you like to prepare a patch? (regression test is required)

I would be very happy to prepare a patch, i will do my best to write a test that's coherent with the current suite

comment:6 by Nicolò Intrieri, 16 months ago

Owner: changed from nobody to Nicolò Intrieri
Status: newassigned

in reply to:  5 comment:7 by Mariusz Felisiak, 16 months ago

I would be very happy to prepare a patch, i will do my best to write a test that's coherent with the current suite

You can check tests in tests.migrations.test_writer.WriterTests, e.g. test_serialize_nested_class().

comment:8 by Nicolò Intrieri, 16 months ago

Has patch: set

comment:9 by GitHub <noreply@…>, 16 months ago

Resolution: fixed
Status: assignedclosed

In e8252fc:

Fixed #34716 -- Fixed serialization of nested class methods in migrations.

Co-authored-by: Nicolò <nicolo.intrieri@…>

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