[wp-trac] [WordPress Trac] #11474: Add validation error reporting system to Settings API
WordPress Trac
wp-trac at lists.automattic.com
Fri Dec 18 16:12:00 UTC 2009
#11474: Add validation error reporting system to Settings API
----------------------------+-----------------------------------------------
Reporter: jeremyclarke | Owner: jeremyclarke
Type: defect (bug) | Status: new
Priority: high | Milestone: 3.0
Component: Administration | Version: 2.9
Severity: normal | Keywords: needs-patch
----------------------------+-----------------------------------------------
Comment(by jeremyclarke):
Replying to [comment:1 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();
}}}
--
Ticket URL: <http://core.trac.wordpress.org/ticket/11474#comment:3>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software
More information about the wp-trac
mailing list