woocommerce/action-scheduler

The function recurring_unique() calls single_unique() with the $args argument in place of the $first argument

jonathanstegall opened this issue ยท 4 comments

In ActionScheduler_ActionFactory.php, there are these lines:

public function recurring_unique( $hook, $args = array(), $first = null, $interval = null, $group = '', $unique = true ) {
    if ( empty( $interval ) ) {
        return $this->single_unique( $hook, $unique, $args, $first, $group );
    }
    $date     = as_get_datetime_object( $first );
    $schedule = new ActionScheduler_IntervalSchedule( $date, $interval );
    $action   = new ActionScheduler_Action( $hook, $args, $schedule, $group );
    return $unique ? $this->store_unique_action( $action ) : $this->store( $action );
}

Then single_unique reads like this:

public function single_unique( $hook, $args = array(), $when = null, $group = '', $unique = true ) {
    $date     = as_get_datetime_object( $when );
    $schedule = new ActionScheduler_SimpleSchedule( $date );
    $action   = new ActionScheduler_Action( $hook, $args, $schedule, $group );
    return $unique ? $this->store_unique_action( $action ) : $this->store( $action );
}

I had a user of the plugin I maintain report this as a potential issue. Here's what they said:

I believe the issue is a difference in the ActionScheduler_ActionFactory.php class between 2.1.2 and 2.2.5. The function recurring_unique() calls single_unique() with the $args argument in place of the $first argument (hence an error when trying to parse an empty array as a string)

For further context, see this forum thread. I've never seen the issue the user is reporting, but it does indeed look to me as though recurring_unique is passing parameters in a different order than single_unique is expecting. I'm interested to see if this is in fact an error that should be fixed, or if we're all missing something.

Update: to clarify, version 2.1.2 of the plugin I maintain was running Action Scheduler 3.4.0, and 2.2.5 is running 3.5.2.

Thanks for flagging this, it does indeed look like a valid problem.

@jonathanstegall Thank you for opening this. I reported the specific issue in that forum thread and everything seems to be working fine when putting those parameters back in the right order. Happy to add anything here if I can help.

This seems like kind of a high priority issue the more I see it. Has anyone had a chance to look at the pull request/see if it's missing anything?

Hi @jonathanstegall,

Thanks for the ping. We agree this is high priority.
I've assigned myself as reviewer for your PR and will be taking a look shortly.
Hopefully we'll have this fixed (thanks to you) soon.