Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18548 closed Cleanup/optimization (fixed)

View changes field when 'unique_together' is not satisfied in a Form

Reported by: anonymous Owned by: nobody
Component: Documentation Version: 1.4
Severity: Normal Keywords:
Cc: timograham@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by aaugustin)

I have a Model called Person with the following constraint: unique_together = ('name', 'country')

Furthermore I have a view to to edit a Person:

def edit_climbing_place(request, person_id):

    person = get_object_or_404(Person, pk=person_id)

    print person.name

    if not request.method == 'POST':
        form     = Person(instance=place)

    else:
        form = Person(request.POST, instance=place)

        if form.is_valid():
            form.save()
            return HttpResponseRedirect(reverse(start_page))
     

    print person.name

    return render_to_response('myProject/edit_person.html',
        {'form' : form, 'person': person},
        context_instance=RequestContext(request))

In the above code if form.is_valid() fails due to the 'unique_together' constraint is not satisfied then the two 'print person.name' outputs are different. In other words if I edit an existing 'Person' (say A) and change the name to a name which matches an already existing person (say B) in the same 'country' then in the above view the variable 'person.name' changes to that of B. All other fields, however, stay the same.

Attachments (1)

18548.diff (600 bytes) - added by timo 2 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by aaugustin

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Fixed formatting (please use preview).

comment:2 Changed 3 years ago by aaugustin

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

You must have edited the code before posting it. As is it will crash for at least two reasons:

  • the Person class is used both as a model and as a form,
  • the place variable is used but is never defined.

The code you've posted can't work and the result you're describing sounds reasonable. I can't guess what your actual code looks like nor why you think the result isn't correct.

In short, this appears to be a support question rather than a bug in Django. Please follow the instructions on this page: TicketClosingReasons/UseSupportChannels. Thanks!

comment:3 Changed 3 years ago by anonymous

  • Resolution invalid deleted
  • Status changed from closed to reopened

Sorry i did edit the code to simplify it a bit but i apparently did it too fast. This should be more clear:

def edit_person(request, person_id):

    person = get_object_or_404(Person, pk=person_id)

    print person.name

    if not request.method == 'POST':
        form = PersonForm(instance=person)

    else:
        form = PersonForm(request.POST, instance=person)

        if form.is_valid():
            form.save()
            return HttpResponseRedirect(reverse(start_page))
     

    print person.name

    return render_to_response('myProject/edit_person.html',
        {'form' : form, 'person': person},
        context_instance=RequestContext(request))

comment:4 Changed 3 years ago by anonymous

I think this is a duplicate of #12896, which was ultimately "fixed" via a documentation note. Possibly that note needs to be extended to indicate that the model cleaning in-place means that part of the requested changes may be applied to the instance before form validation ultimately fails and causes is_valid() to return False?

comment:5 Changed 3 years ago by aaugustin

  • Component changed from Forms to Documentation
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/optimization
  • Version changed from 1.3 to 1.4

Yes, the documentation could state more clearly that the *instance* passed to the form will be modified and should not be reused later in the view, especially if validation fails.

Changed 2 years ago by timo

comment:6 Changed 2 years ago by timo

  • Cc timograham@… added
  • Has patch set

I added the note in the referenced ticket so I think it's clear, but does the attached patch help emphasize the point?

comment:7 Changed 2 years ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from reopened to closed

In fd02bcff4aee885d395f2439efdd522c22e40794:

Fixed #18548 - Clarified note regarding reusing model instances when form validation fails.

comment:8 Changed 2 years ago by Tim Graham <timograham@…>

In 6ebb6f918872c2a714ddb9808984e70daaa95cd8:

[1.4.X] Fixed #18548 - Clarified note regarding reusing model instances when form validation fails.

Backport of fd02bcff4a from master

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