add action scheduler package via composer #3557

rrennick · 2020-01-14T20:34:38Z

This PR loads Action Scheduler 3.1.0 via composer.

It removes the custom AS post store which had the priority setting code.

Detailed test instructions:

Install & activate Action Scheduler master - https://github.com/woocommerce/action-scheduler
Edit the registered version to 3.1.1 https://github.com/woocommerce/action-scheduler/blob/master/action-scheduler.php#L39 (This works because plugins are loaded in alphabetical order.)
Allow the migration to run
Switch to this branch
run composer install
Add the following test snippet

add_action( 'admin_menu', function() {
error_log(ActionScheduler_Versions::instance()->latest_version());	
});

Load a dashboard page
Deactivate Action Scheduler
debug.log should have

[14-Jan-2020 20:07:49 UTC] 3.0.2
[14-Jan-2020 20:07:52 UTC] 3.0.2
[14-Jan-2020 20:07:52 UTC] 3.0.1

Re-import your analytics data
verify order & customer imports processed correctly

Changelog Note:

Dev: Add Action Scheduler 3.1.0

timmyc

[15-Jan-2020 22:01:55 UTC] 3.0.2
[15-Jan-2020 22:02:02 UTC] 3.0.2
[15-Jan-2020 22:02:07 UTC] 3.0.2
[15-Jan-2020 22:02:07 UTC] 3.0.1

Testing well for me, and again really liked the messaging updates during the queue migration in AS 👏

I want to run some more tests on this branch, like do some historical imports and such, but so far this is looking and working great.

src/Schedulers/SchedulerTraits.php

timmyc · 2020-01-16T01:28:42Z

tests/api/reports-import.php

@@ -107,7 +107,7 @@ public function test_import_params() {
 		$order_2->save();

 		// Delete order stats so we can test import API.
-		$wpdb->query( "DELETE FROM {$wpdb->posts} WHERE post_type = 'scheduled-action'" );
+		ReportsSync::clear_queued_actions();


src/Schedulers/SchedulerTraits.php

timmyc · 2020-01-16T22:30:42Z

@rrennick just saw the label change, is this good to test further or were you going to do some more work?

rrennick · 2020-01-17T13:32:13Z

is this good to test further or were you going to do some more work?

I need to address the issue with the unit tests not passing.

rrennick · 2020-01-23T18:47:00Z

Adding a note here on the reasons for the changes to the unit tests:

In the WP Post store, the pending action list is retrieved with a default ordering of action ID ASC. In normal queue processing, the custom store does order actions by action id on the innermost sort. Our unit tests bypass the normal queue processing and just processes a list of retrieved actions. Tests were intermittently failing because 2 orders created for the same customer might not be processed in the same order on every execution.
The PR changes some assumptions to explicit requirements in the batch queue tests.

@timmyc This is ready for another review. Can we add a February release to Zenhub to move this PR to?

claudiosanches · 2020-01-30T17:48:54Z

bin/composer-package-update.sh

+output 3 "Updating package textdomains..."
+
+# Replace text domains within packages with woocommerce
+find ./vendor/action-scheduler -iname '*.php' -exec sed -i.bak -e "s/'action-scheduler' /'woocommerce-admin' /g" -e "s/\"action-scheduler\" /'woocommerce-admin' /g" {} \;


We are improving it on WooCommerce core, so we avoid replacing non-translatable strings: woocommerce/woocommerce#25536

@claudiosanches Thanks for pinging us with this. In case you find it useful, I added a --no-warnings to the node command in our implementation so it wouldn't output a line for every text domain it updated.

rrennick · 2020-02-13T16:50:59Z

@timmyc This is updated to AS 3.1 and ready for another review.

jeffstieler · 2020-02-17T17:06:55Z

Is a specific version of WC required for this?

I'm getting these errors after disabling the AS plugin:

PHP Fatal error:  Uncaught Error: Call to undefined function as_get_scheduled_actions() in /srv/www/wordpress-default/public_html/wp-content/plugins/woocommerce/includes/queue/class-wc-action-queue.php:158
Stack trace:
#0 /srv/www/wordpress-default/public_html/wp-content/plugins/woocommerce-admin/src/Schedulers/ImportScheduler.php(43): WC_Action_Queue->search(Array)
#1 /srv/www/wordpress-default/public_html/wp-content/plugins/woocommerce-admin/src/ReportsSync.php(65): Automattic\WooCommerce\Admin\Schedulers\ImportScheduler::is_importing()
#2 /srv/www/wordpress-default/public_html/wp-content/plugins/woocommerce-admin/src/ReportsSync.php(133): Automattic\WooCommerce\Admin\ReportsSync::is_importing()
#3 /srv/www/wordpress-default/public_html/wp-content/plugins/woocommerce-admin/src/API/Reports/Import/Controller.php(289): Automattic\WooCommerce\Admin\ReportsSync::get_import_stats()
#4 /srv/www/wordpress-default/public_html/wp-includes/rest-api/class-wp-rest-server.php(946): Automattic\WooCommerce\Admin\API\Reports\Import\Controlle in /srv/www/wordpress-default/public_html/wp-content/plugins/woocommerce/includes/queue/class-wc-action-queue.php on line 158

rrennick · 2020-02-17T17:14:24Z

I'm getting these errors after disabling the AS plugin:

Did you run composer install? If you ran that in another branch AS may have been removed.

jeffstieler · 2020-02-17T17:40:18Z

Yes I ran composer install (just did again to be sure even) - I'm getting those PHP errors when loading the Analytics > Settings page.

rrennick · 2020-02-18T17:26:16Z

@jeffstieler To proceed with this PR we would need to include action-scheduler.php before vendor/autoload.php. Do you have any concerns about doing that?

jeffstieler · 2020-02-18T18:14:33Z

To proceed with this PR we would need to include action-scheduler.php before vendor/autoload.php. Do you have any concerns about doing that?

Can you elaborate on why the autoloader isn't able to handle this use case?

rrennick · 2020-02-18T19:06:48Z

Can you elaborate on why the autoloader isn't able to handle this use case?

See woocommerce/action-scheduler#468 (comment)

timmyc · 2020-03-03T01:55:21Z

@rrennick guessing we might be closing this one out?

rrennick · 2020-03-03T12:53:50Z

Due to the issue with composer autoloading we didn't have time in the core release schedule to implement this.

rrennick added [Status] Needs Review tool: monorepo infrastructure status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. focus: performance The issue/PR is related to performance. labels Jan 14, 2020

rrennick force-pushed the try/package-as branch from 4e95339 to 414b70f Compare January 14, 2020 22:38

rrennick removed the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Jan 15, 2020

timmyc reviewed Jan 16, 2020

View reviewed changes

rrennick added status: in progress and removed [Status] Needs Review labels Jan 16, 2020

timmyc reviewed Jan 16, 2020

View reviewed changes

src/Schedulers/SchedulerTraits.php Show resolved Hide resolved

rrennick force-pushed the try/package-as branch from 1d1b0f6 to 09af8bc Compare January 17, 2020 14:10

rrennick mentioned this pull request Jan 20, 2020

move action scheduler to external via composer woocommerce/woocommerce#25404

Merged

6 tasks

rrennick force-pushed the try/package-as branch 2 times, most recently from a77cb54 to dc8fc7e Compare January 22, 2020 20:31

rrennick added 8 commits January 23, 2020 11:16

add action scheduler package via composer

9ebf239

update unit tests to be AS data store agnostic

172eda6

remove post store override, only register AS if WCA is active plugin

ea3f8ac

add comments explaining next() deprecation

ff851b5

update batch queue unit tests to support 3.0 data store

3870c2d

add text domain replacement script

12d4394

move random order test to run last

f8320dc

add order items to stats reset in unit tests

5f0e889

rrennick force-pushed the try/package-as branch from dc8fc7e to 5f0e889 Compare January 23, 2020 15:25

rrennick added 2 commits January 23, 2020 12:54

add queue processing to interval test

8979d82

use consistent scheduled action order in unit tests

b2f3014

rrennick added 2 commits January 23, 2020 14:14

revert add queue processing to interval test

f5acfa6

add action scheduler folder to i18n string parsing

21e2fb9

rrennick added [Status] Needs Review and removed status: in progress labels Jan 23, 2020

rrennick mentioned this pull request Jan 28, 2020

add spacing for text domain replacement when used as a composer dependency woocommerce/action-scheduler#442

Merged

claudiosanches suggested changes Jan 30, 2020

View reviewed changes

use wpTextdomain for text domain replacement

7d6aba1

rrennick force-pushed the try/package-as branch from ec36e35 to 7d6aba1 Compare January 31, 2020 14:47

rrennick changed the base branch from master to version/1.0 February 11, 2020 20:37

rrennick added 2 commits February 13, 2020 10:43

bump AS to 3.1.0, add external to gitignore

05fd9ec

rebase to version/1.0

b8d96ea

rrennick mentioned this pull request Feb 13, 2020

v3.1.0 doesn't work as a composer package woocommerce/action-scheduler#463

Closed

rrennick mentioned this pull request Feb 18, 2020

always hook initialize_latest_version() woocommerce/action-scheduler#468

Closed

rrennick added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. and removed [Status] Needs Review labels Feb 20, 2020

rrennick closed this Mar 3, 2020

rrennick deleted the try/package-as branch March 3, 2020 12:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add action scheduler package via composer #3557

add action scheduler package via composer #3557

rrennick commented Jan 14, 2020 •

edited

Loading

timmyc left a comment

timmyc Jan 16, 2020

timmyc commented Jan 16, 2020

rrennick commented Jan 17, 2020

rrennick commented Jan 23, 2020

claudiosanches Jan 30, 2020

rrennick Jan 30, 2020

rrennick commented Feb 13, 2020

jeffstieler commented Feb 17, 2020

rrennick commented Feb 17, 2020

jeffstieler commented Feb 17, 2020

rrennick commented Feb 18, 2020

jeffstieler commented Feb 18, 2020

rrennick commented Feb 18, 2020

timmyc commented Mar 3, 2020

rrennick commented Mar 3, 2020

add action scheduler package via composer #3557

add action scheduler package via composer #3557

Conversation

rrennick commented Jan 14, 2020 • edited Loading

Detailed test instructions:

Changelog Note:

timmyc left a comment

Choose a reason for hiding this comment

timmyc Jan 16, 2020

Choose a reason for hiding this comment

timmyc commented Jan 16, 2020

rrennick commented Jan 17, 2020

rrennick commented Jan 23, 2020

claudiosanches Jan 30, 2020

Choose a reason for hiding this comment

rrennick Jan 30, 2020

Choose a reason for hiding this comment

rrennick commented Feb 13, 2020

jeffstieler commented Feb 17, 2020

rrennick commented Feb 17, 2020

jeffstieler commented Feb 17, 2020

rrennick commented Feb 18, 2020

jeffstieler commented Feb 18, 2020

rrennick commented Feb 18, 2020

timmyc commented Mar 3, 2020

rrennick commented Mar 3, 2020

rrennick commented Jan 14, 2020 •

edited

Loading