Răsfoiți Sursa

Draw core and Ads stats pie charts with Chart.js 4 (#5843)

* Draw core and Ads stats pie charts with Chart.js 4

The stats block (dashboard) and the Ads promotion summary and ROI blocks
still asked for chart.min.js, which build:chart.js no longer produces.
addJs skips a file it cannot find, so on a page without another
Chart.js block the inline script threw "Chart is not defined".

Load chart.umd.min.js and move the shared chart_stats.html options to
the Chart.js 4 places: plugins.legend and layout.padding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Pad random pie slice colours to six hex digits

'#' . dechex(rand(0, 0xFFFFFF)) gives fewer than six digits for any
value below 0x100000, about one slice in sixteen. The browser rejects
that colour and the slice is filled with the previous one. Use
sprintf('#%06x', ...) in the core stats and Ads promotion charts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Ads: build chart slice colours in one helper

Sonar S1192 flagged the '#%06x' format repeated four times.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Draw chart slice colours with random_int()

Sonar S2245 counts rand() as a vulnerability once it sits in new code,
which failed the security rating gate. The colours need no secure
randomness, but random_int() costs nothing here. The test now checks
200 colours instead of seeding mt_rand, which random_int() ignores.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Andrey Yasko 1 săptămână în urmă
părinte
comite
fde5b56cd9

+ 14 - 6
modules/boonex/ads/classes/BxAdsTemplate.php

@@ -123,14 +123,14 @@ class BxAdsTemplate extends BxBaseModTextTemplate
 
             $aTmplVarsDataLabels[] = $sTitle . ' - ' . $iValue;
             $aTmplVarsDataSet['data'][] = $iValue;
-            $aTmplVarsDataSet['backgroundColor'][] = '#' . dechex(rand(0x000000, 0xFFFFFF));
+            $aTmplVarsDataSet['backgroundColor'][] = $this->_getChartColor();
         }
 
         if($this->_bIsApi){
             return [bx_api_get_block('chart', ['type' => 'pie', 'title' => 'Summary', 'labels' => $aTmplVarsDataLabels, 'data' => $aTmplVarsDataSet])];
         }
 
-        $this->addJs(['chart.min.js']);
+        $this->addJs(['chart.umd.min.js']);
         $this->addCss(['chart.css']);
         return $this->parseHtmlByName('chart_stats.html', [
             'chart_id' => $this->MODULE . '_chart_summary',
@@ -156,7 +156,7 @@ class BxAdsTemplate extends BxBaseModTextTemplate
 
         $aTmplVarsDataLabels[] = $sTitleLocal . ' - ' . $iValueLocal;
         $aTmplVarsDataSet['data'][] = $iValueLocal;
-        $aTmplVarsDataSet['backgroundColor'][] = '#' . dechex(rand(0x000000, 0xFFFFFF));
+        $aTmplVarsDataSet['backgroundColor'][] = $this->_getChartColor();
 
         //--- Get income external (from source)
         $iValueSource = 0;
@@ -175,7 +175,7 @@ class BxAdsTemplate extends BxBaseModTextTemplate
 
                 $aTmplVarsDataLabels[] = $sTitleSource . ' - ' . $iValueSource;
                 $aTmplVarsDataSet['data'][] = $iValueSource;
-                $aTmplVarsDataSet['backgroundColor'][] = '#' . dechex(rand(0x000000, 0xFFFFFF));
+                $aTmplVarsDataSet['backgroundColor'][] = $this->_getChartColor();
             }
         }
 
@@ -185,7 +185,7 @@ class BxAdsTemplate extends BxBaseModTextTemplate
 
         $aTmplVarsDataLabels[] = $sTitleInvestment . ' - ' . $iValueInvestment;
         $aTmplVarsDataSet['data'][] = $iValueInvestment;
-        $aTmplVarsDataSet['backgroundColor'][] = '#' . dechex(rand(0x000000, 0xFFFFFF));
+        $aTmplVarsDataSet['backgroundColor'][] = $this->_getChartColor();
 
         $fRoi = 0;
         if($iValueInvestment > 0)
@@ -195,7 +195,7 @@ class BxAdsTemplate extends BxBaseModTextTemplate
             return [bx_api_get_block('chart', ['type' => 'pie', 'title' => 'ROI', 'labels' => $aTmplVarsDataLabels, 'data' => $aTmplVarsDataSet, 'text' => 'ROI: ' . $fRoi . '%'])];
         }
 
-        $this->addJs(['chart.min.js']);
+        $this->addJs(['chart.umd.min.js']);
         $this->addCss(['chart.css']);
         return $this->parseHtmlByName('entry-roi.html', [
             'chart' => $this->parseHtmlByName('chart_stats.html', [
@@ -383,6 +383,14 @@ class BxAdsTemplate extends BxBaseModTextTemplate
         ));
     }
 
+    /**
+     * Random chart slice colour as #rrggbb.
+     */
+    protected function _getChartColor()
+    {
+        return sprintf('#%06x', random_int(0x000000, 0xFFFFFF));
+    }
+
     protected function _entryBreadcrumb($iCategory, &$oPermalink, &$aTmplVarsItems)
     {
         $CNF = &$this->_oConfig->CNF;

+ 11 - 7
template/chart_stats.html

@@ -6,14 +6,18 @@
                     type: 'doughnut',
                     data: __chart_data__,
                     options: {
-                        legend: {
-                            position: 'bottom'
+                        plugins: {
+                            legend: {
+                                position: 'bottom'
+                            }
                         },
-                        padding: {
-                            left: 0,
-                            right: 0,
-                            top: 0,
-                            bottom: 0
+                        layout: {
+                            padding: {
+                                left: 0,
+                                right: 0,
+                                top: 0,
+                                bottom: 0
+                            }
                         },
                         maintainAspectRatio: false
                     },

+ 2 - 2
template/scripts/BxBaseChartServices.php

@@ -157,11 +157,11 @@ class BxBaseChartServices extends BxDol
             $aTmplVarsDataLabels[] = $sTitleAttr . ' - ' . $iValue;
 
             $aTmplVarsDataSet['data'][] = $iValue;
-            $aTmplVarsDataSet['backgroundColor'][] = '#' . dechex(rand(0x000000, 0xFFFFFF));
+            $aTmplVarsDataSet['backgroundColor'][] = sprintf('#%06x', random_int(0x000000, 0xFFFFFF));
         }
 
         $oTemplate = BxDolTemplate::getInstance();
-        $oTemplate->addJs(array('chart.min.js'));
+        $oTemplate->addJs(array('chart.umd.min.js'));
         $oTemplate->addCss(array('chart.css'));
 
         return $oTemplate->parseHtmlByName('chart_stats.html', array(

+ 97 - 0
tests/units/modules/boonex/ads/classes/BxAdsTemplateTest.php

@@ -0,0 +1,97 @@
+<?php
+
+/**
+ * Ads promotion pie charts (summary and ROI) load Chart.js 4 themselves. addJs skips
+ * a file it cannot find, so the old chart.min.js name left Chart undefined on any page
+ * without another Chart.js block.
+ */
+class BxAdsTemplateTest extends BxDolTestCase
+{
+    protected $_oModule;
+    protected $_iProfileIdBackup;
+    protected $_aPageBackup;
+
+    protected function setUp(): void
+    {
+        bx_import('BxDolModule');
+        $this->_oModule = BxDolModule::getInstance('bx_ads');
+        if (!$this->_oModule)
+            $this->markTestSkipped('bx_ads module is not installed.');
+
+        if (!BxDolDb::getInstance()->isTableExists('bx_ads_entries'))
+            $this->markTestSkipped('bx_ads module is not installed (table bx_ads_entries is missing).');
+
+        $this->_iProfileIdBackup = $this->bxGetProtected($this->_oModule, '_iProfileId', BxBaseModGeneralModule::class);
+        $this->bxSetProtected($this->_oModule, '_iProfileId', 10, BxBaseModGeneralModule::class);
+
+        //--- Start from an empty list so only the files added by the block are seen.
+        $oTemplate = BxDolTemplate::getInstance();
+        $this->_aPageBackup = $this->bxGetProtected($oTemplate, 'aPage');
+        $this->bxSetProtected($oTemplate, 'aPage', array_merge($this->_aPageBackup, ['js_compiled' => []]));
+    }
+
+    protected function tearDown(): void
+    {
+        if ($this->_aPageBackup !== null)
+            $this->bxSetProtected(BxDolTemplate::getInstance(), 'aPage', $this->_aPageBackup);
+
+        if ($this->_oModule && $this->_iProfileIdBackup !== null)
+            $this->bxSetProtected($this->_oModule, '_iProfileId', $this->_iProfileIdBackup, BxBaseModGeneralModule::class);
+    }
+
+    public function testPromotionSummaryLoadsChartJs4()
+    {
+        $s = $this->_oModule->_oTemplate->entryPromotionSummary($this->bxSampleAd());
+
+        $this->assertStringContainsString('id="bx_ads_chart_summary"', $s);
+        $this->assertChartJs4Added();
+    }
+
+    public function testPromotionRoiLoadsChartJs4()
+    {
+        $s = $this->_oModule->_oTemplate->entryPromotionRoi($this->bxSampleAd());
+
+        $this->assertStringContainsString('id="bx_ads_chart_roi"', $s);
+        $this->assertChartJs4Added();
+    }
+
+    public function testPromotionSummaryColoursAreSixDigitHex()
+    {
+        //--- About one draw in 16 is below 0x100000, so 200 colours catch one that lost a leading zero.
+        for ($i = 0; $i < 100; $i++) {
+            $s = $this->_oModule->_oTemplate->entryPromotionSummary($this->bxSampleAd());
+
+            $this->assertSame(1, preg_match('/"backgroundColor":(\[[^\]]*\])/', $s, $aMatches));
+            $aColours = json_decode($aMatches[1], true);
+
+            $this->assertCount(2, $aColours);
+            foreach ($aColours as $sColour)
+                $this->assertMatchesRegularExpression('/^#[0-9a-f]{6}$/', $sColour);
+        }
+    }
+
+    private function assertChartJs4Added(): void
+    {
+        $aPaths = array_column($this->bxGetProtected(BxDolTemplate::getInstance(), 'aPage')['js_compiled'], 'path');
+        $aChart = array_values(array_filter($aPaths, function ($sPath) {
+            return basename($sPath) == 'chart.umd.min.js';
+        }));
+
+        $this->assertCount(1, $aChart, 'The block must add plugins_public/chart.umd.min.js.');
+        $this->assertFileExists($aChart[0]);
+    }
+
+    private function bxSampleAd(): array
+    {
+        return [
+            'id' => 2147483000,
+            'author' => 10,
+            'added' => 1700000000,
+            'impressions' => 120,
+            'clicks' => 7,
+            'source_type' => '',
+            'source' => '',
+            'allow_view_to' => 3,
+        ];
+    }
+}