Skip to content

Issue-39: Allow an implementer-provided callback to be executed after each batch (2.x) #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: 2.x
Choose a base branch
from

Conversation

renatonascalves
Copy link
Contributor

@renatonascalves renatonascalves commented May 7, 2025

Summary

This pull request introduces the ability to allow an implementer-provided callback to be executed after each batch in the after_batch function of the bulk task. Fixes #39. Closes #41.

Description

The bulk task currently has an after_batch function, but it is not pluggable. This enhancement enables implementers to provide their own callbacks to be executed after each batch. The callback can be passed when the bulk task is being set up, allowing greater flexibility and customization. Additionally, several code improvements and updates were made to enhance readability and maintainability.

Implementation Details

  • Added a new protected property $after_batch_callback to the Bulk_Task class to store implementer-provided callbacks.
  • Modified the constructor for improved readability by formatting parameters on separate lines.
  • Removed the Refresh_Database trait from various test classes, including TestPostBulkTask, TestTermBulkTask, and TestUserBulkTask, as database resetting functionality is no longer required or is handled differently.
  • Improved code readability with minor formatting updates, such as adding blank lines and updating PHPDoc comments for clarity.
  • Updated documentation to reflect the new functionality and changes.

Motivation and Context

This feature was requested to address the need for more customizable post-batch processing in bulk tasks. The context for this need arose from this scenario encountered by Alley developers. This enhancement provides developers with the flexibility to implement their own logic in the after_batch process.

How Has This Been Tested?

  • Unit tests have been added to verify the functionality of the implementer-provided callback.
  • Manual testing was performed to ensure the callback is executed correctly with the defined values.
  • New test methods were introduced across test classes to ensure the overall correctness of the updated functionality.

Screenshots (if applicable)

N/A

Types of Changes

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@renatonascalves renatonascalves added enhancement New feature or request php Requires understanding PHP labels May 7, 2025
@renatonascalves renatonascalves marked this pull request as ready for review May 7, 2025 12:47
@@ -19,14 +18,13 @@
* @package alleyinteractive/wp-bulk-task
*/
class TestCsvBulkTask extends Test_Case {
use Refresh_Database;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Test_Case already supports this.

Comment on lines +153 to +155
'cursor' => $this->cursor,
'min_id' => $this->min_id,
'max_id' => $this->max_id,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor is public, but the other two are protected. And they might be needed in the callback. e.g: at every 100 posts, index them in elasticsearch, etc.

array $args,
callable $callable,
string $object_type = 'wp_post',
?callable $after_batch_callback = null,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I implemented this at Bulk_Task construct, but then I changed my mind just because the callback is better together with the run method.

That however forces one to use the wp_post object type. I think that's okay, better to be explicit rather than clever here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request php Requires understanding PHP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant