diff --git a/django/forms/models.py b/django/forms/models.py index 8e9c0de0e7..cc43612bf5 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -464,8 +464,21 @@ class BaseModelFormSet(BaseFormSet): return len(self.get_queryset()) return super(BaseModelFormSet, self).initial_form_count() + def _existing_object(self, pk): + if not hasattr(self, '_object_dict'): + self._object_dict = dict([(o.pk, o) for o in self.get_queryset()]) + return self._object_dict.get(pk) + def _construct_form(self, i, **kwargs): - if i < self.initial_form_count(): + if self.is_bound and i < self.initial_form_count(): + pk_key = "%s-%s" % (self.add_prefix(i), self.model._meta.pk.name) + pk = self.data[pk_key] + pk_field = self.model._meta.pk + pk = pk_field.get_db_prep_lookup('exact', pk) + if isinstance(pk, list): + pk = pk[0] + kwargs['instance'] = self._existing_object(pk) + if i < self.initial_form_count() and not kwargs.get('instance'): kwargs['instance'] = self.get_queryset()[i] return super(BaseModelFormSet, self)._construct_form(i, **kwargs) @@ -604,10 +617,6 @@ class BaseModelFormSet(BaseFormSet): if not self.get_queryset(): return [] - # Put the objects from self.get_queryset into a dict so they are easy to lookup by pk - existing_objects = {} - for obj in self.get_queryset(): - existing_objects[obj.pk] = obj saved_instances = [] for form in self.initial_forms: pk_name = self._pk_field.name @@ -618,7 +627,7 @@ class BaseModelFormSet(BaseFormSet): pk_value = form.fields[pk_name].clean(raw_pk_value) pk_value = getattr(pk_value, 'pk', pk_value) - obj = existing_objects[pk_value] + obj = self._existing_object(pk_value) if self.can_delete: raw_delete_value = form._raw_value(DELETION_FIELD_NAME) should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value) @@ -663,10 +672,13 @@ class BaseModelFormSet(BaseFormSet): return ((not pk.editable) or (pk.auto_created or isinstance(pk, AutoField)) or (pk.rel and pk.rel.parent_link and pk_is_not_editable(pk.rel.to._meta.pk))) if pk_is_not_editable(pk) or pk.name not in form.fields: - try: - pk_value = self.get_queryset()[index].pk - except IndexError: - pk_value = None + if form.is_bound: + pk_value = form.instance.pk + else: + try: + pk_value = self.get_queryset()[index].pk + except IndexError: + pk_value = None if isinstance(pk, OneToOneField) or isinstance(pk, ForeignKey): qs = pk.rel.to._default_manager.get_query_set() else: diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index 231d4781ce..50bc05eef0 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/tests/regressiontests/admin_views/models.py @@ -326,7 +326,6 @@ class GalleryAdmin(admin.ModelAdmin): class PictureAdmin(admin.ModelAdmin): pass - class Language(models.Model): iso = models.CharField(max_length=5, primary_key=True) name = models.CharField(max_length=50) @@ -401,8 +400,25 @@ class WhatsitInline(admin.StackedInline): class FancyDoodadInline(admin.StackedInline): model = FancyDoodad +class Category(models.Model): + collector = models.ForeignKey(Collector) + order = models.PositiveIntegerField() + + class Meta: + ordering = ('order',) + + def __unicode__(self): + return u'%s:o%s' % (self.id, self.order) + +class CategoryAdmin(admin.ModelAdmin): + list_display = ('id', 'collector', 'order') + list_editable = ('order',) + +class CategoryInline(admin.StackedInline): + model = Category + class CollectorAdmin(admin.ModelAdmin): - inlines = [WidgetInline, DooHickeyInline, GrommetInline, WhatsitInline, FancyDoodadInline] + inlines = [WidgetInline, DooHickeyInline, GrommetInline, WhatsitInline, FancyDoodadInline, CategoryInline] admin.site.register(Article, ArticleAdmin) admin.site.register(CustomArticle, CustomArticleAdmin) @@ -426,6 +442,7 @@ admin.site.register(Language, LanguageAdmin) admin.site.register(Recommendation, RecommendationAdmin) admin.site.register(Recommender) admin.site.register(Collector, CollectorAdmin) +admin.site.register(Category, CategoryAdmin) # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2. # That way we cover all four cases: diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 8e7010be9f..99168fdeee 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -16,7 +16,8 @@ from django.utils.html import escape from models import Article, BarAccount, CustomArticle, EmptyModel, \ ExternalSubscriber, FooAccount, Gallery, ModelWithStringPrimaryKey, \ Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast, \ - Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit + Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit, \ + Category try: set @@ -921,6 +922,45 @@ class AdminViewListEditable(TestCase): self.failUnlessEqual(Person.objects.get(name="John Mauchly").alive, False) + def test_list_editable_ordering(self): + collector = Collector.objects.create(id=1, name="Frederick Clegg") + + Category.objects.create(id=1, order=1, collector=collector) + Category.objects.create(id=2, order=2, collector=collector) + Category.objects.create(id=3, order=0, collector=collector) + Category.objects.create(id=4, order=0, collector=collector) + + # NB: The order values must be changed so that the items are reordered. + data = { + "form-TOTAL_FORMS": "4", + "form-INITIAL_FORMS": "4", + + "form-0-order": "14", + "form-0-id": "1", + "form-0-collector": "1", + + "form-1-order": "13", + "form-1-id": "2", + "form-1-collector": "1", + + "form-2-order": "1", + "form-2-id": "3", + "form-2-collector": "1", + + "form-3-order": "0", + "form-3-id": "4", + "form-3-collector": "1", + } + response = self.client.post('/test_admin/admin/admin_views/category/', data) + # Successful post will redirect + self.failUnlessEqual(response.status_code, 302) + + # Check that the order values have been applied to the right objects + self.failUnlessEqual(Category.objects.get(id=1).order, 14) + self.failUnlessEqual(Category.objects.get(id=2).order, 13) + self.failUnlessEqual(Category.objects.get(id=3).order, 1) + self.failUnlessEqual(Category.objects.get(id=4).order, 0) + class AdminSearchTest(TestCase): fixtures = ['admin-views-users','multiple-child-classes'] @@ -1254,11 +1294,24 @@ class AdminInlineTests(TestCase): "fancydoodad_set-2-owner": "1", "fancydoodad_set-2-name": "", "fancydoodad_set-2-expensive": "on", + + "category_set-TOTAL_FORMS": "3", + "category_set-INITIAL_FORMS": "0", + "category_set-0-order": "", + "category_set-0-id": "", + "category_set-0-collector": "1", + "category_set-1-order": "", + "category_set-1-id": "", + "category_set-1-collector": "1", + "category_set-2-order": "", + "category_set-2-id": "", + "category_set-2-collector": "1", } result = self.client.login(username='super', password='secret') self.failUnlessEqual(result, True) - Collector(pk=1,name='John Fowles').save() + self.collector = Collector(pk=1,name='John Fowles') + self.collector.save() def tearDown(self): self.client.logout() @@ -1419,3 +1472,58 @@ class AdminInlineTests(TestCase): self.failUnlessEqual(response.status_code, 302) self.failUnlessEqual(FancyDoodad.objects.count(), 1) self.failUnlessEqual(FancyDoodad.objects.all()[0].name, "Fancy Doodad 1 Updated") + + def test_ordered_inline(self): + """Check that an inline with an editable ordering fields is + updated correctly. Regression for #10922""" + # Create some objects with an initial ordering + Category.objects.create(id=1, order=1, collector=self.collector) + Category.objects.create(id=2, order=2, collector=self.collector) + Category.objects.create(id=3, order=0, collector=self.collector) + Category.objects.create(id=4, order=0, collector=self.collector) + + # NB: The order values must be changed so that the items are reordered. + self.post_data.update({ + "name": "Frederick Clegg", + + "category_set-TOTAL_FORMS": "7", + "category_set-INITIAL_FORMS": "4", + + "category_set-0-order": "14", + "category_set-0-id": "1", + "category_set-0-collector": "1", + + "category_set-1-order": "13", + "category_set-1-id": "2", + "category_set-1-collector": "1", + + "category_set-2-order": "1", + "category_set-2-id": "3", + "category_set-2-collector": "1", + + "category_set-3-order": "0", + "category_set-3-id": "4", + "category_set-3-collector": "1", + + "category_set-4-order": "", + "category_set-4-id": "", + "category_set-4-collector": "1", + + "category_set-5-order": "", + "category_set-5-id": "", + "category_set-5-collector": "1", + + "category_set-6-order": "", + "category_set-6-id": "", + "category_set-6-collector": "1", + }) + response = self.client.post('/test_admin/admin/admin_views/collector/1/', self.post_data) + # Successful post will redirect + self.failUnlessEqual(response.status_code, 302) + + # Check that the order values have been applied to the right objects + self.failUnlessEqual(self.collector.category_set.count(), 4) + self.failUnlessEqual(Category.objects.get(id=1).order, 14) + self.failUnlessEqual(Category.objects.get(id=2).order, 13) + self.failUnlessEqual(Category.objects.get(id=3).order, 1) + self.failUnlessEqual(Category.objects.get(id=4).order, 0)