Opened 8 years ago
Closed 8 years ago
#27688 closed Bug (fixed)
Django message framework add_message() should prefer ducktyping over isinstance
Reported by: | Raffaele Salmaso | Owned by: | Raffaele Salmaso |
---|---|---|---|
Component: | contrib.messages | Version: | dev |
Severity: | Normal | Keywords: | messages |
Cc: | raffaele.salmaso+djangoproject@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Original ticket https://code.djangoproject.com/ticket/21412
django.contrib.messages.add_message check for an HttpRequest object instance, and cannot work as-is with django-rest-framework request wrapper, requiring to use the wrapped object directly
messages.add_message(request._request, ...)
add_message() should not use isinstance
check.
Change History (12)
comment:1 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
comment:2 by , 8 years ago
Triage Stage: | Accepted → Unreviewed |
---|---|
Type: | Bug → Uncategorized |
PR => https://github.com/django/django/pull/7794
I've update original test to be sure that the 'django.contrib.messages.middleware.MessageMiddleware' is not in settings.MIDDLEWARE,
and added a custom request object
comment:3 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
comment:4 by , 8 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:5 by , 8 years ago
Updated check so can be used a custom middleware (if really it is needed).
comment:6 by , 8 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
Looks like the patch was revised after "Ready for checkin" was set. I've left some comments for improvement.
comment:7 by , 8 years ago
Update the PR (mostly rewritten).
Now it is similar to original behaviour.
comment:8 by , 8 years ago
Patch needs improvement: | unset |
---|
comment:10 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:11 by , 8 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Funny, we actually ran into this at work yesterday.