Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #10922 -- Corrected handling of POST data to ensure that the ri…

…ght objects are updated on save when the ordering field is editable. Thanks to Alex Gaynor, Karen Tracy, and Will Hardy for their contributions to this patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@11160 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 7ecb8b08b3a2f35aa1f3f1d979fa5a44bf2b8a8c 1 parent 88da053
@freakboy3742 freakboy3742 authored
View
32 django/forms/models.py
@@ -464,8 +464,21 @@ def initial_form_count(self):
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 @@ def save_existing_objects(self, commit=True):
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 @@ def save_existing_objects(self, commit=True):
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 @@ def pk_is_not_editable(pk):
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:
View
21 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 @@ class CollectorAdmin(admin.ModelAdmin):
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:
View
112 tests/regressiontests/admin_views/tests.py
@@ -16,7 +16,8 @@
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 @@ def test_post_submission(self):
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 @@ def setUp(self):
"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 @@ def test_inherited_inline(self):
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)

0 comments on commit 7ecb8b0

Please sign in to comment.
Something went wrong with that request. Please try again.