Opened 3 years ago

Closed 4 weeks ago

#16978 closed Bug (fixed)

Related models cannot have split() method

Reported by: mitar Owned by: ojii
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 ojii 3 years ago.
tests & fix

Download all attachments as: .zip

Change History (11)

comment:1 Changed 3 years ago by carljm

  • Component changed from Documentation to Database layer (models, ORM)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.3 to SVN

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

comment:2 Changed 3 years ago by ojii

  • Owner changed from nobody to ojii

comment:3 Changed 3 years ago by ojii

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

comment:4 Changed 3 years ago by ojii

  • Owner changed from ojii to nobody

comment:5 Changed 3 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 3 years ago by ojii

  • Needs tests set

Changed 3 years ago by ojii

tests & fix

comment:7 Changed 3 years ago by ojii

  • Has patch set
  • Needs tests unset
  • Owner changed from nobody to ojii

comment:8 Changed 3 years ago by ramiro

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 3 years ago by aaugustin

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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

comment:10 Changed 4 weeks ago by timgraham

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top