#16884 closed New feature (fixed)
Add message level argument to ModelAdmin's message_user
Reported by: | Sergiy Kuzmenko | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | admin |
Cc: | Rafał Jagoda, H0ff1, simon@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | yes |
Description
I think it would be really nice to extend message_user method to allow warning and error messages to be output by admin actions.
So in https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/options.py#L675 method signature would change as follows:
- def message_user(self, request, message): + def message_user(self, request, message, level=INFO):
Semantics and look-n-feel:
- DEBUG, INFO, SUCCESS: Admin action completed successfully; shows the green checkbox image (same as for representing True)
- ERROR: Admin action did not complete; shows the stop sign image (same as for False)
- WARNING: Admin action completed but not as smooth as expected; perhaps there should be a new icon for this (white question mark on red circle?)
Attachments (4)
Change History (22)
comment:1 by , 13 years ago
Component: | Uncategorized → contrib.admin |
---|---|
Triage Stage: | Unreviewed → Accepted |
by , 13 years ago
Attachment: | message-user-level.diff added |
---|
comment:2 by , 13 years ago
comment:3 by , 13 years ago
#15007 asked for the type of the message shown after adding a model instance to be success instead of info. I closed it as duplicate of this one becasue it would provide the feature needed to set different types to messages related to different admin actions.
comment:4 by , 13 years ago
Cc: | added |
---|---|
Has patch: | set |
message.SUCCESS has been added to every success message_user call
and message.ERROR has been added to every message_user call where user try to perform action, but some conditions are not met f.e. action is not selected.
I fixed some tests too.
by , 13 years ago
Attachment: | patch.diff added |
---|
comment:5 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:6 by , 13 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Ready for checkin → Accepted |
Thanks for your work on this. Some tests should be added to test the new functionality, i.e. that the message type can be customized. Icons/colors should also be adjusted for the WARNING type.
by , 13 years ago
Attachment: | patch_2.diff added |
---|
comment:7 by , 13 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Added a new WARNING-Icon and more tests
comment:8 by , 13 years ago
Needs documentation: | set |
---|
message_user is documented: https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.message_user
by , 13 years ago
Attachment: | patch_documentation.diff added |
---|
comment:10 by , 13 years ago
Cc: | added |
---|
follow-up: 12 comment:11 by , 13 years ago
Cc: | added |
---|
If you have to import messages anyway now, isn't it easier to just use messages.error(request, 'foo') instead of self.message_user(request, 'foo', level=messages.ERROR) ?
Especially since message_user doesn't seem to do anything but call messages.info().
follow-up: 13 comment:12 by , 13 years ago
Replying to sbaechler:
If you have to import messages anyway now, isn't it easier to just use messages.error(request, 'foo') instead of self.message_user(request, 'foo', level=messages.ERROR) ?
Especially since message_user doesn't seem to do anything but call messages.info().
Fair, I admit that the message_user benefit is small. It avoids importing the contrib.messages when you use the default level.
So a proposal would be to simply deprecate the method and suggest using the contrib.messages API directly. Other opinions?
follow-up: 14 comment:13 by , 13 years ago
Replying to claudep:
So a proposal would be to simply deprecate the method and suggest using the contrib.messages API directly. Other opinions?
There's a good reason to keep messages_user -- it decouples admin from the messages app. By subclassing ModelAdmin, you can completely replace the use of Django's messages with any other scheme you wish; if you call the messages API directly, this obviously isn't possible.
comment:14 by , 13 years ago
Replying to russellm:
There's a good reason to keep messages_user -- it decouples admin from the messages app. By subclassing ModelAdmin, you can completely replace the use of Django's messages with any other scheme you wish; if you call the messages API directly, this obviously isn't possible.
If by decouple, you mean avoid installing contrib.messages at all, it is probably already ruined by the contrib.messages import in options.py. And if you want to use your own messaging framework, you don't need message_user API for that. The only remaining use-case I can see, would be an external app providing a subclassed ModelAdmin that redirects message_user to another messages framework. At this point, I wonder if this external app would not simply instruct you to send messages with its custom messages framework in the first place. In summary, I'm struggling to find real use cases for that, but of course, I may miss some obvious uses.
comment:15 by , 13 years ago
I understand now that message_user may be considered as a single point of indirection for all contrib.admin internal calls to the API (e.g. subclass ModelAdmin and override message_user to redirect all admin messages). Point taken for the decoupling point.
comment:16 by , 12 years ago
This is implemented in Django 1.5, thus this ticket is already fixed - see https://docs.djangoproject.com/en/1.5/ref/contrib/admin/#django.contrib.admin.ModelAdmin.message_user
comment:17 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:18 by , 12 years ago
For the record this was fixed in edf7ad36faab8d45aafe1f96feaf46794de22fc1.
It appears that CSS classes and images are already there.