Make WordPress Core

Opened 13 years ago

Last modified 6 years ago

#19691 new enhancement

Cannot modify admin messages for /wp-admin/edit.php

Reported by: mikeschinkel's profile mikeschinkel Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Administration Keywords: dev-feedback has-patch 2nd-opinion
Focuses: Cc:

Description

The admin console messages output on line 264 of WordPress 3.3's file /wp-admin/edit.php are not filterable. This causes problems when added row actions need to HTTP GET from to WordPress to modify a post and then display an appropriate message complete with a revert link (like the "Trash" link does.)

An example use-case could be for a custom post type used for both quotes and invoices where a row action might be "Convert Quote to Invoice" where you'd want a message and link displayed at the top of the admin after similar to this:

  • Quote #{$post_id} converted to Invoice. Revert

Currently the only way to accomplish this is to pick hooks before and after the messages are output and use PHP's output buffering; clearly not a "best practice" approach.

In order to address this I'm proposing an 'admin_messages' filter hook to run just before the messages are output:

$messages = apply_filters( 'admin_messages', $messages );

However, since messages are output in numerous locations in the WordPress admin it seemed best to add the hook in every location where messages are output, which is what my patch does. Thus a hook can look at $pagenow or get_current_screen() to decide it is needs to do anything.

Also while searching for places in the admin code that echo messages I found $messages are sometimes an array of HTML where the entire array is echoed and other times the $messages are an array with an index passed via $_GET and only one message will be displayed. For those cases I created another hook 'admin_message' (note that this hook name is singular):

$message = apply_filters( 'admin_message', $message, $messages, $_GET['message'] );

I really only found a specific need for /wp-admin/edit.php today, but it seemed that it would be better for consistency if all messages were made hook filterable. That's why I created a larger patch when all my use-case needs is one new line.

Looking forward to your feedback.

Attachments (3)

admin_messages_hook.diff (5.7 KB) - added by mikeschinkel 13 years ago.
Adds 'admin_messages' and 'admin_message' hooks.
admin_messages_hook-2.diff (6.6 KB) - added by mikeschinkel 13 years ago.
Adds 'admin_messages' hook.
19691.diff (8.9 KB) - added by MikeSchinkel 9 years ago.
Added unique admin message hooks for each use-case.

Download all attachments as: .zip

Change History (12)

@mikeschinkel
13 years ago

Adds 'admin_messages' and 'admin_message' hooks.

#1 @naomicbush
13 years ago

  • Cc naomicbush added

#2 follow-up: @mbijon
13 years ago

  • Cc mike@… added

+1 to add admin_message as-is.

For admin_messages, I'm not sure that requiring plugin devs to test $page now or get_current_screen is a good idea. That's a very different requirement from most filters. Could we have a different hook per-location?


And, if we keep admin_messages like it is now, could it be renamed to more explicitly than just by pluralizing, maybe 'before_all_admin_messages' or 'before_multiple_admin_messages' ?

#3 in reply to: ↑ 2 @mikeschinkel
13 years ago

Replying to mbijon:

For admin_messages, I'm not sure that requiring plugin devs to test $page now or get_current_screen is a good idea. That's a very different requirement from most filters. Could we have a different hook per-location?

While I don't have strong feelings about using my patch as-is, I will disagree that requiring a $pagenow test is different from most filters. I just searched our main plugin code base and we have over 25 places where I had to test $pagenow in order to get the correct behavior.

That said, I don't prefer having to check $pagenow because it requires exception-based programming which is prone to side-effects and errors. One solution might be to use "{$pagenow}-admin_messages" but I'd hate to loose the generic "admin_messages" in case the use-case is not $pagenow specific.

And, if we keep admin_messages like it is now, could it be renamed to more explicitly than just by pluralizing, maybe 'before_all_admin_messages' or 'before_multiple_admin_messages' ?

Good point about making them easier to differentiate, although those hooks names are a bit long, no? How about:

  • admin_message_list and admin_message

Or

  • admin_messages and indexed_admin_message

Or

  • ...?

#4 follow-up: @mbijon
13 years ago

I actually like "{$pagenow}-admin_messages". That's better than making a unique hook per-location.

You're right about checking $pagenow being more common lately too. The generic name is probably simplest given how many hooks I can barely remember at times. Maybe we keep the generic one, but rename to 'before_admin_messages'.

#5 in reply to: ↑ 4 @mikeschinkel
13 years ago

  • Cc mikeschinkel@… added

Replying to mbijon:

I can barely remember at times. Maybe we keep the generic one, but rename to 'before_admin_messages'.

Hmm. Doesn't labeling a hook with 'before_*' imply that there is also an 'after_*'? Having that "before" indicator without an "after" would confuse me more than not.

When I made the patch I was only trying to tackle the plural use-case and threw in the singular use-case because I found it while searching code. Your comments have made me question the difference between the two use-cases (plural and singular) and the best I could come up with is that there is only an implementation difference, not a conceptual one.

So I've updated the patch to always use 'admin_messages', even for the singular case because it's easily conceivable someone might want to add a message to an area that currently only has single messages. Thus the patch converts all single messages to an array and passes thru the 'admin_messages' filter. This created one situation where we ideally need an 'error_messages' filter too, which the patch has too.

In reviewing all the message handling code in WordPress core's admin area it's painfully obvious that it should all be consolidated into a singular message handling function, but I won't tackle that without the core devs blessing first.

@mikeschinkel
13 years ago

Adds 'admin_messages' hook.

#6 @mbijon
13 years ago

I was considering suggesting an after filter too. I couldn't think of a good use-case though.

Getting rid of admin_message and your good points about too-long names makes admin_messages sound better.

+1 on this version

#7 @Offereins
12 years ago

+1 Can this come to 3.6?

Edit: I do favor another approach, while enabling more possibilities for plugin devs. Introducing the do_action( 'before_post_list_table', $post_type ) hook on edit.php, just before inserting the views. This way custom messages can be added as devs do please; errors, updates or otherwise, and also other elements to the edit.php screen. I've got a current project that could use just that.

Last edited 12 years ago by Offereins (previous) (diff)

#8 @chriscct7
10 years ago

  • Keywords needs-refresh added; has-patch 2nd-opinion removed

@MikeSchinkel
9 years ago

Added unique admin message hooks for each use-case.

#9 @MikeSchinkel
9 years ago

  • Keywords has-patch 2nd-opinion added; needs-refresh removed

In revising the patch I decided to take it a different direction. Rather than add one hook that tried to normalize across each use-case I've named a different hook for each use-case. In hindsight I think that makes a lot more sense and will minimize unintentional side efforts when people use these hooks.

It also means I did not have to enable single message locations to output multiple messages, which means no chance for backward compatibility issues.

These are the multi-message filters added:

  • 'site_info_admin_messages'
  • 'new_site_admin_messages'
  • 'site_settings_admin_messages'
  • 'edit_comments_admin_messages'
  • 'nav_menu_admin_messages'
  • 'new_user_admin_errors'
  • 'users_admin_messages'

These are the single message filters added:

  • 'edit_tag_admin_message'
  • 'upload_admin_message'
  • 'widget_admin_message'
  • 'widget_admin_error'
Note: See TracTickets for help on using tickets.