Opened 11 years ago

Closed 41 hours ago

#22977 closed Cleanup/optimization (fixed)

No validation error when ForeignKey related_name clashes with manager name.

Reported by: Russell Keith-Magee Owned by: Mariusz Felisiak
Component: Core (System checks) Version: dev
Severity: Normal Keywords:
Cc: Mariusz Felisiak Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider the following model definition:

class Author(models.Model):
    authors = models.Manager()

    name = models.CharField(max_length=100)

    mentor = models.ForeignKey('self', related_name='authors')

The related name on the the foreign key clashes with the manager, so when you try to issue a query:

    Author.authors.filter(name__startswith='Douglas')

you get an error because "Author.authors has no attribute 'filter'".

The problem is order dependent; if you define the manager *after* the foreign key, you get different errors.

I haven't checked what errors you get if you just have the default Manager, and a related_name of 'objects'.

There may also be problems if you have a field named 'objects'.

I suspect this class of problem could be picked up by the system check framework.

Change History (18)

comment:1 by Konrad Świat, 10 years ago

Owner: changed from nobody to Konrad Świat
Status: newassigned

comment:2 by Konrad Świat, 10 years ago

Has patch: set

comment:3 by Tim Graham, 10 years ago

Patch needs improvement: set

As Loic noted on the PR, we should revisit this after the _meta factor is merged.

comment:4 by Konrad Świat, 10 years ago

Owner: Konrad Świat removed
Status: assignednew

comment:5 by Davide Ceretti, 10 years ago

Owner: set to Davide Ceretti
Status: newassigned
Version: 1.6master

comment:6 by Davide Ceretti, 10 years ago

Given loic's comment at https://github.com/django/django/pull/3359#issuecomment-59642918, what would be the correct implementation of this validation check? Adding it to BaseManager.check method?

comment:7 by Mariusz Felisiak, 3 years ago

Owner: Davide Ceretti removed
Status: assignednew

comment:8 by Anthony Joseph, 4 months ago

Owner: set to Anthony Joseph
Status: newassigned

comment:9 by Mariusz Felisiak, 4 months ago

Cc: Mariusz Felisiak added

comment:10 by Jacob Walls, 4 months ago

Patch needs improvement: unset

comment:11 by Sarah Boyce, 4 months ago

Patch needs improvement: set

comment:12 by Natalia Bidart, 2 weeks ago

Owner: Anthony Joseph removed
Status: assignednew

comment:13 by Mariusz Felisiak, 2 weeks ago

Owner: set to Mariusz Felisiak
Status: newassigned

I will finish it.

in reply to:  13 ; comment:14 by Johanan Oppong Amoateng, 6 days ago

Replying to Mariusz Felisiak:

I will finish it.

Hey Mariusz Felisiak are you still working on this? If not i would like to pick it up

in reply to:  14 comment:15 by Mariusz Felisiak, 6 days ago

Replying to Johanan Oppong Amoateng:

Replying to Mariusz Felisiak:

I will finish it.

Hey Mariusz Felisiak are you still working on this? If not i would like to pick it up

It's only 9 days, pretty early to ask. I'll finish it in the coming days.

comment:16 by Mariusz Felisiak, 4 days ago

Patch needs improvement: unset

New PR.

comment:17 by Sarah Boyce, 2 days ago

Triage Stage: AcceptedReady for checkin

comment:18 by Mariusz Felisiak <felisiak.mariusz@…>, 41 hours ago

Resolution: fixed
Status: assignedclosed

In 6888375c:

Fixed #22977 -- Added system check for clashing managers and reverse related fields.

With thanks to Konrad Świat, Loïc Bistuer, Russell Keith-Magee,
and Mariusz Felisiak.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>

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