Skip to content

[GraphQl] 29168 TierPrices filtering #29675

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.

Sign up for GitHub

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

Merged
magento-engcom-team merged 14 commits into from
Nov 13, 2020
Merged

[GraphQl] 29168 TierPrices filtering #29675

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
017c44e
add filter for tier price value
Usik2203 Aug 19, 2020
603bc9f
add test coverage and some changes
Usik2203 Aug 19, 2020
257ba74
Merge branch '2.4-develop' into 29168-graphql-tier-price
Usik2203 Aug 19, 2020
4f978a4
add some changes to test
Usik2203 Aug 21, 2020
2cfd8a0
remove redundant foreach
Usik2203 Aug 21, 2020
2d9740d
minor fix
Usik2203 Aug 21, 2020
6c285ad
moved part of the logic to separate method
Usik2203 Sep 2, 2020
d535303
minor fix
Usik2203 Sep 3, 2020
f498653
add some changes in method names
Usik2203 Sep 6, 2020
b2021d5
Merge branch '2.4-develop' into 29168-graphql-tier-price
Usik2203 Nov 11, 2020
30901be
CR recommendations
Usik2203 Nov 11, 2020
6b4383b
minor changes
Usik2203 Nov 11, 2020
58eaf18
Revert changes
Usik2203 Nov 11, 2020
9f9164f
Merge branch '2.4-develop' into 29168-graphql-tier-price
Usik2203 Nov 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 76 additions & 29 deletions app/code/Magento/CatalogCustomerGraphQl/Model/Resolver/PriceTiers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@

namespace Magento\CatalogCustomerGraphQl\Model\Resolver;

use Magento\Catalog\Api\Data\ProductTierPriceInterface;
use Magento\CatalogCustomerGraphQl\Model\Resolver\Customer\GetCustomerGroup;
use Magento\CatalogCustomerGraphQl\Model\Resolver\Product\Price\Tiers;
use Magento\CatalogCustomerGraphQl\Model\Resolver\Product\Price\TiersFactory;
use Magento\CatalogGraphQl\Model\Resolver\Product\Price\Discount;
use Magento\CatalogGraphQl\Model\Resolver\Product\Price\ProviderPool as PriceProviderPool;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\GraphQl\Config\Element\Field;
use Magento\Framework\GraphQl\Query\Resolver\ValueFactory;
use Magento\Framework\GraphQl\Query\ResolverInterface;
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\GraphQl\Query\Resolver\ValueFactory;
use Magento\Framework\Pricing\PriceCurrencyInterface;
use Magento\CatalogCustomerGraphQl\Model\Resolver\Product\Price\Tiers;
use Magento\CatalogCustomerGraphQl\Model\Resolver\Product\Price\TiersFactory;
use Magento\CatalogCustomerGraphQl\Model\Resolver\Customer\GetCustomerGroup;
use Magento\Store\Api\Data\StoreInterface;
use Magento\CatalogGraphQl\Model\Resolver\Product\Price\Discount;
use Magento\CatalogGraphQl\Model\Resolver\Product\Price\ProviderPool as PriceProviderPool;
use Magento\Catalog\Api\Data\ProductTierPriceInterface;

/**
* Resolver for price_tiers
Expand Down Expand Up @@ -120,42 +120,89 @@ function () use ($productId, $context) {

Copy link
Contributor

@cpartica cpartica Nov 11, 2020

Choose a reason for hiding this comment

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

can you fix types on $this->tiers->addProductFilter($productId) ? \Magento\CatalogCustomerGraphQl\Model\Resolver\Product\Price\Tiers::addProductFilter has no strict types
public function addProductFilter($productId): void
would be
public function addProductFilter(int $productId): void

Copy link
Contributor Author

@Usik2203 Usik2203 Nov 11, 2020

Choose a reason for hiding this comment

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

Done

$productPrice = $this->tiers->getProductRegularPrice($productId) ?? 0.0;
$tierPrices = $this->tiers->getProductTierPrices($productId) ?? [];

return $this->formatProductTierPrices($tierPrices, $productPrice, $store);
return $this->formatAndFilterTierPrices($tierPrices, $productPrice, $store);
}
);
}

/**
* Format tier prices for output
* Format and filter tier prices for output
*
* @param ProductTierPriceInterface[] $tierPrices
* @param float $productPrice
* @param StoreInterface $store
* @return array
*/
private function formatProductTierPrices(array $tierPrices, float $productPrice, StoreInterface $store): array
{
private function formatAndFilterTierPrices(
array $tierPrices,
float $productPrice,
StoreInterface $store
): array {
$tiers = [];
$qtyCache = [];

foreach ($tierPrices as $key => $tierPrice) {
$this->formatTierPrices($productPrice, $store, $tierPrice, $tiers);
$this->filterTierPrices($tierPrices, $key, $tierPrice, $qtyCache, $tiers);
}
return $tiers;
}

/**
* Format tier prices for output
*
* @param float $productPrice
* @param StoreInterface $store
* @param ProductTierPriceInterface $tierPrice
* @param array $tiers
*/
private function formatTierPrices(float $productPrice, StoreInterface $store, &$tierPrice, &$tiers)
Copy link
Contributor

@cpartica cpartica Nov 11, 2020

Choose a reason for hiding this comment

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

we kind of prohibit passing params by reference &$tierPrice, &$tiers
You better want a function that returns something. You rarely want a function that returns 2 things.
formatTierPrices can return $tiers for example.
Objects are passed by reference by default so you don't need &$tierPrice to do $tierPrice->setValue
You can also do that outside formatTierPrices and don't set anything in tierPrice, just get
Plus you only need getValue and getQty and getPercentageValue from it. You can use those params directly not by passing the whole tierPrice.
From store you only need CurrencyCode not store.
Suddenly your method becomes: formatTierPricesValues(productPrice, tierPriceValue, TierPricePercentage, tierPriceQty): array (where array is $tiers)

Copy link
Contributor Author

@Usik2203 Usik2203 Nov 11, 2020

Choose a reason for hiding this comment

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

Done

{
$tierPrice->setValue($this->priceCurrency->convertAndRound($tierPrice->getValue()));
$percentValue = $tierPrice->getExtensionAttributes()->getPercentageValue();
if ($percentValue && is_numeric($percentValue)) {
$discount = $this->discount->getDiscountByPercent($productPrice, (float)$percentValue);
} else {
$discount = $this->discount->getDiscountByDifference($productPrice, (float)$tierPrice->getValue());
}

foreach ($tierPrices as $tierPrice) {
$tierPrice->setValue($this->priceCurrency->convertAndRound($tierPrice->getValue()));
$percentValue = $tierPrice->getExtensionAttributes()->getPercentageValue();
if ($percentValue && is_numeric($percentValue)) {
$discount = $this->discount->getDiscountByPercent($productPrice, (float)$percentValue);
$tiers[] = [
"discount" => $discount,
"quantity" => $tierPrice->getQty(),
"final_price" => [
"value" => $tierPrice->getValue(),
"currency" => $store->getCurrentCurrencyCode()
]
];
}

/**
* Filter the lowest price for each quantity
*
* @param array $tierPrices
* @param int $key
* @param ProductTierPriceInterface $tierPriceItem
* @param array $qtyCache
* @param array $tiers
*/
private function filterTierPrices(
array $tierPrices,
int $key,
ProductTierPriceInterface $tierPriceItem,
array &$qtyCache,
Copy link
Contributor

@cpartica cpartica Nov 11, 2020

Choose a reason for hiding this comment

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

$qtyCache doesn't need to be passed by reference
it can be part of your object \Magento\CatalogCustomerGraphQl\Model\Resolver\PriceTiers
by declaring a private $productTiersByQuanity[] //you can dcument that this acts like a cache
it can hold prices like $productTiersByQuanity[customerGroup][productId][qty] so it can be used for multiple products and if called multiple times for the same product, you can just fetch it and not even need to recalculate it.
not sure if customerGroup is required. probably not. you'l only get the relevant tiers for current customerGroup
of course filterTierPrices : array which is the filtered tiers, and you can pass formatted tiers from previous function return

Copy link
Contributor Author

@Usik2203 Usik2203 Nov 11, 2020

Choose a reason for hiding this comment

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

Done

array &$tiers
) {
$qty = $tierPriceItem->getQty();
if (isset($qtyCache[$qty])) {
$priceQty = $qtyCache[$qty];
if ((float)$tierPriceItem->getValue() < (float)$tierPrices[$priceQty]->getValue()) {
unset($tiers[$priceQty]);
$qtyCache[$priceQty] = $key;
} else {
$discount = $this->discount->getDiscountByDifference($productPrice, (float)$tierPrice->getValue());
unset($tiers[$key]);
}

$tiers[] = [
"discount" => $discount,
"quantity" => $tierPrice->getQty(),
"final_price" => [
"value" => $tierPrice->getValue(),
"currency" => $store->getCurrentCurrencyCode()
]
];
} else {
$qtyCache[$qty] = $key;
}
return $tiers;
}
}
22 changes: 19 additions & 3 deletions dev/tests/api-functional/testsuite/Magento/GraphQl/CatalogCustomer/PriceTiersTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
namespace Magento\GraphQl\CatalogCustomer;

use Magento\GraphQl\GetCustomerAuthenticationHeader;
use Magento\TestFramework\TestCase\GraphQlAbstract;
use Magento\Store\Api\StoreRepositoryInterface;
use Magento\TestFramework\Helper\Bootstrap;
use Magento\TestFramework\ObjectManager;
use Magento\TestFramework\TestCase\GraphQlAbstract;

class PriceTiersTest extends GraphQlAbstract
{
Expand Down Expand Up @@ -64,10 +64,12 @@ public function testLoggedInCustomer()
);

$itemTiers = $response['products']['items'][0]['price_tiers'];
$this->assertCount(3, $itemTiers);
$this->assertCount(5, $itemTiers);
$this->assertEquals(9.25, $this->getValueForQuantity(2, $itemTiers));
$this->assertEquals(8.25, $this->getValueForQuantity(3, $itemTiers));
$this->assertEquals(7.25, $this->getValueForQuantity(5, $itemTiers));
$this->assertEquals(9.00, $this->getValueForQuantity(7, $itemTiers));
$this->assertEquals(7.25, $this->getValueForQuantity(8, $itemTiers));
}

/**
Expand Down Expand Up @@ -95,12 +97,26 @@ public function testSecondStoreViewWithCurrencyRate()
);

$itemTiers = $response['products']['items'][0]['price_tiers'];
$this->assertCount(3, $itemTiers);
$this->assertCount(5, $itemTiers);
$this->assertEquals(round(9.25 * $rate, 2), $this->getValueForQuantity(2, $itemTiers));
$this->assertEquals(round(8.25 * $rate, 2), $this->getValueForQuantity(3, $itemTiers));
$this->assertEquals(round(7.25 * $rate, 2), $this->getValueForQuantity(5, $itemTiers));
}

/**
* @magentoApiDataFixture Magento/Catalog/_files/simple_product_with_tier_prices_for_multiple_groups.php
*/
public function testGetLowestPriceForGuest()
{
$productSku = 'simple';
$query = $this->getProductSearchQuery($productSku);
$response = $this->graphQlQuery($query);
$itemTiers = $response['products']['items'][0]['price_tiers'];
$this->assertCount(2, $itemTiers);
$this->assertEquals(round(8.25, 2), $this->getValueForQuantity(7, $itemTiers));
$this->assertEquals(round(7.25, 2), $this->getValueForQuantity(8, $itemTiers));
}

/**
* Get the tier price value for the given product quantity
*
Expand Down
31 changes: 31 additions & 0 deletions .../testsuite/Magento/Catalog/_files/simple_product_with_tier_prices_for_multiple_groups.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Magento\Store\Api\WebsiteRepositoryInterface;
use Magento\TestFramework\Helper\Bootstrap;
use Magento\TestFramework\Workaround\Override\Fixture\Resolver;
use Magento\Customer\Model\Group;

Resolver::getInstance()->requireDataFixture('Magento/Catalog/_files/product_simple.php');

Expand Down Expand Up @@ -57,6 +58,36 @@
'percentage_value'=> null,
'qty'=> 5,
'value'=> 7
],
[
'customer_group_id' => Group::CUST_GROUP_ALL,
'percentage_value'=> null,
'qty'=> 7,
'value'=> 9.25
],
[
'customer_group_id' => '1',
'percentage_value'=> null,
'qty'=> 7,
'value'=> 9.00
],
[
'customer_group_id' => Group::NOT_LOGGED_IN_ID,
'percentage_value'=> null,
'qty'=> 7,
'value'=> 8.25
],
[
'customer_group_id' => Group::CUST_GROUP_ALL,
'percentage_value'=> null,
'qty'=> 8,
'value'=> 7.25
],
[
'customer_group_id' => Group::NOT_LOGGED_IN_ID,
'percentage_value'=> null,
'qty'=> 8,
'value'=> 9
]
];
$productTierPrices = [];
Expand Down