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

Adds test for PHPCS

.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

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);
     }
-
 }

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);
 

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;

Menu.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

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);
     }
-
 }

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)) {

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());
     }
-
 }

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);
     }
-}
+}

Updates/0.0.2.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.0.3.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.0.4.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.0.5.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.0.6.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.0.7.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

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',

Updates/0.1.10.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.1.17.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

Updates/0.1.8.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

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)) {

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>

tests/Fixtures/CustomAlerts.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

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('[email protected]', '[email protected]'),
         $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('[email protected]', '[email protected]')
-    )
-    {
+    ) {
         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);
     }
-}
+}