Editing without using id hidden field in form?

Question

I am not sure if this is the cleanest way of writing an edit. But after hours of research this is the best I could come up with. However I don't like the fact that I have to store the id inside a hiddenfield just to retrieve it again as POST to actually update the model.

Is there a more efficient way of doing this?

def edit_contact_view(request):
    profile = request.user.get_profile()
    if 'id' in request.GET:
        try:
            id = request.GET['id']
            contacts = profile.company.contact_set.all()
            form = ContactsForm(profile.company, instance=contacts.get(id=id))
            form.data['id'] = id
            variables = RequestContext(request, {'form':form })
            return render_to_response("contact.html", variables)
        except Contact.DoesNotExist:
            raise Http404(_(u'Contact not found'))
    else:
        if request.method == 'POST':
            form = ContactsForm(profile.company, request.POST)
            if form.is_valid():
                contacts = profile.company.contact_set.all()
                contact = contacts.get(id=form.cleaned_data['id'])
                contact.last_name = form.cleaned_data['last_name']
                contact.save()
    return HttpResponseRedirect('/')

score 4 · Accepted Answer · edited May 23 '17 at 10:29

Yes, you are making this more complicated than it needs to be.

Firstly, in Django it's more usual to pass the id in the URL itself, rather than in a GET parameter - eg /contact/edit/3/ rather than /contact/edit?id=3. See for example this question to see how to configure URLs like that.

Secondly, whichever way you do it, there's no need to pass the id in a hidden variable since it's already available from the URL. You can always get the instance from there.

Thirdly, I presume that ContactForm is a ModelForm, but you're not using the save functionality, which simplifies things still further.

Putting it all together:

def edit_contact_view(request, id=None):
    profile = request.user.get_profile()
    if id is not None:
        contact = get_object_or_404(Contact, pk=id)
    else:
        contact = Contact(company=profile.company)
    if request.POST:
       form = ContactForm(request.POST, instance=contact)
       if form.is_valid():
           contact = form.save()
           return redirect(...)

    else:
        form = ContactForm(instance=contact)
    return render_to_response('contact.html', {'form': form})

thanks a lot Daniel, this solution is very clean and complete. I have implemented all your three suggestions and it works beautifully. So much shorter and it combines both add & edit in one. The only thing is when I add a `Contact` the code sees there is no id, hence creates a new instance of `Contact` to pass it into a form. Once it comes back as POST, it creates again a new instance of `Contact` and passes it into the form incl. the request.POST. This looks a bit confusing to me, why we create a new instance of Contact and pass it again into the POST form. — Houman, Jun 30 '12 at 07:16
It's just to simplify the pattern - otherwise you need an extra if/else around both form instantiations. It's just an unsaved instance in any case, so there's no db call. — Daniel Roseman, Jun 30 '12 at 08:02

score 2 · Answer 2 · answered Jun 29 '12 at 18:10

2

I must be missing something here but why don't you encode the id in the url as is standard?

i.e. in urls.py:

url('^/contact/?P<contact_id>[0-9]+)/edit/$', edit_contact_view, name='edit_contact_view')

your view:

def edit_contact_view(request, contact_id):
    profile = request.user.get_profile()
    contact = get_object_or_404(Contact, id=contact_id)
    if contact not in profile.company.contact_set.all():
        return Http404

and in your template

<form method="POST">
   {{ form.as_p }}
   ...
</form>

answered Jun 29 '12 at 18:10

scytale

12,346
3
32
46

i believe `get_object_or_404(profile.company.contact_set.all(), id=contact_id)` should be `get_object_or_404(Contact, pk=contact_id)` – Francis Yaconiello Jun 29 '12 at 18:13
good point - get_object_or_404 takes a model class, not a queryset. – scytale Jun 29 '12 at 18:14
1

also I use pk instead of id or whatever you may haver overridden your primary key to be, just a tiny bit less model dependent. in general i agree with your approach however. (http://stackoverflow.com/questions/2165865/django-queries-id-vs-pk) – Francis Yaconiello Jun 29 '12 at 18:15
2

Actually `get_object_or_404` *can* take a queryset. – Chris Pratt Jun 29 '12 at 18:49

Editing without using id hidden field in form?

2 Answers2