Merge pull request #187 from matomo-org/spice-psr

AltamashShaikh · web-flow · commit 777121919216 · 2024-10-16T13:03:29.000+05:30
Adds test for PHPCS
diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml
@@ -0,0 +1,43 @@
+name: PHPCS check
+
+on: pull_request
+
+permissions:
+  actions: read
+  checks: read
+  contents: read
+  deployments: none
+  issues: read
+  packages: none
+  pull-requests: read
+  repository-projects: none
+  security-events: none
+  statuses: read
+
+jobs:
+  phpcs:
+    name: PHPCS
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          lfs: false
+          persist-credentials: false
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: '7.4'
+          tools: cs2pr
+      - name: Install dependencies
+        run:
+          composer init --name=matomo/customalerts --quiet;
+          composer --no-plugins config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true -n;
+          composer config repositories.matomo-coding-standards vcs https://github.com/matomo-org/matomo-coding-standards -n;
+          composer require matomo-org/matomo-coding-standards:dev-master;
+          composer install --dev --prefer-dist --no-progress --no-suggest
+      - name: Check PHP code styles
+        id: phpcs
+        run: ./vendor/bin/phpcs --report-full --standard=phpcs.xml --report-checkstyle=./phpcs-report.xml
+      - name: Show PHPCS results in PR
+        if: ${{ always() && steps.phpcs.outcome == 'failure' }}
+        run: cs2pr ./phpcs-report.xml --prepend-filename
diff --git a/API.php b/API.php
@@ -158,7 +158,6 @@ private function filterAdditionalEmails($additionalEmails)
         }
 
         foreach ($additionalEmails as &$email) {
-
             $email = trim($email);
             if (empty($email)) {
                 $email = false;
@@ -173,7 +172,6 @@ private function filterPhoneNumbers($phoneNumbers)
         $availablePhoneNumbers = (new \Piwik\Plugins\MobileMessaging\Model())->getActivatedPhoneNumbers(Piwik::getCurrentUserLogin());
 
         foreach ($phoneNumbers as $key => &$phoneNumber) {
-
             $phoneNumber = trim($phoneNumber);
 
             if (!in_array($phoneNumber, $availablePhoneNumbers)) {
@@ -278,5 +276,4 @@ public function getTriggeredAlerts($idSites)
 
         return $this->getModel()->getTriggeredAlerts($idSites, $login);
     }
-
 }
diff --git a/Controller.php b/Controller.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -105,7 +106,6 @@ private function findSiteName($alert)
         $idSite = $this->findSiteId($alert);
 
         if (!empty($idSite)) {
-
             return Site::getNameFor($idSite);
         }
     }
@@ -157,14 +157,12 @@ public function formatAlerts($triggeredAlerts, $format)
                 return $view->render();
 
             case 'sms':
-
                 $view                  = new View('@CustomAlerts/smsTriggeredAlerts');
                 $view->triggeredAlerts = $this->enrichTriggeredAlerts($triggeredAlerts);
 
                 return $view->render();
 
             case 'text':
-
                 $view                  = new View('@CustomAlerts/textTriggeredAlerts');
                 $view->triggeredAlerts = $this->enrichTriggeredAlerts($triggeredAlerts);
 
diff --git a/CustomAlerts.php b/CustomAlerts.php
@@ -11,7 +11,6 @@
 
 use Piwik\Common;
 use Piwik\Container\StaticContainer;
-use Piwik\Option;
 use Piwik\Piwik;
 use Piwik\Plugins\SitesManager\API as SitesManagerApi;
 use Piwik\Scheduler\Task;
diff --git a/Menu.php b/Menu.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Model.php b/Model.php
@@ -20,7 +20,6 @@
  */
 class Model
 {
-
     public static function install()
     {
         $tableAlert = "`idalert` INT NOT NULL PRIMARY KEY ,
@@ -434,5 +433,4 @@ public function deleteTriggeredAlertsForSite($idSite)
     {
         $this->getDb()->query("DELETE FROM " . Common::prefixTable("alert_triggered") . " WHERE idsite = ?", $idSite);
     }
-
 }
diff --git a/Notifier.php b/Notifier.php
@@ -72,7 +72,6 @@ private function groupAlertsPerEmailRecipient($triggeredAlerts)
         $alertsPerEmail = array();
 
         foreach ($triggeredAlerts as $triggeredAlert) {
-
             $emails = $this->getEmailRecipientsForAlert($triggeredAlert);
 
             foreach ($emails as $mail) {
@@ -189,7 +188,6 @@ private function groupAlertsPerSmsRecipient($triggeredAlerts)
         $recipients = array();
 
         foreach ($triggeredAlerts as $triggeredAlert) {
-
             $phoneNumbers = $triggeredAlert['phone_numbers'];
 
             if (empty($phoneNumbers)) {
diff --git a/Processor.php b/Processor.php
@@ -15,7 +15,6 @@
 use Piwik\Context;
 use Piwik\DataTable;
 use Piwik\Date;
-use Piwik\Option;
 use Piwik\Piwik;
 use Piwik\Plugins\API\ProcessedReport;
 use Piwik\Scheduler\RetryableException;
@@ -453,5 +452,4 @@ protected function triggerAlert($alert, $idSite, $valueNew, $valueOld)
     {
         $this->getModel()->triggerAlert($alert['idalert'], $idSite, $valueNew, $valueOld, Date::now()->getDatetime());
     }
-
 }
diff --git a/Tasks.php b/Tasks.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -67,4 +68,4 @@ private function runAlerts($period, $idSite)
         $this->processor->processAlerts($period, (int) $idSite);
         $this->notifier->sendNewAlerts($period, (int) $idSite);
     }
-}
+}
diff --git a/Updates/0.0.2.php b/Updates/0.0.2.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.0.3.php b/Updates/0.0.3.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.0.4.php b/Updates/0.0.4.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.0.5.php b/Updates/0.0.5.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.0.6.php b/Updates/0.0.6.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.0.7.php b/Updates/0.0.7.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.0.8.php b/Updates/0.0.8.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -39,8 +40,10 @@ public function getMigrations(Updater $updater)
         $triggeredTable = Common::prefixTable('alert_triggered');
 
         return array(
-            $this->migration->db->sql("RENAME TABLE `" . Common::prefixTable('alert_log') . "` TO `" . Common::prefixTable('alert_triggered') . "`",
-                array(DbMigration::ERROR_CODE_DUPLICATE_COLUMN, DbMigration::ERROR_CODE_TABLE_NOT_EXISTS, DbMigration::ERROR_CODE_TABLE_EXISTS)),
+            $this->migration->db->sql(
+                "RENAME TABLE `" . Common::prefixTable('alert_log') . "` TO `" . Common::prefixTable('alert_triggered') . "`",
+                array(DbMigration::ERROR_CODE_DUPLICATE_COLUMN, DbMigration::ERROR_CODE_TABLE_NOT_EXISTS, DbMigration::ERROR_CODE_TABLE_EXISTS)
+            ),
             $this->migration->db->addColumns('alert_triggered', array(
                 'name'              => 'VARCHAR(100) NOT NULL',
                 'login'             => 'VARCHAR(100) NOT NULL',
diff --git a/Updates/0.1.10.php b/Updates/0.1.10.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.1.17.php b/Updates/0.1.17.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Updates/0.1.8.php b/Updates/0.1.8.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/Validator.php b/Validator.php
@@ -44,7 +44,6 @@ public function filterPhoneNumbers($phoneNumbers)
         $availablePhoneNumbers = (new \Piwik\Plugins\MobileMessaging\Model())->getActivatedPhoneNumbers(Piwik::getCurrentUserLogin());
 
         foreach ($phoneNumbers as $key => &$phoneNumber) {
-
             $phoneNumber = trim($phoneNumber);
 
             if (!in_array($phoneNumber, $availablePhoneNumbers)) {
diff --git a/phpcs.xml b/phpcs.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<ruleset name="customAlerts" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vendor/squizlabs/php_codesniffer/phpcs.xsd">
+
+    <description>Matomo Coding Standard for CustomAlerts plugin</description>
+
+    <arg name="extensions" value="php" />
+
+    <file>.</file>
+
+    <exclude-pattern>tests/javascript/*</exclude-pattern>
+    <exclude-pattern>*/vendor/*</exclude-pattern>
+
+    <rule ref="Matomo"></rule>
+
+    <rule ref="Generic.Files.LineLength">
+        <properties>
+            <property name="lineLimit" value="250" />
+        </properties>
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+
+    <rule ref="Squiz.Classes.ValidClassName.NotCamelCaps">
+        <!-- Classnames for our update files don't match PascalCase, this can't be changed easily -->
+        <exclude-pattern>Updates/*</exclude-pattern>
+    </rule>
+
+    <rule ref="PSR1.Methods.CamelCapsMethodName.NotCamelCaps">
+        <!-- Allow using method name without camel caps in tests as long as some methods are named test_* -->
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+
+    <rule ref="PSR1.Classes.ClassDeclaration.MultipleClasses">
+        <!-- Allow using multiple classes in one file for tests -->
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+</ruleset>
diff --git a/tests/Fixtures/CustomAlerts.php b/tests/Fixtures/CustomAlerts.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
diff --git a/tests/Integration/ApiTest.php b/tests/Integration/ApiTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -18,7 +19,6 @@
  */
 class ApiTest extends BaseTest
 {
-
     public function setUp(): void
     {
         parent::setUp();
@@ -39,17 +39,29 @@ protected function createAlert(
         $reportCondition = 'matches_exactly',
         $emails = array('test1@example.com', 'test2@example.com'),
         $comparedTo = 1
-    )
-    {
+    ) {
         if (is_null($idSites)) {
             $idSites = $this->idSite;
         }
 
         // those should be dropped by the api as they do not exist in Piwik
         $phoneNumbers = array('+1234567890', '1234567890');
 
-        $id = $this->api->addAlert($name, $idSites, $period, 0, $emails, $phoneNumbers, $metric, $metricCondition,
-            $metricMatched = 5, $comparedTo, $report, $reportCondition, 'Piwik');
+        $id = $this->api->addAlert(
+            $name,
+            $idSites,
+            $period,
+            0,
+            $emails,
+            $phoneNumbers,
+            $metric,
+            $metricCondition,
+            $metricMatched = 5,
+            $comparedTo,
+            $report,
+            $reportCondition,
+            'Piwik'
+        );
         return $id;
     }
 
@@ -164,8 +176,7 @@ protected function assertIsAlert(
         $report = 'MultiSites_getOne',
         $reportCondition = 'matches_exactly',
         $reportMatched = 'Piwik'
-    )
-    {
+    ) {
         if (is_null($idSites)) {
             $idSites = array($this->idSite);
         }
@@ -211,8 +222,7 @@ protected function editAlert(
         $metricCondition = 'less_than',
         $reportCondition = 'matches_exactly',
         $emails = array('test1@example.com', 'test2@example.com')
-    )
-    {
+    ) {
         if (is_null($idSites)) {
             $idSites = $this->idSite;
         }
@@ -221,8 +231,22 @@ protected function editAlert(
         $phoneNumbers = array('+1234567890', '1234567890');
         $comparedTo   = 1;
 
-        $id = $this->api->editAlert($id, $name, $idSites, $period, 0, $emails, $phoneNumbers, $metric, $metricCondition,
-            $metricMatched = 5, $comparedTo, $report, $reportCondition, 'Piwik');
+        $id = $this->api->editAlert(
+            $id,
+            $name,
+            $idSites,
+            $period,
+            0,
+            $emails,
+            $phoneNumbers,
+            $metric,
+            $metricCondition,
+            $metricMatched = 5,
+            $comparedTo,
+            $report,
+            $reportCondition,
+            'Piwik'
+        );
         return $id;
     }
 
@@ -571,4 +595,4 @@ public function test_getTriggeredAlerts_ShouldReturnAllThatMatchesLoginAndIdSite
         $triggeredAlerts         = $this->api->getTriggeredAlerts(array(1));
         $this->assertCount(0, $triggeredAlerts);
     }
-}
+}
diff --git a/tests/Integration/BaseTest.php b/tests/Integration/BaseTest.php
diff --git a/tests/Integration/ControllerTest.php b/tests/Integration/ControllerTest.php
diff --git a/tests/Integration/CustomAlertsTest.php b/tests/Integration/CustomAlertsTest.php
diff --git a/tests/Integration/ModelTest.php b/tests/Integration/ModelTest.php
diff --git a/tests/Integration/NotifierTest.php b/tests/Integration/NotifierTest.php
diff --git a/tests/Integration/ProcessorTest.php b/tests/Integration/ProcessorTest.php
diff --git a/tests/Integration/ValidatorTest.php b/tests/Integration/ValidatorTest.php
diff --git a/tests/System/CheckDirectDependencyUseCommandTest.php b/tests/System/CheckDirectDependencyUseCommandTest.php

Original file line number	Diff line number	Diff line change
`@@ -158,7 +158,6 @@ private function filterAdditionalEmails($additionalEmails)`
`158`	`158`	`}`
`159`	`159`
`160`	`160`	`foreach ($additionalEmails as &$email) {`
`161`		`-`
`162`	`161`	`$email = trim($email);`
`163`	`162`	`if (empty($email)) {`
`164`	`163`	`$email = false;`
`@@ -173,7 +172,6 @@ private function filterPhoneNumbers($phoneNumbers)`
`173`	`172`	`$availablePhoneNumbers = (new \Piwik\Plugins\MobileMessaging\Model())->getActivatedPhoneNumbers(Piwik::getCurrentUserLogin());`
`174`	`173`
`175`	`174`	`foreach ($phoneNumbers as $key => &$phoneNumber) {`
`176`		`-`
`177`	`175`	`$phoneNumber = trim($phoneNumber);`
`178`	`176`
`179`	`177`	`if (!in_array($phoneNumber, $availablePhoneNumbers)) {`
`@@ -278,5 +276,4 @@ public function getTriggeredAlerts($idSites)`
`278`	`276`
`279`	`277`	`return $this->getModel()->getTriggeredAlerts($idSites, $login);`
`280`	`278`	`}`
`281`		`-`
`282`	`279`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`<?php`
	`2`	`+`
`2`	`3`	`/**`
`3`	`4`	`* Matomo - free/libre analytics platform`
`4`	`5`	`*`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,6 @@`
`20`	`20`	`*/`
`21`	`21`	`class Model`
`22`	`22`	`{`
`23`		`-`
`24`	`23`	`public static function install()`
`25`	`24`	`{`
`26`	`25`	$tableAlert = "`idalert` INT NOT NULL PRIMARY KEY ,
`@@ -434,5 +433,4 @@ public function deleteTriggeredAlertsForSite($idSite)`
`434`	`433`	`{`
`435`	`434`	`$this->getDb()->query("DELETE FROM " . Common::prefixTable("alert_triggered") . " WHERE idsite = ?", $idSite);`
`436`	`435`	`}`
`437`		`-`
`438`	`436`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,6 @@`
`15`	`15`	`use Piwik\Context;`
`16`	`16`	`use Piwik\DataTable;`
`17`	`17`	`use Piwik\Date;`
`18`		`-use Piwik\Option;`
`19`	`18`	`use Piwik\Piwik;`
`20`	`19`	`use Piwik\Plugins\API\ProcessedReport;`
`21`	`20`	`use Piwik\Scheduler\RetryableException;`
`@@ -453,5 +452,4 @@ protected function triggerAlert($alert, $idSite, $valueNew, $valueOld)`
`453`	`452`	`{`
`454`	`453`	`$this->getModel()->triggerAlert($alert['idalert'], $idSite, $valueNew, $valueOld, Date::now()->getDatetime());`
`455`	`454`	`}`
`456`		`-`
`457`	`455`	`}`