Opened 7 years ago

Closed 6 years ago

#9758 closed (invalid)

modelformset_factory forms generate error when using queryset

Reported by: cyberjack17@… Owned by: nobody
Component: Forms Version: 1.0
Severity: Keywords:
Cc: carljm Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

What I'm trying to do:

  • I'm building a formset with exactly seven entries, one for each day of the last week.
  • I use modelformset_factory to create my form and specify a queryset to filter the particular item and date range.

What I see:

  • The page displays the seven entries as expected. The first seven days are created fine and can be edited at any point.
  • If I try to edit additional entries, I see the following error:

(Hidden field id) Status report with this None already exists.

  • The kicker is that if I remove the queryset limitation, this error goes away. Ie, my formset includes all entries, not just the seven I want.
  • Without the queryset, I can add / edit as many items and weeks as I want.

Here's a minimal example, which assumes you already have data stored in StatusReport.

class StatusReport(models.Model):
   item          = models.IntegerField()
   report_date   = models.DateField()
   status        = models.CharField(max_length=10)

view:

def detail(request, item_id):

    # removed sanity checks, entry creation, etc

    StatusFormSet = modelformset_factory(StatusReport, extra=0)
    if request.method == 'POST':
        formset = StatusFormSet(request.POST)
        if formset.is_valid():
            formset.save()
    else:
        ### with error
        #limited_reports = StatusReport.objects.filter(item=item_id)
        #formset = StatusFormSet(queryset=limited_reports)
        ### without error
        formset = StatusFormSet()
    return render_to_response('manage_items.html',{'formset': formset})

and generic template

<form method="POST" action="">
    <table>
        {{ formset }}
    </table>
    <input type = "submit" value = "Submit">
</form>

Change History (4)

comment:1 Changed 7 years ago by kmtracey

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

So I answered on the mailing list (http://groups.google.com/group/django-users/browse_thread/thread/ec6b24a405ea5abe/2a7de9ae3d974bb3) that I thought the problem was that the same queryset wasn't being passed in during POST as had been used during GET, resulting in the edited instances not being correctly matched up with the actual db instances. And that is what's happening, but I'm not sure if the error is in the app code or Django. Can someone who is more familiar with ModelForm[sets] give some input on whether it is required to pass instance/queryset data in when creating the form during POST processing?

What's happening in the case of no queryset being passed in for the formset during POST is that the individual forms (which have hidden ID fields) are being given 'instance' attributes that do not match the instances used to initially create the forms during GET. I've not looked at how this happens, I'm just going by what I see happening in django.forms.models.validate_unique. Say I have a formset with one StatusReport for pk=3 being posted. When no queryset is passed in to the formset creation during POST, in validate_unique we wind up attempting to check for the uniqueness of the hidden id (pk) field. We exclude from the lookup the object with pk of self.instance, but (in the case I traced through) self.instance had pk=1 (1st in the default unfiltered queryset used when no queryset is specified during formset creation?). So validate_unique finds there's already an object in the db with pk=3, and adds an error for that field which winds up printing out as "(Hidden field id) Status report with this None already exists." The "None" is the non-existent label for the hidden id field.

A way to "fix" it (the one I mentioned on the mailing list) is to pass in the same queryset during POST as was used during GET, but that strikes me as somewhat fragile since how can the view code guarantee no changes have been made to the DB in the time between GET and POST that will affect correctly matching up DB instances with the submitted form data? Perhaps an entirely unrelated form has been submitted in the interim that adds a new object that will result in the filtered queryset during POST having more entries than it had during GET, say. The submitted data includes the pk's (though I don't know if this is a requirement or just how it happens by default?). Shouldn't the submitted pk's be what is used during validate_unique when attempting to exclude "this object" from the lookup for uniqueness checking?

Even if it's required that the same instance/queryset be passed in during GET and POST I think the doc needs clarification of that, and validate_unique (or some other code) should be changed to generate a more coherent error than what's happening now. Having validate_unique run with self.instance pointing to an entirely different object than is contained in the POST data seems like an error that should be either raised explicitly or caught earlier. Feedback from someone with more a clue in this area would be helpful...

BTW this error has been reported at least one other time on the mailing list: http://groups.google.com/group/django-users/browse_thread/thread/37c0672759298e57/b3e109015f4e63d3 without any real resolution.

comment:2 Changed 7 years ago by kmtracey

See #9076 also for how the current match-formset-post-data-to-existing-db-instances approach is fragile. That ticket notes how for some DBs if no order is specified, the actual order returned for a queryset may not be the same from call to call. So the queryset retrieved for GET may be in a different order than the one retrieved during POST (even if they contain the exact same entries) and the formset winds up with instances not correctly matched to POST data.

comment:3 Changed 7 years ago by carljm

  • Cc carljm added

comment:4 Changed 6 years ago by kmtracey

  • Resolution set to invalid
  • Status changed from new to closed

In the interval since this was originally opened, the docs have been expanded/clarified to note that it is expected that the queryset argument be passed during both GET and POST processing: http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#using-a-custom-queryset. That makes this particular problem a programming error, since the error was observed only in the case where the queryset was being passed on GET but not POST.

Also in the interval #10163 was closed as a dupe of this, which it isn't really. The admin does pass queryset in on both GET and POST. The problem in #10163 is the ordering may be unspecified, and thus differ from call to call, particularly on PostgreSQL. I'm going to close this as invalid and re-open #10163 to address that issue. #10163 is also a little easier to deal with as it is a problem that can be observed with the existing test suite.

Note: See TracTickets for help on using tickets.
Back to Top