WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 5 years ago

#11474 closed enhancement

Add validation error reporting system to Settings API — at Initial Version

Reported by: jeremyclarke Owned by: jeremyclarke
Milestone: 3.0 Priority: high
Severity: normal Version: 2.9
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description

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.

Change History (1)

@jeremyclarke6 years ago

Settings Validation Interim Hack Solution

Note: See TracTickets for help on using tickets.