Add validation error reporting system to Settings API

[jeremyclarke]

Problem

As discussed in this wp-hackers email thread - ​http://groups.google.com/group/wp-hackers/browse_thread/thread/1d70363875bd2e32 - the Settings API (which is used to add settings sections and fields to admin settings pages elegantly and automatically) has a distinct lack of ability to return useful information to users about any validation errors that occurred during the validation callback function given in register_setting().

The main problem is that the validation occurs on a seperate pageload from the final return to your settings page that the user sees (it happens on options.php, the target of forms used in the Settings API, when update_option() is called on the registered setting and thus its validation filter is run). This means that there is no way for a developer using the Settings API to designate error messages for the user from within the validation callback, because any GLOBALS, objects or other variables set during that function will be destroyed when the user is forwarded back to the original page. This forwarding, which causes the message 'Settings Saved' to be shown, regardless of how validation went, happens in options.php with the following code:

$goback = add_query_arg( 'updated', 'true', wp_get_referer() );
wp_redirect( $goback );	

Importance

This missing feature is also a huge defect of the Settings API, both because it gives no sane way for eager developers to add errors to their own settings sections/pages, but also because it fails to inspire them to do so with an elegant framework. IMHO error reporting to users should be a first-class citizen of the Settings API, and pushing error reporting in the faces of developers will cause an overall usability improvement for all WordPress plugins, not just those who's authors were already concerned about reporting validation errors.

Put another way: Usability improvements to the Settings API through easy error reporting should be treated like security improvements through mandatory validation functions: Both are functionalities that remind/inspire developers to do what they should be doing anyway, but often forget about/don't think to do on their own.

The lack of error reporting also exposes itself within the default admin options in WordPress, where no error reporting is done on the core options when they are invalid. The admin_email option in Settings > General is a perfect example. If you give it an invalid email address (no @ sign) it silently removes the entire email address during validation, then shows a 'Settings Saved' message on the resulting screen, along with an empty admin_email field. This is a very dangerous situation to say the least. Like most other core admin options a message about why validation failed is vital, but without an API way to show these errors it is essentially impossible for core settings to output errors. If my solution below is added to core outputting an error for admin_email will involve a 1-line patch to its validation.

Temporary Solution Until 3.0

This should definitely be added by the time 3.0 arrives but that probably won't be for a long time from now (dec17-09). Until then it's still very important that error messaging be possible while using the Settings API.

With the help of Ade Walker and Otto on wp-hackers I worked out an interim solution using a special field inside your option array that you check and remove while on your settings page after the processing has occurred. It is most assuredly a hack and not ideal, but it is a good interim solution for those who want to start using the Settings API but don't want to give up messages. If my solution outlined below is accepted it should be easy to convert from the hack to the API once its ready.

Code exposing the interim solution is available in this pastie.org paste: ​http://pastie.org/747439

It should also be attached to this ticket as settings_validation_errors_interim_hack.txt

Solution

This needs a solution regardless of whether I'm the one to think of it, but here is an idea.

Functions Outline

// Add errors to a global array while validating options
add_settings_error( $setting, $name, $message, $type = 'error' );

// Fetch Settings errors that have been defined on the current pageload
get_settings_errors( $setting = '' );	

// Display settings errors saved during the previous pageload
settings_errors( $setting = '' );

Specifics

A function can be added to the API with the express purpose of defining errors to show to the user when they are returned to the settings page. It should have the ability to handle multiple errors and preferably also have the ability to handle failure AND success messages, so that plugins can do all their messaging from inside the validation function (which inspires further use of validation functions even outside other elements of the Settings API involved in pages).

Something like:

add_settings_error( $setting, $name, $message, $type = 'error' );

• $setting The id of the setting the error is related to.
• $name A unique identifier for the error message like 'admin_email_fail' (used in a css class on the message and in the resulting messages array). Might be unnecessary but seems like a good idea to include.
• $message The actual message text to show
• $type One of: error, success, warning etc (ideally it would map directly to the CSS classes used on error messages).

Alternately the function could be given a more general name, though I think it might be less obvious what its for and 'error' might work better despite its mild innacuracy:

add_settings_message( $setting, $name, $message, $type = 'error' );

Either way this function would save the message/error array to a global variable comparable to $wp_settings_fields (which holds registered settings fields), maybe $wp_settings_errors (or again, $wp_settings_messages).

A complementary function would be used to fetch the errors from the global:

get_settings_errors( $setting = '' );	

• $setting An optional setting id in case you want to fetch only errors/messages related to one setting.

Passing the data back

The get_settings_errors() function would then be called in options.php just before the redirection occurs after saving options, and if errors were present it would use that data for the redirect rather than just 'updated', 'true'. Something like:

if ($errors = get_settings_errors()) {
	$goback = add_query_arg( 'settings_errors', $errors, wp_get_referer() );
} else {
	$goback = add_query_arg( 'updated', 'true', wp_get_referer() );
}
wp_redirect( $goback );		

Acutally looking at it now it seems like this won't work. We can't really pass back an array using add_query_arg(). This implies we might need to use some database storage for the settings, maybe set_transient(), that we could save the $errors to, then show them from on the next pageload.

I don't know what would be the best ways to do this, please advise. Is there some way we could pass back the array directly and more simply than using the database?

Without using Database: message registration (awkward)

One non-database solution would be to specifically register each error message with another function like register_settings_error($setting, $error_name, $error_text). Then when redirecting back to the settings page we could return a list of comma-separated $error_names with add_query_arg() that could later be shown with something like settings_error($setting, $error_name).

This strikes me as overkill though, and doubles the number of functions people need to learn to use the messaging system. If we can avoid 'message registration' then I think we should as developers will only have to know add_settings_error() to make use of it. If others think message registration is a good idea though then I'm into it, as it would make things even cleaner and more organized in exchange for the increased complexity.

DB-Based solution: set_transient('settings_errors')

The problem with using the database to store the errors sent back from options.php is that you could end up with situations where, for whatever reason, the messages are not shown to the user right away (e.g. option was saved automatically elsewhere in admin or theme), and end up being displayed later when the user returns to the settings page but hasn't submitted anything.

Thus we'd need to link the saved errors with the subsequent pageload explicitly. Maybe 'nonces' could solve it by verifying that the fields were just submitted, I'm not hip on nonces (I learned the Settings API to avoid them!) so let me know if that would be best.

Based on a quick look at uses of delete_transient() in core, transients seem to be commonly used for tasks like this, including reporting on updated plugins and core updates across pageloads. A transient based solution could go like this, replacing the current logic in options.php that I pasted at the top:

if ($errors = get_settings_errors()) {
	// Save the $errors into a transient with a short expiration
	set_transient('settings_errors', $errors, 60);
	// Forward back to the settings page with a 'settings_errors' flag
	$goback = add_query_arg( 'settings_errors', 'true', wp_get_referer() );
} else {
	$goback = add_query_arg( 'updated', 'true', wp_get_referer() );
}
wp_redirect( $goback );		

If we did it this way then upon return to the original settings page we'd have access to $_GET['settings_errors']. If it is 'true', we'd know that there were fresh errors to show stored in the 'settings_errors' transient. It would also ensure that only that subsequent pageload would show those particular errors and the transient would quickly expire and become inactive/replaced by other settings manipulations. Just in case we would also delete the transient after displaying the errors.

Displaying the errors

The last missing piece would be a display function to output the errors in standardized WP format (in case it ever changes from the current <div class="error"> system):

settings_errors( $setting = '' );

• $setting Optionally limit the displayed errors to only those of a specific option id (not used in my imagined core system but a useful addition for people using the function elsewhere)

I propose this name as comparable to settings_fields($settings), which needs to be called inside settings pages that use the Settings API, other ideas for the name of this function are welcome.

settings_errors() would run get_transient('settings_errors') to fetch any current settings errors, then display them one by one using the current html and css for admin errors. It would use the $type argument from add_settings_error() to style the message appropriately. After display it would delete any values in the 'settings_errors' transient.

The idea would be to use this during the admin_notices hook, or after the 'updated'='true' checking done in in /wp-admin/options-head.php. Something like:

if (isset($_GET['settings_errors'])) 
	settings_errors();

This would mean that developers using the API would have their messages automatically displayed at the top of their settings page, and would have done so with only one function call that was conveniently done in the midst of their option validation.

Conclusion

HOO-AH! This is one damn long ticket. Maybe I should have just coded it first but IMHO the work is mostly done by now and maybe it was easier with writing than coding. Either way, please give me some feedback on this and let me know if I'm way off course. A patch will probably be easy enough to put together that I'll actually do it, if not hopefully I gave enough detail for someone to take care of it before feature freeze on 3.0.

[jeremyclarke]

Settings Validation Interim Hack Solution

[studiograsshopper]

Replying to jeremyclarke:

I really like this, and agree that the Settings API needs an error reporting system.

A question: what would happen if the user decides not to fix the cause of the validation error immediately? Say, he/she goes off and does something else, then navigates back to the plugin's settings page, for example after the transient has expired. Would it be useful to automatically re-run the register_setting() callback on page load? It seems to me desirable that the error messages are persistent until the cause(s) of the error(s) is(are) fixed.

[jeremyclarke]

Replying to studiograsshopper:

A question: what would happen if the user decides not to fix the cause of the validation error immediately? Say, he/she goes off and does something else, then navigates back to the plugin's settings page, for example after the transient has expired. Would it be useful to automatically re-run the register_setting() callback on page load? It seems to me desirable that the error messages are persistent until the cause(s) of the error(s) is(are) fixed.

This system would activate any time update_option('your_option') is run. The idea is specifically to give feedback on invalid options that were submitted and destroyed instead of saved during validation. Usually you wouldn't want to leave half-formed or dangerous data in the options array, so you remove it and show an error.

In that situation the only persistent error you could have is a missing mandatory setting, like what happens with Akismet and its API key. IMHO situations like that are very rare, usually a default can be used in emergencies. In the rare cases where intense communication is needed I think it is fair to ask plugin developers to add their own admin_notices hook like Akismet does.

If a setting is important, I'd recommend a bit of extra logic in the add_settings_field() callback function that displays the form elements for that value. Validate the value and if its empty/imperfect and important, show a <div class="error"> block with a message about how important it is to have it set. I think it is at least theoretically fair to keep this seperate from the rest of the Settings Errors system in this ticket, as it isnt' directly related to information about setting validation, but rather about mandatory settings etc.

That said, maybe I'm not making the display function, settings_errors(), smart enough. Maybe it should look not just at get_transient('settings_errors') but ALSO at global $wp_settings_errors to find errors to display. This way a developer could decide to re-validate the data on each pageload specifically to show errors that they have built into their validator that give feedback about empty errors etc.

Using your own validation callback function directly it might look like this:

// Run the validation on existing option to populate $wp_settings_errors
my_settings_validation_callback(get_option('my_option'));	
// Show the settings errors
settings_errors('my_option');

Probably a valid addition to what is outlined in the ticket. When I work on the patch I'll keep it in mind and try to make sure that example works by making settings_errors() work in both contexts (after saving and on the same pageload as a validation session). It's also probably worth making sure that get_settings_errors() also works in both contexts (i.e. that it checks both the global and the transient) and that settings_errors() uses get_settings_errors() to fetch its errors either way.

Question: If get_settings_errors('my_option') is run (e.g. if someone manually puts settings_errors('my_option') into their settings page), should it automatically run a validation? I think maybe the solution is to check various things in order. like:

get_settings_errors( 'my_option' )

• Check global $wp_settings_errors[ 'my_option' ] and return if it exists.
• If $_GET[ 'settings_errors' ] check the get_transient value.
• If we still have nothing run the validation callback for my_option then check global $wp_settings_errors again.

It kind of gets insane but it might be a useful way to simplify the process. It would allow settings_errors('my_option') to be called on its own and in most cases work as expected. One case I think might come up is duplicate messages, some error showing in the default admin_notices one as well as in your custom settings_errors() call. Maybe we'd just need to tell people: if you use settings_errors() on your own time, make sure to do it with a check for submission errors first:

if (!$_GET['settings_errors']) settings_errors();

[studiograsshopper]

Replying to jeremyclarke:
All valid points, and I understand the distinction you are making between validation and compulsory settings.
I like the get_settings_errors( 'my_option') proposal. I think this will meet the needs I was thinking of in my earlier question.

[westi]

Marking this as an enhancement not a bug as this is not a bug in the settings code you would just like to add more functionality to it.

[hakre]

Related: #11517

[jeremyclarke]

Patch to add error reporting and documentation to Settings API

[jeremyclarke]

The patch above adds error reporting to the Settings API mostly as I described in the ticket and other comments. I've carefully documented all the functions I added so looking at the diff and files should mostly explain it. I also added documentation to many of the other Settings API functions so the system should be easier to pick up without a tutorial.

Testing the system

To test this you need to have a settings page set up with add_settings_sections() and add_settings_field(). You can add calls to add_settings_error() inside your setting's validation callback function when you notice a problem with a setting. The resulting error messages should show up when the page reloads. If there were no errors it should show the same old Settings Saved message.

The output errors have the same format as the old Settings Saved message: div->p->strong, The div has a custom ID and several classes for CSS. It is set up to take advantage of the 'error' and 'updated' CSS classes from the admin, which is why the two $type's for errors are error and updated. Error will be red and updated will be yellow.

The other use-case for this is to add settings errors inside your validator that check the data even when its not being saved. Things like checking for a missing option and complaining until it gets set. If you use settings_errors() yourself inside your setting page function you can make it show these errors with its $sanitize function, which will re-sanitize the setting without saving it then show you any errors that resulted. Using the $hide_on_update setting is a good idea if you do this, as it will stop the manual error display from showing if they are already showing in the default post-submission process.

New globals and storage

• $wp_settings_errors - Global storage array to hold errors logged during the current pageview
• set_transient('settings_errors') transient storage array used to pass errors back from processing on options.php

wp-admin/includes/template.php

Add new API functions and update docs

• Register a settings error to be displayed to the user

  add_settings_error( $setting, $id, $message, $type = 'error' )
  

• Fetch settings errors registered by add_settings_error()

  get_settings_errors( $setting = '', $sanitize = FALSE )
  

• Display settings errors registered by add_settings_error()

  settings_errors($setting = '', $sanitize = FALSE, $hide_on_update = FALSE )
  

wp-admin/options-head.php

• Show settings errors in options-head.php so it always runs on settings pages.

Previously there was a generic 'Settings saved' message that would show anytime $_GET[ 'updated' ], even if the saved settings had been validated out of existence. Now a call to settings_errors() is there, so that any errors registered so far that pageload or on the previous pageload get shown. The generic message is now registered as a settings error using the API in wp-admin/options.php

wp-admin/options.php

• Use settings errors to return back to settings pages instead of just returning 'updated'.

This file has the place where the user's settings page submission is forwarded back to the page they started on. It needed to be modified to save any settings registered during the setting processing (using its $sanitize_callback function as defined in register_setting()) ) into a transient before forwarding back. The patch also adds the 'settings saved' message to the registered errors before saving them, so that the generic updated message will automatically be shown when settings_errors() is called on the settings page.

wp-includes/formatting.php

• Use settings errors during the validation of the admin_email option hard-coded in sanitize_option()

This is an example of how the system is used as well as a fix for a particularly big hole in how settings saving currently works. Without this patch users are able to enter an empty or even just malformed (no @ sign etc.) admin email value and effectively erase all signs of an admin email, without even being shown an error to indicate that they have erased the admin email which is a bad idea.

Once the system is set in place it will be goood to consider what other core settings might need better error messaging about innapropriate values.

WP_ERROR

Anyone have an opinion about this? The current patch doesn't use WP_Error class. Should it? Anyone have an idea for how it should be used?

Still TO DO

These are things i know still need doing. Please let me know if you think something else is missing. I will make a follow up patch that includes them and responds to any feedback.

• add a filter to get_settings_errors()
• add an action to settings_errors()

Hopefully this can get in before the feature freeze. I will probably be able update it more based on any requests.

[jeremyclarke]

I spent some time trying to integrate WP_Error class into this system but its not very well designed for this purpose. Specifically, the raw data it saves is organized but inconvenient to access, and there is no display function to show errors.

After investigating how WP_Error errors are shown in the admin I found the show_message() function which is sometimes used by core but is flimsy and insufficient for working with complex WP_Error objects. Ticket #12254 is an outline of how show_message can be fixed to work with any WP_Error object and make showing errors from inside very simple. If a patch to that ticket was committed I think integrating WP_Error into the settings API would make a lot more sense.

For now I think its better off the way it is and should be committed in this format. If/when show_message is updated it will be easy and back-compatible to convert it to use WP_Error.

[ryan]

(In [13177]) Option validation error reporting. Props jeremyclarke. see #11474

[ryan]

Fixed some warnings and added a function_exists check on the call to add_settings_error in sanitize_option. add_settings_error is defined in wp-admin/includes but is being called from a function in wp-includes. One warning remains. add_settings_error references $title, which is not defined anywhere. $id is defined by not used. I wasn't sure of the intent so I marked the line with an @todo and left it as is.

[ptahdunbar]

Replying to ryan:

add_settings_error references $title, which is not defined anywhere. $id is defined by not used. I wasn't sure of the intent so I marked the line with an @todo and left it as is.

With WP_DEBUG on, it breaks all forms that rely on options.php. I'd use $id until a proper fix is patched.

[hakre]

Commit was done after feature freeze. Revert?

[jeremyclarke]

I'll fix the patch very soon, sorry guys. I told Ryan on IRC but didn't reply here.

[jeremyclarke]

Fix bugs in Settings API Errors patch

[jeremyclarke]

The new patch fixes the problem with the undefined index. Sorry about that, rboren was right that I had stupidly changed the function's argument without checking my uses of it. I've replaced the $id/$title argument with one called $code, which is how the slug-names of errors are referred to in WP_Error class, so this system is that much closer to integrating with WP_Error.

rboren: I also changed the way the function_exists() call is done in sanitize_option() because your setup was actually returning "1" as the admin email when it was valid (because it was setting $value to the result of the &&). I moved the actual sanitize_email() onto its own line and everything seems to be working.

ptahdunbar really sorry that it was actually breaking things for you. If I'd known I would have hussled with the patch.

rboren let me know if there's anything else wrong with it.

[ryan]

(In [13197]) add_settings_error fixes. Props jeremyclarke. fixes #11474

[pulser1983]

Hi Friends,

I Have a suggestion regarding get_settings_errors($setting).

Suppose get_settings_errors($setting) give me this output.

Array
(
    [0] => Array
        (
            [setting] => rt-option
            [code] => email 
            [message] =>  email is invalid.
            [type] => error
        )

    [1] => Array
        (
            [setting] => rt-option
            [code] => name
            [message] => name cannot be empty.
            [type] => error
        )

)

I couldn't find a fay to fetch the message for name directly.

My proposal is get_settings_errors() should accept three argument, example..

 get_settings_errors( $setting = '', $code = '', $sanitize = FALSE )

This way user can print error message near the input box directly. Also printing the error message on header is not always helpful.

I hope you take my point into consideration.

Thanks
Radhe

[westi]

Please don't re-open completed enhancements which have already shipped.

Please open a new ticket with your suggestions - you question is probably best asked in the support forums first