bulk_edit_posts() function needs an action hook

[helgatheviking]

Currently, the only method that I can find to save additional data in via bulk edit requires using an .ajax() call.

This could be avoided if bulk_edit_posts() had a simple action hook at the end before it returns it's results.

do_action( 'bulk_edit_posts', $post_data );

return array( 'updated' => $updated, 'skipped' => $skipped, 'locked' => $locked );

[helgatheviking]

Some JS triggers in inline-edit-posts.js wouldn't go astray either. For example:

t.trigger( "setBulk-inlineedit" );

[helen]

I'm enjoying your bulk/quick edit tickets very much - just want to note (as I probably have elsewhere) that this is one of those times where we'd be better off with a revamp and that any additions to what exists need to be considered ever-more carefully in that light. A Backbone-ified bulk/quick edit has been tossed around as an idea a couple times, if you want to look into that.

[helgatheviking]

I'm flattered! Even if I didn't ever think that tickets could be 'enjoyed'. I've been revamping one of my plugins that relies heavily on quick/bulk edit and have come to realize that those areas need some love... particularly more hooks/filters for developers. Unfortunately, I don't know the first thing about backbone so I don't know that I'd be much use there.

Pertaining to this issue... so are you saying that admin hooks/triggers will not be added in favor of a possible, eventual overhaul? It seems a shame to lose out on some hooks while that maybe happens.

All that said, my digging did eventually bring me to the revelation that bulk edit is processed by a loop that hits wp_insert_post() so actually the ability to save bulk edit details doesn't require ajax at all and can run on the exact same save_post routine that quick edit and meta boxes already use.

[Mte90]

The patch is very simple, contain the hook with a little bit of documentation (that require a feedback) in the bulk_edit_posts() function.

[afercia]

28112.diff​ the docblock needs some coding standards.

[mrasharirfan]

I have applied WordPress coding standards to the patch.

[desrosj]

With the plan for 5.0.1 to be a 2-week quick release in mind, I am going to punt this to 5.0.2.

28112.2.diff​ is still applying cleanly for me to trunk, but a new patch will be needed if this will be committed directly to the 5.0 branch.

[itowhid06]

Updated @since to reflect actual version the hook is introduce

[desrosj]

Going to punt this to 5.1. It is outside of the scope for 5.0.3 (block
editor bugs, regressions, and major bugs).

[pento]

I'm not entirely sure what the bulk_edit_posts action is supposed to do. It's passed $post_data, which changes for each post, but it happens outside of the post loop, so it's only called once with the data from the last post edited.

[mensmaximus]

Replying to pento:

I'm not entirely sure what the bulk_edit_posts action is supposed to do. It's passed $post_data, which changes for each post, but it happens outside of the post loop, so it's only called once with the data from the last post edited.

As far as I understand @helgatheviking wants to save additional data added to the bulk edit meta box by avoiding an ajax call. Assuming this to be the case the patch provided won't work for that purpose as @pento already mentioned.

Imho such a hook is not necessary at all because there are three action hooks available to further manipulate a post after it got updated: edit_post_{$post->post_type}, edit_post and post_updated

[mensmaximus]

Replying to mensmaximus:

Imho such a hook is not necessary at all because there are three action hooks available to further manipulate a post after it got updated: edit_post_{$post->post_type}, edit_post and post_updated

I did some testing and found none of these action hooks can be utilized to save custom data from bulk edit because $_POST is empty if wp_insert_post() gets invoked by bulk_edit_posts().

To save custom data from bulk edit you can use the filter hook wp_insert_post_data. It passes $postarr as second argument containing all values from $_POST.

[Mte90]

This ticket is still valid after all those years?

[costdev]

I believe @mensmaximus is correct regarding available hooks:

• ​wp_insert_attachment_data for attachments.
• ​wp_insert_post_data for everything else.

However, hooking these filters means the callback will run on each insert - something that might be undesirable. For this reason, the proposed action hook would be more appropriate.

The action hook in the patch may need to use $shared_data, because $post_data is extended with data for each post in the foreach loop, and this additional data would only be for the last post in the loop. @helgatheviking, @Mte90, what do you think about this?

[helgatheviking]

Is there a way to detect if wp_insert_post_data() is running only in the quick edit? If so, that seems more than sufficient.

If not, after all these years, I don't see the harm in adding a hook. Needing the other array variable makes sense, but I certainly couldn't tell you what my test results were like back in 2014. :)

[Mte90]

Just as reference today the function is on ​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-admin/includes/post.php#L471

Looking at the code $shared_post_data is a copy of $post_data used inside the loop but $post_data is used to save the data inside posts so seems the best choice.

Trac ticket: https://core.trac.wordpress.org/ticket/28112

[audrasjb]

Thanks for the PR. I refreshed it, fixed some WPCS issues, and improved the related docblock.

@costdev I think we can probably consider this for 6.2, isn't it? :)

[costdev]

@audrasjb Sounds good to me! Milestoning.

I've also dropped a minor comment on the PR I'd appreciate your input on.

[audrasjb]

I added a commit to the PR to rephrase the docblock, thanks for the review @costdev!

Self assigning for commit consideration.

[audrasjb]

In 55108:

Quick/Bulk Edit: Add an action hook on bulk_edit_posts().

This changeset introduces the bulk_edit_posts action hook, triggered after processing the post data for bulk edit and before it returns its results. For example, it allows developers to save additional data without having to perform any .ajax() call.

Props helgatheviking, helen, Mte90, afercia, mrasharirfan, desrosj, itowhid06, pento, mensmaximus, audrasjb, costdev.
Fixes #28112.

Committed in https://core.trac.wordpress.org/changeset/55108

[SergeyBiryukov]

Thanks for the commit!

I believe comment:17 raises a good point: the current implementation of the action accepts $post_data, which contains the submitted form results, plus some data for the last post in the loop, but the latter is not mentioned in the hook documentation and seems confusing for future maintenance, as it conflates two different data sets.

I think passing $shared_post_data instead, as suggested in comment:22, would be the correct thing to do here. If we do feel strongly about also including the post data processed in the loop, it should be consistently collected for each post and passed as the second parameter.

Also, if my reading of comment:3 is correct, it seems like this action might not actually be needed after all, as the use case from the ticket description can be achieved via existing hooks, e.g save_post?

All that said, my digging did eventually bring me to the revelation that bulk edit is processed by a loop that hits wp_insert_post() so actually the ability to save bulk edit details doesn't require ajax at all and can run on the exact same save_post routine that quick edit and meta boxes already use.

With the comment above, and also Helen's note in comment:2:

just want to note (as I probably have elsewhere) that this is one of those times where we'd be better off with a revamp and that any additions to what exists need to be considered ever-more carefully in that light.

are we sure there is still enough demand for this hook? I'm all for making core more flexible, but also not sure if it's a good idea to introduce hooks just because we can, as they will need to be maintained indefinitely due to WP's backward compatibility promise.

[audrasjb]

Replying to SergeyBiryukov:

Also, if my reading of comment:3 is correct, it seems like this action might not actually be needed after all, as the use case from the ticket description can be achieved via existing hooks, e.g save_post?

All that said, my digging did eventually bring me to the revelation that bulk edit is processed by a loop that hits wp_insert_post() so actually the ability to save bulk edit details doesn't require ajax at all and can run on the exact same save_post routine that quick edit and meta boxes already use.

I might be wrong, but I think the issue if that there is no way to determine the context of the process (in this case: running a bulk edit process).

[Mte90]

If we are talking about focus it is better a dedicated hook to avoid to use save_post that is executed in all of them.
You can have different settings in the panel in the bulk mode as example, this let the developer just to focus on what to do on bulk.

[SergeyBiryukov]

Replying to audrasjb:

I might be wrong, but I think the issue if that there is no way to determine the context of the process (in this case: running a bulk edit process).

It appears that $_GET['bulk_edit'] is set during bulk edit, so that might be a way to check for the context. Then again, what would be a use case of only saving some data on bulk edit but not on regular post edit?

[Mte90]

It is passed so much that since 2016 that I don't remember why I did that patch, but I remember that it was for something I was looking into.

Apart from what kind of data, I think that it is more simple for a dev to use that new filter instead looking on wp_insert_post and the $_GET variable.
As usually you don't have any guide or tutorial on how to do something so specific on WordPress as the docs are just for function but contain also list of hooks.

If someone is looking on how to save data on bulk it is more easy to look for a hook that investigate the WordPress internals.

[webcommsat]

Bug review January 30, 2023:

• The discussion found it is useful to have the whole information at once, but questioned whether the implementation was the ideal way.
• Concerns on implementation resulting in an excessive load on the memory which could be a pain point for some projects.
• If we need this implementation is needed for bulk editing.
• Comment #35 mentions that we don't need this hook and Sergey's solution for bulk edit.

As there is also the question of maintaining hooks in the future, Sergey's solution appears to provide a way forward in many situations. Suggestion that a documentation would be useful if how to use bulk edit in this situation. This ticket could be reopened if reporters / users had additional needs, rather than single case usage.

• Included a link in Slack for any other comments.

Scrub attendees: webcommsat, marybaum, ogleckler, robinwpdeveloper

[costdev]

@audrasjb @SergeyBiryukov With 6.2 Beta 1 being released later today, what's the next step for this ticket?

Does [55108] need to be reverted pending confirmation that this hook is still needed, that the right variable is passed, and that the docs are correct?

Additional props: @mukesh27

[SergeyBiryukov]

Replying to costdev:

Does [55108] need to be reverted pending confirmation that this hook is still needed, that the right variable is passed, and that the docs are correct?

I think that would be preferred for now, though curious to see what others think.

[costdev]

@SergeyBiryukov As there is still uncertainty about this and 6.2 Beta 1 is fast approaching, I'd suggest a revert and re-milestoning for 6.3 so that we can aim to land this next cycle once we get a final idea of whether this enhancement is needed, and how it should be implemented.

[SergeyBiryukov]

In 55265:

Quick/Bulk Edit: Remove the bulk_edit_posts action for now.

This reverts [55108] pending confirmation that this hook is still needed, that the right variable is passed, and that the docs are correct.

The necessity and implementation of this hook will be reconsidered in a future release.

See #28112.

[marybaum]

For 6.2 this needs neither a dev note nor docs, because it won't be part of the release. If it becomes part of 6.3, then it will need dev note and docs.

Atm, it's not clear that the hook would offer new functionality.

[Mte90]

Right now the code was removed after adding it because we don't agree if adding or not.

[oglekler]

I changed my mind, I think we should add this action, it can be useful. But use cases will not appear until it will be in the core. We are in 9 days to the Beta, and this is probably simplest from all other enhancements we have. So, it can be in Beta 1 in no time.

I tested the patch, even if it is a simple one, it is still applicable.

Here the data which is going to this action:

Array
(
    [s] => 
    [post_status] => draft
    [post_type] => page
    [_wpnonce] => 0276e53341
    [_wp_http_referer] => /wp-admin/edit.php?post_type=page&paged=1
    [action] => edit
    [m] => 0
    [paged] => 1
    [bulk_edit] => Update
    [post_view] => list
    [screen] => edit-page
    [post] => Array
        (
            [0] => 24
            [1] => 25
            [2] => 26
        )

    [action2] => edit
    [post_ID] => 26
    [post_mime_type] => 
    [comment_status] => open
    [ping_status] => closed
    [post_author] => 1
    [ID] => 26
    [user_ID] => 1
)

One of the use cases can be - update of these posts in some other place via webhook.

[SergeyBiryukov]

Replying to oglekler:

I changed my mind, I think we should add this action, it can be useful.

Fair enough :)

But the current patch still conflates two different data sets, as noted in comment:32:

• The submitted form data
• Some data for the last post in the loop

This can be confusing for developers, as they might assume all the data applies to all posts, whereas some of it in fact only applies to the last one. Moreover, the list of post IDs can be incorrect, as some of the posts might be skipped and not updated because they are locked or due to insufficient permissions.

If we're going to implement this hook, I believe it should receive:

• The list of post IDs which were actually updated
• The submitted form data that applies to all of the updated posts

Would 28112.4.diff​ address the potential use cases here?

[oglekler]

@SergeyBiryukov, yes, I think $shared_post_data is more accurate, but you've missed $ of this variable in the patch. $shared_post_data['post'] and $updated should be equal, but better to keep $updated. I would have expected that if nothing has changed, the post ID will not be added to $updated, but this isn't the case, even if nothing has changed in post parameters, it's still going to this array.

[costdev]

Added missing $ and a PHPUnit test to ensure the action is always fired in future.

[oglekler]

Patch is ready for review

[mukesh27]

Thanks @costdev for the PR.

@audrasjb Could you please take a look before beta so we can ship this also?

[oglekler]

PR looks fine for me, ready for commit. Please proceed.

[oglekler]

@audrasjb can you please take a look? This ticket was reopened 5 months ago, and I cannot imagine any side effects from this patch.

[SergeyBiryukov]

In 56091:

Quick/Bulk Edit: Add an action hook to bulk_edit_posts() function.

This changeset introduces the bulk_edit_posts action hook, triggered after processing the post data for bulk edit and before the function returns its results. For example, it allows developers to save additional data without having to perform any .ajax() calls.

Follow-up to [8973].

Props helgatheviking, helen, Mte90, afercia, mrasharirfan, desrosj, itowhid06, pento, mensmaximus, audrasjb, costdev, webcommsat, marybaum, oglekler, mukesh27, SergeyBiryukov.
Fixes #28112.

Thanks for the PR! Merged in r56091.