jpwatts / django-positions

A Django field for custom model ordering.
BSD 3-Clause "New" or "Revised" License
284 stars 71 forks source link

cannot use a manyToManyField for a collection #4

Closed aronchi closed 14 years ago

aronchi commented 14 years ago

If I try to add a collection named with a manyToManyField (like a category for a Product), I get this error:

'Product' instance needs to have a primary key value before a many-to-many relationship can be used.

I've used this code in my model: position= PositionField(_('position'), collection='categories')

jpwatts commented 14 years ago

Thanks for pointing this out; I've never tried using a ManyToManyField as a collection. It definitely doesn't work and the error message isn't helpful, either.

I'm going to generalize and say that any time you want to use a ManyToManyField as a collection, what you really need is an intermediate model with a PositionField. In your example, position is really an attribute of the relationship between a Product and a Category, rather than of a Product itself.

class Category(models.Model):
    ...

class Product(models.Model):
    categories = models.ManyToManyField(Category, through='ProductCategory')

class ProductCategory(models.Model):
    product = models.ForeignKey(Product)
    category = models.ForeignKey(Category)
    position = PositionField(collection=('product', 'category'))

If you can think of any cases where the generalization doesn't hold, I'd love to hear about them. Otherwise, I'll probably change PositionField to raise an exception if a ManyToManyField is provided in the collection argument. At least that way the error message will be related to the actual problem, rather than a Django implementation detail.

jpwatts commented 14 years ago

Closed by 650cb0eb68bd8f49c7c5d4957ea724ffbc7104b1. Documents proper usage of PositionField for many-to-many relationships.

jpwatts commented 14 years ago

I documented this in 650cb0eb68bd8f49c7c5d4957ea724ffbc7104b1. The example I gave above wasn't very good—sorry about that. I also added positions.examples.store to demonstrate proper usage and test that everything works as expected.