4

I have a error handling mechanism setup in one of my plugins to add notices and errors to the admin area, just like the core does. It works fine in most cases, but there are some situations (like saving a custom post type) where it doesn't. I'm guessing that a redirect is happening behind the scenes, and the messages are being printed before the redirect happens, so that they appear to never show up.

So, I'm guessing this is what's happening

  1. User edits a custom post type and hits Publish
  2. My post_updated callback is called, which validates and saves the custom fields
  3. The callback adds an error message
  4. Wordpress does a redirect to a some page to do some processing
  5. My admin_notices callback is called, which prints and clears the messages
  6. Wordpress redirects back to the post
  7. My admin_notices callback is called again, but there are no messages to print because they were printed in step #5

Under normal circumstances steps 4 and 5 don't happen, so everything works fine, but I think when Wordpress saves posts it introduces the extra redirect. Is something I can do to make sure this always works? I was thinking I might be able to check something inside printMessages() and return immediately if it's at step 4, but I'm not sure what.

These two questions may shed some light on the problem, but don't fully give a solution: Add validation and error handling when saving custom fields?, How to display admin error notice if settings saved succesfully?.

Here's the code:

/**
 * Constructor
 * @author Ian Dunn <[email protected]>
 */
public function __construct()
{
    // Initialize variables
    $defaultOptions         = array( 'updates' => array(), 'errors' => array() );
    $this->options          = array_merge( get_option( self::PREFIX . 'options', array() ), $defaultOptions );
    $this->updatedOptions   = false;
    $this->userMessageCount = array( 'updates' => 0, 'errors' => 0 );
    // more

    add_action( 'admin_notices',    array($this, 'printMessages') );
    add_action( 'post_updated',     array($this, 'saveCustomFields') );

    // does other stuff
}

/**
 * Saves values of the the custom post type's extra fields
 * @author Ian Dunn <[email protected]>
 */
public function saveCustomFields()
{
    // does stuff

    if( true ) // if there was an error
        $this->enqueueMessage( 'foo', 'error' );
}

/**
 * Displays updates and errors
 * @author Ian Dunn <[email protected]>
 */
public function printMessages()
{
    foreach( array('updates', 'errors') as $type )
    {
        if( $this->options[$type] && ( self::DEBUG_MODE || $this->userMessageCount[$type] ) )
        {
            echo '<div id="message" class="'. ( $type == 'updates' ? 'updated' : 'error' ) .'">';
            foreach($this->options[$type] as $message)
                if( $message['mode'] == 'user' || self::DEBUG_MODE )
                    echo '<p>'. $message['message'] .'</p>';
            echo '</div>';

            $this->options[$type] = array();
            $this->updatedOptions = true;
            $this->userMessageCount[$type] = 0;
        }
    }
}

/**
 * Queues up a message to be displayed to the user
 * @author Ian Dunn <[email protected]>
 * @param string $message The text to show the user
 * @param string $type 'update' for a success or notification message, or 'error' for an error message
 * @param string $mode 'user' if it's intended for the user, or 'debug' if it's intended for the developer
 */
protected function enqueueMessage($message, $type = 'update', $mode = 'user')
{
    array_push($this->options[$type .'s'], array(
        'message' => $message,
        'type' => $type,
        'mode' => $mode
    ) );

    if($mode == 'user')
        $this->userMessageCount[$type . 's']++;

    $this->updatedOptions = true;
}

/**
 * Destructor
 * Writes options to the database
 * @author Ian Dunn <[email protected]>
 */
public function __destruct()
{
    if($this->updatedOptions)
        update_option(self::PREFIX . 'options', $this->options);
}   

Update: The updated code with the accepted answer has been committed to core.php in the plugin's trunk in case anyone wants to see a full working copy. The next stable release that will have it is 1.2.

Update 2: I've abstracted this functionality into a self-contained library that you can include in your plugin. Core is discussing including similar functionality in #11515.

3
  • Are you completely sure this is caused by redirect? You can try something like Better HTTP Redirect to confirm.
    – Rarst
    Commented Jun 27, 2011 at 16:51
  • @Rarst - I'm not sure, so I'll try out that plugin. Thanks :)
    – Ian Dunn
    Commented Jun 27, 2011 at 17:28
  • I installed the plugin and confirmed that a redirect is happening. That doesn't 100% prove that the redirect is the problem, but it would make sense. I still need to figure out a way to solve the problem, though.
    – Ian Dunn
    Commented Jun 28, 2011 at 3:41

2 Answers 2

2
+50

There are few things that I have pointed out in the code below too:

  1. You were overwriting the options read from get_option using array_merge
  2. You had hard-coded the Message Counts.
  3. saving options in __destruct just does not work. (I don't have any clue yet, may be experts will shed some light on it.

I have marked all the sections where I have made the changes with HKFIX, with a bit of description:

/**
 * Constructor
 * @author Ian Dunn <[email protected]>
 */
public function __construct()
{

    // Initialize variables
    $defaultOptions         = array( 'updates' => array(), 'errors' => array() );

    /* HKFIX: array_merge was overwriting the values read from get_option, 
     * moved $defaultOptions as first argument to array_merge */
    $this->options          = array_merge( $defaultOptions, get_option( self::PREFIX . 'options', array() ) );
    $this->updatedOptions   = false;

    /* HKFIX: the count for update and error messages was hardcoded,
     * which was ignoring the messages already in the options table read above
     * later in print the MessageCounts is used in loop
     * So I updated to set the count based on the options read from get_option */
    $this->userMessageCount = array();
    foreach ( $this->options as $msg_type => $msgs ) {
        $this->userMessageCount[$msg_type] = count( $msgs );
    }
    // more

    add_action( 'admin_notices',    array($this, 'printMessages') );
    add_action( 'post_updated',     array($this, 'saveCustomFields') );

    // does other stuff
}

/**
 * Saves values of the the custom post type's extra fields
 * @author Ian Dunn <[email protected]>
 */
public function saveCustomFields()
{
    // does stuff

    /* HKFIX: this was false, so changed it to true, may be not a fix but thought I should mention ;) */
    if( true )
        $this->enqueueMessage( 'foo', 'error' );

}

/**
 * Displays updates and errors
 * @author Ian Dunn <[email protected]>
 */
public function printMessages()
{

    foreach( array('updates', 'errors') as $type )
    {
        if( $this->options[$type] && ( self::DEBUG_MODE || $this->userMessageCount[$type] ) )
        {
            echo '<div id="message" class="'. ( $type == 'updates' ? 'updated' : 'error' ) .'">';
            foreach($this->options[$type] as $message)
                if( $message['mode'] == 'user' || self::DEBUG_MODE )
                    echo '<p>'. $message['message'] .'</p>';
            echo '</div>';

            $this->options[$type] = array();
            $this->updatedOptions = true;
            $this->userMessageCount[$type] = 0;

        }
    }

    /* HKFIX: Save the messages, can't wait for destruct */
    if ( $this->updatedOptions ) {
        $this->saveMessages();
    }

}

/**
 * Queues up a message to be displayed to the user
 * @author Ian Dunn <[email protected]>
 * @param string $message The text to show the user
 * @param string $type 'update' for a success or notification message, or 'error' for an error message
 * @param string $mode 'user' if it's intended for the user, or 'debug' if it's intended for the developer
 */
protected function enqueueMessage($message, $type = 'update', $mode = 'user')
{

    array_push($this->options[$type .'s'], array(
        'message' => $message,
        'type' => $type,
        'mode' => $mode
    ) );


    if($mode == 'user')
        $this->userMessageCount[$type . 's']++;

    /* HKFIX: save the messages, can't wait for destruct */
    $this->saveMessages();
}

/* HKFIX: Dedicated funciton to save messages 
 * Can also be called from destruct if that is really required */
public function saveMessages() 
{
        update_option(self::PREFIX . 'options', $this->options);
}

/**
 * Destructor
 * Writes options to the database
 * @author Ian Dunn <[email protected]>
 */
public function __destruct()
{
    /* HKFIX: Can't rely on saving options in destruct, this just does not work */
        // its very late to call update_options in destruct
        //update_option(self::PREFIX . 'options', $this->options);

}
7
  • I'll try those changes out, but I don't think any of them really affect the core problem, which is the redirect. printMessages() is called each time the plugin is instantiated, so the messages are printed before the redirect and cleared, so they never get seen after the redirect.
    – Ian Dunn
    Commented Jun 28, 2011 at 14:30
  • I have checked it and it is working for me. The redirect actually does not call functions hooked to admin_notices but it does instantiate the plugin. And your construct was overwriting the options and hence they were never getting printed. Commented Jun 28, 2011 at 14:33
  • I put all those changes into my code and it didn't change anything (see pastebin.com/jGjsk4SL). Everything works fine when there isn't a redirect, so I don't think that the array_merge, destruct, etc are the problems. The only time it doesn't work is when there's a redirect.
    – Ian Dunn
    Commented Jun 28, 2011 at 18:45
  • You forgot to update the options in enqueueMessage, see: pastebin.com/C5GXWcx9 Commented Jun 28, 2011 at 19:58
  • Ah, ok, I missed that one. I don't want to call it in enqueueMessage() because that could result in several database queries instead of just one at the end. So, instead I hooked into the shutdown event and that seems to be working. I want to do some more testing to make sure, but it looks like this will work. Thanks :)
    – Ian Dunn
    Commented Jun 28, 2011 at 21:04
0

I currently don't have a clue what's going on with your plugin, so I point you at two things:

wp_parse_args() is a nice way to merge defaults with other arguments.

private $defaults;

function wpse20130_parse_us( $args );
{
    $new_args = wp_parse_args( $this->defaults, $args );
    return $new_args;
}

And this Plugin is a little closer to how core handles errors (straight out of my head - may contain errors itself):

EDIT: Test Plugin

<?php
/**
Plugin Name:    WPSE Show Error on post
Plugin URI:     https://github.com/franz-josef-kaiser/
Description:    Example for the useage of the WP Error class in a plugin
Author:         Franz Josef Kaiser
Author URI:     https://github.com/franz-josef-kaiser
Version:        0.1
License:        GPL v2 - http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
*/

// Secure: doesn't allow to load this file directly
if( ! class_exists('WP') ) 
{
    header( 'Status: 403 Forbidden' );
    header( 'HTTP/1.1 403 Forbidden' );
    exit;
}

if ( ! class_exists('wpse20130Error') )
{

class wpse20130Error
{
    private $args = array();

    private $error_msg;

    const TEXTDOMAIN = 'textdomain';

    function __construct()
    {
        $this->wpse20130_input( $this->args );

        add_action( 'admin_notices', array($this, 'wpse20130_trigger_error') );
    }

    function wpse20130_input( $args )
    {
        if ( ! isset( $args['some_important_value'] ) )
            $this->error_msg = sprintf(
                __(
                    'You have to specify the some_important_value inside the %2$s function.'.'<br />'.
                    'Error triggered inside: file name %1$s (line number %3$s)'
                    ,self::TEXTDOMAIN
                )
                ,__FILE__
                ,__FUNCTION__
                ,__LINE__
            );
        }

    function wpse20130_trigger_error()
    {
        // Trigger Errors if we got some
        if ( isset( $this->error_msg ) )
        {
            $error = new WP_Error( 'input_data', $this->error_msg );
            if ( is_wp_error( $error ) ) 
            {
                $output = 
                    '<div id="error-'.$error->get_error_code().'" class="error error-notice">'.
                        $error->get_error_message().
                    '</div>';

                // die & print error message
                echo $output;
            }
        }
    }
} // END Class wpse20130Error

new wpse20130Error();
} // endif;
?>

Give it a try. :)

2
  • I don't think there's anything in that code that helps me. I don't want to die() or echo the error immediately, I want to hook into admin_notices so that it'll be displayed in the normal message/error location. I also want to be able to call it from any place in the code, not just when saving a post, so it has to run every time the plugin is instantiated. I think using WP_Error() is a good idea, but it doesn't solve the redirect issue.
    – Ian Dunn
    Commented Jun 16, 2011 at 14:50
  • See edit. I reworked it as a plugin to show you how it works with the WP Error class. I know it doesn't solve your issue, but i already stated that in my first line in the answer. I have to get around this myself, so maybe i can give you a better answer in a week or so.
    – kaiser
    Commented Jun 16, 2011 at 18:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.