It essentially is saying that if the current variation cannot be the minimum then just continue to the next item. However, this ignores the fact that the variation could be maximum and we'll never make that determination. This leads to this function only ever returning the first variation as the maximum.
It never sets the $shortest_initial_interval
In the wcs_calculate_min_max_variations() it attempts to find if the current variation has the shortest interval, however it never sets it.
The product with the highest price and shortest period and interval is considered the most expensive. Thats the base case and it makes sense.
The elseif though also requires that the variation price be equal to the highest price and either have the shortest period & shortest interval which to me sounds like the original if 🤔 and the second condition just checks if its' the shortest interval. If you think about it this whole if else block could be simplified.
In any case the is_max logic is too limited. If we never find a product with the highest initial cost and shortest period, we'll basically never find the highest price.
[!important]
Looking further into I can't see a use of the maximum variation data anyway. We can probably get rid of it or look into fixing these bugs to make sure it returns a reasonable, recognisable and understandable maximum priced variation.
Create a variable subscription product with 3 variations:
10 / month
25 / 3 months
100 / year
View the product meta in the database and note that the maximum variation data is the same as the minimum
Expected behavior
It's hard to know how this function intends to calculate the maximum priced variation product. It doesn't appear to make use of any price / day calculation or anything. But perhaps looking into that might be the simplest approach.
[!Note]
Given the maximum variation data isn't currently used, I've requested that the original submitter of this bug provide more information about what their intent is so we can better understand the priority and possibly a fix too. This is why this has a needs-prioritisation label.
Describe the bug
The
wcs_calculate_min_max_variations()function takes an array of product variation data and is tasked with finding the minimum and maximum price data.This function is super convoluted (~300 lines long) and hard to follow at times.
While looking at it I think I've found 3 bugs.
This
continueshould be removedhttps://github.com/Automattic/woocommerce-subscriptions-core/blob/a923da64ef4e5c4b7747fa81f4b496ca4dfb6298/includes/wcs-product-functions.php#L287-L290
It essentially is saying that if the current variation cannot be the minimum then just continue to the next item. However, this ignores the fact that the variation could be maximum and we'll never make that determination. This leads to this function only ever returning the first variation as the maximum.
It never sets the
$shortest_initial_intervalIn the
wcs_calculate_min_max_variations()it attempts to find if the current variation has the shortest interval, however it never sets it.https://github.com/Automattic/woocommerce-subscriptions-core/blob/a923da64ef4e5c4b7747fa81f4b496ca4dfb6298/includes/wcs-product-functions.php#L285
Something like this here should fix that.
The
$is_maxhas limited logicThe section designed to set the maximum priced variation is limited and may even be confused.
https://github.com/Automattic/woocommerce-subscriptions-core/blob/a923da64ef4e5c4b7747fa81f4b496ca4dfb6298/includes/wcs-product-functions.php#L335-L352
The product with the highest price and shortest period and interval is considered the most expensive. Thats the base case and it makes sense.
The
elseifthough also requires that the variation price be equal to the highest price and either have the shortest period & shortest interval which to me sounds like the originalif🤔 and the second condition just checks if its' the shortest interval. If you think about it this whole if else block could be simplified.In any case the is_max logic is too limited. If we never find a product with the highest initial cost and shortest period, we'll basically never find the highest price.
Ticket: 7457511-zd-a8c Slack: p1702972851798119-slack-C7U3Y3VMY
To Reproduce
Expected behavior
It's hard to know how this function intends to calculate the maximum priced variation product. It doesn't appear to make use of any price / day calculation or anything. But perhaps looking into that might be the simplest approach.