Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

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

message-user-level.diff (782 bytes ) - added by Sergiy Kuzmenko 13 years ago.
patch.diff (9.5 KB ) - added by Rafał Jagoda 13 years ago.
patch_2.diff (12.0 KB ) - added by H0ff1 13 years ago.
patch_documentation.diff (1.4 KB ) - added by H0ff1 13 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 by Alex Gaynor, 13 years ago

Component: Uncategorizedcontrib.admin
Triage Stage: UnreviewedAccepted

by Sergiy Kuzmenko, 13 years ago

Attachment: message-user-level.diff added

comment:2 by Sergiy Kuzmenko, 13 years ago

It appears that CSS classes and images are already there.

comment:3 by Ramiro Morales, 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 Rafał Jagoda, 13 years ago

Cc: Rafał Jagoda 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 Rafał Jagoda, 13 years ago

Attachment: patch.diff added

comment:5 by Radosław Sieradzki, 13 years ago

Triage Stage: AcceptedReady for checkin

comment:6 by Julien Phalip, 13 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

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 H0ff1, 13 years ago

Attachment: patch_2.diff added

comment:7 by H0ff1, 13 years ago

Needs tests: unset
Patch needs improvement: unset

Added a new WARNING-Icon and more tests

by H0ff1, 13 years ago

Attachment: patch_documentation.diff added

comment:9 by H0ff1, 13 years ago

Needs documentation: unset

attached documentation-patch

comment:10 by H0ff1, 13 years ago

Cc: H0ff1 added

comment:11 by Simon Bächler, 13 years ago

Cc: simon@… 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().

in reply to:  11 ; comment:12 by Claude Paroz, 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?

in reply to:  12 ; comment:13 by Russell Keith-Magee, 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.

in reply to:  13 comment:14 by Claude Paroz, 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 Claude Paroz, 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 Tome Cvitan, 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 Tome Cvitan, 12 years ago

Resolution: fixed
Status: newclosed

comment:18 by Aymeric Augustin, 12 years ago

For the record this was fixed in edf7ad36faab8d45aafe1f96feaf46794de22fc1.

Note: See TracTickets for help on using tickets.
Back to Top