Code

Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#529 closed enhancement (fixed)

Add support for GenericForeignKey

Reported by: adrian Owned by: adrian
Component: Contrib apps Version:
Severity: critical Keywords: rthml tab space editor js
Cc: hi-world, cup Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

sopel had the idea of a GenericForeignKey, which would abstract the concept of "content_type_id" and "object_id". Basically, it'd be a way to relate an object to "one of several types of objects."

For example, we currently have this in the Comment model:

class Comment(meta.Model):
    # ...
    content_type = meta.ForeignKey(core.ContentType)
    object_id = meta.IntegerField()

That could be replaced with this:

class Comment(meta.Model):
    # ...
    content_object = meta.GenericForeignKey()

With that, Comment objects would get an automatic get_content_object() method, which would return whatever object was related, regardless of its type.

This is a messy problem, so we'd have to figure out a couple of loose ends:

  • Does every other object in the system get a get_comment_list method?
  • Do we enforce referential integrity, so that, for example, all the appropriate comments would be deleted if a story was deleted?

Attachments (0)

Change History (15)

comment:1 Changed 9 years ago by sopel

Regarding: Does every other object in the system get a get_comment_list method?

It should be possible to specify what models you can relate to from a given GenericFK, so you'd say: object = GenericForeignKey(Laptop, Server, Desktop). This way the above problem goes away.

This also solves the problem of our object instances being cleanly and quickly removed when a Laptop is removed. Without specifying the models that we relate to, every model on deletion would have to do a DELETE from each of the tables of models with GenericForeignKey's. And if we knew what models a GenericForeignKey can relate to - we would only issue one DELETE - to the right table.

comment:2 Changed 9 years ago by robert@…

This seems kind of evil from a relational point of view...

What is the disadvantage of having a Comment superclass ( that could be table-less),
and just subclassing for each thing you want to have comments on?

class Comment(meta.AbstractModel):

... some fields...

class LaptopComment(Comment):

content_object = meta.ForeignKey(Laptop)

class ServerComment(Comment):

content_object = meta.ForeignKey(Server)

To make this seamless, superclasses should aggregate list methods of their subclasses.
Also, I'm not sure what the right thing to do is if the target of a ForeignKey has subclasses...
I don't think any solution is great there.

comment:3 Changed 9 years ago by Boffbowsh

  • Cc paul.bowsher@… added

comment:5 Changed 9 years ago by Boffbowsh

After a bit of discussion on IRC with rjwittams, I have a solution. Basically, I need this functionality by the end of the week, and am going to have to shortcut it until a better solution is discussed.

Essentially, i'd like to have this model:

from django.core import meta

class Comment (meta.Model):
	body = meta.TextField()
	
	self.get_subject = self.get_commentobject().get_comment_subject
	
class BlogEntry (meta.Model):
	body = meta.TextField()
	who = meta.CharField(max_length=32)
	
class DocEntry (meta.Model):
	body = meta.TextField()
	function = meta.CharField(max_length=32)

class FAQEntry (meta.Model):
	question = meta.TextField()
	answer = meta.TextField()
	
class CommentObject (meta.Model):
	comment = meta.OneToOneField(Comment)
	blogentry = meta.OneToOneField(BlogEntry, null=True)
	docentry = meta.OneToOneField(DocEntry, null=True)
	faqentry = meta.OneToOneField(FAQEntry, null=True)
	
	def get_comment_subject(self):
		for field in (blogentry, docentry, faqentry):
			if field:
				return field
		return None

Shortcutted to:
{{{from django.core import meta

class Comment (meta.Model):

body = meta.TextField()
subject = meta.GenericForeignKey([BlogEntry, DocEntry, FAQEntry])


class BlogEntry (meta.Model):

body = meta.TextField()
who = meta.CharField(max_length=32)


class DocEntry (meta.Model):

body = meta.TextField()
function = meta.CharField(max_length=32)

class FAQEntry (meta.Model):

question = meta.TextField()
answer = meta.TextField()

}}}

comment.get_subject() should return either a BlogEntry, FAQEntry or DocEntry. Not entirely sure how this should look in the admin interface though.

comment:6 Changed 9 years ago by Boffbowsh

Preview function is my friend

from django.core import meta

class Comment (meta.Model):
	body = meta.TextField()
	subject = meta.GenericForeignKey([BlogEntry, DocEntry, FAQEntry])
	
class BlogEntry (meta.Model):
	body = meta.TextField()
	who = meta.CharField(max_length=32)
	
class DocEntry (meta.Model):
	body = meta.TextField()
	function = meta.CharField(max_length=32)

class FAQEntry (meta.Model):
	question = meta.TextField()
	answer = meta.TextField()

comment:7 Changed 9 years ago by Boffbowsh

Ignore my last two comments, they're wrong. rjwittams' workaround is:

from django.core import meta

class Comment (meta.Model):
	body = meta.TextField()

	def get_parent(self):
	    if not self.getters:
		getters = [self.get_comment_reply, self.get_blogcomment, self.get_faqcomment, self.get_doccomment]
	    for getter in self.getters:
		ext = getter()
		if ext:
		   return ext.parent      

class BlogEntry (meta.Model):
	body = meta.TextField()
	who = meta.CharField(max_length=32)
	
class DocEntry (meta.Model):
	body = meta.TextField()
	function = meta.CharField(max_length=32)

class FAQEntry (meta.Model):
	question = meta.TextField()
	answer = meta.TextField()
	
class BlogComment (meta.Model):
	comment = meta.OneToOneField(Comment)
	parent = meta.ForeignKey(BlogEntry)

class CommentReply (meta.Model):
	comment = meta.OneToOneField(Comment)
	parent = meta.ForeignKey(Comment)

class DocComment(meta.Model):
	comment = meta.OneToOneField(Comment)
	parent =  meta.ForeignKey(DocEntry)
	
class FaqComment(meta.Model):
	comment = meta.OneToOneField(Comment)
	parent = meta.ForeignKey(FAQEntry)

comment:8 Changed 8 years ago by Here

  • Type defect deleted

comment:9 Changed 8 years ago by jacob

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

(In [3134]) Added generic foreign key support to Django. Much thanks to Ian Holsman and
Luke Plant -- most of this code is theirs. Documentation is to follow; for now
see the example/unit test. Fixes #529.

comment:10 Changed 8 years ago by anonymous

  • Type set to defect

comment:11 Changed 8 years ago by anonymous

  • Cc paul.bowsher@… removed

Remove CC to prevent spam - sorry

comment:12 Changed 8 years ago by anonymous

  • Component changed from Metasystem to Contrib apps
  • milestone set to Version 1.0
  • priority changed from low to high
  • Severity changed from normal to critical
  • Type changed from defect to enhancement

comment:13 Changed 8 years ago by hi-world cup

  • Cc hi-world, cup added
  • Keywords rthml tab space editor js added
  • Summary changed from Add support for GenericForeignKey to hi-world cup

comment:14 Changed 8 years ago by adrian

  • Summary changed from hi-world cup to Add support for GenericForeignKey

comment:15 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

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.