Opened 10 years ago

Closed 10 years ago

#21967 closed Cleanup/optimization (fixed)

ModelFormMixin have extra logic in get_form_kwargs that crash view

Reported by: lagovas.lagovas@… Owned by: Ryan Kaskel
Component: Generic views Version: dev
Severity: Normal Keywords: generic edit ModelFormMixin
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

When view inherit FormView and ModelFormMixin, have correct form_class and model fields, view will crash with AttributeError at /
'AddNoteView' object has no attribute 'object'

in this code:

class ModelFormMixin:
    ....
    def get_form_kwargs(self):
        """
        Returns the keyword arguments for instantiating the form.
        """
        kwargs = super(ModelFormMixin, self).get_form_kwargs()
        kwargs.update({'instance': self.object})
        return kwargs

It's due to accessing to self.object before form.save(). Even in get request, when object can't be exist.

To check this you need inherit from FormView and ModelFormMixin:

class AddNoteView(FormView, ModelFormMixin):

template_name = 'notes.html'
form_class = NoteForm
success_url = reverse_lazy('index')
model = Note

Attachments (1)

Untitled.png (35.9 KB ) - added by lagovas.lagovas@… 10 years ago.
screenshot of traceback

Download all attachments as: .zip

Change History (5)

by lagovas.lagovas@…, 10 years ago

Attachment: Untitled.png added

screenshot of traceback

comment:1 by Baptiste Mispelon, 10 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 1.6master

Hi,

If you're combining ModelFormMixin and FormView, I think you'd be better off using a CreateView.
And if you find yourself writing complex code, it might be a sign that it's time to drop Django's generic mixins and write your own view from scratch (inheriting from View).

Having said that, the implementation of ModelFormMixin.get_form_class has some code that makes sure self.object exists so for consistency's sake, I'll mark this as accepted.

The change is pretty straightforward so I'll also mark the ticket as easy picking.

Thanks.

comment:2 by Ryan Kaskel, 10 years ago

Owner: changed from nobody to Ryan Kaskel
Status: newassigned

comment:3 by Ryan Kaskel, 10 years ago

Has patch: set

comment:4 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 75a96f06e916dda9124da9ddf765f3b8d5530914:

Fixed #21967: Added check for object in ModelFormMixin.get_form_kwargs.

Thanks lagovas.lagovas at gmail.com for the report.

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