Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fatal error in purchasing a subscription with PRB and ECE when the subscription's price or sign-up fee is a string #3617

Merged
merged 8 commits into from
Nov 28, 2024

Conversation

Mayisha
Copy link
Contributor

@Mayisha Mayisha commented Nov 21, 2024

Fixes #2966

In get_product_price function for PRB/ECE we need $product->get_price(), wc_get_price_including_tax, wc_get_price_excluding_tax, WC_Subscriptions_Product::get_sign_up_fee for different scenarios. These functions may return the result in different types as per the corresponding function definitions int/float/string.

When the types are different the following code causes a fatal error-

$product_price = $product->get_price() + WC_Subscriptions_Product::get_sign_up_fee( $product );

Changes proposed in this Pull Request:

  • casting the prices to float to avoid the fatal error.

Testing instructions

  • The error is not always reproducible. So we should test subscription purchase
  • Create a few products if you don't have them already-
    • A subscription product with no signup fee
    • A variable subscription product with a signup fee
    • A simple subscription product with a signup fee
  • Purchase these type of products with PRB button. Confirm that there is no fatal error.
  • Enable the ECE feature flag.
  • Purchase these type of products with ECE button. Confirm that there is no fatal error.

@Mayisha Mayisha requested review from a team and wjrosa and removed request for a team November 21, 2024 06:49
@Mayisha Mayisha requested a review from wjrosa November 22, 2024 12:11
@@ -366,16 +366,16 @@ public function get_button_label() {
* @since 5.2.0
*
* @param object $product WC_Product_* object.
* @return integer Total price.
* @return float Total price.
*/
public function get_product_price( $product ) {
$product_price = $product->get_price();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$product_price = $product->get_price();
$product_price = (float) $product->get_price();

}

return $product_price;
return (float) $product_price;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (float) $product_price;
return $product_price;

*/
public function get_product_price( $product ) {
$product_price = $product->get_price();
// Add subscription sign-up fees to product price.
if ( in_array( $product->get_type(), [ 'subscription', 'subscription_variation' ] ) && class_exists( 'WC_Subscriptions_Product' ) ) {
$product_price = $product->get_price() + WC_Subscriptions_Product::get_sign_up_fee( $product );
$product_price = (float) $product_price + (float) WC_Subscriptions_Product::get_sign_up_fee( $product );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$product_price = (float) $product_price + (float) WC_Subscriptions_Product::get_sign_up_fee( $product );
$product_price += (float) WC_Subscriptions_Product::get_sign_up_fee( $product );

Copy link
Contributor

@wjrosa wjrosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, Mayisha. Works as intended!

I left some optional suggestions (which I believe makes the code easier to read).

@Mayisha Mayisha merged commit c03c136 into develop Nov 28, 2024
35 checks passed
@Mayisha Mayisha deleted the fix/2966-prb-sub-fatal branch November 28, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal error with PRBs and Subscriptions when the subscription's price or sign-up fee is a string
2 participants