Opened 13 months ago

Closed 13 months ago

#35224 closed Cleanup/optimization (fixed)

Make GenericForeignKey a subclass of Field

Reported by: Adam Johnson Owned by: Adam Johnson
Component: contrib.contenttypes Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:17886 build:failure, 17863 merged, 17865 unmerged

Description (last modified by Adam Johnson)

When introduced in bca5327b21eb2e3ee18292cbe532d6d0071201d8, GenericForeignKey was created as its own class. Since then, it has grown to resemble a field, particularly in fb48eb05816b1ac87d58696cdfe48be18c901f16.

GenericForeignKey *not* being a Field leads to several issues:

  1. It is added as a “field” and returned by Model._meta.get_fields(), a type confusion ​reflected in django-stubs.
  2. It duplicates code, such as _check_field_name().
  3. It misses methods like __repr__().

I think we should make it a subclass of Field.

(GenericRelation has always been a subclass of Field.)

Change History (10)

comment:1 by Adam Johnson, 13 months ago

Description: modified (diff)

comment:2 by Adam Johnson, 13 months ago

Description: modified (diff)
Summary: Make GenericForeignKey subclass Field → Make GenericForeignKey a subclass of Field

comment:3 by Adam Johnson, 13 months ago

Has patch: set

comment:4 by Simon Charette, 13 months ago

Triage Stage: Unreviewed → Accepted

I'm eager to see what the test suite says but it seems like a worth cleanup!

comment:5 by Dylan Patrick Go, 13 months ago

Owner: changed from nobody to Dylan Patrick Go

comment:6 by Adam Johnson, 13 months ago

There’s an open PR, the tests all pass.

comment:7 by Dylan Patrick Go, 13 months ago

Owner: Dylan Patrick Go removed
Status: assigned → new

comment:8 by Adam Johnson, 13 months ago

Owner: set to Adam Johnson
Status: new → assigned

Dylan, I already attached a PR to this ticket.

comment:9 by Mariusz Felisiak, 13 months ago

Triage Stage: Accepted → Ready for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

Resolution: → fixed
Status: assigned → closed

In 6002df06:

Fixed #35224 -- Made GenericForeignKey inherit from Field.

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