From 46e30c950b619ccd95ad8a4960b404da55fa345f Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sun, 21 Jun 2026 16:26:13 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20error=20handling=20and=20data=20integrity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add missing language field to batch-generated records - Wrap batch insert in try-catch to handle duplicate key races - Add logging to all empty catch blocks (script.php, MokoOG license check) - Guard loadShopProduct() with try-catch for missing MokoSuiteShop tables - Guard reviews query in JsonLdBuilder for missing #__mokoshop_reviews --- .../src/Controller/BatchController.php | 34 +++++++++++-------- .../src/Extension/MokoOG.php | 28 +++++++++------ .../src/Helper/JsonLdBuilder.php | 30 +++++++++------- source/script.php | 12 +++++-- 4 files changed, 63 insertions(+), 41 deletions(-) diff --git a/source/packages/com_mokoog/src/Controller/BatchController.php b/source/packages/com_mokoog/src/Controller/BatchController.php index 468984b..68141f4 100644 --- a/source/packages/com_mokoog/src/Controller/BatchController.php +++ b/source/packages/com_mokoog/src/Controller/BatchController.php @@ -90,6 +90,7 @@ class BatchController extends BaseController $articles = $db->loadObjectList(); $created = 0; + $skipped = 0; $now = Factory::getDate()->toSql(); foreach ($articles as $article) { @@ -98,23 +99,28 @@ class BatchController extends BaseController $ogImage = $this->extractImage($article); $record = (object) [ - 'content_type' => 'com_content', - 'content_id' => (int) $article->id, - 'og_title' => $ogTitle, - 'og_description' => $ogDescription, - 'og_image' => $ogImage, - 'og_type' => 'article', - 'seo_title' => '', + 'content_type' => 'com_content', + 'content_id' => (int) $article->id, + 'og_title' => $ogTitle, + 'og_description' => $ogDescription, + 'og_image' => $ogImage, + 'og_type' => 'article', + 'seo_title' => '', 'meta_description' => $article->metadesc ?: '', - 'robots' => '', - 'canonical_url' => '', - 'published' => 1, - 'created' => $now, - 'modified' => $now, + 'robots' => '', + 'canonical_url' => '', + 'language' => '*', + 'published' => 1, + 'created' => $now, + 'modified' => $now, ]; - $db->insertObject('#__mokoog_tags', $record); - $created++; + try { + $db->insertObject('#__mokoog_tags', $record); + $created++; + } catch (\RuntimeException $e) { + $skipped++; + } } echo new JsonResponse([ diff --git a/source/packages/plg_system_mokoog/src/Extension/MokoOG.php b/source/packages/plg_system_mokoog/src/Extension/MokoOG.php index 312d96a..37e826e 100644 --- a/source/packages/plg_system_mokoog/src/Extension/MokoOG.php +++ b/source/packages/plg_system_mokoog/src/Extension/MokoOG.php @@ -619,7 +619,8 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface 'warning' ); } catch (\Throwable $e) { - // Don't break admin over a license check + // Don't break admin over a license check, but log for debugging + \Joomla\CMS\Log\Log::add('MokoOG license check: ' . $e->getMessage(), \Joomla\CMS\Log\Log::WARNING, 'mokoog'); } } @@ -638,17 +639,22 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface return $cache[$productId]; } - $db = Factory::getDbo(); - $query = $db->getQuery(true) - ->select('p.id, p.sku, p.price, p.currency, p.stock_qty') - ->select('c.title AS name, c.introtext AS description, c.images') - ->from($db->quoteName('#__mokosuite_crm_products', 'p')) - ->join('LEFT', $db->quoteName('#__content', 'c') . ' ON c.id = p.article_id') - ->where($db->quoteName('p.id') . ' = ' . $productId) - ->where($db->quoteName('p.published') . ' = 1'); + try { + $db = Factory::getDbo(); + $query = $db->getQuery(true) + ->select('p.id, p.sku, p.price, p.currency, p.stock_qty') + ->select('c.title AS name, c.introtext AS description, c.images') + ->from($db->quoteName('#__mokosuite_crm_products', 'p')) + ->join('LEFT', $db->quoteName('#__content', 'c') . ' ON c.id = p.article_id') + ->where($db->quoteName('p.id') . ' = ' . $productId) + ->where($db->quoteName('p.published') . ' = 1'); - $db->setQuery($query); - $cache[$productId] = $db->loadObject(); + $db->setQuery($query); + $cache[$productId] = $db->loadObject(); + } catch (\RuntimeException $e) { + // MokoSuiteShop tables may not exist + $cache[$productId] = null; + } return $cache[$productId]; } diff --git a/source/packages/plg_system_mokoog/src/Helper/JsonLdBuilder.php b/source/packages/plg_system_mokoog/src/Helper/JsonLdBuilder.php index 9f7f975..7f0598a 100644 --- a/source/packages/plg_system_mokoog/src/Helper/JsonLdBuilder.php +++ b/source/packages/plg_system_mokoog/src/Helper/JsonLdBuilder.php @@ -224,21 +224,25 @@ class JsonLdBuilder ]; // Aggregate rating from reviews if available - $reviewQuery = $db->getQuery(true) - ->select('COUNT(*) AS review_count, AVG(rating) AS avg_rating') - ->from($db->quoteName('#__mokoshop_reviews')) - ->where($db->quoteName('product_id') . ' = ' . $productId) - ->where($db->quoteName('status') . ' = ' . $db->quote('approved')); + try { + $reviewQuery = $db->getQuery(true) + ->select('COUNT(*) AS review_count, AVG(rating) AS avg_rating') + ->from($db->quoteName('#__mokoshop_reviews')) + ->where($db->quoteName('product_id') . ' = ' . $productId) + ->where($db->quoteName('status') . ' = ' . $db->quote('approved')); - $db->setQuery($reviewQuery); - $rating = $db->loadObject(); + $db->setQuery($reviewQuery); + $rating = $db->loadObject(); - if ($rating && (int) $rating->review_count > 0) { - $schema['aggregateRating'] = [ - '@type' => 'AggregateRating', - 'ratingValue' => round((float) $rating->avg_rating, 1), - 'reviewCount' => (int) $rating->review_count, - ]; + if ($rating && (int) $rating->review_count > 0) { + $schema['aggregateRating'] = [ + '@type' => 'AggregateRating', + 'ratingValue' => round((float) $rating->avg_rating, 1), + 'reviewCount' => (int) $rating->review_count, + ]; + } + } catch (\RuntimeException $e) { + // Reviews table may not exist if MokoSuiteShop reviews module not installed } return $schema; diff --git a/source/script.php b/source/script.php index b50d096..315a788 100644 --- a/source/script.php +++ b/source/script.php @@ -82,7 +82,9 @@ class Pkg_MokoOGInstallerScript $key = $db->loadResult(); if (!empty($key)) { $this->savedDownloadKey = $key; } } - catch (\Throwable $e) {} + catch (\Throwable $e) { + \Joomla\CMS\Log\Log::add('MokoOG saveDownloadKey: ' . $e->getMessage(), \Joomla\CMS\Log\Log::WARNING, 'mokoog'); + } } private function restoreDownloadKey(): void @@ -112,7 +114,9 @@ class Pkg_MokoOGInstallerScript )->execute(); } } - catch (\Throwable $e) {} + catch (\Throwable $e) { + \Joomla\CMS\Log\Log::add('MokoOG restoreDownloadKey: ' . $e->getMessage(), \Joomla\CMS\Log\Log::WARNING, 'mokoog'); + } } private function warnMissingLicenseKey(): void @@ -147,6 +151,8 @@ class Pkg_MokoOGInstallerScript 'warning' ); } - catch (\Throwable $e) {} + catch (\Throwable $e) { + \Joomla\CMS\Log\Log::add('MokoOG warnMissingLicenseKey: ' . $e->getMessage(), \Joomla\CMS\Log\Log::WARNING, 'mokoog'); + } } }