#21967 closed Cleanup/optimization (fixed)

ModelFormMixin have extra logic in get_form_kwargs that crash view

Reported by: lagovas.lagovas@… Owned by: ryankask
Component: Generic views Version: master
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@… 14 months ago.
screenshot of traceback

Download all attachments as: .zip

Change History (5)

Changed 14 months ago by lagovas.lagovas@…

screenshot of traceback

comment:1 Changed 14 months ago by bmispelon

  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/optimization
  • Version changed from 1.6 to master

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 Changed 14 months ago by ryankask

  • Owner changed from nobody to ryankask
  • Status changed from new to assigned

comment:3 Changed 14 months ago by ryankask

  • Has patch set

comment:4 Changed 14 months ago by Tim Graham <timograham@…>

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

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