Opened 5 years ago

Closed 22 months ago

#16978 closed Bug (fixed)

Related models cannot have split() method

Reported by: Mitar Owned by: Jonas Obrist
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: mmitar@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Related models cannot have split() method because add_lazy_relation (in db/models/fields/related.py) assumes that something with split() method must be string. I would propose that:

        try:
            app_label, model_name = relation.split(".")
        except ValueError:

is changed to:

        try:
            if isinstance(relation, ModelBase):
                raise ValueError
            app_label, model_name = relation.split(".")
        except ValueError:

Attachments (1)

16978.patch (2.4 KB) - added by Jonas Obrist 5 years ago.
tests & fix

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by Carl Meyer

Component: DocumentationDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Version: 1.3SVN

Not sure the proposed patch is the best option, but this should certainly be fixed.

comment:2 Changed 5 years ago by Jonas Obrist

Owner: changed from nobody to Jonas Obrist

comment:3 Changed 5 years ago by Jonas Obrist

I tried to reproduce the problem, but couldn't do so, what is needed to have this problem happen?

comment:4 Changed 5 years ago by Jonas Obrist

Owner: changed from Jonas Obrist to nobody

comment:5 Changed 5 years ago by Mitar

You have to check when add_lazy_relation is called with class as first argument. I find two such cases:

  • in RelatedField's contribute_to_class if other._meta.pk is None
  • if order_with_respect_to is configured

I do not remember how I triggered the first case. But the second case happens with, for example:

class RelatedModel(models.Model):
    foobar = models.IntegerField()

    def split(self):
        pass

class SomeModel(models.Model):
    related_field = models.ForeignKey(RelatedModel)

    class Meta:
        order_with_respect_to = 'related_field'

comment:6 Changed 5 years ago by Jonas Obrist

Needs tests: set

Changed 5 years ago by Jonas Obrist

Attachment: 16978.patch added

tests & fix

comment:7 Changed 5 years ago by Jonas Obrist

Has patch: set
Needs tests: unset
Owner: changed from nobody to Jonas Obrist

comment:8 Changed 5 years ago by Ramiro Morales

Triage Stage: AcceptedReady for checkin

comment:9 Changed 5 years ago by Aymeric Augustin

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Apparently, the test passes without the fix in django/db/models/fields/related.py.

comment:10 Changed 22 months ago by Tim Graham

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top