Ticket #15574: inline-formsets-15574.diff

File inline-formsets-15574.diff, 18.9 KB (added by Mathieu Pillard, 12 years ago)

  • AUTHORS 

    diff --git a/AUTHORS b/AUTHORS
index e3aec98..7195154 100644
    a b answer newbie questions, and generally made Django that much better:  
    397397    Todd O'Bryan <toddobryan@mac.com>
    398398    Selwin Ong <selwin@ui.co.id>
    399399    Gerardo Orozco <gerardo.orozco.mosqueda@gmail.com>
     400    Paul Oswald <pauloswald@gmail.com>
    400401    Christian Oudard <christian.oudard@gmail.com>
    401402    oggie rob <oz.robharvey@gmail.com>
    402403    oggy <ognjen.maric@gmail.com>
    … … answer newbie questions, and generally made Django that much better:  
    413414    phil@produxion.net
    414415    phil.h.smith@gmail.com
    415416    Gustavo Picon
     417    Mathieu Pillard <django@virgule.net>
    416418    Michael Placentra II <someone@michaelplacentra2.net>
    417419    plisk
    418420    Daniel Poelzleithner <http://poelzi.org/>

  • django/forms/models.py 

    diff --git a/django/forms/models.py b/django/forms/models.py
index cd8f027..c53a8a1 100644
    a b class BaseModelFormSet(BaseFormSet):  
    421421        self.initial_extra = kwargs.pop('initial', None)
    422422        defaults = {'data': data, 'files': files, 'auto_id': auto_id, 'prefix': prefix}
    423423        defaults.update(kwargs)
     424        self._ignored_forms_count = 0
    424425        super(BaseModelFormSet, self).__init__(**defaults)
    425426
    426427    def initial_form_count(self):
    427428        """Returns the number of forms that are required in this FormSet."""
    428429        if not (self.data or self.files):
    429430            return len(self.get_queryset())
    430         return super(BaseModelFormSet, self).initial_form_count()
     431        # Because the super total_form_count method will use the management_form
     432        # data to return the total, we need to compensate by removing the number
     433        # of ignored forms.
     434        return super(BaseModelFormSet, self).initial_form_count() - self._ignored_forms_count
     435
     436    def total_form_count(self):
     437        # Because the super total_form_count() method will use the management_form
     438        # data to return the total, we need to compensate by removing the number
     439        # of ignored forms.       
     440        return super(BaseModelFormSet, self).total_form_count() - self._ignored_forms_count
    431441
    432442    def _existing_object(self, pk):
    433443        if not hasattr(self, '_object_dict'):
    434444            self._object_dict = dict([(o.pk, o) for o in self.get_queryset()])
    435445        return self._object_dict.get(pk)
    436446
     447    def _construct_forms(self):
     448        self.forms = []
     449        ignored_forms_count = 0
     450        for i in xrange(self.total_form_count()):
     451            try:
     452                form = self._construct_form(i)
     453                self.forms.append(form)
     454            except ValidationError:
     455                ignored_forms_count += 1
     456
     457        # Now that we finished looping over the forms, we can set
     458        # ignored_forms - that will affect all *_form_count() methods,
     459        self._ignored_forms_count = ignored_forms_count
     460
    437461    def _construct_form(self, i, **kwargs):
    438462        if self.is_bound and i < self.initial_form_count():
    439463            # Import goes here instead of module-level because importing
    … … class BaseModelFormSet(BaseFormSet):  
    448472                pk = pk[0]
    449473            kwargs['instance'] = self._existing_object(pk)
    450474        if i < self.initial_form_count() and not kwargs.get('instance'):
    451             kwargs['instance'] = self.get_queryset()[i]
     475            if not self.is_bound:
     476                kwargs['instance'] = self.get_queryset()[i]
     477            else:
     478                # This shouldn't happen: bound forms, because they contain the data,
     479                # should already have a pk and therefore an instance.
     480                # This means something is wrong, either the form was tampered with
     481                # or the object has been deleted since. We can't build the form
     482                # with the data we have, so immediately raise an error
     483                raise ValidationError('Invalid pk when constructing form from data')
    452484        if i >= self.initial_form_count() and self.initial_extra:
    453485            # Set initial values for extra forms
    454486            try:

  • tests/regressiontests/admin_inlines/models.py 

    diff --git a/tests/regressiontests/admin_inlines/models.py b/tests/regressiontests/admin_inlines/models.py
index f2add00..91f17f5 100644
    a b Testing of admin inline formsets.  
    55from django.db import models
    66from django.contrib.contenttypes.models import ContentType
    77from django.contrib.contenttypes import generic
     8from django.contrib import admin
    89
    910
    1011class Parent(models.Model):
    … … class Holder2(models.Model):  
    5657    dummy = models.IntegerField()
    5758
    5859
     60# Objects for bug #15574 Test
     61class Thing(models.Model):
     62    description = models.CharField(max_length=256)
     63
     64class ThingItem(models.Model):
     65    description = models.CharField(max_length=256)
     66    thing = models.ForeignKey(Thing)
     67
     68class ThingItemInline(admin.TabularInline):
     69    model = ThingItem
     70    extra = 0
     71
     72class ThingAdmin(admin.ModelAdmin):
     73    inlines = [ThingItemInline,]
     74
     75admin.site.register(Thing, ThingAdmin)
     76
     77
    5978class Inner2(models.Model):
    6079    dummy = models.IntegerField()
    6180    holder = models.ForeignKey(Holder2)

  • tests/regressiontests/admin_inlines/tests.py 

    diff --git a/tests/regressiontests/admin_inlines/tests.py b/tests/regressiontests/admin_inlines/tests.py
index 8b620cc..296a8f4 100644
    a b from django.contrib.admin.tests import AdminSeleniumWebDriverTestCase  
    44from django.contrib.admin.helpers import InlineAdminForm
    55from django.contrib.auth.models import User, Permission
    66from django.contrib.contenttypes.models import ContentType
     7from django.core.urlresolvers import reverse
    78from django.test import TestCase
    89
    910# local test models
    1011from .admin import InnerInline
    1112from .models import (Holder, Inner, Holder2, Inner2, Holder3, Inner3, Person,
    1213    OutfitItem, Fashionista, Teacher, Parent, Child, Author, Book, Profile,
    13     ProfileCollection)
     14    ProfileCollection, Thing, ThingItem)
    1415
    1516
    1617class TestInline(TestCase):
    … … class TestInlineAdminForm(TestCase):  
    200201        parent_ct = ContentType.objects.get_for_model(Parent)
    201202        self.assertEqual(iaf.original.content_type, parent_ct)
    202203
     204class TestEditingInlineViews(TestCase):
     205    """
     206    Make sure that concurrent editing does not result in an error for the user
     207
     208    Refs #15574
     209
     210    These tests simulate two test clients by submitting slightly different data
     211    twice. This would be equivalent to working with two tabs open in a browser,
     212    or two different users working separately. Individually each of these forms
     213    is valid but the second one is invalid when submitted one after the other.
     214    """   
     215    fixtures = ['admin-views-users.xml']
     216
     217    def setUp(self):
     218        result = self.client.login(username='super', password='secret')
     219        self.failUnlessEqual(result, True)
     220        # Set up a thing to be modified in the test
     221        self.thing = Thing.objects.create(description="Parent object")
     222        self.thing_item_0 = ThingItem.objects.create(description="Item #0", thing=self.thing)
     223        self.thing_item_1 = ThingItem.objects.create(description="Item #1", thing=self.thing)
     224
     225    def tearDown(self):
     226        self.client.logout()
     227        self.thing_item_0.delete()
     228        self.thing_item_1.delete()
     229        self.thing.delete()
     230
     231    def test_concurrent_editing_first(self):
     232        """
     233        Test concurrent editing when a user deletes the first inline in a list;
     234        second user is simply editing the same Thing object.
     235        """
     236        data = {
     237            'description': [u'A new description #2'],
     238            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     239            'thingitem_set-TOTAL_FORMS': [u'2'],
     240            'thingitem_set-INITIAL_FORMS': [u'2'],
     241            'thingitem_set-0-DELETE': [u''],
     242            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     243            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     244            'thingitem_set-0-description': [u'New item #0 description'],
     245            'thingitem_set-1-DELETE': [u''],
     246            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     247            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     248            'thingitem_set-1-description': [u'Deleted item #1 description'],
     249        }
     250        self._do_test_concurrent_editing_views(deleted_idx=0, data=data)
     251        self.assertEqual(self.thing.thingitem_set.count(), 1)
     252
     253    def test_concurrent_editing_first_delete_twice(self):
     254        """
     255        Test concurrent editing when a user deletes the first inline in a list;
     256        second user is also deleting the same inline.
     257        """
     258        data = {
     259            'description': [u'A new description #3'],
     260            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     261            'thingitem_set-TOTAL_FORMS': [u'2'],
     262            'thingitem_set-INITIAL_FORMS': [u'2'],
     263            'thingitem_set-0-DELETE': [u'on'],
     264            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     265            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     266            'thingitem_set-0-description': [u'New item #0 description'],
     267            'thingitem_set-1-DELETE': [u''],
     268            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     269            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     270            'thingitem_set-1-description': [u'Deleted item #1 description'],
     271        }
     272        self._do_test_concurrent_editing_views(deleted_idx=0, data=data)
     273        self.assertEqual(self.thing.thingitem_set.count(), 1)
     274
     275    def test_concurrent_editing_first_add_another(self):
     276        """
     277        Test concurrent editing when a user deletes the first inline in a list;
     278        second user is adding another inline on the same change_view.
     279        """
     280        data = {
     281            'description': [u'A new description #3'],
     282            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     283            'thingitem_set-TOTAL_FORMS': [u'2'],
     284            'thingitem_set-INITIAL_FORMS': [u'2'],
     285            'thingitem_set-0-DELETE': [u'on'],
     286            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     287            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     288            'thingitem_set-0-description': [u'New item #0 description'],
     289            'thingitem_set-1-DELETE': [u'on'],
     290            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     291            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     292            'thingitem_set-1-description': [u'Deleted item #1 description'],
     293        }
     294        self._do_test_concurrent_editing_views(deleted_idx=0, data=data)
     295        self.assertEqual(self.thing.thingitem_set.count(), 0)
     296
     297    def test_concurrent_editing_first_delete_different(self):
     298        """
     299        Test concurrent editing when a user deletes the first inline in a list;
     300        second user is also deleting another inline on the same change_view.
     301        """
     302        data = {
     303            'description': [u'A new description #3'],
     304            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     305            'thingitem_set-TOTAL_FORMS': [u'3'],
     306            'thingitem_set-INITIAL_FORMS': [u'2'],
     307            'thingitem_set-0-DELETE': [u''],
     308            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     309            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     310            'thingitem_set-0-description': [u'New item #0 description'],
     311            'thingitem_set-1-DELETE': [u''],
     312            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     313            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     314            'thingitem_set-1-description': [u'Deleted item #1 description'],
     315            'thingitem_set-2-DELETE': [u''],
     316            'thingitem_set-2-id': [u''],
     317            'thingitem_set-2-thing': [u'%s' % (self.thing.pk,)],
     318            'thingitem_set-2-description': [u'New item #2 description'],
     319        }
     320        self._do_test_concurrent_editing_views(deleted_idx=0, data=data)
     321        self.assertEqual(self.thing.thingitem_set.count(), 2)
     322
     323    def test_concurrent_editing_last(self):
     324        """
     325        Test concurrent editing when a user deletes the last inline in a list;
     326        second user is simply editing the same Thing object.
     327        """
     328        data = {
     329            'description': [u'A new description #2'],
     330            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     331            'thingitem_set-TOTAL_FORMS': [u'2'],
     332            'thingitem_set-INITIAL_FORMS': [u'2'],
     333            'thingitem_set-0-DELETE': [u''],
     334            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     335            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     336            'thingitem_set-0-description': [u'New item #0 description'],
     337            'thingitem_set-1-DELETE': [u''],
     338            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     339            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     340            'thingitem_set-1-description': [u'Deleted item #1 description'],
     341        }
     342        self._do_test_concurrent_editing_views(deleted_idx=1, data=data)
     343        self.assertEqual(self.thing.thingitem_set.count(), 1)
     344
     345    def test_concurrent_editing_last_delete_twice(self):
     346        """
     347        Test concurrent editing when a user deletes the last inline in a list;
     348        second user is also deleting the same inline.
     349        """
     350        data = {
     351            'description': [u'A new description #3'],
     352            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     353            'thingitem_set-TOTAL_FORMS': [u'2'],
     354            'thingitem_set-INITIAL_FORMS': [u'2'],
     355            'thingitem_set-0-DELETE': [u''],
     356            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     357            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     358            'thingitem_set-0-description': [u'New item #0 description'],
     359            'thingitem_set-1-DELETE': [u'on'],
     360            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     361            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     362            'thingitem_set-1-description': [u'Deleted item #1 description'],
     363        }
     364        self._do_test_concurrent_editing_views(deleted_idx=1, data=data)
     365        self.assertEqual(self.thing.thingitem_set.count(), 1)
     366
     367    def test_concurrent_editing_last_add_another(self):
     368        """
     369        Test concurrent editing when a user deletes the last inline in a list;
     370        second user is adding another inline on the same change_view.
     371        """
     372        data = {
     373            'description': [u'A new description #3'],
     374            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     375            'thingitem_set-TOTAL_FORMS': [u'2'],
     376            'thingitem_set-INITIAL_FORMS': [u'2'],
     377            'thingitem_set-0-DELETE': [u'on'],
     378            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     379            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     380            'thingitem_set-0-description': [u'New item #0 description'],
     381            'thingitem_set-1-DELETE': [u'on'],
     382            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     383            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     384            'thingitem_set-1-description': [u'Deleted item #1 description'],
     385        }
     386        self._do_test_concurrent_editing_views(deleted_idx=1, data=data)
     387        self.assertEqual(self.thing.thingitem_set.count(), 0)
     388
     389    def test_concurrent_editing_last_delete_different(self):
     390        """
     391        Test concurrent editing when a user deletes the last inline in a list;
     392        second user is also deleting another inline on the same change_view.
     393        """
     394        data = {
     395            'description': [u'A new description #3'],
     396            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     397            'thingitem_set-TOTAL_FORMS': [u'3'],
     398            'thingitem_set-INITIAL_FORMS': [u'2'],
     399            'thingitem_set-0-DELETE': [u''],
     400            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     401            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     402            'thingitem_set-0-description': [u'New item #0 description'],
     403            'thingitem_set-1-DELETE': [u''],
     404            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     405            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     406            'thingitem_set-1-description': [u'Deleted item #1 description'],
     407            'thingitem_set-2-DELETE': [u''],
     408            'thingitem_set-2-id': [u''],
     409            'thingitem_set-2-thing': [u'%s' % (self.thing.pk,)],
     410            'thingitem_set-2-description': [u'New item #2 description'],
     411        }
     412        self._do_test_concurrent_editing_views(deleted_idx=1, data=data)
     413        self.assertEqual(self.thing.thingitem_set.count(), 2)
     414
     415    def _do_test_concurrent_editing_views(self, deleted_idx=0, data=None):
     416        thingitem_set_0_DELETE = u''
     417        thingitem_set_1_DELETE = u''
     418
     419        if deleted_idx == 0:
     420            thingitem_set_0_DELETE = u'on'
     421        elif deleted_idx == 1:
     422            thingitem_set_1_DELETE = u'on'
     423
     424        first_form_data = {
     425            'description': [u'A new description'],
     426            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     427            'thingitem_set-TOTAL_FORMS': [u'2'],
     428            'thingitem_set-INITIAL_FORMS': [u'2'],
     429            'thingitem_set-0-DELETE': [thingitem_set_0_DELETE],
     430            'thingitem_set-0-id': [u'%s' % (self.thing_item_0.pk,)],
     431            'thingitem_set-0-thing': [u'%s' % (self.thing.pk,)],
     432            'thingitem_set-0-description': [u'New item #0 description'],
     433            'thingitem_set-1-DELETE': [thingitem_set_1_DELETE],
     434            'thingitem_set-1-id': [u'%s' % (self.thing_item_1.pk,)],
     435            'thingitem_set-1-thing': [u'%s' % (self.thing.pk,)],
     436            'thingitem_set-1-description': [u'Deleted item #1 description'],
     437        }
     438        edit_url = 'admin:%s_%s_change' %(self.thing._meta.app_label,  self.thing._meta.module_name)
     439        view_url = 'admin:%s_%s_changelist' %(self.thing._meta.app_label,  self.thing._meta.module_name)
     440
     441        # Sanity check
     442        self.assertEqual(self.thing.thingitem_set.count(), 2)
     443
     444        # Submit first form
     445        response = self.client.post(reverse(edit_url, args=[self.thing.pk]), first_form_data)
     446
     447        # Check our first edit was accepted
     448        self.thing = Thing.objects.get(pk=self.thing.pk)
     449        self.assertRedirects(response, reverse(view_url), 302, 200)
     450        self.assertEqual(self.thing.thingitem_set.count(), 1)
     451        self.assertEqual(self.thing.description, first_form_data['description'][0])
     452
     453        # Submit the second form (this represents either the same user submitting
     454        # another tab that he had loaded before, or a second user).
     455        response = self.client.post(reverse(edit_url, args=[self.thing.pk]), data)
     456
     457        # Check our second edit was handled
     458        self.thing = Thing.objects.get(pk=self.thing.pk)
     459        self.assertRedirects(response, reverse(view_url), 302, 200)
     460        self.assertEqual(self.thing.description, data['description'][0])
     461
     462
    203463class TestInlinePermissions(TestCase):
    204464    """
    205465    Make sure the admin respects permissions for objects that are edited
Back to Top