Project-Pro/shopPRO
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ver. 0.293: Code review fixes — 6 repositories, 16 fixes

Browse Source 
- ArticleRepository: SQL injection fix (addslashes→parameterized), DRY refactor topArticles/newsListArticles
- AttributeRepository: dead class_exists('\S') blocking cache/temp clear
- CategoryRepository: dead class_exists('\S') blocking SEO link generation (critical)
- BannerRepository: parameterize $today in SQL + null guard on query()
- BasketCalculator: null guard checkProductQuantityInStock + optional DI params
- PromotionRepository: null guard on $basket (production fatal)
- OrderRepository/ShopBasketController/ajax.php: explicit DI in BasketCalculator callers

614 tests, 1821 assertions (+4 new)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Jacek Pyziak
2026-02-19 01:07:39 +01:00
parent 29821bccf2
commit 054b1b4a34
19 changed files with 297 additions and 218 deletions
Download Patch File Download Diff File Expand all files Collapse all files

132
autoload/Domain/Article/ArticleRepository.php
View File

@@ -844,13 +844,14 @@ class ArticleRepository
/**
* Pobiera artykuly opublikowane w podanym zakresie dat.
*/
public function articlesByDateAdd( string $dateStart, string $dateEnd ): array
public function articlesByDateAdd( string $dateStart, string $dateEnd, string $langId = 'pl' ): array
{
$stmt = $this->db->query(
'SELECT id FROM pp_articles '
. 'WHERE status = 1 '
. 'AND date_add BETWEEN \'' . addslashes( $dateStart ) . '\' AND \'' . addslashes( $dateEnd ) . '\' '
. 'ORDER BY date_add DESC'
. 'AND date_add BETWEEN :date_start AND :date_end '
. 'ORDER BY date_add DESC',
[':date_start' => $dateStart, ':date_end' => $dateEnd]
);
$articles = [];
@@ -858,7 +859,7 @@ class ArticleRepository
if ( is_array( $rows ) ) {
foreach ( $rows as $row ) {
$articles[] = $this->articleDetailsFrontend( $row['id'], 'pl' );
$articles[] = $this->articleDetailsFrontend( $row['id'], $langId );
}
}
@@ -889,24 +890,18 @@ class ArticleRepository
return null;
}
$results = $this->db->select('pp_articles_langs', '*', [
$langRow = $this->db->get('pp_articles_langs', '*', [
'AND' => ['article_id' => $articleId, 'lang_id' => $langId]
]);
if (is_array($results)) {
foreach ($results as $row) {
if ($row['copy_from']) {
$results2 = $this->db->select('pp_articles_langs', '*', [
'AND' => ['article_id' => $articleId, 'lang_id' => $row['copy_from']]
]);
if (is_array($results2)) {
foreach ($results2 as $row2) {
$article['language'] = $row2;
}
}
} else {
$article['language'] = $row;
}
if ($langRow) {
if ($langRow['copy_from']) {
$copyRow = $this->db->get('pp_articles_langs', '*', [
'AND' => ['article_id' => $articleId, 'lang_id' => $langRow['copy_from']]
]);
$article['language'] = $copyRow ? $copyRow : $langRow;
} else {
$article['language'] = $langRow;
}
}
@@ -955,7 +950,7 @@ class ArticleRepository
return unserialize($objectData);
}
$results = $this->db->query(
$stmt = $this->db->query(
'SELECT * FROM ( '
. 'SELECT '
. 'a.id, date_modify, date_add, o, '
@@ -971,12 +966,14 @@ class ArticleRepository
. 'INNER JOIN pp_articles AS a ON a.id = ap.article_id '
. 'INNER JOIN pp_articles_langs AS al ON al.article_id = ap.article_id '
. 'WHERE '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = \'' . $langId . '\' '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = :lang_id '
. ') AS q1 '
. 'WHERE q1.title IS NOT NULL '
. 'ORDER BY q1.' . $order . ' '
. 'LIMIT ' . (int)$from . ',' . (int)$limit
)->fetchAll();
. 'LIMIT ' . (int)$from . ',' . (int)$limit,
[':lang_id' => $langId]
);
$results = $stmt ? $stmt->fetchAll() : [];
if (is_array($results) && !empty($results)) {
foreach ($results as $row) {
@@ -1003,7 +1000,7 @@ class ArticleRepository
return (int)unserialize($objectData);
}
$results = $this->db->query(
$stmt = $this->db->query(
'SELECT COUNT(0) FROM ( '
. 'SELECT '
. 'a.id, '
@@ -1019,10 +1016,12 @@ class ArticleRepository
. 'INNER JOIN pp_articles AS a ON a.id = ap.article_id '
. 'INNER JOIN pp_articles_langs AS al ON al.article_id = ap.article_id '
. 'WHERE '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = \'' . $langId . '\' '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = :lang_id '
. ') AS q1 '
. 'WHERE q1.title IS NOT NULL'
)->fetchAll();
. 'WHERE q1.title IS NOT NULL',
[':lang_id' => $langId]
);
$results = $stmt ? $stmt->fetchAll() : [];
$count = isset($results[0][0]) ? (int)$results[0][0] : 0;
@@ -1106,14 +1105,30 @@ class ArticleRepository
* Pobiera najpopularniejsze artykuly ze strony (wg views DESC, z Redis cache).
*/
public function topArticles(int $pageId, int $limit, string $langId): ?array
{
return $this->fetchArticlesByPage('topArticles', $pageId, $limit, $langId, 'views DESC');
}
/**
* Pobiera najnowsze artykuly ze strony (wg date_add DESC, z Redis cache).
*/
public function newsListArticles(int $pageId, int $limit, string $langId): ?array
{
return $this->fetchArticlesByPage('newsListArticles', $pageId, $limit, $langId, 'date_add DESC');
}
/**
* Wspolna logika dla topArticles/newsListArticles (z Redis cache).
*/
private function fetchArticlesByPage(string $cachePrefix, int $pageId, int $limit, string $langId, string $orderBy): ?array
{
$cacheHandler = new \Shared\Cache\CacheHandler();
$cacheKey = "ArticleRepository::topArticles:{$pageId}:{$limit}:{$langId}";
$cacheKey = "ArticleRepository::{$cachePrefix}:{$pageId}:{$limit}:{$langId}";
$objectData = $cacheHandler->get($cacheKey);
if (!$objectData) {
$articlesData = $this->db->query(
$stmt = $this->db->query(
'SELECT * FROM ( '
. 'SELECT '
. 'a.id, date_add, views, '
@@ -1129,61 +1144,14 @@ class ArticleRepository
. 'INNER JOIN pp_articles AS a ON a.id = ap.article_id '
. 'INNER JOIN pp_articles_langs AS al ON al.article_id = ap.article_id '
. 'WHERE '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = \'' . $langId . '\' '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = :lang_id '
. ') AS q1 '
. 'WHERE q1.title IS NOT NULL '
. 'ORDER BY q1.views DESC '
. 'LIMIT 0, ' . (int)$limit
)->fetchAll(\PDO::FETCH_ASSOC);
$cacheHandler->set($cacheKey, $articlesData);
} else {
$articlesData = unserialize($objectData);
}
$articles = null;
if (\Shared\Helpers\Helpers::is_array_fix($articlesData)) {
foreach ($articlesData as $row) {
$articles[] = $this->articleDetailsFrontend((int)$row['id'], $langId);
}
}
return $articles;
}
/**
* Pobiera najnowsze artykuly ze strony (wg date_add DESC, z Redis cache).
*/
public function newsListArticles(int $pageId, int $limit, string $langId): ?array
{
$cacheHandler = new \Shared\Cache\CacheHandler();
$cacheKey = "ArticleRepository::newsListArticles:{$pageId}:{$limit}:{$langId}";
$objectData = $cacheHandler->get($cacheKey);
if (!$objectData) {
$articlesData = $this->db->query(
'SELECT * FROM ( '
. 'SELECT '
. 'a.id, date_add, '
. '( CASE '
. 'WHEN copy_from IS NULL THEN title '
. 'WHEN copy_from IS NOT NULL THEN ( '
. 'SELECT title FROM pp_articles_langs '
. 'WHERE lang_id = al.copy_from AND article_id = a.id '
. ') '
. 'END ) AS title '
. 'FROM '
. 'pp_articles_pages AS ap '
. 'INNER JOIN pp_articles AS a ON a.id = ap.article_id '
. 'INNER JOIN pp_articles_langs AS al ON al.article_id = ap.article_id '
. 'WHERE '
. 'status = 1 AND page_id = ' . (int)$pageId . ' AND lang_id = \'' . $langId . '\' '
. ') AS q1 '
. 'WHERE q1.title IS NOT NULL '
. 'ORDER BY q1.date_add DESC '
. 'LIMIT 0, ' . (int)$limit
)->fetchAll(\PDO::FETCH_ASSOC);
. 'ORDER BY q1.' . $orderBy . ' '
. 'LIMIT 0, ' . (int)$limit,
[':lang_id' => $langId]
);
$articlesData = $stmt ? $stmt->fetchAll(\PDO::FETCH_ASSOC) : [];
$cacheHandler->set($cacheKey, $articlesData);
} else {

10
autoload/Domain/Attribute/AttributeRepository.php
View File

@@ -938,14 +938,8 @@ class AttributeRepository
private function clearTempAndCache(): void
{
if (class_exists('\S')) {
if (method_exists('\S', 'delete_dir')) {
\Shared\Helpers\Helpers::delete_dir('../temp/');
}
if (method_exists('\S', 'delete_cache')) {
\Shared\Helpers\Helpers::delete_cache();
}
}
\Shared\Helpers\Helpers::delete_dir('../temp/');
\Shared\Helpers\Helpers::delete_cache();
}
private function normalizeDecimal(float $value, int $precision = 2): float

24
autoload/Domain/Banner/BannerRepository.php
View File

@@ -331,13 +331,15 @@ class BannerRepository
}
$today = date('Y-m-d');
$results = $this->db->query(
$stmt = $this->db->query(
"SELECT id, name FROM pp_banners "
. "WHERE status = 1 "
. "AND (date_start <= '{$today}' OR date_start IS NULL) "
. "AND (date_end >= '{$today}' OR date_end IS NULL) "
. "AND home_page = 0"
)->fetchAll();
. "AND (date_start <= :today1 OR date_start IS NULL) "
. "AND (date_end >= :today2 OR date_end IS NULL) "
. "AND home_page = 0",
[':today1' => $today, ':today2' => $today]
);
$results = $stmt ? $stmt->fetchAll() : [];
$banners = null;
if (is_array($results) && !empty($results)) {
@@ -370,15 +372,17 @@ class BannerRepository
}
$today = date('Y-m-d');
$results = $this->db->query(
$stmt = $this->db->query(
"SELECT * FROM pp_banners "
. "WHERE status = 1 "
. "AND (date_start <= '{$today}' OR date_start IS NULL) "
. "AND (date_end >= '{$today}' OR date_end IS NULL) "
. "AND (date_start <= :today1 OR date_start IS NULL) "
. "AND (date_end >= :today2 OR date_end IS NULL) "
. "AND home_page = 1 "
. "ORDER BY date_end ASC "
. "LIMIT 1"
)->fetchAll();
. "LIMIT 1",
[':today1' => $today, ':today2' => $today]
);
$results = $stmt ? $stmt->fetchAll() : [];
$banner = null;
if (is_array($results) && !empty($results)) {

32
autoload/Domain/Basket/BasketCalculator.php
View File

@@ -26,22 +26,32 @@ class BasketCalculator
return $count . ' produktów';
}
public static function summaryPrice($basket, $coupon = null)
/**
* @param string|null $langId Language ID (falls back to global $lang_id if null)
* @param \Domain\Product\ProductRepository|null $productRepo (falls back to $GLOBALS['mdb'] if null)
*/
public static function summaryPrice($basket, $coupon = null, $langId = null, $productRepo = null)
{
global $lang_id;
if ($langId === null) {
global $lang_id;
$langId = $lang_id;
}
if ($productRepo === null) {
$productRepo = new \Domain\Product\ProductRepository($GLOBALS['mdb']);
}
$summary = 0;
$productRepo = new \Domain\Product\ProductRepository($GLOBALS['mdb']);
if (is_array($basket)) {
foreach ($basket as $position) {
$product = $productRepo->findCached((int)$position['product-id'], $lang_id);
$product = $productRepo->findCached((int)$position['product-id'], $langId);
$product_price_tmp = self::calculateBasketProductPrice(
(float)($product['price_brutto_promo'] ?? 0),
(float)($product['price_brutto'] ?? 0),
$coupon,
$position
$position,
$productRepo
);
$summary += $product_price_tmp['price_new'] * $position['quantity'];
}
@@ -71,6 +81,9 @@ class BasketCalculator
public static function checkProductQuantityInStock($basket, bool $message = false)
{
if ( !is_array( $basket ) || empty( $basket ) )
return false;
$result = false;
$productRepo = new \Domain\Product\ProductRepository($GLOBALS['mdb']);
@@ -115,9 +128,14 @@ class BasketCalculator
* Calculate product price in basket (with coupon + promotion discounts).
* Migrated from \shop\Product::calculate_basket_product_price()
*/
public static function calculateBasketProductPrice( float $price_brutto_promo, float $price_brutto, $coupon, $basket_position )
/**
* @param \Domain\Product\ProductRepository|null $productRepo (falls back to $GLOBALS['mdb'] if null)
*/
public static function calculateBasketProductPrice( float $price_brutto_promo, float $price_brutto, $coupon, $basket_position, $productRepo = null )
{
$productRepo = new \Domain\Product\ProductRepository($GLOBALS['mdb']);
if ($productRepo === null) {
$productRepo = new \Domain\Product\ProductRepository($GLOBALS['mdb']);
}
// Produkty przecenione
if ( $price_brutto_promo )

10
autoload/Domain/Category/CategoryRepository.php
View File

@@ -668,18 +668,12 @@ class CategoryRepository
private function refreshCategoryArtifacts(): void
{
if (class_exists('\\S')) {
\Shared\Helpers\Helpers::htacces();
\Shared\Helpers\Helpers::delete_dir('../temp/');
}
\Shared\Helpers\Helpers::htacces();
\Shared\Helpers\Helpers::delete_dir('../temp/');
}
private function normalizeSeoLink($value): ?string
{
if (!class_exists('\\S')) {
return $this->toNullableString($value);
}
$seo = \Shared\Helpers\Helpers::seo((string)$value);
$seo = trim((string)$seo);

6
autoload/Domain/Order/OrderRepository.php
View File

@@ -560,7 +560,8 @@ class OrderRepository
$transport = ( new \Domain\Transport\TransportRepository( $this->db ) )->findActiveByIdCached( $transport_id );
$payment_method = ( new \Domain\PaymentMethod\PaymentMethodRepository( $this->db ) )->findActiveById( (int)$payment_id );
$basket_summary = \Domain\Basket\BasketCalculator::summaryPrice($basket, $coupon);
$productRepo = new \Domain\Product\ProductRepository($this->db);
$basket_summary = \Domain\Basket\BasketCalculator::summaryPrice($basket, $coupon, $lang_id, $productRepo);
$order_number = $this->generateOrderNumber();
$order_date = date('Y-m-d H:i:s');
$hash = md5($order_number . time());
@@ -619,7 +620,6 @@ class OrderRepository
if (is_array($basket)) {
foreach ($basket as $basket_position) {
$attributes = '';
$productRepo = new \Domain\Product\ProductRepository($this->db);
$product = $productRepo->findCached($basket_position['product-id'], $lang_id);
if (is_array($basket_position['attributes'])) {
@@ -649,7 +649,7 @@ class OrderRepository
}
}
$product_price_tmp = \Domain\Basket\BasketCalculator::calculateBasketProductPrice((float)$product['price_brutto_promo'], (float)$product['price_brutto'], $coupon, $basket_position);
$product_price_tmp = \Domain\Basket\BasketCalculator::calculateBasketProductPrice((float)$product['price_brutto_promo'], (float)$product['price_brutto'], $coupon, $basket_position, $productRepo);
$this->db->insert('pp_shop_order_products', [
'order_id' => $order_id,

3
autoload/Domain/Promotion/PromotionRepository.php
View File

@@ -453,6 +453,9 @@ class PromotionRepository
public function findPromotion( $basket )
{
if ( !is_array( $basket ) || empty( $basket ) )
return is_array( $basket ) ? $basket : [];
foreach ( $basket as $key => $val )
{
unset( $basket[$key]['discount_type'] );