Opened 4 years ago

Closed 2 years ago

Last modified 2 years ago

#16884 closed New feature (fixed)

Add message level argument to ModelAdmin's message_user

Reported by: shelldweller Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords: admin
Cc: rafallo, 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 shelldweller 4 years ago.
patch.diff (9.5 KB) - added by rafallo 3 years ago.
patch_2.diff (12.0 KB) - added by H0ff1 3 years ago.
patch_documentation.diff (1.4 KB) - added by H0ff1 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 4 years ago by Alex

  • Component changed from Uncategorized to contrib.admin
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by shelldweller

comment:2 Changed 4 years ago by shelldweller

It appears that CSS classes and images are already there.

comment:3 Changed 4 years ago by ramiro

#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 Changed 3 years ago by rafallo

  • Cc rafallo 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.

Changed 3 years ago by rafallo

comment:5 Changed 3 years ago by rsiera

  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 3 years ago by julien

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to 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.

Changed 3 years ago by H0ff1

comment:7 Changed 3 years ago by H0ff1

  • Needs tests unset
  • Patch needs improvement unset

Added a new WARNING-Icon and more tests

Changed 3 years ago by H0ff1

comment:9 Changed 3 years ago by H0ff1

  • Needs documentation unset

attached documentation-patch

comment:10 Changed 3 years ago by H0ff1

  • Cc H0ff1 added

comment:11 follow-up: Changed 3 years ago by sbaechler

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

comment:12 in reply to: ↑ 11 ; follow-up: Changed 3 years ago by claudep

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?

comment:13 in reply to: ↑ 12 ; follow-up: Changed 3 years ago by russellm

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 in reply to: ↑ 13 Changed 3 years ago by claudep

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 Changed 3 years ago by claudep

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 Changed 2 years ago by tome

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 Changed 2 years ago by tome

  • Resolution set to fixed
  • Status changed from new to closed

comment:18 Changed 2 years ago by aaugustin

For the record this was fixed in edf7ad36faab8d45aafe1f96feaf46794de22fc1.

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