Opened 14 years ago

Last modified 9 years ago

#15574 new Bug

IndexError: list index out of range caused by inline formsets

Reported by: Paul Oswald Owned by: nobody
Component: Forms Version: dev
Severity: Normal Keywords: inline formset
Cc: Patrick Kranzlmueller, Paul Oswald, Alan Justino da Silva, django@…, nik@…, joseph@…, cmawebsite@…, milos.urbanek@…, carsten.fuchs@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Note: I originally reported this as ticket #14642 but after discussion on the mailing list it has been decided that they are perhaps different issues. See http://groups.google.com/group/django-developers/browse_thread/thread/8c66461f1712dbb9

I was getting this error in my production system and I thought it was my forms. Turns out I could eliminate my forms as a source of the issue by using the admin. I've changed the summary to match. I reduced it down to this test case in Django 1.2.x:

models.py:

class Thingy(models.Model):
    description = models.CharField(max_length=256)

class ThingyItem(models.Model):
    thingy = models.ForeignKey(Thingy)
    description = models.CharField(max_length=256)

admin.py:

class ThingyItemInline(admin.TabularInline):
    model = ThingyItem
    extra = 0

class ThingyAdmin(admin.ModelAdmin):
    inlines = [ThingyItemInline,]

admin.site.register(Thingy, ThingyAdmin)
admin.site.register(ThingyItem)

Now do the following:

  • Create a new Thingy with several ThingyItems? in the admin and save it.
  • Open the edit page.
  • Open the edit page for the same thingy in a second browser window.
  • Check the "Delete" button on the last ThingyItem? and save it in the second window.
  • Now go back to the first form and save it

When I do this, I get:

Traceback (most recent call last):
  File "/Users/poswald/Projects/django/tests/regressiontests/admin_inlines/tests.py", line 211, in test_concurrent_editing_views
    response = self.client.post(reverse(edit_url, args=[self.thing.pk]), data[1])
  File "/Users/poswald/Projects/django/django/test/client.py", line 455, in post
    response = super(Client, self).post(path, data=data, content_type=content_type, **extra)
  File "/Users/poswald/Projects/django/django/test/client.py", line 256, in post
    return self.request(**r)
  File "/Users/poswald/Projects/django/django/core/handlers/base.py", line 111, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "/Users/poswald/Projects/django/django/contrib/admin/options.py", line 308, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/Users/poswald/Projects/django/django/utils/decorators.py", line 93, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/poswald/Projects/django/django/views/decorators/cache.py", line 79, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/Users/poswald/Projects/django/django/contrib/admin/sites.py", line 190, in inner
    return view(request, *args, **kwargs)
  File "/Users/poswald/Projects/django/django/utils/decorators.py", line 28, in _wrapper
    return bound_func(*args, **kwargs)
  File "/Users/poswald/Projects/django/django/utils/decorators.py", line 93, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/poswald/Projects/django/django/utils/decorators.py", line 24, in bound_func
    return func(self, *args2, **kwargs2)
  File "/Users/poswald/Projects/django/django/db/transaction.py", line 217, in inner
    res = func(*args, **kwargs)
  File "/Users/poswald/Projects/django/django/contrib/admin/options.py", line 978, in change_view
    queryset=inline.queryset(request))
  File "/Users/poswald/Projects/django/django/forms/models.py", line 680, in __init__
    queryset=qs)
  File "/Users/poswald/Projects/django/django/forms/models.py", line 416, in __init__
    super(BaseModelFormSet, self).__init__(**defaults)
  File "/Users/poswald/Projects/django/django/forms/formsets.py", line 47, in __init__
    self._construct_forms()
  File "/Users/poswald/Projects/django/django/forms/formsets.py", line 108, in _construct_forms
    self.forms.append(self._construct_form(i))
  File "/Users/poswald/Projects/django/django/forms/models.py", line 689, in _construct_form
    form = super(BaseInlineFormSet, self)._construct_form(i, **kwargs)
  File "/Users/poswald/Projects/django/django/forms/models.py", line 440, in _construct_form
    kwargs['instance'] = self.get_queryset()[i]
  File "/Users/poswald/Projects/django/django/db/models/query.py", line 173, in __getitem__
    return self._result_cache[k]
IndexError: list index out of range

It seems that the inline formset code is a bit brittle. An error is thrown when the data submitted in the management form is not in agreement with the state of the database. I'd like to make an automated test case for this but I'm not sure how to do that sequence of steps in the test client. If anyone can shed some light onto what the formset code is *supposed* to do in this situation it would be useful. I imagine the options are the second form overrides the first or something like a form validation error is raised that indicates the form is stale.

Attachments (3)

inline-formset-test.diff (6.1 KB ) - added by Paul Oswald 14 years ago.
inline-formsets-15574.diff (18.9 KB ) - added by Mathieu Pillard 13 years ago.
inline_formsets_15574_v2.diff (21.9 KB ) - added by milosu 10 years ago.

Download all attachments as: .zip

Change History (40)

by Paul Oswald, 14 years ago

Attachment: inline-formset-test.diff added

comment:1 by Ramiro Morales, 14 years ago

Triage Stage: UnreviewedAccepted

Discussion in #11313 (list_editable is also implemented with model formsets) and the report of the OP about observing the same problems with custom non-admin formsets seem to point this multiple simultaneous edition safety problem is a limitation of Django model formsets.

comment:2 by Luke Plant, 14 years ago

Type: Bug

comment:3 by Luke Plant, 14 years ago

Severity: Normal

comment:4 by Paul Oswald, 14 years ago

Easy pickings: unset
milestone: 1.4
Triage Stage: AcceptedDesign decision needed

Setting to 'design decision needed'. I would appreciate if someone would explain what the formset code is supposed to do in this situation.

comment:5 by Jacob, 13 years ago

milestone: 1.4

Milestone 1.4 deleted

comment:6 by Patrick Kranzlmueller, 13 years ago

Cc: Patrick Kranzlmueller added
UI/UX: unset

comment:7 by Paul Oswald, 13 years ago

Cc: Paul Oswald added

comment:8 by Alan Justino da Silva, 13 years ago

Cc: Alan Justino da Silva added

comment:9 by Mathieu Pillard, 13 years ago

Cc: django@… added
Triage Stage: Design decision neededUnreviewed
Version: 1.3-betaSVN

I've been bitten by this in production, did some digging and have a possible solution.

This problem has several variants:

  • If there are multiple relations and one of the relation other than the last one is deleted outside the current change view, you will get a "Please correct the error below" message... without showing where the error occured, because it's happening on a pk field, which is hidden.
  • If there is only one relation or there are multiple relations but only the last one is deleted, you'll get the IndexError
  • In addition, if you try to delete an already deleted relation, you'll get a ValidatorError, which isn't caught because it doesn't happen inside a form's clean() method, and will generate a 500 error.

Since the user has no way to correct/avoid this himself, I think the correct answer
to this problem is to simply ignore the forms in question. The worst thing that could
happen is that he'll lose any modification made to the concerned form(s), but that's
better than losing all his work on the page itself.

I'm attaching a patch that does exactly that, with a lot of tests to cover the different
variants. The heart of the problem lies in the fact that django BaseModelFormSet tries to find
an instance for each form in the formset even if it doesn't make sense sometimes.
Because ModelForms will consider a None instance as a request for creation, the
best way to handle this, IMHO, is to bail early and remember how many forms are
problematic.

Edit: Note that my patch is built on top of attachment:inline-formset-test.diff and therefore probably makes it obsolete.

Last edited 13 years ago by Mathieu Pillard (previous) (diff)

by Mathieu Pillard, 13 years ago

Attachment: inline-formsets-15574.diff added

comment:10 by Mathieu Pillard, 13 years ago

Has patch: set

comment:11 by Anssi Kääriäinen, 13 years ago

I don't think it is a good idea to just throw the deleted model's form data out and save the rest as if nothing went wrong. A concurrent delete should lead to validation error of "It seems B was deleted concurrently, verify changes and try again.". (Or just "Concurrent modification. Please verify changes and save again").

comment:12 by Mathieu Pillard, 13 years ago

The problem is, AFAIK django doesn't have any mechanism to deal with this kind of errors. Showing an error to the user is possible, but he won't be able to correct the problem, since the simple presence of the deleted inline in data will trigger this issue. He has no way of forcing django to ignore this form (which he may or may not have touched at all in the first place)

comment:13 by Anssi Kääriäinen, 13 years ago

Triage Stage: UnreviewedDesign decision needed

So, lets add some mechanism for that :)

I really think that skipping part of the submitted data and claiming that everything went well is plain evil. Even a 500 error is better than doing a partial save without any notification.

Is it possible to just raise a ValidationError("Concurrent Modification. Verify changes."), show the remaining forms for the user (remove the deleted ones), and let the user decide what to do. That is, make it explicit to the user what is saved and what is not saved.

It would of course be better to show the deleted forms and give the user the ability to resurrect them, but that will be more complicated to implement.

I am putting this back to DDN triage stage, as it seems pretty clear some design decision is needed.

comment:14 by Marco Bazzani, 12 years ago

Patch doesn't apply on 1.3.1 is it for 1.4 ?

comment:15 by anonymous, 12 years ago

visik7: my patch was made for trunk, but it shouldn't be hard to adapt to 1.3.1.

Note to those who want to use my patch as a base: there is a case that doesn't work: if the user, when using the django admin with the patch, submits a form a deleted relation but also containing another error, he'll get the form again to get a chance to correct the error, but the formset management form will output TOTAL_FORMS/INITIAL_FORMS inconsistent with the number of forms, and therefore the user won't be able to submit the form.

comment:16 by lk <lara.khoury@…>, 12 years ago

I am getting the following: "list index out of range" exception when updating object with inline formset

My code is as follows:

class ProjectUpdateView(UpdateView):
form_class = ProjectForm
template_name = 'projects/project_create.html'
success_url = reverse_lazy('projects-list')

def get_object(self, queryset=None):
    obj = Project.objects.get(id=self.kwargs['pk'])
    return obj

def get_context_data(self, **kwargs):
    context = super(ProjectUpdateView, self).get_context_data(**kwargs)
    project = Project.objects.get(id=self.kwargs['pk'])

    if self.request.POST:
        context['reward_formset'] = ProjectRewardFormSet(self.request.POST, self.request.FILES, instance=project)  
    else:
        context['reward_formset'] = ProjectRewardFormSet(instance=project)
    return context

def form_valid(self, form):
    context = self.get_context_data()
    reward_formset = context['reward_formset']
    if reward_formset.is_valid():
        self.object = form.save(commit=False)
        self.object.owner = self.request.user
        self.object.save()
        reward_formset.instance = self.object
        reward_formset.save()
        return HttpResponseRedirect(self.get_success_url())
    else:
        return self.render_to_response(self.get_context_data(form=form)) 

And the traceback as follows:

/home/lk/.venv/repo/lib/python2.7/site-packages/django/core/handlers/base.py in get_response
111.                         response = callback(request, *callback_args, **callback_kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/contrib/auth/decorators.py in _wrapped_view
20.                 return view_func(request, *args, **kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/views/generic/base.py in view
48.             return self.dispatch(request, *args, **kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/views/generic/base.py in dispatch
69.         return handler(request, *args, **kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/views/generic/edit.py in post
172.         return super(BaseCreateView, self).post(request, *args, **kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/views/generic/edit.py in post
138.             return self.form_valid(form)
...
▶ Local vars
/home/lk/.venv/repo/repo/../repo/projects/views.py in form_valid
22.         context = self.get_context_data()
...
▶ Local vars
/home/lk/.venv/repo/repo/../repo/projects/views.py in get_context_data
39.             context['reward_formset'] = ProjectRewardFormSet(self.request.POST)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/forms/models.py in __init__
697.                                                 queryset=qs, **kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/forms/models.py in __init__
424.         super(BaseModelFormSet, self).__init__(**defaults)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/forms/formsets.py in __init__
50.         self._construct_forms()
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/forms/formsets.py in _construct_forms
115.             self.forms.append(self._construct_form(i))
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/forms/models.py in _construct_form
706.         form = super(BaseInlineFormSet, self)._construct_form(i, **kwargs)
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/forms/models.py in _construct_form
451.             kwargs['instance'] = self.get_queryset()[i]
...
▶ Local vars
/home/lk/.venv/repo/lib/python2.7/site-packages/django/db/models/query.py in __getitem__
190.             return self._result_cache[k]
...
▶ Local vars

I have read the following issue but I am not able to figure out what I should change exactly in my code! I have been facing since exception since days and I am really stuck! I would appreciate a quick and detailed reply! Many thanks in advance!

comment:17 by Karen Tracey, 12 years ago

The initial code you include doesn't match the code in your traceback. Traceback includes:

▶ Local vars
/home/lk/.venv/repo/repo/../repo/projects/views.py in get_context_data
39.             context['reward_formset'] = ProjectRewardFormSet(self.request.POST)
...

which is not passing an instance in with the post data. That's likely a problem.

comment:18 by lk <lara.khoury@…>, 12 years ago

That is so weird!!! I just noticed that it always goes to the CreateView not the UpdateView even though I am redirecting the url to the UpdateView

class ProjectCreateView(CreateView):
    form_class = ProjectForm
    template_name = 'projects/project_create.html'
    success_url = reverse_lazy('projects-list')

    def form_valid(self, form):
        context = self.get_context_data()
        reward_formset = context['reward_formset']
        if reward_formset.is_valid():
            self.object = form.save(commit=False)
            self.object.owner = self.request.user
            self.object.save()
            reward_formset.instance = self.object
            reward_formset.save()
            action.send(self.request.user, verb='created', target=self.object)
            return HttpResponseRedirect(self.get_success_url())
        else:
            return self.render_to_response(self.get_context_data(form=form))

    def get_context_data(self, **kwargs):
        context = super(ProjectCreateView, self).get_context_data(**kwargs)

        if self.request.POST:
            context['reward_formset'] = ProjectRewardFormSet(self.request.POST)
        else:
            context['reward_formset'] = ProjectRewardFormSet()

        return context




class ProjectUpdateView(UpdateView):
    form_class = ProjectForm
    template_name = 'projects/project_create.html'
    success_url = reverse_lazy('projects-list')    

    def form_valid(self, form):
        context = self.get_context_data()
        reward_formset = context['reward_formset']
        if reward_formset.is_valid():
            self.object = form.save(commit=False)
            self.object.owner = self.request.user
            self.object.save()
            reward_formset.instance = self.object
            reward_formset.save()
            return HttpResponseRedirect(self.get_success_url())
        else:
            return self.render_to_response(self.get_context_data(form=form)) 

    def get_object(self, queryset=None):
        obj = Project.objects.get(id=self.kwargs['pk'])
        return obj

    def get_context_data(self, **kwargs):
        context = super(ProjectUpdateView, self).get_context_data(**kwargs)
        project = Project.objects.get(id=self.kwargs['pk'])

        if self.request.POST:
            context['reward_formset'] = ProjectRewardFormSet(self.request.POST, self.request.FILES, instance=project)  
        else:
            context['reward_formset'] = ProjectRewardFormSet(instance=project)
        return context    

Have anyone faced this before?

comment:19 by Karen Tracey, 12 years ago

You'll likely have better luck getting a response posting on django-users or #django IRC, this is now veering pretty far off the bug that is this ticket.

comment:20 by chriscog, 12 years ago

Just ran into this issue myself, and reported in #19408. I get the "IndexError" if I was to simultaneously delete the last item in a formset, but get an uncaught ValidationError exception - uncaught, in that it returns a 500 code rather than a redbox in the form, if I simultaneously delete middle items.

I agree that there's no real good solution, here.

Clearly if there's simultaneous deletes, then "WELL, NOT A PROBLEM". However, if one form attempts a delete, and the other an edit, then it's not so clear. The best solution would be to come up with some kind of validation error for the subsequent attempt, but what, exactly, to display is the question. However, given that there is currently NO checking for simultaneous edits. (the 2nd submitted edits will override the first), I think its safe to say that we don't have to be "Perfect" here... if someone wants perfection, then they're going to have to arrange some kind of funky locking themselves, and Django should be relied on to just "do something that makes sense".

So, in that vein, here's my proposal:

  1. No matter what, Django should cope with the case that a row disappears, so the "IndexError" should never happen.
  1. A delete followed by a re-delete on the same form results in a delete with no warning.
  1. A delete followed by an edit results in a validation error for the second action, the edit. However, since the row is _gone_, the error is an error on the form set saying that "<Object x> has been recently deleted, and your edits have been discarded". Simply re-submitting the form will now work.
  1. An edit followed by a delete (if this can even be detected) results in a validation error on the form being deleted. The form shows the post-edit values. The message is "<Object x> has recently been changed. You may attempt to re-delete it."
  1. An edit followed by an edit (again, if this is even detected) will just save the new edits. Given the "absolute best option" would be to somehow show the newly-changed values while keeping the user's edits so he can validate it is a very tall order, I would just fall back on saving the 2nd set of edits, and making this behaviour well documented.

comment:21 by dangtrinhnt@…, 12 years ago

Has patch: unset

Hi everyone,

I got the same problem with Django 1.1. But, the real problem is my app'd worked well before I added 3 foreignkey field to the model. My code as follow:

models.py:

class PtcEvent(models.Model):
	name = models.CharField(max_length=50)
	desc = models.TextField(verbose_name='description')
	grades = models.ManyToManyField(Grade)
	view_start = models.DateTimeField(default=NOW)
	view_end = models.DateTimeField(default=NOW + timedelta(days=30))
	edit_start = models.DateTimeField(default=NOW)
	edit_end = models.DateTimeField(default=NOW + timedelta(days=25))
	
	active = models.BooleanField(default=True)
	granularity = models.SmallIntegerField(default=30)
	# buffer minutes will simply display the booking time length shorter: granularity - buffer_minutes
	buffer_minutes = models.SmallIntegerField(default=5)

        # the new 3 fields
	default_notification_mail_body = models.ForeignKey(EmailTemplate, null=True, \
					related_name='notification_mail_template')
	default_teacher_confirmation_mail_body = models.ForeignKey(EmailTemplate, null=True, \
					related_name='teacher_confirmation_mail_template')
	default_parent_confirmation_mail_body = models.ForeignKey(EmailTemplate, null=True, \
					related_name='parent_confirmation_mail_template') 

	def __unicode__(self):
		return u'%s (%s)' % (self.name, self.view_start.strftime(DATE_FORMAT))
	
	def get_status(self):
		now = datetime.now()
		if self.view_start <= now and now < self.view_end:
			if self.edit_start <= now and now < self.edit_end:
				return EVENT_LIVE
			else:
				return EVENT_VIEW
		return EVENT_CLOSED
	
	def get_status_text(self):
		return EVENT_STATUS_TEXT[self.get_status()]
	
	class Meta:
		ordering = ['-view_start','name']
		verbose_name = "PTC Event"



class PtcPeriod(models.Model):
    start_date = models.DateField(default=date.today() + timedelta(days=30))
    start_time = models.TimeField(default=time(8,0))
    end_time = models.TimeField(default=time(12,0))
    teacher_blocks = models.SmallIntegerField(default=1)
    ptc_event = models.ForeignKey(PtcEvent)

    def __unicode__(self):
        return u'%s [%s, %s-%s]' % (self.ptc_event.name, self.start_date.strftime(DATE_FORMAT), self.start_time.strftime(TIME_FORMAT), self.end_time.strftime(TIME_FORMAT))

    def start(self):
        return datetime.combine(self.start_date, self.start_time)

    def end(self):
        return datetime.combine(self.start_date, self.end_time)

    def slot_list(self):
        result = []
        step = timedelta(minutes=self.ptc_event.granularity)
        start_time = self.start()
        end_time = self.end()

        cur_time = start_time
        while (cur_time < end_time):
            result.append(cur_time)
            cur_time = cur_time + step

        return result

    def slot_list_text(self):
        result = [ v.strftime(DATETIME_FORMAT) for v in self.slot_list()]
        return result


    class Meta:
        ordering = ['-start_date']
        verbose_name = "PTC Period"

forms.py:

class PtcEventForm(ModelForm):
	# view_start and edit_start = datetime when the ptcevent is created
	view_start = DateTimeField(widget=SplitSelectDateTimeWidget(hour_step=1, \
					minute_step=15, second_step=60, twelve_hr=True))
	view_end = DateTimeField(widget=SplitSelectDateTimeWidget(hour_step=1, \
					minute_step=15, second_step=60, twelve_hr=True))
	edit_start = DateTimeField(widget=SplitSelectDateTimeWidget(hour_step=1, \
					minute_step=15, second_step=60, twelve_hr=True))
	edit_end = DateTimeField(widget=SplitSelectDateTimeWidget(hour_step=1, \
					minute_step=15, second_step=60, twelve_hr=True))
	grades = ModelMultipleChoiceField(widget=ColumnCheckboxSelectMultiple(), required=True, queryset=Grade.objects.all())
		
	class Meta:
		model = PtcEvent
		exclude = ('active', 'granularity', 'buffer_minutes',)

PtcPeriodFormSetBase_edit = inlineformset_factory(PtcEvent, PtcPeriod, extra=0, max_num=10, can_delete=True)

class PtcPeriodFormSet_edit(PtcPeriodFormSetBase_edit):
	def add_fields(self, form, index):
		super(PtcPeriodFormSet_edit, self).add_fields(form, index)
		form.fields['start_date'].widget = SelectDateWidget()
		form.fields['start_time'].widget = SelectTimeWidget(hour_step=1, \
					minute_step=15, second_step=60, twelve_hr=True)
		form.fields['end_time'].widget = SelectTimeWidget(hour_step=1, \
					minute_step=15, second_step=60, twelve_hr=True)
		form.fields['teacher_blocks'].widget.attrs['size'] = 2

views.py:

def edit_ptc_event(request, ptcevent_id):
	current_selected_event_id = get_current_selected_event_id(request)
	all_ptc_events = get_all_active_events()
	staff = request.user
	page_title = "Edit PTC Event"
	if request.method == 'POST':
		action = request.POST.get('action')
		ptc_event_form = PtcEventForm(request.POST, request.FILES)
		if ptc_event_form.is_valid():
			if action == u'save':
				ptc_event = ptc_event_form.save(commit=False)
				log_info("ptc event edit %s" % str(ptc_event))
				ptc_period_formset = PtcPeriodFormSet_edit(request.POST, request.FILES, instance=ptc_event)
				log_info("ptc_period_formset %s" % str(ptc_period_formset))
				if ptc_period_formset.is_valid():
					ptc_event.save()
					ptc_event_form.save_m2m() # important!
					ptc_period_formset.save()
			return redirect('conferences.staff.views.manage_ptc_events')
		else:
			ptc_period_formset = PtcPeriodFormSet_edit(request.POST, request.FILES)
	else:
		ptc_event = PtcEvent.objects.get(pk=ptcevent_id)
		ptc_event_form = PtcEventForm(instance=ptc_event)
		ptc_period_formset = PtcPeriodFormSet_edit(instance=ptc_event)
	return render_to_response('staff/add_new_ptc_event.html', locals(), context_instance=RequestContext(request))

add_new_ptc_event.html template:

{% extends "staff/base.html" %}
{% block head_extra %}
<script src="/site_media/js/jquery.min.js"></script>
<script src="/site_media/js/addformtoformset.js"></script>
{% endblock %}

{% block actionarea %}
<br>
<table class="main-title" width="100%">
	<tr>
		<td><h1>{{ page_title }}</h1></td>
		<td class="main-button"><a href="/staff/ptcevents/all/"><button class="mini button gray">Back</button></a></td>
	</tr>
</table>
<br>
	{% if ptc_event_form %}

		<span class="noted">(*) All fields are required!</span>
		<form method="post" action="" accept-charset="utf-8">
			<table id="add-ptc-event" class="table-wrapper vertical" width="99%" border="0">
				<tr>
					<th>Name</th>
					<td>
						<span class="error">{{ ptc_event_form.name.errors }}</span>
						{{ ptc_event_form.name }}
					</td>
				</tr>
				<tr>
					<th>Description</th>
					<td>
						<span class="error">{{ ptc_event_form.desc.errors }}</span>
						{{ ptc_event_form.desc }}
					</td>
				</tr>

				<tr>
					<th>View start</th>
					<td>
						<span class="error">{{ ptc_event_form.view_start.errors }}</span>
						{{ ptc_event_form.view_start }}
					</td>
				</tr>

				<tr>
					<th>View end</th>
					<td>
						<span class="error">{{ ptc_event_form.view_end.errors }}</span>
						{{ ptc_event_form.view_end }}
					</td>
				</tr>

				<tr>
					<th>Edit start</th>
					<td>
						<span class="error">{{ ptc_event_form.edit_start.errors }}</span>
						{{ ptc_event_form.edit_start }}
					</td>
				</tr>

				<tr>
					<th>Edit end</th>
					<td>
						<span class="error">{{ ptc_event_form.edit_end.errors }}</span>
						{{ ptc_event_form.edit_end }}
					</td>
				</tr>

				<tr>
					<th>Grades</th>
					<td>
						<span class="error">{{ ptc_event_form.grades.errors }}</span>
						{{ ptc_event_form.grades }}
					</td>
				</tr>

				<tr>
					<th>Notification mail template</th>
					<td>
						<span class="error">{{ ptc_event_form.default_notification_mail_body.errors }}</span>
						{{ ptc_event_form.default_notification_mail_body }}
					</td>
				</tr>

				<tr>
					<th>Teacher confirmation mail template</th>
					<td>
						<span class="error">{{ ptc_event_form.default_teacher_confirmation_mail_body.errors }}</span>
						{{ ptc_event_form.default_teacher_confirmation_mail_body }}
					</td>
				</tr>

				<tr>
					<th>Parent confirmation mail template</th>
					<td>
						<span class="error">{{ ptc_event_form.default_parent_confirmation_mail_body.errors }}</span>
						{{ ptc_event_form.default_parent_confirmation_mail_body }}
					</td>
				</tr>



				<!-- Ptc Period Formset-->
				
				<tr id="vertical-nohover">
					<td colspan="2">
						<table class="table-wrapper" id="add-ptc-period-table" width="99%">
							<tr><td colspan="5" class="radio title">PTC Periods</td></tr>
							<tr>
								<td class="column">Start date</td>
								<td class="column">Start time</td>
								<td class="column">End time</td>
								<td class="column">Teacher blocks</td>
								<td class="column">Action</td>
							</tr>
						{{ ptc_period_formset.management_form }}
						{% for ptc_period_form in ptc_period_formset.forms %}
							<tr class="item">
								<td class="radio">
									{{ ptc_period_form.id }}
									{{ ptc_period_form.start_date }}
								</td>
								<td class="radio">
									{{ ptc_period_form.start_time }}
								</td>
								<td class="radio">
									{{ ptc_period_form.end_time }}
								</td>
								<td class="radio textfield">
									{{ ptc_period_form.teacher_blocks }}
								</td>
								<td><a class="delete mini button gray radio" href="#">Delete</a></td>
									{{ ptc_period_form.ptc_event.as_hidden }}
							</tr>
						{% endfor %}
							<tr>								
								<td colspan="5" class="radio" id="addbtn" style="padding-right: 4px;"><a id="add" href="#" class="mini button blue">Add</a></td>
							</tr>
						</table>
					</td>
				</tr>
				
				<tr>
					<th>Action</th>
					<td>
						<button type="reset" value="Reset" class="medium button gray">Reset</button></a>
						<button type="submit" name="action" value="save" class="medium button blue">Save</button>
					</td>
				</tr>
			</table>
			<br>
		</form>
	{% else %}
		<p>There's something wrong with the Database! Please contact the System Administrator for Support!</p>
	{% endif %}
	
	<br>
	<br>
{% endblock %}

comment:22 by Aymeric Augustin, 12 years ago

Triage Stage: Design decision neededAccepted

This is obviously a bug, moving back to accepted.

comment:23 by pie.or.paj@…, 11 years ago

Is there any workaround for this issue?
Maybe a way to disable caching for formsets.
I do have more formset-caching problems aswell:
When a formset is saved the old (and now outdated)
formset is often served instead if a fresh.

comment:24 by nik@…, 11 years ago

Cc: nik@… added

comment:25 by joseph@…, 11 years ago

Cc: joseph@… added

comment:26 by naktinis@…, 11 years ago

One workaround is to override your ModelAdmin's change_view to catch an IndexError, clean the nonexistent entries from request.POST and rerun super's change_view. When cleaning, make sure to reduce 'foo_set-TOTAL_FORMS' etc. values.

This ticket's been open for three years, by the way.

in reply to:  26 comment:27 by anonymous, 11 years ago

Replying to naktinis@…:

One workaround is to override your ModelAdmin's change_view to catch an IndexError, clean the nonexistent entries from request.POST and rerun super's change_view. When cleaning, make sure to reduce 'foo_set-TOTAL_FORMS' etc. values.

This ticket's been open for three years, by the way.

could you be so kind and post a snippet of this?

comment:28 by Avraham Seror <tovmeod@…>, 11 years ago

Tried:

def change_view(self, request, object_id, form_url='', extra_context=None):
        try:
            return super(MyModelAdmin, self).change_view(request, object_id, form_url, extra_context)
        except IndexError:
            for key, value in request.POST.items():
                if key.startswith('templatepage'):
                    request.POST.pop(key)
            return super(TemplateAdmin, self).change_view(request, object_id, form_url, extra_context)

but is gives me ValidationError: ManagementForm data is missing or has been tampered with

Last edited 9 years ago by Tim Graham (previous) (diff)

comment:29 by Anssi Kääriäinen, 11 years ago

I think some of the errors in this ticket (for example double delete) are now fixed in master. See efb0100ee67931329f17bc9988ecd5f0619cea14.

I am not sure what happens in edit deleted case.

comment:30 by Avraham Seror <tovmeod@…>, 11 years ago

Well, for those stuck in 1.4 in the meantime this is the workaround I'm using:

the problem with my previous snippet was that I was removing too much, I considered using a regex but that would be overkill.
in my case I'm removing everything so now I'm simply doing as below instead of a loop:

request.POST['templatepage_set-TOTAL_FORMS'] = '0'

in any case, is this fix in 1.6?

Last edited 9 years ago by Tim Graham (previous) (diff)

comment:31 by Anssi Kääriäinen, 11 years ago

No, the fix isn't in 1.6.

comment:32 by Tim Graham, 10 years ago

See #22689 for a duplicate (or at least similar) issue.

in reply to:  30 comment:33 by Pavel Savchenko, 10 years ago

Replying to Avraham Seror <tovmeod@…>:

Well, for those stuck in 1.4 in the meantime this is the workaround I'm using:

the problem with my previous snippet was that I was removing too much, I considered using a regex but that would be overkill.
in my case I'm removing everything so now I'm simply doing as below instead of a loop:

request.POSTtemplatepage_set-TOTAL_FORMS = '0'

in any case, is this fix in 1.6?

For anyone who's still wondering; the above mentioned fix in master seems to have been included in 1.7.

comment:34 by Collin Anderson, 10 years ago

Cc: cmawebsite@… added

comment:35 by milosu, 10 years ago

Cc: milos.urbanek@… added

comment:36 by milosu, 10 years ago

This bug is very annoying in production.
Moreover from my observations under Django 1.4, which I'm running, the patch in https://github.com/django/django/commit/efb0100ee67931329f17bc9988ecd5f0619cea14 does not solve anything.
The IndexError does not happen with that patch, but ValidationError due the the invalid pkey is thrown instead in the form and attempt to re-submit the form yields "MultiValueKeyDictError" exception due to the missing pkey ID data.

My solution is to detect the case, when the form object was deleted, remove its form data from the formset and to throw a ValidationError at the formset level.

The user can then check the form - which is already missing the deleted object - and re-submit it.

Patch against Django 1.4 attached. Hope it helps.

by milosu, 10 years ago

comment:37 by Carsten Fuchs, 9 years ago

Cc: carsten.fuchs@… added
Note: See TracTickets for help on using tickets.
Back to Top