Migrate annotations to a separate db table, update model, update tests (#23564)

* Create a model for annotations in a separate db table
* Add index in a separate db call
* Update version to 5.5.0-b2
* Update controller to work with slightly changed API
* Update test suite and API to work together correctly
* Use custom migrations to migrate and purge annotations
* Adjust sanitisation to sanitise on annotation output
* Update PHPStan baseline
* Add system test for annotation add
* Allow to test APIs without providing a date
* Add system test for Annotations.getAll API call when no date is provided
* Ensure we handle today/yesterday dates correctly when adding/saving annotations
* Add tests for today/yesterday date handling
* Add annotations with potentially harmful characters/strings
* Add annotations UI test to cover potentially dangerous characters

---------

Co-authored-by: Nathan Gavin <nathangavin987@gmail.com>
Co-authored-by: Marc Neudert <marc@innocraft.com>

Author: 3 people

core/Common.php

@@ -1246,4 +1246,24 @@ protected static function checkValidLanguagesIsSet($validLanguages)
         }
         return $validLanguages;
     }
+
+    /**
+     * Flatten variously nested arrays into a single flat list of values
+     *
+     * @param array $array
+     * @return array
+     */
+    public static function flattenArray(array $array): array
+    {
+        $result = [];
+        foreach ($array as $value) {
+            if (is_array($value)) {
+                $result = array_merge($result, static::flattenArray($value));
+            } else {
+                $result[] = $value;
+            }
+        }
+
+        return $result;
+    }
 }

core/Db/Schema/Mysql.php

@@ -393,6 +393,17 @@ public function getTablesCreateSql()
                                       UNIQUE KEY unique_plugin_version_title (`plugin_name`, `version`, `title`(100))                            
                                   ) $tableOptions
             ",
+            'annotations'         => "CREATE TABLE `{$prefixTables}annotations` (
+                                      `id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT,
+                                      `idsite` INTEGER UNSIGNED NOT NULL,
+                                      `date` DATETIME NOT NULL,
+                                      `note` TEXT NOT NULL,
+                                      `starred` TINYINT(1) NOT NULL DEFAULT 0,
+                                      `user` VARCHAR(100) NOT NULL,
+                                      PRIMARY KEY(`id`),
+                                      INDEX index_idsite_date (`idsite`, `date`)                            
+                                  ) $tableOptions
+            ",
         );
 
         return $tables;

core/Piwik.php

@@ -599,7 +599,7 @@ public static function isUserHasWriteAccess($idSites)
     /**
      * Checks that the current user has view access to the requested list of sites
      *
-     * @param int|array $idSites The list of site IDs to check access for.
+     * @param int|array|string $idSites The list of site IDs to check access for.
      * @throws Exception if the current user does not have view access to every site in the list.
      * @api
      */

core/Site.php

@@ -418,7 +418,7 @@ public function getCreatorLogin()
      * @param string|array $ids Comma separated idSite list, eg, `'1,2,3,4'` or an array of IDs, eg,
      *                          `array(1, 2, 3, 4)`.
      * @param bool|string $_restrictSitesToLogin Implementation detail. Used only when running as a scheduled task.
-     * @return array An array of valid, unique integers.
+     * @return array<string>|array<int> An array of valid, unique integers.
      */
     public static function getIdSitesFromIdSitesString($ids, $_restrictSitesToLogin = false)
     {

core/Updates/5.5.0-b2.php

@@ -0,0 +1,50 @@
+<?php
+
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link    https://matomo.org
+ * @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Updates;
+
+use Piwik\Plugins\CoreAdminHome\Commands\MigrateAnnotations;
+use Piwik\Plugins\CoreAdminHome\Commands\PurgeLegacyAnnotations;
+use Piwik\Updater;
+use Piwik\Updater\Migration\Custom as CustomMigration;
+use Piwik\Updates as PiwikUpdates;
+use Piwik\Updater\Migration;
+
+/**
+ * Update for version 5.5.0-b2
+ */
+class Updates_5_5_0_b2 extends PiwikUpdates
+{
+    /**
+     * Migrations
+     *
+     * @param Updater $updater
+     *
+     * @return Migration[]
+     */
+    public function getMigrations(Updater $updater)
+    {
+        $migrations = [];
+
+        $migrateCmd = './console core:matomo550-migrate-annotations';
+        $annotationsMigrate = new CustomMigration([MigrateAnnotations::class, 'migrate'], $migrateCmd);
+        $migrations[] = $annotationsMigrate;
+
+        $purgeCmd = './console core:matomo550-purge-legacy-annotations';
+        $annotationsPurge = new CustomMigration([PurgeLegacyAnnotations::class, 'purge'], $purgeCmd);
+        $migrations[] = $annotationsPurge;
+
+        return $migrations;
+    }
+
+    public function doUpdate(Updater $updater)
+    {
+        $updater->executeMigrations(__FILE__, $this->getMigrations($updater));
+    }
+}

core/Version.php

@@ -22,7 +22,7 @@ final class Version
      * The current Matomo version.
      * @var string
      */
-    public const VERSION = '5.5.0-b1';
+    public const VERSION = '5.5.0-b2';
 
     public const MAJOR_VERSION = 5;
 

phpstan-baseline.neon

@@ -3718,31 +3718,6 @@ parameters:
 			count: 1
 			path: plugins/Actions/VisitorDetails.php
 
-		-
-			message: "#^PHPDoc tag @param for parameter \\$lastN with type bool\\|int is not subtype of native type int\\|null\\.$#"
-			count: 1
-			path: plugins/Annotations/API.php
-
-		-
-			message: "#^Parameter \\#1 \\$idSites of static method Piwik\\\\Piwik\\:\\:checkUserHasViewAccess\\(\\) expects array\\|int, string given\\.$#"
-			count: 2
-			path: plugins/Annotations/API.php
-
-		-
-			message: "#^Parameter \\#5 \\$starred of method Piwik\\\\Plugins\\\\Annotations\\\\AnnotationList\\:\\:update\\(\\) expects int\\|null, bool\\|null given\\.$#"
-			count: 1
-			path: plugins/Annotations/API.php
-
-		-
-			message: "#^Cannot call method getTimestamp\\(\\) on string\\|false\\.$#"
-			count: 1
-			path: plugins/Annotations/AnnotationList.php
-
-		-
-			message: "#^Method Piwik\\\\Plugins\\\\Annotations\\\\Annotations\\:\\:getDateRangeForPeriod\\(\\) should return array\\<Piwik\\\\Date\\> but returns array\\<int, false\\>\\.$#"
-			count: 1
-			path: plugins/Annotations/Annotations.php
-
 		-
 			message: "#^Parameter \\#1 \\$array of function reset is passed by reference, so it expects variables only\\.$#"
 			count: 1
@@ -4858,11 +4833,6 @@ parameters:
 			count: 1
 			path: plugins/Events/VisitorDetails.php
 
-		-
-			message: "#^Parameter \\#1 \\$idSites of static method Piwik\\\\Piwik\\:\\:checkUserHasViewAccess\\(\\) expects array\\|int, string given\\.$#"
-			count: 3
-			path: plugins/ExamplePlugin/API.php
-
 		-
 			message: "#^Parameter \\#3 \\$escapeComment of static method Piwik\\\\Plugins\\\\Diagnostics\\\\Diagnostic\\\\DiagnosticResult\\:\\:informationalResult\\(\\) expects bool, string given\\.$#"
 			count: 1
@@ -5958,11 +5928,6 @@ parameters:
 			count: 1
 			path: plugins/Referrers/API.php
 
-		-
-			message: "#^Parameter \\#1 \\$idSites of static method Piwik\\\\Piwik\\:\\:checkUserHasViewAccess\\(\\) expects array\\|int, string given\\.$#"
-			count: 3
-			path: plugins/Referrers/API.php
-
 		-
 			message: "#^Parameter \\#1 \\$name of method Piwik\\\\DataTable\\:\\:getColumn\\(\\) expects string, int given\\.$#"
 			count: 1
@@ -6688,11 +6653,6 @@ parameters:
 			count: 1
 			path: plugins/VisitTime/API.php
 
-		-
-			message: "#^Parameter \\#1 \\$idSites of static method Piwik\\\\Piwik\\:\\:checkUserHasViewAccess\\(\\) expects array\\|int, string given\\.$#"
-			count: 1
-			path: plugins/VisitTime/API.php
-
 		-
 			message: "#^Parameter \\#1 \\$idsite of static method Piwik\\\\Site\\:\\:getTimezoneFor\\(\\) expects int, string given\\.$#"
 			count: 1