Summary

Add a mixin for updating models with a ModelForm

Review Request #7493 — Created July 5, 2015 and submitted July 13, 2015, 5:24 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.5.x

Bugs

Depends On

~~7494~~

Blocks

7483

Commit

5ace999...

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done

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

Issues

Description	From	Last Updated
Should use :py:attr: to reference form_class.	chipx86	July 6, 2015, 8:30 p.m.
:py:class: for ModelForm.	chipx86	July 6, 2015, 8:30 p.m.
:py:class: here too.	chipx86	July 6, 2015, 8:30 p.m.
:py:meth: for these.	chipx86	July 6, 2015, 8:31 p.m.
:py:class: and :py:attr:	chipx86	July 6, 2015, 8:33 p.m.
Blank line between these.	chipx86	July 6, 2015, 8:33 p.m.
:py:class:, and trailing period.	chipx86	July 6, 2015, 8:33 p.m.
What does this do? Just provide a default? What happens if the form doesn't support extra_data?	chipx86	July 6, 2015, 8:33 p.m.
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 …	chipx86	July 6, 2015, 8:33 p.m.
Blank line between these.	chipx86	July 6, 2015, 8:33 p.m.
Should have a trailing period.	chipx86	July 6, 2015, 8:33 p.m.
In this case, it'd be better to do: from djblets.webapi.resources.mixins.forms import ( UpdateFormMixin as DjbletsUpdateFormMixin) That leaves room for additional …	chipx86	July 11, 2015, 2:24 a.m.
django.db.models.Model	chipx86	July 11, 2015, 2:24 a.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/mixins.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/mixins.py

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.)

reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should use :py:attr: to reference form_class.
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
:py:class: for ModelForm.
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
:py:class: here too.
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
:py:meth: for these.
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
:py:class: and :py:attr:
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
:py:class:, and trailing period.
```

reviewboard/webapi/mixins.py (Diff revision 1)

The issue has been resolved. Show all issues

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

brennie July 6, 2015, 8:17 p.m.

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

reviewboard/webapi/mixins.py (Diff revision 1)

The issue has been resolved. 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?

reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
reviewboard/webapi/mixins.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should have a trailing period.
```

Change Summary:

Now refactored into Djblets.

Description:

		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.

Depends On:

+	~~7494 - Add a mixin for updating models with a ModelForm~~

Commit:

+	ec0ab8fb7aa8499dc0fba35894196dee80e7359c

Diff:

Revision 2 (+48)

Show changes

reviewboard/webapi/mixins.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/mixins.py



Tool: Pyflakes
Processed Files:
    reviewboard/webapi/mixins.py

Commit:

-	ec0ab8fb7aa8499dc0fba35894196dee80e7359c
+	9d72aac2831365eb7ca27f5b61dfaaf8c89118ba

Diff:

Revision 3 (+50)

Show changes

reviewboard/webapi/mixins.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/mixins.py



Tool: Pyflakes
Processed Files:
    reviewboard/webapi/mixins.py

Change Summary:

Rebase off updated Djblets changes.

Description:

		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
	~	`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.

Commit:

-	9d72aac2831365eb7ca27f5b61dfaaf8c89118ba
+	5ace99904af65e1f9b4f1dee54ca1c29e47b5eb3

Diff:

Revision 4 (+51 -1)

Show changes

reviewboard/webapi/mixins.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/mixins.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/mixins.py

reviewboard/webapi/mixins.py (Diff revision 4)

The issue has been resolved. 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.

reviewboard/webapi/mixins.py (Diff revision 4)
The issue has been resolved. Show all issues
```
django.db.models.Model
```

Diff:

Revision 5 (+51 -1)

Show changes

reviewboard/webapi/mixins.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/mixins.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/mixins.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (b85d536)