[Django] #12521: ModelAdmin save_model request.user idiom is broken by model validation

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation
----------------------------------+-----------------------------------------
Reporter: simon | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Keywords: | Stage: Unreviewed
Has_patch: 0 |
----------------------------------+-----------------------------------------
The common (documented) pattern for automatically setting the "author"
field on an object to request.user by over-riding the save_model method
on a ModelAdmin subclass breaks with the new model validation code. Here
are the example models.py and admin.py files:

{{{
from django.db import models

class Entry(models.Model):
title = models.CharField(max_length = 255)
body = models.TextField()
published = models.DateTimeField(auto_now_add = True)
author = models.ForeignKey('auth.User')

def __unicode__(self):
return self.title
}}}
admin.py:
{{{
from models import Entry
from django.contrib import admin

class EntryAdmin(admin.ModelAdmin):
list_display = ('title', 'published', 'author')
exclude = ('author',)

def save_model(self, request, obj, form, change):
obj.author = request.user
obj.save()

admin.site.register(Entry, EntryAdmin)
}}}
And here's the exception:
{{{
Traceback:
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/core/handlers/base.py" in get_response
99. response = callback(request, *callback_args,
**callback_kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/contrib/admin/options.py" in wrapper
237. return self.admin_site.admin_view(view)(*args,
**kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/utils/decorators.py" in __call__
36. return self.decorator(self.func)(*args, **kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/utils/decorators.py" in _wrapped_view
86. response = view_func(request, *args, **kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/utils/decorators.py" in __call__
36. return self.decorator(self.func)(*args, **kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
70. response = view_func(request, *args, **kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/contrib/admin/sites.py" in inner
187. return view(request, *args, **kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/utils/decorators.py" in _wrapped_view
86. response = view_func(request, *args, **kwargs)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/db/transaction.py" in _commit_on_success
295. res = func(*args, **kw)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/contrib/admin/options.py" in add_view
760. if form.is_valid():
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/forms/forms.py" in is_valid
120. return self.is_bound and not bool(self.errors)
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/forms/forms.py" in _get_errors
111. self.full_clean()
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/forms/forms.py" in full_clean
286. self.cleaned_data = self.clean()
File "/Users/simon/Development/playing-with-django-1.2/ve/lib/python2.6
/site-packages/django/forms/models.py" in clean
270. raise UnresolvableValidationError(e.message_dict)

Exception Type: UnresolvableValidationError at /admin/blog/entry/add/
Exception Value:
}}}
The validation error is "This field cannot be null".

This pattern is documented here:
http://docs.djangoproject.com/en/dev/ref/contrib/admin/#modeladmin-methods
- we should at the very least update the documentation, but even having
done that we'd still break existing code.

--
Ticket URL: <http://code.djangoproject.com/ticket/12521>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

-------------------------------------------+--------------------------------
Reporter: simon | Owner: jkocherhans
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Unreviewed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
-------------------------------------------+--------------------------------
Changes (by jkocherhans):

* status: new => assigned
* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => jkocherhans
* needs_docs: => 0
* has_patch: 0 => 1

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:1>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1

Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |

---------------------------------------------+------------------------------
Changes (by jkocherhans):

* stage: Unreviewed => Design decision needed

Comment:

See #12513 and [http://groups.google.com/group/django-
developers/browse_thread/thread/cee43f109e0cf6b this thread] on django-dev
for more info.

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:2>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by EmilStenstrom):

* cc: e...@kth.se (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:3>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by jezdez):

* cc: jezdez (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:4>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by knutin):

* cc: knu...@gmail.com (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:5>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by zbyte64):

* cc: zby...@gmail.com (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:6>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by isagalaev):

* cc: Man...@SoftwareManiacs.Org (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:7>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: isagalaev

Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1

Needs_docs: 0 | Needs_tests: 1

Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by isagalaev):

* owner: jkocherhans => isagalaev
* status: assigned => new
* needs_tests: 0 => 1

Comment:

I'm about to add some tests to jkocherhans' patch. Honza, your patch is
supposedly does the same thing, can you comment if you want yours as a
replacement or not?

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:8>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: isagalaev

Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Comment (by Honza_Kral):

Replying to [comment:8 isagalaev]:

> I'm about to add some tests to jkocherhans' patch. Honza, your patch is
supposedly does the same thing, can you comment if you want yours as a
replacement or not?

yes, the only problem with my patch, afaik, i sthat it only looks at
exclude and not in at fields in django/forms/models.py line 252. As I said
earlier, it's a quick patch (part of it copy paste, which is plain wrong)
just to show the direction to go.

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:9>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: tolano
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by tolano):

* cc: ari...@gmail.com (added)
* owner: isagalaev => tolano

* status: new => assigned

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:10>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: isagalaev

Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by tolano):

* owner: tolano => isagalaev

* status: assigned => new

Comment:

What happened? I didn't change the owner nor the status! I'll try to set
it back.

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:11>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by isagalaev):

* owner: isagalaev => jkocherhans

Comment:

Added a couple of tests that fail with current trunk. Some comments:

* admin_views tests are testing UserCreationForm from contrib.auth. It
used not to break after model-validation merge because it was correcting
model instance as a side effect of validation. It's not rewritten in more
idiomatic way applying its non-form data in `save()` (and currently it
correctly fails due to missing "password" field in the form)
* model_forms tests are exercising different ways to set field on a
ModelForm and also make sure that the form still provides an incomplete
and unsaved instance of a model

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:12>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by jkocherhans):

* status: new => assigned

Comment:

My current patch is almost there I think. I still need to talk to Honza
about where the `model.validate()` hook should be called, but other than
that I thinks it's ready. It also fixes #12520, #12537, and #12552.

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:13>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Comment (by isagalaev):

Confirming that current patch fixes my use-case with PreviewForm.

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:14>

[Django]

#12521: ModelAdmin save_model request.user idiom is broken by model validation

---------------------------------------------+------------------------------
Reporter: simon | Owner: jkocherhans

Status: closed | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: fixed | Keywords:

Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------+------------------------------

Changes (by lukeplant):

* status: assigned => closed
* resolution: => fixed

Comment:

This was fixed in [12206] by Joseph Kocherhans (typo in commit message
says #12512 instead of #12521).

--
Ticket URL: <http://code.djangoproject.com/ticket/12521#comment:15>