[Django] #36973: fields.E348 check should check against the accessor_name instead of the model name

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Type: Bug
Status: new | Component: Core
| (System checks)
Version: 6.0 | Severity: Normal
Keywords: System checks, | Triage Stage:
fields.E348, related_name | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
when running manage.py I hit erroneous fields.E348 check. The system check
is supposed to examine if ManyToOne/ManyToMany field <related_name> for
<model>.<field name> clashes with the name of a model manager. However,
instead of checking the related_name (i.e. <fieldname>_set or manually set
related_name=<related_name>) it gets the actual field name of the foreign
key.

As shown in the original check function below

{{{
def _check_conflict_with_managers(self):
errors = []
manager_names = {manager.name for manager in self.opts.managers}
for rel_objs in self.model._meta.related_objects:
related_object_name = rel_objs.name
if related_object_name in manager_names:
field_name = f"{self.model._meta.object_name}.{self.name}"
errors.append(
checks.Error(
f"Related name '{related_object_name}' for
'{field_name}' "
"clashes with the name of a model manager.",
hint=(
"Rename the model manager or change the
related_name "
f"argument in the definition for field
'{field_name}'."
),
obj=self,
id="fields.E348",
)
)
return errors
}}}

it compares a model's managers' name and its related object field name.

{{{ related_object_name = rel_objs.name }}}

**Patch:**
It should use the .accessor_name instead of .name
{{{
diff --git a/django/db/models/fields/related.py
b/django/db/models/fields/related.py
index f1bc664007..a3ef912a8e 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -723,7 +723,7 @@ class ForeignObject(RelatedField):
errors = []
manager_names = {manager.name for manager in self.opts.managers}
for rel_objs in self.model._meta.related_objects:
- related_object_name = rel_objs.name
+ related_object_name = rel_objs.accessor_name
if related_object_name in manager_names:
field_name =
f"{self.model._meta.object_name}.{self.name}"
errors.append(
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36973>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------

Reporter: nick.ma@… | Owner: Shubh Rai
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: System checks, | Triage Stage:
fields.E348, related_name | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Shubh Rai):

* component: Core (System checks) => Database layer (models, ORM)
* owner: (none) => Shubh Rai
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:1>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Shubh Rai
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: System checks, | Triage Stage:
fields.E348, related_name | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Shubh Rai):

* cc: Shubh Rai (added)

Comment:

I have made a pr: https://github.com/django/django/pull/20846 . Please
feel free to tell me any changes that are required , any feedback would be
appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:2>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Shubh Rai
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: System checks, | Triage Stage: Accepted
fields.E348, related_name |

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Clifford Gama):

* easy: 1 => 0
* stage: Unreviewed => Accepted

Comment:

While investigating this ticket, I discovered that the patch added in
#22977 only identifies a clash between an accessor and a manager if the
relationship is self-referential. The attempt to fix that issue made it
easier to understand the issue reported here. I opened an alternative PR
to fix both issues.
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:3>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Shubh Rai
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: System checks, | Triage Stage: Accepted
fields.E348, related_name |
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_tests: 0 => 1

Comment:

I think we can add one test case for the issue discovered in review, but
otherwise this is good to go.
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:4>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Shubh Rai
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: System checks, | Triage Stage: Ready for
fields.E348, related_name | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin

Comment:

Ah, it was already accounted for!
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:5>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Shubh Rai
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |

Severity: Release blocker | Resolution:
Keywords: System checks, | Triage Stage: Accepted
fields.E348, related_name |
Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_docs: 0 => 1
* severity: Normal => Release blocker
* stage: Ready for checkin => Accepted

Comment:

Actually, this is a bug in 6.0, so we need a release note so we can
backport before Tuesday.
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:6>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------

Reporter: nick.ma@… | Owner: Clifford
| Gama

Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: System checks, | Triage Stage: Accepted
fields.E348, related_name |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* owner: Shubh Rai => Clifford Gama

--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:7>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Clifford
| Gama
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: System checks, | Triage Stage: Ready for
fields.E348, related_name | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_docs: 1 => 0

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:8>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Clifford
| Gama

Type: Bug | Status: closed

Component: Database layer | Version: 6.0
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: System checks, | Triage Stage: Ready for
fields.E348, related_name | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"fcf916884d25ed430bd7cedaea2b10035c2aa3b6" fcf91688]:
{{{#!CommitTicketReference repository=""
revision="fcf916884d25ed430bd7cedaea2b10035c2aa3b6"
Fixed #36973 -- Made fields.E348 check detect further clashes between
managers and related_names.

Clashes were only detected for self-referential relationships, i.e.
ForeignKey("self").

Refs #22977. Bug in 6888375c53476011754f778deabc6cdbfa327011.

Thanks JaeHyuckSa for the thorough review!
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:9>

[Django]

#36973: fields.E348 check should check against the accessor_name instead of the
model name
-------------------------------------+-------------------------------------
Reporter: nick.ma@… | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: System checks, | Triage Stage: Ready for
fields.E348, related_name | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"4eb38f6994f233feeac5852cbee2ae6663a9b1ef" 4eb38f6]:
{{{#!CommitTicketReference repository=""
revision="4eb38f6994f233feeac5852cbee2ae6663a9b1ef"
[6.0.x] Fixed #36973 -- Made fields.E348 check detect further clashes

between managers and related_names.

Clashes were only detected for self-referential relationships, i.e.
ForeignKey("self").

Refs #22977. Bug in 6888375c53476011754f778deabc6cdbfa327011.

Thanks JaeHyuckSa for the thorough review!

Backport of fcf916884d25ed430bd7cedaea2b10035c2aa3b6 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36973#comment:10>