search

woocommerce / action-scheduler-custom-tables

This plugin is no longer needed. The custom table code is now part of Action Scheduler version 3.0 and newer. If you're using the latest Action Scheduler, you have the most performant schema available.
https://actionscheduler.org
14 stars 2 forks source link

Fix dependency check #49

Closed thenbrent closed 5 years ago

thenbrent commented 5 years ago

PR #42 introduced a dependency check to make sure AS 2.1.0 or newer was active before loading. The logic on the check was wrong - we need dependencies to be met, not for them to be unmet.

Fixes #46

JPry commented 5 years ago

This change looks correct, but I'm concerned by the breaking tests. I'm investigating those before merging.

JPry commented 5 years ago

I did some digging into this, and it looks like the trouble here has to do with the use of the priority of 0.5. This doesn't behave as we expect. In the internals of the WP_Hook class, the add_filter() function has this line:

$priority_existed = isset( $this->callbacks[ $priority ] );

With a priority value of 0.5, the variable $priority_existed is set to true, and the callback is attached to priority 0. Because this plugin is loaded before the main Action Scheduler plugin, the dependency check is triggered before Action Scheduler has actually registered its version.

I opened Prospress/action-scheduler#221 as a result. With that change, I think we ought to also change our main init hook to the action_scheduler_pre_init hook instead.

thenbrent commented 5 years ago

Thanks for doing the hard work on this one @JPry. I knew that 0.5 priority was a hack.

I was hoping to avoid bumping the dependency higher, but I think that's unavoidable and using the hook in Prospress/action-scheduler#221 is definitely the best approach, so we can do that instead. :)

I've created https://github.com/Prospress/action-scheduler-custom-tables/issues/50 to address that separately to this PR.

thenbrent commented 5 years ago

Closing this as it's being handled as part of #53.