Skip to content

Commit 81eb1c8

Browse files
committed
fix(amount-factory): address second review — zero-discount guard, shouldNotReceive, order test
- Guard zero-discount in from_store_api_cart(): pass null instead of Money(0.00) when discount_minor === 0, matching the from_wc_cart() and from_wc_order() behaviour and fixing the assertNull test assertion. - Replace shouldReceive('get_total') with shouldNotReceive() in testFromWcCartTotalDerivesFromComponentsNotWcGetTotal so the test actually enforces that get_total() is never called. - Add testFromWcOrderTotalDerivesFromComponentsNotGetTotal() with the same shouldNotReceive guard, covering the from_wc_order() rounding fix. - Fix misleading 'never used for value' comment on total_price() mock.
1 parent 31f3ffb commit 81eb1c8

2 files changed

Lines changed: 34 additions & 4 deletions

File tree

modules/ppcp-api-client/src/Factory/AmountFactory.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public function from_store_api_cart( CartTotals $cart_totals ): Amount {
149149
null,
150150
null,
151151
null,
152-
$make( $discount_minor ),
152+
$discount_minor > 0 ? $make( $discount_minor ) : null,
153153
)
154154
);
155155
}

tests/PHPUnit/ApiClient/Factory/AmountFactoryTest.php

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,9 @@ public function testFromWcCartTotalDerivesFromComponentsNotWcGetTotal(): void
182182
$cart->shouldReceive( 'get_shipping_total' )->andReturn( 5.00 );
183183
$cart->shouldReceive( 'get_total_tax' )->andReturn( 1.07 );
184184
$cart->shouldReceive( 'get_discount_total' )->andReturn( 0.0 );
185-
// get_total() deliberately returns a mismatched value to prove it is not used.
186-
$cart->shouldReceive( 'get_total' )->withAnyArgs()->andReturn( 19.39 );
185+
// Enforce that get_total() is never consulted — the total is derived solely
186+
// from breakdown components. If it were called, the test would fail.
187+
$cart->shouldNotReceive( 'get_total' );
187188

188189
$woocommerce = Mockery::mock( \WooCommerce::class );
189190
$session = Mockery::mock( \WC_Session::class );
@@ -225,7 +226,7 @@ public function testFromStoreApiCart(
225226
$totals->shouldReceive( 'total_shipping' )->andReturn( $make( $shipping ) );
226227
$totals->shouldReceive( 'total_tax' )->andReturn( $make( $tax ) );
227228
$totals->shouldReceive( 'total_discount' )->andReturn( $make( $discount ) );
228-
$totals->shouldReceive( 'total_price' )->andReturn( $make( 0 ) ); // never used for value
229+
$totals->shouldReceive( 'total_price' )->andReturn( $make( 0 ) ); // used for currency metadata only
229230

230231
$result = $this->testee->from_store_api_cart( $totals );
231232
$breakdown = $result->breakdown();
@@ -371,6 +372,35 @@ public function testFromWcOrderDiscountIsNull()
371372
$this->assertNull($result->breakdown()->discount());
372373
}
373374

375+
/**
376+
* Proves that from_wc_order() derives the total from breakdown components
377+
* and never calls get_total(). Mirrors the equivalent from_wc_cart() test.
378+
*/
379+
public function testFromWcOrderTotalDerivesFromComponentsNotGetTotal(): void
380+
{
381+
$order = Mockery::mock( \WC_Order::class );
382+
// Enforce get_total() is never consulted.
383+
$order->shouldNotReceive( 'get_total' );
384+
$order->shouldReceive( 'get_payment_method' )->andReturn( PayPalGateway::ID );
385+
$order->shouldReceive( 'get_meta' )->andReturn( null );
386+
$order->shouldReceive( 'get_currency' )->andReturn( $this->currency );
387+
// subtotal=$13.33, fees=0, shipping=$5.00, tax=$1.07 → total should be $19.40.
388+
$order->shouldReceive( 'get_subtotal' )->andReturn( 13.33 );
389+
$order->shouldReceive( 'get_total_fees' )->andReturn( 0.0 );
390+
$order->shouldReceive( 'get_shipping_total' )->andReturn( 5.00 );
391+
$order->shouldReceive( 'get_total_tax' )->andReturn( 1.07 );
392+
$order->shouldReceive( 'get_total_discount' )->andReturn( 0.0 );
393+
$this->itemFactory->shouldReceive( 'from_wc_order' )->andReturn( [] );
394+
395+
$result = $this->testee->from_wc_order( $order );
396+
397+
$this->assertSame( '19.40', $result->value_str() );
398+
$this->assertSame( '13.33', $result->breakdown()->item_total()->value_str() );
399+
$this->assertSame( '5.00', $result->breakdown()->shipping()->value_str() );
400+
$this->assertSame( '1.07', $result->breakdown()->tax_total()->value_str() );
401+
$this->assertNull( $result->breakdown()->discount() );
402+
}
403+
374404
/**
375405
* @dataProvider dataFromPayPalResponse
376406
* @param $response

0 commit comments

Comments
 (0)