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.

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

Jump to bottom

Add a column id_shop in product_sale #29547

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Add a column id_shop in product_sale #29547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e2e9449
Add a column id_shop in product_sale
lmeyer1 Sep 6, 2022
99a36f8
product_sale: fix trivial errors
lmeyer1 Sep 6, 2022
cc2e79f
Update database structure
lmeyer1 Sep 6, 2022
78b99fa
Add the argument idShop where required
lmeyer1 Sep 6, 2022
93f37a6
Make id_shop optional and working in shop group or all shops context
lmeyer1 Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion classes/PaymentModule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ public function validateOrder(

foreach ($this->context->cart->getProducts() as $product) {
if ($order_status->logable) {
ProductSale::addProductSale((int) $product['id_product'], (int) $product['cart_quantity']);
ProductSale::addProductSale((int) $product['id_product'], (int) $product['cart_quantity'], $order->id_shop);
}
}

Expand Down
80 changes: 53 additions & 27 deletions classes/ProductSale.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ class ProductSaleCore
public static function fillProductSales()
{
$sql = 'REPLACE INTO ' . _DB_PREFIX_ . 'product_sale
(`id_product`, `quantity`, `sale_nbr`, `date_upd`)
SELECT od.product_id, SUM(od.product_quantity), COUNT(od.product_id), NOW()
FROM ' . _DB_PREFIX_ . 'order_detail od GROUP BY od.product_id';
(`id_product`, `id_shop`,`quantity`, `sale_nbr`, `date_upd`)
SELECT od.product_id, od.id_shop, SUM(od.product_quantity), COUNT(od.product_id), NOW()
FROM ' . _DB_PREFIX_ . 'order_detail od GROUP BY od.product_id, od.id_shop';

return Db::getInstance()->execute($sql);
}
Expand All @@ -53,8 +53,8 @@ public static function getNbSales()
{
$sql = 'SELECT COUNT(ps.`id_product`) AS nb
FROM `' . _DB_PREFIX_ . 'product_sale` ps
LEFT JOIN `' . _DB_PREFIX_ . 'product` p ON p.`id_product` = ps.`id_product`
' . Shop::addSqlAssociation('product', 'p', false) . '
INNER JOIN `' . _DB_PREFIX_ . 'product` p ON p.`id_product` = ps.`id_product`
' . Shop::addSqlAssociation('product', 'p') . ' and product_shop.id_shop = ps.id_shop
WHERE product_shop.`active` = 1';

return (int) Db::getInstance(_PS_USE_SQL_SLAVE_)->getValue($sql);
Expand Down Expand Up @@ -111,8 +111,8 @@ public static function getBestSales($idLang, $pageNumber = 0, $nbProducts = 10,
DATEDIFF(p.`date_add`, DATE_SUB("' . date('Y-m-d') . ' 00:00:00",
INTERVAL ' . (int) $interval . ' DAY)) > 0 AS new'
. ' FROM `' . _DB_PREFIX_ . 'product_sale` ps
LEFT JOIN `' . _DB_PREFIX_ . 'product` p ON ps.`id_product` = p.`id_product`
' . Shop::addSqlAssociation('product', 'p', false);
INNER JOIN `' . _DB_PREFIX_ . 'product` p ON ps.`id_product` = p.`id_product`
' . Shop::addSqlAssociation('product', 'p') . ' and product_shop.id_shop = ps.id_shop';
if (Combination::isFeatureActive()) {
$sql .= ' LEFT JOIN `' . _DB_PREFIX_ . 'product_attribute_shop` product_attribute_shop
ON (p.`id_product` = product_attribute_shop.`id_product` AND product_attribute_shop.`default_on` = 1 AND product_attribute_shop.id_shop=' . (int) $context->shop->id . ')';
Expand Down Expand Up @@ -193,8 +193,8 @@ public static function getBestSalesLight($idLang, $pageNumber = 0, $nbProducts =
product_shop.`date_add` > "' . date('Y-m-d', strtotime('-' . (Configuration::get('PS_NB_DAYS_NEW_PRODUCT') ? (int) Configuration::get('PS_NB_DAYS_NEW_PRODUCT') : 20) . ' DAY')) . '" as new,
product_shop.`on_sale`, product_attribute_shop.minimal_quantity AS product_attribute_minimal_quantity
FROM `' . _DB_PREFIX_ . 'product_sale` ps
LEFT JOIN `' . _DB_PREFIX_ . 'product` p ON ps.`id_product` = p.`id_product`
' . Shop::addSqlAssociation('product', 'p') . '
INNER JOIN `' . _DB_PREFIX_ . 'product` p ON ps.`id_product` = p.`id_product`
' . Shop::addSqlAssociation('product', 'p') . ' and product_shop.id_shop = ps.id_shop
LEFT JOIN `' . _DB_PREFIX_ . 'product_attribute_shop` product_attribute_shop
ON (p.`id_product` = product_attribute_shop.`id_product` AND product_attribute_shop.`default_on` = 1 AND product_attribute_shop.id_shop=' . (int) $context->shop->id . ')
LEFT JOIN `' . _DB_PREFIX_ . 'product_attribute` pa ON (product_attribute_shop.id_product_attribute=pa.id_product_attribute)
Expand Down Expand Up @@ -235,28 +235,45 @@ public static function getBestSalesLight($idLang, $pageNumber = 0, $nbProducts =
*
* @param int $productId Product ID
* @param int $qty Quantity
* @param int $idShop Shop ID
*
* @return bool Indicates whether the sale was successfully added
*/
public static function addProductSale($productId, $qty = 1)
public static function addProductSale($productId, $qty = 1, $idShop = null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jolelievre, @kpodemski
I now changed like suggested.

But for these two functions addProductSale and removeProductSale working in shop group or all shop context is hurting the statistics. In both cases, we either add or remove one sale for each shop. This is duplicating sales or removing them duplicated.

We could argue, that it would be better do nothing, if $isShop is null, because in this case we can assess exactly the error: one sale has not be added or removed. When adding or removing on an array of shops, this array could be big, and the error introduced would be big too. (Suppose we have 20 shops, we would save 20 sales instead of 1).

Copy link
Contributor

Choose a reason for hiding this comment

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

@lmeyer1, you are right. Unfortunately, I don't see any way to keep "BC Break-free" different way

We could:

  • introduce a new method addProductSaleForShop
  • deprecate or leave the old one
  • change the use of addProductSale for addProductSaleForShop
  • but still, there will be a problem with those who run addProductSale because id_shop will be NULL, and we won't get any information about it in methods getting sales

Maybe we should wait for PS 9.0 and change it then?

{
return Db::getInstance()->execute('
if (is_null($idShop)) {
$idShops = Shop::getContextListShopID();
} else {
$idShops = [$idShop];
}
$success = true;
foreach ($idShops as $idShop) {
$success = $success && Db::getInstance()->execute('
INSERT INTO ' . _DB_PREFIX_ . 'product_sale
(`id_product`, `quantity`, `sale_nbr`, `date_upd`)
VALUES (' . (int) $productId . ', ' . (int) $qty . ', 1, NOW())
(`id_product`, `id_shop`, `quantity`, `sale_nbr`, `date_upd`)
VALUES (' . (int) $productId . ', ' . (int) $idShop . ', ' . (int) $qty . ', 1, NOW())
ON DUPLICATE KEY UPDATE `quantity` = `quantity` + ' . (int) $qty . ', `sale_nbr` = `sale_nbr` + 1, `date_upd` = NOW()');
}

return $success;
}

/**
* Get number of sales.
*
* @param int $idProduct Product ID
* @param int $idShop Shop ID
*
* @return int Number of sales for the given Product
*/
public static function getNbrSales($idProduct)
public static function getNbrSales($idProduct, $idShop = null)
{
$result = Db::getInstance()->getRow('SELECT `sale_nbr` FROM ' . _DB_PREFIX_ . 'product_sale WHERE `id_product` = ' . (int) $idProduct);
if (is_null($idShop)) {
$idShops = Shop::getContextListShopID();
} else {
$idShops = [$idShop];
}
$result = Db::getInstance()->getRow('SELECT `sale_nbr` FROM ' . _DB_PREFIX_ . 'product_sale WHERE `id_product` = ' . (int) $idProduct . ' and `id_shop` in (' . implode(', ', $idShops) . ')');
if (empty($result) || !array_key_exists('sale_nbr', $result)) {
return -1;
}
Expand All @@ -269,23 +286,32 @@ public static function getNbrSales($idProduct)
*
* @param int $idProduct Product ID
* @param int $qty Quantity
* @param int $idShop Shop ID
*
* @return bool Indicates whether the product sale has been successfully removed
*/
public static function removeProductSale($idProduct, $qty = 1)
public static function removeProductSale($idProduct, $qty = 1, $idShop = null)
{
$totalSales = ProductSale::getNbrSales($idProduct);
if ($totalSales > 1) {
return Db::getInstance()->execute(
'
UPDATE ' . _DB_PREFIX_ . 'product_sale
SET `quantity` = CAST(`quantity` AS SIGNED) - ' . (int) $qty . ', `sale_nbr` = CAST(`sale_nbr` AS SIGNED) - 1, `date_upd` = NOW()
WHERE `id_product` = ' . (int) $idProduct
);
} elseif ($totalSales == 1) {
return Db::getInstance()->delete('product_sale', 'id_product = ' . (int) $idProduct);
if (is_null($idShop)) {
$idShops = Shop::getContextListShopID();
} else {
$idShops = [$idShop];
}
$success = true;
foreach ($idShops as $idShop) {
$totalSales = ProductSale::getNbrSales($idProduct, $idShop);
if ($totalSales > 1) {
$success = $success && Db::getInstance()->execute(
'
UPDATE ' . _DB_PREFIX_ . 'product_sale
SET `quantity` = CAST(`quantity` AS SIGNED) - ' . (int) $qty . ', `sale_nbr` = CAST(`sale_nbr` AS SIGNED) - 1, `date_upd` = NOW()
WHERE `id_product` = ' . (int) $idProduct . ' and `id_shop` = ' . (int) $idShop
);
} elseif ($totalSales == 1) {
$success = $success && Db::getInstance()->delete('product_sale', 'id_product = ' . (int) $idProduct . ' and `id_shop` = ' . (int) $idShop);
}
}

return true;
return $success;
}
}
4 changes: 2 additions & 2 deletions classes/order/OrderHistory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public function changeIdOrderState($new_order_state, $id_order, $use_existing_pa
if (Validate::isLoadedObject($old_os)) {
// if becoming logable => adds sale
if ($new_os->logable && !$old_os->logable) {
ProductSale::addProductSale($product['product_id'], $product['product_quantity']);
ProductSale::addProductSale($product['product_id'], $product['product_quantity'], (int) $order->id_shop);
// @since 1.5.0 - Stock Management
if (!Pack::isPack($product['product_id']) &&
in_array($old_os->id, $error_or_canceled_statuses) &&
Expand All @@ -230,7 +230,7 @@ public function changeIdOrderState($new_order_state, $id_order, $use_existing_pa
}
} elseif (!$new_os->logable && $old_os->logable) {
// if becoming unlogable => removes sale
ProductSale::removeProductSale($product['product_id'], $product['product_quantity']);
ProductSale::removeProductSale($product['product_id'], $product['product_quantity'], (int) $order->id_shop);

// @since 1.5.0 - Stock Management
if (!Pack::isPack($product['product_id']) &&
Expand Down
3 changes: 2 additions & 1 deletion install-dev/data/db_structure.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1838,10 +1838,11 @@ CREATE TABLE `PREFIX_product_lang` (
/* info about number of products sold */
CREATE TABLE `PREFIX_product_sale` (
`id_product` int(10) unsigned NOT NULL,
`id_shop` int(10) unsigned NOT NULL,
`quantity` int(10) unsigned NOT NULL DEFAULT '0',
`sale_nbr` int(10) unsigned NOT NULL DEFAULT '0',
`date_upd` date DEFAULT NULL,
PRIMARY KEY (`id_product`),
PRIMARY KEY (`id_product`, `id_shop`),
KEY `quantity` (`quantity`)
) ENGINE=ENGINE_TYPE DEFAULT CHARSET=utf8mb4 COLLATION;

Expand Down