Add a mixin for updating models with a ModelForm

Review Request #7493 — Created July 5, 2015 and submitted

Information

Review Board
release-2.5.x
5ace999...

Reviewers

The UpdateFormMixin is a mixin for a WebAPIResource that allows a
resource to specify a form class (which should be a subclass of
django.forms.ModelForm) to use for creating and updating of model
instances. This allows the resource that are very basic wrappers around
models to have their logic further simplified.

This mixin is required for allowing partial updating of models.
Traditionally, a ModelForm can take an instance parameter
specifying an already existing model instance to update, but will still
require all required field to specified in the form data. This mixin
takes care of that by extracting missing form data from the instance
and inserting it into the form data in the proper format (such as
setting it to be a list of primary keys in the case of a
ModelMultipleChoiceField).

This mixin also allows resources to specify special functions for
parsing values out of the recieved form data. This is useful, e.g., for
automatically converting human-readable values accepted by the API to
the value the database expects for a ChoiceField.

This is a re-export of the mixin from Djblets with some extra
functionality added.

Used the mixin in a model. It worked as expected.

Description From Last Updated

Should use :py:attr: to reference form_class.

chipx86chipx86

:py:class: for ModelForm.

chipx86chipx86

:py:class: here too.

chipx86chipx86

:py:meth: for these.

chipx86chipx86

:py:class: and :py:attr:

chipx86chipx86

Blank line between these.

chipx86chipx86

:py:class:, and trailing period.

chipx86chipx86

What does this do? Just provide a default? What happens if the form doesn't support extra_data?

chipx86chipx86

So Django's form support actually has a nifty function we probably want to use: django.forms.models.model_to_dict. This takes an instance, a …

chipx86chipx86

Blank line between these.

chipx86chipx86

Should have a trailing period.

chipx86chipx86

In this case, it'd be better to do: from djblets.webapi.resources.mixins.forms import ( UpdateFormMixin as DjbletsUpdateFormMixin) That leaves room for additional …

chipx86chipx86

django.db.models.Model

chipx86chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
  2. 
      
chipx86
  1. How about moving this over to Djblets? This would be useful outside of Review Board (and I'm working to take all the neat "core" stuff and make it part of Djblets anyway.)

  2. 
      
chipx86
  1. 
      
  2. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    Should use :py:attr: to reference form_class.

  3. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    :py:class: for ModelForm.

  4. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    :py:class: here too.

  5. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    :py:meth: for these.

  6. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    :py:class: and :py:attr:

  7. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
     
    Show all issues

    Blank line between these.

  8. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    :py:class:, and trailing period.

  9. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    What does this do? Just provide a default? What happens if the form doesn't support extra_data?

    1. That actually doesn't really do anything and shouldn't be there.

  10. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
     
    Show all issues

    So Django's form support actually has a nifty function we probably want to use: django.forms.models.model_to_dict.

    This takes an instance, a list of form fields, excluded fields, and returns a dictionary that can be passed in to a form's initial=. Sounds like this is what we want?

  11. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
     
    Show all issues

    Blank line between these.

  12. reviewboard/webapi/mixins.py (Diff revision 1)
     
     
    Show all issues

    Should have a trailing period.

  13. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/webapi/mixins.py (Diff revision 4)
     
     
     
    Show all issues

    In this case, it'd be better to do:

    from djblets.webapi.resources.mixins.forms import (
        UpdateFormMixin as DjbletsUpdateFormMixin)
    

    That leaves room for additional imports to take place on following lines.

  3. reviewboard/webapi/mixins.py (Diff revision 4)
     
     
    Show all issues

    django.db.models.Model

  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/mixins.py
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.5.x (b85d536)