Code

Opened 6 years ago

Closed 16 months ago

#8565 closed Cleanup/optimization (wontfix)

Comments should use object_id instead of object_pk

Reported by: baumer1122 Owned by: nobody
Component: contrib.comments Version: master
Severity: Normal Keywords: comments generic object_pk object_id
Cc: pete@… Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.contrib.comments.models.BaseCommentAbstractModel uses object_pk to create a generic relation

I may be missing something, but straying from the expected and documented default of object_id seems to only create extra confusion. I noticed this when trying to add a GenericTabularInline

Attachments (0)

Change History (6)

comment:1 Changed 6 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

See also #8554

comment:2 Changed 6 years ago by jacob

  • Resolution set to duplicate
  • Status changed from new to closed

Dup of #8554.

comment:3 Changed 2 years ago by anonymous

  • Easy pickings unset
  • Resolution duplicate deleted
  • Severity set to Normal
  • Status changed from closed to reopened
  • Type set to Uncategorized
  • UI/UX unset

I don't see how this is a duplicate of 8554, which has to do with casting the output of object_pk to text. The issue here is the use of "object_pk" instead of "object_id" has the field name. This non-standard field name requires anyone who extends it and uses GenericRelation back to use object_id_field="object_pk". This isn't documented anywhere either.

comment:4 Changed 2 years ago by anonymous

  • Type changed from Uncategorized to Bug

comment:5 Changed 2 years ago by lrekucki

  • Triage Stage changed from Unreviewed to Someday/Maybe
  • Type changed from Bug to Cleanup/optimization

This isn't documented anywhere either.

How about https://docs.djangoproject.com/en/1.4/ref/contrib/comments/models/ ?

I don't really see an issue here. Sure, it would be better if the original author used the default name, but having a different name doesn't break anything. Changing this would mean, *every* project using contrib.comments would have to migrate or monkey patch, so I don't think this is worth the trouble. Maybe in 2.0 ?

comment:6 Changed 16 months ago by aaugustin

  • Resolution set to wontfix
  • Status changed from reopened to closed

This is a bit annoying, but changing the name doesn't seem worth it at this point.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.