Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
85 stars 31 forks source link

wcs_get_subscription_period_strings function doesn't allow for removing periods without PHP warning in PHP 8.1 #363

Open skunkbad opened 1 year ago

skunkbad commented 1 year ago

Description of the bug Time periods cannot be removed using the woocommerce_subscription_periods filter that is in the wcs_get_subscription_period_strings function of wcs-time-functions.php. Doing so produces PHP warnings anytime the function is called. The PHP warning comes from the fact that once we remove keys from the periods, and since your code doesn't check for the presence of the key before trying to use it, we get a lot of undefined array key warnings filling up our error logs.

To Reproduce

add_filter( 'woocommerce_subscription_periods',  'filterFrequencyPeriods', 333 );
// We only want months, No days, No weeks, No Years
function filterFrequencyPeriods( $periods, $number )
{
    $newPeriods = array_filter( $periods, function( $k ){
      return $k == 'month';
    }, ARRAY_FILTER_USE_KEY);

    return $newPeriods;
}

Expected behavior I expect to remove periods by key without generating PHP warnings

Actual behavior PHP warnings filling up the error logs

Product impact WooCommerce Subscriptions

Additional context Perhaps if your wcs_get_subscription_period_strings function's return statement could check if the array key exists, we could proceed without warnings. Something like this might work:

return ( ! empty( $period ) && isset( $translated_periods[ $period ] ) ) 
    ? $translated_periods[ $period ] 
    : $translated_periods;

Also, we are a paying customer. Can verify this if necessary.

lkraav commented 1 year ago

Can you paste your exact PHP warning @skunkbad?

I'm also trying to limit available subscription periods to just [ 'month', 'year' ].

foreach( [
        'woocommerce_subscription_available_time_periods',
        'woocommerce_subscription_periods',
        'woocommerce_subscription_trial_periods'
] as $filter ) {
        add_filter( $filter, __NAMESPACE__ . '\cxl_remove_subscription_periods' );
}

function cxl_remove_subscription_periods( array $time_periods ): array {

        foreach ( [ 'day', 'week' ] as $tp ) {
                unset( $time_periods[ $tp ] );
        }

        return $time_periods;

}

I see a problem at wcs_get_non_cached_subscription_ranges() https://github.com/Automattic/woocommerce-subscriptions-core/blob/121ef4b833aa9ae47e91e22389fca8d30057deba/includes/wcs-time-functions.php#L85 which hardcodes period names. This should respect one of the known 3 filters listed above.

Would love to get a core team comment on whether I'm understanding this correctly?

I could file a PR that runs this array through apply_filters woocommerce_subscription_periods (seems like the most general name to me), is that the way to go?

skunkbad commented 1 year ago

@ikraav, this is only one line from our error log:

[Sat Jan 14 11:30:32.426422 2023] [proxy_fcgi:error] [pid 6650:tid 140067316237888] [client 127.0.0.1:36524] AH01071: Got error 'PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:  Undefined array key "day" in /var/www/htdocs/wildfish/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/wcs-time-functions.php on line 43PHP message: PHP Warning:
lkraav commented 1 year ago

Yeah same thing.

Although in my case, I discovered the issue is more nuanced: since we have 7-day trials, I can't fully eliminate day subscription period.

My main goal was to just stop CS agents from making mistakes with creating custom subscriptions, where the default subscription period would be day.

Even after finding woocommerce_subscription_can_date_be_updated filter, which sits close enough to admin billing panel, I still had to use a crutch with add_filter_once(), because apparently more things on subscription edit page will trigger this filter.

But it works for removing day and week from subscription edit billing panel.

use WP_Screen;

// @see \WCS_Meta_Box_Schedule::output()
add_action( 'current_screen', function( WP_Screen $current_screen ): void {

    if ( 'shop_subscription' !== $current_screen->id ) {
        return;
    }

    if ( filter_input( INPUT_GET, 'enable_day_week_period' ) ) {
        return;
    }

    add_filter( 'woocommerce_subscription_can_date_be_updated', static function( bool $can_date_be_updated ): bool {

        add_filter_once( 'woocommerce_subscription_periods', function( array $time_periods ): array {
            return array_diff( $time_periods, [ 'day', 'week' ] );
        } );

        return $can_date_be_updated;

    } );

} );
skunkbad commented 1 year ago

@ikraav, for the purpose of trying to get my issue fixed, I'd prefer to not get sidetracked. We pay for the subscriptions plugin and expect a certain level of support. When raising the issue over at woocommerce.com, I was told that by the customer service person that they are not developers and I could come here to report the issue. Since we pay for this plugin, I don't expect to have to do the work to fix it, otherwise I feel like it should be a free plugin. Hopefully somebody will see this issue that cares about customers, and say/do something.

skunkbad commented 1 year ago

If we could at least have the wcs_get_subscription_period_strings function wrapped in an if statement checking if the function already exists, then at least we could define it elsewhere. It's frustrating to not get anything done when the fix is so simple.