Skip to content

Commit

Permalink
Revert "Removed useless code, two GFK to one model don't work anyways…"
Browse files Browse the repository at this point in the history 
This reverts commit 188182b.
  • Loading branch information
@apollo13
apollo13 committed Nov 25, 2013
1 parent e0d3de7 commit 20a8063
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion taggit/managers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ def post_through_setup(self, cls):
self.rel.to = self.through._meta.get_field("tag").rel.to
self.related = RelatedObject(self.through, cls, self)
if self.use_gfk:
tagged_items = GenericRelation(self.through)
self.__class__._related_name_counter += 1
related_name = '+%d' % self.__class__._related_name_counter
tagged_items = GenericRelation(self.through, related_name=related_name)
tagged_items.contribute_to_class(cls, 'tagged_items')

def save_form_data(self, instance, value):
Expand Down

10 comments on commit 20a8063

@viciu
Copy link

@viciu viciu commented on 20a8063 Nov 27, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for reverting commit 188182b? After reverting, deletion of tagged item in admin causes

AttributeError: 'MyCustomTaggedItem' object has no attribute '+1'.

I tested this on Django Version: 1.5.5

With version 0.11 it works fine.

@apollo13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vici: Can you please open a issue with full traceback? The reason was that it would fail on 1.4 and 1.5 if you had more than one relation https://travis-ci.org/alex/django-taggit/builds/14516600

@apollo13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, technically I just should forbid two generic taggable relations -- but this seemed like the easier way out; patches + test welcome

@apollo13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@viciu: Can you confirm that acbbba8 fixes this for you?

@viciu
Copy link

@viciu viciu commented on 20a8063 Dec 1, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apollo13: Thanks! I confirm, after your last commit this works for me. As suggested, I've created #172 for the reference.

@konrad1234
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting this same problem for Django 1.6. Can anyone confirm whether or not this is a problem for them as well?

@viciu
Copy link

@viciu viciu commented on 20a8063 Dec 13, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konrad1234: have you tried just released Django 1.6.1?

@konrad1234
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just updated but it didn't fix the problem.

@apollo13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to use the develop branch till I got a new release…

@konrad1234
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks! That commit works for Django 1.6.1.

Please sign in to comment.