Skip to content

Commit d71696c

Browse files
arraypressclaude
andcommitted
Show what the field set refused
A required field left empty, or a value a rule turned down, used to save as nothing and say nothing. The refusal now goes to add_settings_error() under the field key, the stored value stays, and on the way back from options.php the field is marked and its message shown beside it. Core adds "Settings saved." only when nothing was registered, so a rejected field is never reported as saved. A whole-value write keeps a refused field's stored value too, which the field set promises. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Kwx2STJ4EaVaJofEJJ9Zo
1 parent 99ac942 commit d71696c

4 files changed

Lines changed: 305 additions & 17 deletions

File tree

‎README.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ the per-type sanitising follow from it.
1919
* Group fields into tabs and sections, saving one tab without touching others
2020
* Read and write a value by name, without knowing how the option is shaped
2121
* Sanitise per field type, rather than writing a callback for each
22+
* Refuse a value that is missing or malformed, and say which field
2223
* Encrypt a field, so an API key is not stored in the clear
2324
* Export, import and reset from Screen Options
2425
* Show a field only when another is set
@@ -58,6 +59,22 @@ $name = get_setting_field_value( 'my_plugin', 'site_name' );
5859
Field types come from `arraypress/wp-field-kit`, so anything it renders can go
5960
in a settings page.
6061

62+
## Validation
63+
64+
Sanitising coerces; it does not refuse. A field that must hold something says
65+
`'required' => true`, and one that must hold a particular shape names a rule
66+
with `validate` — `email`, `url`, `numeric`, `integer`, `slug`, `alphanumeric`,
67+
or a callable returning `true`, a message or a `WP_Error`:
68+
69+
```php
70+
'contact' => [ 'type' => 'text', 'label' => __( 'Contact', 'my-plugin' ), 'required' => true, 'validate' => 'email' ],
71+
```
72+
73+
A value that fails is not stored. The field keeps what it held, every other
74+
field on the tab is saved, and after the redirect the message appears as an
75+
error notice at the top of the page — in place of "Settings saved." — and
76+
again under the field itself.
77+
6178
## Requirements
6279

6380
* PHP 8.3 or later

‎src/SettingFields.php‎

Lines changed: 87 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,48 @@ public function sanitize( mixed $input ): array {
426426
// eaten by the save that stores it.
427427
$set->save( wp_slash( $input ) );
428428

429-
return $this->ordered_like_stored( $collector->values() );
429+
return $this->ordered_like_stored( $this->with_refused( $set, $collector->values() ) );
430+
}
431+
432+
/**
433+
* Report each field the set refused, and keep what it held.
434+
*
435+
* The messages go to the Settings API under this page's option name,
436+
* which is where options.php looks once every option is updated: it adds
437+
* "Settings saved." only when nothing has been registered, carries
438+
* whatever has into the transient the next load reads, and
439+
* settings_errors() prints them at the top of the page. A refused email
440+
* address is therefore reported as such rather than announced as saved.
441+
* Guarded, because the Settings API is admin-only and this gate also
442+
* runs for a write from cron or a webhook, where there is no page to
443+
* report to.
444+
*
445+
* The stored value is put back by hand for the write that would
446+
* otherwise lose it. A form save seeds the collector with everything
447+
* stored and the set leaves a failing field alone, so it survives on its
448+
* own; a whole-value write seeds no field at all, and without this the
449+
* value would be dropped — which is the one thing the kit promises a
450+
* refused value is not.
451+
*
452+
* @param FieldSet $set The set that just saved.
453+
* @param array<string, mixed> $values What it collected.
454+
*
455+
* @return array<string, mixed>
456+
*/
457+
private function with_refused( FieldSet $set, array $values ): array {
458+
$stored = $this->options->values();
459+
460+
foreach ( $set->errors() as $key => $message ) {
461+
if ( array_key_exists( $key, $stored ) ) {
462+
$values[ $key ] = $stored[ $key ];
463+
}
464+
465+
if ( function_exists( 'add_settings_error' ) ) {
466+
add_settings_error( (string) $this->config['option_name'], (string) $key, $message, 'error' );
467+
}
468+
}
469+
470+
return $values;
430471
}
431472

432473
/**
@@ -618,7 +659,9 @@ public function render_page(): void {
618659
echo '<div class="wrap field-kit__settings">';
619660

620661
// No argument: core adds its own "Settings saved." under the `general`
621-
// slug, and asking for this page's slug alone would hide it.
662+
// slug, and asking for this page's slug alone would hide it. A field
663+
// the last save refused is printed here as well, and again under the
664+
// field itself.
622665
settings_errors();
623666

624667
printf( '<form method="post" action="%s">', esc_url( admin_url( 'options.php' ) ) );
@@ -634,7 +677,7 @@ public function render_page(): void {
634677
esc_attr( $tab )
635678
);
636679

637-
$this->render_sections( $tab );
680+
$this->render_sections( $tab, $this->field_errors() );
638681

639682
if ( $this->config['submit_button'] ) {
640683
submit_button();
@@ -643,14 +686,41 @@ public function render_page(): void {
643686
echo '</form></div>';
644687
}
645688

689+
/**
690+
* The messages the last save registered against this page's fields.
691+
*
692+
* Read back from the Settings API rather than from the set that saved,
693+
* because that set belonged to the previous request: options.php saves,
694+
* stores what was registered in a transient and redirects, and
695+
* get_settings_errors() is what reads that transient on the load that
696+
* follows. Only codes that name a field are kept — the import and reset
697+
* notices are registered under the same slug and are not about a field.
698+
*
699+
* @return array<string, string> Keyed by field.
700+
*/
701+
private function field_errors(): array {
702+
$errors = [];
703+
704+
foreach ( (array) get_settings_errors( (string) $this->config['option_name'] ) as $error ) {
705+
$code = (string) ( $error['code'] ?? '' );
706+
707+
if ( 'error' === (string) ( $error['type'] ?? '' ) && isset( $this->fields[ $code ] ) ) {
708+
$errors[ $code ] = (string) ( $error['message'] ?? '' );
709+
}
710+
}
711+
712+
return $errors;
713+
}
714+
646715
/**
647716
* Render one tab's sections and fields.
648717
*
649-
* @param string $tab Tab slug.
718+
* @param string $tab Tab slug.
719+
* @param array<string, string> $errors Messages from the last save, keyed by field.
650720
*
651721
* @return void
652722
*/
653-
private function render_sections( string $tab ): void {
723+
private function render_sections( string $tab, array $errors ): void {
654724
$set = $this->set( $tab );
655725
$fields = $set->fields();
656726
$shown = [];
@@ -680,15 +750,15 @@ private function render_sections( string $tab ): void {
680750
printf( '<p class="description">%s</p>', wp_kses_post( (string) $section['description'] ) );
681751
}
682752

683-
$this->render_table( $set, $in_section, (string) $slug );
753+
$this->render_table( $set, $in_section, (string) $slug, $errors );
684754
}
685755

686756
// A field no section claimed still has to appear, or a typo in a
687757
// section name silently removes it from the page.
688758
$loose = array_diff_key( $fields, array_flip( $shown ) );
689759

690760
if ( [] !== $loose ) {
691-
$this->render_table( $set, $loose, '' );
761+
$this->render_table( $set, $loose, '', $errors );
692762
}
693763
}
694764

@@ -699,10 +769,11 @@ private function render_sections( string $tab ): void {
699769
* @param Field[] $fields The fields to render.
700770
* @param string $scope The section these belong to, or empty for the
701771
* fields no section claimed.
772+
* @param array<string, string> $errors Messages from the last save, keyed by field.
702773
*
703774
* @return void
704775
*/
705-
private function render_table( FieldSet $set, array $fields, string $scope ): void {
776+
private function render_table( FieldSet $set, array $fields, string $scope, array $errors ): void {
706777
$layout = Sections::split( $fields );
707778

708779
if ( [] !== $layout ) {
@@ -712,13 +783,13 @@ private function render_table( FieldSet $set, array $fields, string $scope ): vo
712783
// first one's panels.
713784
$sections = Sections::render(
714785
$layout,
715-
function ( array $group ) use ( $set ): string {
786+
function ( array $group ) use ( $set, $errors ): string {
716787
if ( [] === $group ) {
717788
return '';
718789
}
719790

720791
ob_start();
721-
$this->render_rows( $set, $group );
792+
$this->render_rows( $set, $group, $errors );
722793

723794
return (string) ob_get_clean();
724795
},
@@ -730,18 +801,19 @@ function ( array $group ) use ( $set ): string {
730801
return;
731802
}
732803

733-
$this->render_rows( $set, $fields );
804+
$this->render_rows( $set, $fields, $errors );
734805
}
735806

736807
/**
737808
* A set of fields as one settings table.
738809
*
739-
* @param FieldSet $set The field set.
740-
* @param Field[] $fields The fields to render.
810+
* @param FieldSet $set The field set.
811+
* @param Field[] $fields The fields to render.
812+
* @param array<string, string> $errors Messages from the last save, keyed by field.
741813
*
742814
* @return void
743815
*/
744-
private function render_rows( FieldSet $set, array $fields ): void {
816+
private function render_rows( FieldSet $set, array $fields, array $errors ): void {
745817
echo '<table class="form-table" role="presentation"><tbody>';
746818

747819
foreach ( $fields as $field ) {
@@ -793,7 +865,7 @@ private function render_rows( FieldSet $set, array $fields ): void {
793865
// The header cell is the visible heading here, so the kit
794866
// draws none: a group keeps its legend, hidden, so the
795867
// grouping is still announced without appearing twice.
796-
$set->render_field( $field, '', false ) // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- the kit escapes as it builds.
868+
$set->render_field( $field, $errors[ $field->key() ] ?? '', false ) // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- the kit escapes as it builds.
797869
);
798870
}
799871

‎tests/SettingFieldsTest.php‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,168 @@ public function test_the_email_bridge_speaks_the_shape_compose_reads(): void {
11331133
$this->assertSame( [ 'subject' => 'Only this' ], get_setting_fields_email( 'sf_demo', 'receipt' ) );
11341134
}
11351135

1136+
/**
1137+
* A required field left empty is refused, and keeps what it held.
1138+
*
1139+
* `required` used to be an HTML attribute and nothing more: a crafted
1140+
* request walked past it and the stored value was deleted. The set now
1141+
* refuses it, and this library's part is to say so through the Settings
1142+
* API — under the option name, keyed by the field — where options.php
1143+
* will find it.
1144+
*/
1145+
public function test_a_required_field_left_empty_is_refused_and_kept(): void {
1146+
$page = $this->validated_page();
1147+
1148+
$this->submit( $page, 'one', [ 'first' => 'kept' ] );
1149+
$this->submit( $page, 'one', [ 'first' => '' ] );
1150+
1151+
$this->assertSame( 'kept', $page->get_value( 'first' ) );
1152+
$this->assertSame(
1153+
[
1154+
[
1155+
'setting' => 'sf_demo',
1156+
'code' => 'first',
1157+
'message' => 'First is required.',
1158+
'type' => 'error',
1159+
],
1160+
],
1161+
get_settings_errors( 'sf_demo' )
1162+
);
1163+
}
1164+
1165+
/**
1166+
* After the redirect, the refused field is marked and explained.
1167+
*
1168+
* The set that refused it belonged to the request that saved.
1169+
* options.php stored what was registered in a transient and redirected,
1170+
* so the page has to read it back through the Settings API and hand the
1171+
* message to the field — which is the difference between a notice at
1172+
* the top and a field the user can find.
1173+
*/
1174+
public function test_the_page_marks_the_refused_field_after_the_redirect(): void {
1175+
$page = $this->validated_page();
1176+
1177+
$this->submit( $page, 'one', [ 'first' => '' ] );
1178+
$this->redirect_back();
1179+
1180+
$html = $this->render( $page );
1181+
1182+
// Once in the notice at the top, once under the field.
1183+
$this->assertSame( 2, substr_count( $html, 'First is required.' ) );
1184+
$this->assertStringContainsString( 'notice-error', $html );
1185+
$this->assertStringContainsString( 'field-kit__error', $html );
1186+
$this->assertStringContainsString( 'aria-invalid="true"', $html );
1187+
1188+
// And not announced as saved: core adds that only when nothing was
1189+
// registered, which is what registering the error is for.
1190+
$this->assertStringNotContainsString( 'Settings saved.', $html );
1191+
}
1192+
1193+
/**
1194+
* A save that refuses nothing is still announced as saved.
1195+
*/
1196+
public function test_a_clean_save_is_announced_as_saved(): void {
1197+
$page = $this->validated_page();
1198+
1199+
$this->submit(
1200+
$page,
1201+
'one',
1202+
[
1203+
'first' => 'fine',
1204+
'contact' => 'dev@example.test',
1205+
]
1206+
);
1207+
$this->redirect_back();
1208+
1209+
$html = $this->render( $page );
1210+
1211+
$this->assertStringContainsString( 'Settings saved.', $html );
1212+
$this->assertStringNotContainsString( 'field-kit__error', $html );
1213+
}
1214+
1215+
/**
1216+
* A refused value in a write that is not a submission is kept too.
1217+
*
1218+
* A form save seeds the collector with everything stored, so the set
1219+
* leaving a failing field alone is enough. A whole-value write seeds no
1220+
* field at all, and the value has to be put back by hand — or a plugin
1221+
* calling update_option() with one bad address would drop the good one.
1222+
*/
1223+
public function test_a_refused_value_survives_a_whole_value_write(): void {
1224+
$page = $this->validated_page();
1225+
1226+
$this->submit(
1227+
$page,
1228+
'one',
1229+
[
1230+
'first' => 'kept',
1231+
'contact' => 'dev@example.test',
1232+
]
1233+
);
1234+
1235+
$_POST = [];
1236+
1237+
update_option(
1238+
'sf_demo',
1239+
[
1240+
'first' => 'changed',
1241+
'contact' => 'not-an-address',
1242+
]
1243+
);
1244+
1245+
$this->assertSame( 'changed', $page->get_value( 'first' ) );
1246+
$this->assertSame( 'dev@example.test', $page->get_value( 'contact' ) );
1247+
$this->assertSame( 'contact', get_settings_errors( 'sf_demo' )[0]['code'] );
1248+
}
1249+
1250+
/**
1251+
* A page with a required field and a validated one.
1252+
*
1253+
* @return SettingFields
1254+
*/
1255+
private function validated_page(): SettingFields {
1256+
return $this->page(
1257+
[
1258+
'fields' => [
1259+
'first' => [
1260+
'type' => 'text',
1261+
'label' => 'First',
1262+
'tab' => 'one',
1263+
'required' => true,
1264+
],
1265+
'contact' => [
1266+
'type' => 'text',
1267+
'label' => 'Contact',
1268+
'tab' => 'one',
1269+
'validate' => 'email',
1270+
],
1271+
],
1272+
]
1273+
);
1274+
}
1275+
1276+
/**
1277+
* What options.php does once every option is updated, then the next load.
1278+
*
1279+
* Modelled rather than stubbed because the decision is core's and the
1280+
* point is to pin what this library feeds it: "Settings saved." is added
1281+
* only when nothing has been registered, and everything registered rides
1282+
* the transient to the load that follows, which carries settings-updated.
1283+
*
1284+
* @return void
1285+
*/
1286+
private function redirect_back(): void {
1287+
if ( ! count( get_settings_errors() ) ) {
1288+
add_settings_error( 'general', 'settings_updated', 'Settings saved.', 'success' );
1289+
}
1290+
1291+
set_transient( 'settings_errors', get_settings_errors(), 30 );
1292+
1293+
// The next request: a fresh global, and the argument the redirect carries.
1294+
$GLOBALS['sf_errors'] = [];
1295+
$_GET['settings-updated'] = 'true';
1296+
}
1297+
11361298
/**
11371299
* Render the page and return what it printed.
11381300
*

0 commit comments

Comments
 (0)