Skip to content
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

restore action handling to WC status, use data store to delete actions #487

Merged
merged 2 commits into from Mar 13, 2020

Conversation

rrennick
Copy link
Collaborator

Closes #485

This PR restores the action handling to the Scheduled Actions screen under WooCommerce -> Status -> Scheduled Actions. The handling was removed in #354.

While investigating this I noticed that the bulk delete action attempts to delete the actions from the default data store. The PR updates that function to use the data store to delete the actions.

Testing

  1. In master confirm that neither the bulk delete action or the action links work in the WC status page
  2. Switch to this branch
  3. Update action-scheduler.php to artificially bump the version
-if ( ! function_exists( 'action_scheduler_register_3_dot_1_dot_2' ) ) {
+if ( ! function_exists( 'action_scheduler_register_3_dot_1_dot_3' ) ) {
 
        if ( ! class_exists( 'ActionScheduler_Versions' ) ) {
                require_once( 'classes/ActionScheduler_Versions.php' );
                add_action( 'plugins_loaded', array( 'ActionScheduler_Versions', 'initialize_latest_version' ), 1, 0 );
        }
 
-       add_action( 'plugins_loaded', 'action_scheduler_register_3_dot_1_dot_2', 0, 0 );
+       add_action( 'plugins_loaded', 'action_scheduler_register_3_dot_1_dot_3', 0, 0 );
 
-       function action_scheduler_register_3_dot_1_dot_2() {
+       function action_scheduler_register_3_dot_1_dot_3() {
                $versions = ActionScheduler_Versions::instance();
-               $versions->register( '3.1.2', 'action_scheduler_initialize_3_dot_1_dot_2' );
+               $versions->register( '3.1.3', 'action_scheduler_initialize_3_dot_1_dot_3' );
        }
 
-       function action_scheduler_initialize_3_dot_1_dot_2() {
+       function action_scheduler_register_3_dot_1_dot_3() {
                $versions = ActionScheduler_Versions::instance();
-               $versions->register( '3.1.2', 'action_scheduler_initialize_3_dot_1_dot_2' );
+               $versions->register( '3.1.3', 'action_scheduler_initialize_3_dot_1_dot_3' );
        }
 
-       function action_scheduler_initialize_3_dot_1_dot_2() {
+       function action_scheduler_initialize_3_dot_1_dot_3() {
                require_once( 'classes/abstracts/ActionScheduler.php' );
                ActionScheduler::init( __FILE__ );
        }
 
        // Support usage in themes - load this version if no plugin has loaded a version yet.
        if ( did_action( 'plugins_loaded' ) && ! class_exists( 'ActionScheduler' ) ) {
-               action_scheduler_initialize_3_dot_1_dot_2();
+               action_scheduler_initialize_3_dot_1_dot_3();
                do_action( 'action_scheduler_pre_theme_init' );
                ActionScheduler_Versions::initialize_latest_version();
        }
  1. Check the WC status report to verify the bumped version is active
  2. Test action links and bulk actions in both the Tools and WooCommerce -> Status Scheduled Actions screens work.

@rrennick rrennick added this to the 3.1.3 milestone Mar 12, 2020
@@ -91,6 +91,7 @@ public function process_admin_ui() {
*/
public function render_admin_ui() {
$table = new ActionScheduler_ListTable( ActionScheduler::store(), ActionScheduler::logger(), ActionScheduler::runner() );
$table->process_actions();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the call to process_actions() be removed from process_admin_ui()? It looks like this will fire twice when going to Tools > Scheduled Actions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jeffstieler Thanks for the suggestion. The PR has been updated.

@jeffstieler
Copy link
Contributor

jeffstieler commented Mar 12, 2020

Testing now @rrennick - please update the version in package.json as well.

EDIT: or remove the version bump if that's just for testing..

Copy link
Contributor

@jeffstieler jeffstieler left a comment

Choose a reason for hiding this comment

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

Code looks good and tests well.

If the version bump was meant to happen in this PR, please update package.json as well. Otherwise, please revert the version bump in action-scheduler.php.

@jeffstieler jeffstieler added has feedback Issues for which we requested feedback from the author and received it. Status: Needs Merge and removed Status: Needs Review labels Mar 12, 2020
@thenbrent
Copy link
Contributor

thenbrent commented Mar 12, 2020

The PR updates that function to use the data store to delete the actions.

That's likely related to #484 and #488.

@rrennick
Copy link
Collaborator Author

The PR updates

@thenbrent That was a typo The should have been This. It's the small change in the abstract list table.

If the version bump was meant to happen in this PR

@jeffstieler Thanks for catching that.

@rrennick rrennick merged commit ab32ca6 into master Mar 13, 2020
@rrennick rrennick deleted the issue_485 branch March 13, 2020 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has feedback Issues for which we requested feedback from the author and received it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run action link does not work in WC status page
3 participants