Ticket #15574: inline_formsets_15574_v2.diff

File inline_formsets_15574_v2.diff, 21.9 KB (added by milosu, 11 years ago)

  • django/forms/formsets.py 

    diff --git a/django/forms/formsets.py b/django/forms/formsets.py
index 1e40a2c..7ed2336 100644
    a b MAX_NUM_FORM_COUNT = 'MAX_NUM_FORMS'  
    1919ORDERING_FIELD_NAME = 'ORDER'
    2020DELETION_FIELD_NAME = 'DELETE'
    2121
     22class BrokenFormError(Exception):
     23    pass
     24
    2225class ManagementForm(Form):
    2326    """
    2427    ``ManagementForm`` is used to keep track of how many form instances
    … … class ManagementForm(Form):  
    3134        self.base_fields[MAX_NUM_FORM_COUNT] = IntegerField(required=False, widget=HiddenInput)
    3235        super(ManagementForm, self).__init__(*args, **kwargs)
    3336
     37    def revalidate(self):
     38        self._errors = None
     39        if not self.is_valid():
     40            raise ValidationError('ManagementForm data is missing or has been tampered with')
     41
    3442class BaseFormSet(StrAndUnicode):
    3543    """
    3644    A collection of instances of the same Form class.
    … … class BaseFormSet(StrAndUnicode):  
    124132    def _construct_forms(self):
    125133        # instantiate all the forms and put them in self.forms
    126134        self.forms = []
    127         for i in xrange(self.total_form_count()):
    128             self.forms.append(self._construct_form(i))
     135        form_index = 0
     136        initial_count = self.initial_form_count()
     137        total_count = self.total_form_count()
     138        orig_total_count = total_count
     139        for i in xrange(orig_total_count):
     140            try:
     141                self.forms.append(self._construct_form(form_index))
     142                form_index += 1
     143            except BrokenFormError:
     144                # wipe out all data of the broken form
     145                key_prefix = '%s-%s' % (self.prefix, str(form_index))
     146                for key in self.data.keys():
     147                    if key.startswith(key_prefix):
     148                        del self.data[key]
     149                # shift the data IDs of the remaining forms in the formset by one
     150                n = 0
     151                start = i + 1
     152                for j in range(start, total_count):
     153                    key_prefix = '%s-%s' % (self.prefix, str(j))
     154                    new_key_prefix = '%s-%s' % (self.prefix, str(form_index + n))
     155                    for key in self.data.keys():
     156                        if key.startswith(key_prefix):
     157                            new_key = key.replace(key_prefix, new_key_prefix)
     158                            self.data[new_key] = self.data[key]
     159                            del self.data[key]
     160                    n += 1
     161                # decrement the number of forms in the formset by one
     162                total_count -= 1
     163                initial_count -= 1
     164                self.data['%s-%s' % (self.prefix, INITIAL_FORM_COUNT)] = initial_count
     165                self.data['%s-%s' % (self.prefix, TOTAL_FORM_COUNT)] = total_count
     166                self.management_form.revalidate()
    129167
    130168    def _construct_form(self, i, **kwargs):
    131169        """

  • django/forms/models.py 

    diff --git a/django/forms/models.py b/django/forms/models.py
index 88a5511..84eb276 100644
    a b from django.core.exceptions import ValidationError, NON_FIELD_ERRORS, FieldError  
    99from django.core.validators import EMPTY_VALUES
    1010from django.forms.fields import Field, ChoiceField
    1111from django.forms.forms import BaseForm, get_declared_fields
    12 from django.forms.formsets import BaseFormSet, formset_factory
     12from django.forms.formsets import BaseFormSet, formset_factory, BrokenFormError
    1313from django.forms.util import ErrorList
    1414from django.forms.widgets import (SelectMultiple, HiddenInput,
    1515    MultipleHiddenInput, media_property)
    … … class BaseModelFormSet(BaseFormSet):  
    434434        self.initial_extra = kwargs.pop('initial', None)
    435435        defaults = {'data': data, 'files': files, 'auto_id': auto_id, 'prefix': prefix}
    436436        defaults.update(kwargs)
     437        self._object_deleted = False
    437438        super(BaseModelFormSet, self).__init__(**defaults)
    438439
    439440    def initial_form_count(self):
    … … class BaseModelFormSet(BaseFormSet):  
    460461            if isinstance(pk, list):
    461462                pk = pk[0]
    462463            kwargs['instance'] = self._existing_object(pk)
     464            if kwargs['instance'] is None:
     465                self._object_deleted = True
     466                raise BrokenFormError()
    463467        if i < self.initial_form_count() and not kwargs.get('instance'):
    464468            kwargs['instance'] = self.get_queryset()[i]
    465469        if i >= self.initial_form_count() and self.initial_extra:
    … … class BaseModelFormSet(BaseFormSet):  
    511515
    512516    def clean(self):
    513517        self.validate_unique()
     518        if self._object_deleted:
     519            raise ValidationError(ugettext(u'Some object on this formset was already deleted by another transaction. Please re-submit the form.'))
    514520
    515521    def validate_unique(self):
    516522        # Collect unique_checks and date_checks to run from all the forms.

  • tests/regressiontests/admin_inlines/models.py 

    diff --git a/tests/regressiontests/admin_inlines/models.py b/tests/regressiontests/admin_inlines/models.py
index f2add00..40d5235 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 029b032..fdf4ec6 100644
    a b from tlp.admin.tests import AdminSeleniumWebDriverTestCase  
    44from tlp.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        self.assertContains(response, 'Please correct the error below.')
     458
     459        total_forms = int(data['thingitem_set-TOTAL_FORMS'][0]) - 1
     460        initial_forms = int(data['thingitem_set-INITIAL_FORMS'][0]) - 1
     461
     462        invalid_form_data = {
     463            'description': data['description'],
     464            'thingitem_set-MAX_NUM_FORMS': [u'0'],
     465            'thingitem_set-TOTAL_FORMS': [str(total_forms)],
     466            'thingitem_set-INITIAL_FORMS': [str(initial_forms)],
     467        }
     468        for key, value in data.iteritems():
     469            current_form_id = key.split('-')
     470            if len(current_form_id) < 3:
     471                continue
     472            current_form_id = int(current_form_id[1])
     473            if deleted_idx == 0 and '-0-' in key:
     474                continue
     475            if deleted_idx == 1 and '-1-' in key:
     476                continue
     477            new_form_id = current_form_id
     478            if current_form_id > 0:
     479                new_form_id -= 1
     480            new_key = key.replace(str(current_form_id), str(new_form_id))
     481            invalid_form_data[new_key] = data[key]
     482
     483        if deleted_idx == 0:
     484            preserved_item_id = self.thing_item_1.pk
     485            deleted_item_id = self.thing_item_0.pk
     486        elif deleted_idx == 1:
     487            preserved_item_id = self.thing_item_0.pk
     488            deleted_item_id = self.thing_item_1.pk
     489
     490        self.assertContains(response, 'name="thingitem_set-TOTAL_FORMS" value="%s"' % total_forms)
     491        self.assertContains(response, 'name="thingitem_set-INITIAL_FORMS" value="%s"' % initial_forms)
     492        self.assertContains(response, '<ul class="errorlist"><li>Some object on this formset was already deleted by another transaction. Please re-submit the form.</li>')
     493
     494        if initial_forms > 0:
     495            self.assertContains(response, 'name="thingitem_set-0-id" value="%s" id="id_thingitem_set-0-id"' % preserved_item_id)
     496        self.assertNotContains(response, '-id" value="%s" id="id_thingitem_set' % deleted_item_id)
     497        self.assertNotContains(response, 'name="thingitem_set-%s-id"' % str(total_forms + 1))
     498
     499        response = self.client.post(reverse(edit_url, args=[self.thing.pk]), invalid_form_data)
     500
     501        # Check our second edit was handled
     502        self.thing = Thing.objects.get(pk=self.thing.pk)
     503        self.assertRedirects(response, reverse(view_url), 302, 200)
     504        self.assertEqual(self.thing.description, data['description'][0])
     505
     506
    203507class TestInlinePermissions(TestCase):
    204508    """
    205509    Make sure the admin respects permissions for objects that are edited
Back to Top