jpwatts / django-positions

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

Retrieve next_sibling per collection on a deleted instance, not just once #55

Closed amywieliczka closed 6 years ago

amywieliczka commented 7 years ago

Delete assumes a given model has only one PositionField and reorders that singular PositionField. In the instance of many position fields, though, each must be reordered respectively.

class ExhibitItem(models.Model):
    exhibit = models.ForeignKey(Exhibit, on_delete=models.CASCADE, blank=True, null=True)
    lesson_plan = models.ForeignKey(LessonPlan, on_delete=models.CASCADE, blank=True, null=True)
    historical_essay = models.ForeignKey(HistoricalEssay, on_delete=models.CASCADE, blank=True, null=True)
    exhibit_order = PositionField(collection='exhibit')
    lesson_plan_order = PositionField(collection='lesson_plan')
    historical_essay_order = PositionField(collection='historical_essay')

An ExhibitItem may be 3rd in the exhibit, 12th in the lesson plan, and not exist in any historical essays. When that ExhibitItem is deleted, the 4th item in the exhibit becomes the 3rd item, AND the 13th item in the lesson plan becomes the 12th.

forrestp commented 6 years ago

@amywieliczka I know this is pretty old, but I finally took a deeper look at your PR.

I like your general approach, but I'm worried this only solves the bug you saw for PositionFields with a single reference field as a collection, while this library technically supports using multiple other fields as the collection.

I have issued a new PR (#58) with a similar implementation that should handle this other case, as well as caching the next_sibling_pk label to avoid dynamically generating the same string in 4 different places. Can you confirm that this still solves the bug for your use case?

forrestp commented 6 years ago

I merged in my implementation, closing this for now. Please re-open if there are concerns that the fix in #58 is incorrect.