-
Notifications
You must be signed in to change notification settings - Fork 225
Adaptive pricing support check for products in the cart #5017
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
base: develop
Are you sure you want to change the base?
Changes from 4 commits
4f65803
b9f46af
2c959ca
72f39bc
15217ac
64f4aec
fe9756c
7d8ef82
b9e042b
24d1079
08701fc
9c8a580
84306f5
076eb1c
17ed001
84ec96d
4027bdf
383ad96
c7d230a
87e09a4
e375895
3acbf53
7d680a5
e860f18
f05e803
7bbdabc
0767965
b6e5ef5
06c8b9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1105,6 +1105,57 @@ public static function has_cart_or_checkout_on_current_page() { | |
| return is_cart() || is_checkout() || has_block( 'woocommerce/cart' ) || has_block( 'woocommerce/checkout' ); | ||
| } | ||
|
|
||
| /** | ||
| * Returns whether adaptive pricing is supported for the current checkout. | ||
| * | ||
| * When on the checkout page, adaptive pricing is not supported if the cart contains | ||
| * any of: subscription products or pre-order (charge upon release) products. | ||
| * | ||
| * @return bool True if adaptive pricing is supported for the current checkout, false otherwise. | ||
| * @since 10.5.0 | ||
| */ | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public static function is_adaptive_pricing_supported(): bool { | ||
|
|
||
| // False if checkout session feature flag is disabled. | ||
| if ( ! WC_Stripe_Feature_Flags::is_checkout_sessions_available() ) { | ||
| return false; | ||
| } | ||
|
|
||
| // False if adaptive pricing option is disabled. | ||
| if ( 'yes' !== WC_Stripe_Helper::get_settings( null, 'adaptive_pricing' ) ) { | ||
Mayisha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
| } | ||
|
|
||
| // False if not on the checkout page. | ||
| if ( ! is_checkout() && ! has_block( 'woocommerce/checkout' ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( ! WC()->cart || WC()->cart->is_empty() ) { | ||
| return true; | ||
| } | ||
|
|
||
| foreach ( WC()->cart->get_cart() as $cart_item_key => $cart_item ) { | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $product = apply_filters( 'woocommerce_cart_item_product', $cart_item['data'], $cart_item, $cart_item_key ); | ||
|
|
||
| if ( ! is_object( $product ) || ! ( $product instanceof WC_Product ) ) { | ||
| continue; | ||
| } | ||
|
|
||
| // Subscriptions are not supported with adaptive pricing. | ||
| if ( class_exists( 'WC_Subscriptions_Product' ) && WC_Subscriptions_Product::is_subscription( $product ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| // Pre-order (charge upon release) is not supported with adaptive pricing. | ||
| if ( class_exists( 'WC_Pre_Orders_Product' ) && WC_Pre_Orders_Product::product_is_charged_upon_release( $product ) ) { | ||
|
||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Return true if the current_tab and current_section match the ones we want to check against. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| <?php | ||
| /** | ||
| * Pre-Orders WC_Pre_Orders_Product helper. | ||
| */ | ||
|
|
||
| /** | ||
| * Class WC_Pre_Orders_Product. | ||
| * | ||
| * This helper class should ONLY be used for unit tests. | ||
| */ | ||
| class WC_Pre_Orders_Product { | ||
|
|
||
| /** | ||
| * The mocked value for product_is_charged_upon_release(). | ||
| * | ||
| * @var bool | ||
| */ | ||
| public static bool $product_is_charged_upon_release_result = false; | ||
|
|
||
| /** | ||
| * Set the value to be returned from product_is_charged_upon_release(). | ||
| * | ||
| * @param bool $result The value to return from product_is_charged_upon_release(). | ||
| * @return void | ||
| */ | ||
| public static function set_is_pre_order( bool $result ): void { | ||
| self::$product_is_charged_upon_release_result = $result; | ||
| } | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Determine if a product is charged upon release (pre-order). | ||
| * | ||
| * @param WC_Product $product The product to check. | ||
| * @return bool | ||
| */ | ||
| public static function product_is_charged_upon_release( $product ): bool { | ||
| return self::$product_is_charged_upon_release_result; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -784,15 +784,15 @@ public function test_is_webhook_url( $url, $webhook_url, $expected_result ) { | |
| */ | ||
| public function is_webhook_url_provider() { | ||
| return [ | ||
| 'webhook URLs with mismatched protocol should match' => [ 'https://example.com/?wc-api=wc_stripe', 'http://example.com/?wc-api=wc_stripe', true ], | ||
| 'webhook URLs with mismatched host should not match' => [ 'https://example.com/?wc-api=wc_stripe', 'https://test.example.com/?wc-api=wc_stripe', false ], | ||
| 'webhook URLs with mismatched path should not match' => [ 'https://example.com/foo?wc-api=wc_stripe', 'https://example.com/bar?wc-api=wc_stripe', false ], | ||
| 'webhook URL with empty path should match' => [ 'https://example.com/', 'https://example.com/?wc-api=wc_stripe', true ], | ||
| 'webhook URL with empty query string should match' => [ 'https://example.com/test/', 'https://example.com/test/', true ], | ||
| 'webhook URLs with mismatched protocol should match' => [ 'https://example.com/?wc-api=wc_stripe', 'http://example.com/?wc-api=wc_stripe', true ], | ||
| 'webhook URLs with mismatched host should not match' => [ 'https://example.com/?wc-api=wc_stripe', 'https://test.example.com/?wc-api=wc_stripe', false ], | ||
| 'webhook URLs with mismatched path should not match' => [ 'https://example.com/foo?wc-api=wc_stripe', 'https://example.com/bar?wc-api=wc_stripe', false ], | ||
| 'webhook URL with empty path should match' => [ 'https://example.com/', 'https://example.com/?wc-api=wc_stripe', true ], | ||
| 'webhook URL with empty query string should match' => [ 'https://example.com/test/', 'https://example.com/test/', true ], | ||
| 'webhook URL with empty comparison query should not match' => [ 'https://example.com/test/?foo=bar', 'https://example.com/test/', false ], | ||
| 'webhook URL with missing parameter should not match' => [ 'https://example.com/test/?wc-api=wc_stripe', 'https://example.com/test/?wc-api=wc_stripe&foo=bar', false ], | ||
| 'webhook URL with wrong parameter should not match' => [ 'https://example.com/test/?wc-api=wc_stripe_BAD', 'https://example.com/test/?wc-api=wc_stripe', false ], | ||
| 'webhook URL with extra parameters should match' => [ 'https://example.com/test/?wc-api=wc_stripe&foo=bar', 'https://example.com/test/?wc-api=wc_stripe', true ], | ||
| 'webhook URL with missing parameter should not match' => [ 'https://example.com/test/?wc-api=wc_stripe', 'https://example.com/test/?wc-api=wc_stripe&foo=bar', false ], | ||
| 'webhook URL with wrong parameter should not match' => [ 'https://example.com/test/?wc-api=wc_stripe_BAD', 'https://example.com/test/?wc-api=wc_stripe', false ], | ||
| 'webhook URL with extra parameters should match' => [ 'https://example.com/test/?wc-api=wc_stripe&foo=bar', 'https://example.com/test/?wc-api=wc_stripe', true ], | ||
Mayisha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ]; | ||
| } | ||
|
|
||
|
|
@@ -882,7 +882,7 @@ public function test_get_localized_error_message_from_response( string $error_ty | |
| */ | ||
| public function provide_test_get_localized_error_message_from_response(): array { | ||
| return [ | ||
| 'card_error with localized message' => [ | ||
| 'card_error with localized message' => [ | ||
| 'error_type' => 'card_error', | ||
| 'error_code' => 'invalid_cvc', | ||
| 'error_message' => 'Mock invalid CVC', | ||
|
|
@@ -891,14 +891,14 @@ public function provide_test_get_localized_error_message_from_response(): array | |
| ], | ||
| 'expected_message' => "The card's security code is invalid.", | ||
| ], | ||
| 'card_error without localized message' => [ | ||
| 'card_error without localized message' => [ | ||
| 'error_type' => 'card_error', | ||
| 'error_code' => 'unexpected_error_code', | ||
| 'error_message' => 'Unexpected error', | ||
| 'localized_data' => [], | ||
| 'expected_message' => 'Unexpected error', | ||
| ], | ||
| 'other error with localized message' => [ | ||
| 'other error with localized message' => [ | ||
| 'error_type' => 'invalid_request_error', | ||
| 'error_code' => 'amount_too_small', | ||
| 'error_message' => 'Amount too small', | ||
|
|
@@ -936,31 +936,31 @@ public function test_get_localized_error_message_from_response_with_unexpected_d | |
| */ | ||
| public function provide_test_get_localized_error_message_from_response_with_unexpected_data(): array { | ||
| return [ | ||
| 'String response' => [ | ||
| 'String response' => [ | ||
| 'response' => 'Unexpected data', | ||
| 'expected_message' => '', | ||
| ], | ||
| 'Integer response' => [ | ||
| 'Integer response' => [ | ||
| 'response' => 123, | ||
| 'expected_message' => '', | ||
| ], | ||
| 'Float response' => [ | ||
| 'Float response' => [ | ||
| 'response' => 123.45, | ||
| 'expected_message' => '', | ||
| ], | ||
| 'Boolean response' => [ | ||
| 'Boolean response' => [ | ||
| 'response' => true, | ||
| 'expected_message' => '', | ||
| ], | ||
| 'Array response' => [ | ||
| 'Array response' => [ | ||
| 'response' => [ 'error' => 'Unexpected data' ], | ||
| 'expected_message' => '', | ||
| ], | ||
| 'Object response with string error' => [ | ||
| 'response' => (object) [ 'error' => 'Unexpected data' ], | ||
| 'expected_message' => '', | ||
| ], | ||
| 'Object response with array error' => [ | ||
| 'Object response with array error' => [ | ||
| 'response' => (object) [ 'error' => [ 'message' => 'Unexpected data' ] ], | ||
| 'expected_message' => '', | ||
| ], | ||
|
Comment on lines
939
to
966
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: it looks like the whitespace for the array keys was unintentionally removed. |
||
|
|
@@ -995,7 +995,7 @@ public function provide_test_get_localized_error_message_from_response_with_unex | |
| 'Object response with object error, type, and object message property' => [ | ||
| 'response' => (object) [ | ||
| 'error' => (object) [ | ||
| 'type' => 'card_error', | ||
| 'type' => 'card_error', | ||
| 'message' => (object) [ 'test' => 'Unexpected error' ], | ||
| ], | ||
| ], | ||
|
|
@@ -1095,22 +1095,141 @@ public function test_build_line_items( bool $itemized = false, array $expected_i | |
| $this->assertSame( $expected_items, $actual ); | ||
| } | ||
|
|
||
| /** | ||
| * Test for `is_adaptive_pricing_supported` – cart content and preconditions. | ||
| * | ||
| * @param bool $feature_flag Feature flag enabled. | ||
| * @param bool $is_checkout Checkout page. | ||
| * @param string $adaptive_pricing Adaptive pricing setting. | ||
| * @param string $cart_product_type Cart product type. | ||
| * @param bool $expected Expected result. | ||
| * @return void | ||
| * @dataProvider provide_is_adaptive_pricing_supported | ||
| */ | ||
| public function test_is_adaptive_pricing_supported( bool $feature_flag, bool $is_checkout, string $adaptive_pricing, ?string $cart_product_type, bool $expected ): void { | ||
| $stripe_settings = WC_Stripe_Helper::get_stripe_settings(); | ||
| $stripe_settings['adaptive_pricing'] = $adaptive_pricing; | ||
| $stripe_settings['optimized_checkout_element'] = 'yes'; | ||
| $stripe_settings['capture'] = 'yes'; | ||
| $stripe_settings['pmc_enabled'] = 'yes'; | ||
| WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings ); | ||
|
|
||
| update_option( \WC_Stripe_Feature_Flags::CHECKOUT_SESSIONS_FEATURE_FLAG_NAME, $feature_flag ? 'yes' : 'no' ); | ||
|
|
||
| $is_checkout_filter = function () use ( $is_checkout ) { | ||
| return $is_checkout; | ||
| }; | ||
| add_filter( 'woocommerce_is_checkout', $is_checkout_filter ); | ||
|
|
||
| if ( 'subscription' === $cart_product_type ) { | ||
| \WC_Subscriptions_Product::set_is_subscription( true ); | ||
| } else { | ||
| \WC_Subscriptions_Product::set_is_subscription( false ); | ||
| } | ||
|
|
||
| if ( 'pre-order' === $cart_product_type ) { | ||
| \WC_Pre_Orders_Product::set_is_pre_order( true ); | ||
| } else { | ||
| \WC_Pre_Orders_Product::set_is_pre_order( false ); | ||
| } | ||
Mayisha marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| WC()->cart->empty_cart(); | ||
| $product = null; | ||
| if ( ! empty( $cart_product_type ) ) { | ||
| $product = WC_Helper_Product::create_simple_product(); | ||
| WC()->cart->add_to_cart( $product->get_id(), 1 ); | ||
| } | ||
|
|
||
| $actual = WC_Stripe_Helper::is_adaptive_pricing_supported(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a thought, it may be worth adding tests that check for additional cases that would exercise more logic, including the following:
TIL that pre-orders can't be in a cart with other products.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. |
||
|
|
||
| // Cleanup. | ||
| WC()->cart->empty_cart(); | ||
| remove_filter( 'woocommerce_is_checkout', $is_checkout_filter ); | ||
| WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings ); | ||
| update_option( \WC_Stripe_Feature_Flags::CHECKOUT_SESSIONS_FEATURE_FLAG_NAME, 'no' ); | ||
|
|
||
| if ( isset( $product ) && $product ) { | ||
| $product->delete( true ); | ||
| } | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| $this->assertSame( $expected, $actual ); | ||
| } | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Data provider for `test_is_adaptive_pricing_supported`. | ||
| * | ||
| * @return array | ||
| */ | ||
| public function provide_is_adaptive_pricing_supported(): array { | ||
| return [ | ||
| 'feature flag disabled' => [ | ||
| 'feature_flag' => false, | ||
| 'is_checkout' => true, | ||
| 'adaptive_pricing' => 'yes', | ||
| 'cart_product_type' => 'simple', | ||
| 'expected' => false, | ||
| ], | ||
| 'adaptive pricing disabled' => [ | ||
| 'feature_flag' => true, | ||
| 'is_checkout' => true, | ||
| 'adaptive_pricing' => 'no', | ||
| 'cart_product_type' => 'simple', | ||
| 'expected' => false, | ||
| ], | ||
| 'not on checkout' => [ | ||
| 'feature_flag' => true, | ||
| 'is_checkout' => false, | ||
| 'adaptive_pricing' => 'yes', | ||
| 'cart_product_type' => 'simple', | ||
| 'expected' => false, | ||
| ], | ||
| 'empty cart' => [ | ||
| 'feature_flag' => true, | ||
| 'is_checkout' => true, | ||
| 'adaptive_pricing' => 'yes', | ||
| 'cart_product_type' => null, | ||
| 'expected' => true, | ||
| ], | ||
| 'simple product only' => [ | ||
| 'feature_flag' => true, | ||
| 'is_checkout' => true, | ||
| 'adaptive_pricing' => 'yes', | ||
| 'cart_product_type' => 'simple', | ||
| 'expected' => true, | ||
| ], | ||
| 'subscription in cart' => [ | ||
| 'feature_flag' => true, | ||
| 'is_checkout' => true, | ||
| 'adaptive_pricing' => 'yes', | ||
| 'cart_product_type' => 'subscription', | ||
| 'expected' => false, | ||
| ], | ||
| 'pre-order in cart' => [ | ||
| 'feature_flag' => true, | ||
| 'is_checkout' => true, | ||
| 'adaptive_pricing' => 'yes', | ||
| 'cart_product_type' => 'pre-order', | ||
| 'expected' => false, | ||
| ], | ||
| ]; | ||
| } | ||
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
Mayisha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Data provider for `test_build_line_items`. | ||
| * | ||
| * @return array | ||
| */ | ||
| public function provide_test_build_line_items(): array { | ||
| return [ | ||
| 'itemized' => [ | ||
| 'itemized' => [ | ||
| 'itemized' => true, | ||
| 'expected items' => [ | ||
| [ | ||
| 'label' => 'Dummy Product', | ||
| 'amount' => 1000, | ||
| ], | ||
| [ | ||
| 'label' => 'Tax', | ||
| 'label' => 'Tax', | ||
| 'amount' => 0, | ||
| ], | ||
| [ | ||
|
|
@@ -1125,7 +1244,7 @@ public function provide_test_build_line_items(): array { | |
| ], | ||
| ], | ||
| ], | ||
| 'non-itemized' => [ | ||
| 'non-itemized' => [ | ||
| 'itemized' => false, | ||
| 'expected items' => array_merge( | ||
| [ | ||
|
|
@@ -1134,7 +1253,7 @@ public function provide_test_build_line_items(): array { | |
| 'amount' => 1000, | ||
| ], | ||
| [ | ||
| 'label' => 'Tax', | ||
| 'label' => 'Tax', | ||
| 'amount' => 0, | ||
| ], | ||
| [ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think it would help if the specific items were listed on their own lines.
Also, is it OK to handle products using a deposit? I don't think we save the payment method in that case, but I worry that Deposits sets the expectation that it is $X today and $Y later, when the actual amounts in local currency may not be $Y on any given future date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I have added deposits in the unsupported list in my latest commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be good to make sure that decision/exclusion is pushed up to the requirements discussion/doc as well.