Make WordPress Core

Opened 7 years ago

Last modified 3 years ago

#43019 new enhancement

Hook to validate post form data before save

Reported by: henrywright's profile henry.wright Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version:
Component: Database Keywords: has-patch needs-testing
Focuses: Cc:

Description

There doesn't seem to be a hook available to validate form data before a post is saved. In my case I'm considering using JavaScript to validate the post title, content and meta but would prefer to do this validation server side.

Can we add a hook?

Attachments (2)

43019.diff (535 bytes) - added by danieltj 7 years ago.
Pre insert post hook
43019.2.diff (832 bytes) - added by henry.wright 7 years ago.

Download all attachments as: .zip

Change History (15)

#1 @danieltj
7 years ago

  • Keywords reporter-feedback added

I'm just curious, but why do you need to validate those fields prior to saving as that already happens on post save? Having said that, there is a hook called pre_post_update that is run when something is updated, although not when something is saved for the first time mind you.

Useful links for this hook:

  1. https://core.trac.wordpress.org/browser/tags/4.9/src/wp-includes/post.php#L3371
  2. https://developer.wordpress.org/reference/hooks/pre_post_update/

There are some filter hooks you could use though within the wp_insert_post function if you wanted to check data that is submitted, but like I said; it's validated when run through the function.

If you're wanting to validate custom meta data for a post, you can still use the save_post hook and do your own custom validation there but it depends on your use case as to whether that's helpful or not.

#2 @henry.wright
7 years ago

  • Keywords reporter-feedback removed

Hi @danieltj

I'm just curious, but why do you need to validate those fields prior to saving as that already happens on post save?

Perhaps I should have been more clear. A hook will allow for custom validation to be done. If WordPress validates already then that's great but the key word here is custom.

Having said that, there is a hook called pre_post_update that is run when something is updated

As you mentioned in your comment, the pre_post_update hook will execute only if $update evaluates to true. The hook I'm requesting should apply also to new posts.

If you're wanting to validate custom meta data for a post, you can still use the save_post hook and do your own custom validation there but it depends on your use case as to whether that's helpful or not.

save_post is executed after the post has been saved. This won't be helpful if you need to perform custom validation before the post is added to the database.

#3 @danieltj
7 years ago

  • Keywords has-patch added

Fair enough - I'm all for adding in extra hooks - I was just curious the use-case for it. I'll add a patch for a proposed hook, not sure if anything else is needed apart from the post data being passed through it.

@danieltj
7 years ago

Pre insert post hook

#4 @henry.wright
7 years ago

Thanks for the diff @danieltj

I think we should provide a way of bailing early if the data isn't valid. Please see the incoming 43019.2.diff. Grateful for your thoughts

@henry.wright
7 years ago

#5 @johnbillion
7 years ago

  • Keywords ux-feedback needs-testing added

Thanks for the patch. What does the user journey look like if the data that they've entered is considered invalid? Do they see a generic error message stating "The data supplied isn't valid"? Are the fields repopulated with their invalid data or does their data get lost?

Screenshots or a very short screencast could help.

#6 @henry.wright
7 years ago

Thanks John for the feedback.

The user journey isn't very good with my patch 43019.2.diff. They will see the generic error message you mentioned. I think the patch could be fleshed out to allow the callback to return a custom error message.

I believe currently the field data is lost if it's invalid. I think this should also be changed.

#7 @foresmac
6 years ago

This would be super useful. Maybe the way to do it is to have the callback return an array of WP_Error for each custom field that fails validation? If it's empty, then there are no custom field errors.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


5 years ago

#9 @karmatosed
5 years ago

  • Keywords ux-feedback removed

I am removing ux-feedback as a keyword because this doesn't need the design team to give feedback right now. This was discussed in this week's design triage meeting.

#10 @eseyfried
4 years ago

I am having the same challenge as @henrywright. I was looking for a similar hook that fires before the post saves and would allow me to halt the save process if my custom validation fails. This patch seems to cover this scenario. I am curious to know if this will be accepted into Core. This thread seemed to have died off. Would someone please post an update?

#11 @rafdizzle86
4 years ago

  • Component changed from General to Database
  • Severity changed from normal to major
  • Version set to 5.6

I've found a "work-around" for updates. You can hook into the filter wp_insert_post_data and return a null or some other falsey value that is not an array. This is because a few lines down there's a call to $wpdb->update( $wpdb->posts, $data, $where ) which does a validation on $data in the first few lines:

if ( ! is_array( $data ) || ! is_array( $where ) ) {
	return false;
}

In the same hook you can update an option with a notification as to why you failed the update - in the following request you can display that notification using the admin_notices hook.

I haven't looked that much into $wpdb->insert() - however it seems more complicated as it calls a _insert_replace_helper method that does some SQL data validation.

Hopefully this helps other folks who are wondering why WordPress and WooCommerce don't have a simple hook for data validation and halting the save. Seems like a rudimentary hook that should be part of any database save process!

WooCommerce devs: please include an explicit filter that allows 3rd party developers to validate data and halt the save process if that data is invalid!

Version 1, edited 4 years ago by rafdizzle86 (previous) (next) (diff)

#12 @hellofromTonya
4 years ago

  • Version 5.6 deleted

Removing 5.6 as the Version, as the ticket creation predates the 5.6 release.

#13 @Starbuck
3 years ago

After research I came here looking for the requested feature.

In option.php, a similar feature was added for ticket:38903, where filter pre_update_option returns a filtered option value, and if that value is the same as the original, it does not go through the update.

So in my code if there is an issue with user-entered data I reset the value to the original and set an admin notice. (For better UX the choice to reset or leave the invalid value is configurable.)

The "side effect" functionality in option.php and post.php, with wp_insert_post_data (and wp_insert_attachment_data) as suggested in comment:11, is adequate but not at all elegant.

@rafdizzle86 was correct that the $wpdb->update function returns false if $data is null. But a little further down in post.php, if $update=false I believe $wpdb->insert will choke on bad $data.

So rather than relying on this side effect, which is subject to break, I'm proposing a new consistent hook for both post types and options, to be executed immediately before $wpdb->update and $wpdb->insert. This will clearly allow for a final validation of data prior to the write operation:

Ref: https://core.trac.wordpress.org/browser/branches/5.8/src/wp-includes/option.php#L487
New:

$verified = apply_filters('final_option_verification', $option, $update_args);
$result = $verified && $wpdb->update( $wpdb->options, $update_args, array( 'option_name' => $option ) );
if ( ! $result ) { // existing next lines
  return false;
}

Ref: https://core.trac.wordpress.org/browser/branches/5.8/src/wp-includes/post.php#L4196
New:

$verified = apply_filters('final_post_verification', $post_ID, $data);
if ( !$verified || false === $wpdb->update( $wpdb->posts, $data, $where ) ) { ...

Ref: https://core.trac.wordpress.org/browser/branches/5.8/src/wp-includes/post.php#L4219
New:

$verified = apply_filters('final_post_verification', $post_ID, $data);
if ( !$verified || false === $wpdb->insert( $wpdb->posts, $data ) ) { ...

A better and consistent naming pattern for the filter might be: "final_{$type}_verification". Types can include:

  • post_insert
  • post_update
  • attachment_insert
  • attachment_update
  • post_delete
  • post_trash

This would allow the hook to be used for many other purposes in-core, and can be adopted as a final confirmation standard outside of the core, like 'final_customer_archive_verification'.

This ticket as been waiting for 4 years for processing and might be resolved with just a few carefully-placed lines of code. I've never submitted a PR here but I can implement and test this functionality if it seems worthy, and will read-up on how to properly submit the PR. Thanks.

Note: I didn't mention metadata or CPTs but the same changes apply.
Ref: https://core.trac.wordpress.org/browser/branches/5.8/src/wp-includes/meta.php#L103
In that case and elsewhere, there are already checks ($check var above the insert of meta data). A new feature like this shouldn't be redundant with existing features, and care should be taken to ensure Prepare statements, other filters, key generation, etc are still performed in the proper sequence.

Last edited 3 years ago by Starbuck (previous) (diff)
Note: See TracTickets for help on using tickets.