From c93aa0bede24596df491f370d416729af70c8618 Mon Sep 17 00:00:00 2001 From: Altamash Shaikh Date: Fri, 11 Oct 2024 21:31:45 +0530 Subject: [PATCH 1/2] Adds test for PHPCS --- .github/workflows/phpcs.yml | 43 +++++++++++++++++++++++++++++++++++++ phpcs.xml | 36 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 .github/workflows/phpcs.yml create mode 100644 phpcs.xml diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml new file mode 100644 index 0000000..9fa80c6 --- /dev/null +++ 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/migration --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/phpcs.xml b/phpcs.xml new file mode 100644 index 0000000..1986007 --- /dev/null +++ b/phpcs.xml @@ -0,0 +1,36 @@ + + + + Matomo Coding Standard for Migration plugin + + + + . + + tests/javascript/* + */vendor/* + + + + + + + + tests/* + + + + + Updates/* + + + + + tests/* + + + + + tests/* + + \ No newline at end of file From 96e70663d8c8880fdee4a2f987778baf4a3b5305 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Fri, 18 Oct 2024 11:26:13 +1300 Subject: [PATCH 2/2] Making PHPCS changes --- Commands/Migrate.php | 3 ++- Db/BatchQuery.php | 1 + Migration.php | 1 + Migrations.php | 15 ++++++++++++--- Migrations/AnnotationsMigration.php | 1 + Migrations/ArchiveMigration.php | 2 +- Migrations/BaseMigration.php | 3 ++- Migrations/CustomDimensionMigration.php | 1 + Migrations/GoalsMigration.php | 1 + Migrations/LogActionMigration.php | 1 + Migrations/LogMigration.php | 10 +++++----- Migrations/Provider.php | 7 +++++-- Migrations/Request.php | 2 +- Migrations/SegmentsMigration.php | 1 + Migrations/SiteMigration.php | 1 + Migrations/SiteSettingMigration.php | 1 + Migrations/SiteUrlMigration.php | 1 + TargetDb.php | 1 + tests/Fixtures/MigrationFixture.php | 15 ++++++++------- .../Migrations/LogActionMigrationTest.php | 2 +- tests/Integration/TargetDbTest.php | 2 +- tests/System/Commands/MigrateTest.php | 3 +-- 22 files changed, 50 insertions(+), 25 deletions(-) diff --git a/Commands/Migrate.php b/Commands/Migrate.php index a44fa3e..7b94947 100644 --- a/Commands/Migrate.php +++ b/Commands/Migrate.php @@ -1,4 +1,5 @@ askForConfirmation('Are you sure you want to migrate the data for idSite '.(int) $idSite.'. (y/N)'); + return $this->askForConfirmation('Are you sure you want to migrate the data for idSite ' . (int) $idSite . '. (y/N)'); } } diff --git a/Db/BatchQuery.php b/Db/BatchQuery.php index eb593f7..99edf08 100644 --- a/Db/BatchQuery.php +++ b/Db/BatchQuery.php @@ -1,4 +1,5 @@ log('Processed ' . $migration->getName()); } } catch (\Exception $e) { - //Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than an approximation maintained by PDO. If a query that is subject to "implicit commit" is executed, PDO::inTransaction() will subsequently return false, as a transaction is no longer active + /** + * Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than + * an approximation maintained by PDO. If a query that is subject to "implicit commit" is executed, + * PDO::inTransaction() will subsequently return false, as a transaction is no longer active + */ //inTransaction check fixes warning raised due to implicit commit change if ($this->isInTransaction($targetDb)) { $targetDb->rollBack(); @@ -51,7 +56,11 @@ public function migrate($migrations, Request $request, TargetDb $targetDb) } throw $e; } - //Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than an approximation maintained by PDO. If a query that is subject to "implicit commit" is executed, PDO::inTransaction() will subsequently return false, as a transaction is no longer active + /** + * Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than an + * approximation maintained by PDO. If a query that is subject to "implicit commit" is executed, + * PDO::inTransaction() will subsequently return false, as a transaction is no longer active + */ //inTransaction check fixes warning raised due to implicit commit change if ($this->isInTransaction($targetDb)) { $targetDb->commit(); @@ -72,7 +81,7 @@ public function onLog($callback) private function isInTransaction($targetDb) { - $inTransactionMethodExists = method_exists($targetDb->getDb()->getConnection(),'inTransaction'); + $inTransactionMethodExists = method_exists($targetDb->getDb()->getConnection(), 'inTransaction'); return (!$inTransactionMethodExists || $targetDb->getDb()->getConnection()->inTransaction()); } diff --git a/Migrations/AnnotationsMigration.php b/Migrations/AnnotationsMigration.php index 775ea69..728bc6f 100644 --- a/Migrations/AnnotationsMigration.php +++ b/Migrations/AnnotationsMigration.php @@ -1,4 +1,5 @@ sourceIdSite)); + $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable($tableUnprefixed) . ' WHERE ' . $idSiteColumnName . ' = ?', array($request->sourceIdSite)); $this->log(sprintf('Found %s %s', count($rows), $entityName)); diff --git a/Migrations/CustomDimensionMigration.php b/Migrations/CustomDimensionMigration.php index 587af2a..a37fa06 100644 --- a/Migrations/CustomDimensionMigration.php +++ b/Migrations/CustomDimensionMigration.php @@ -1,4 +1,5 @@ generateQuery('SELECT * FROM ' . Common::prefixTable('log_visit') . ' WHERE idsite = ? ORDER BY idvisit ASC', array($request->sourceIdSite)) as $visitRows) { + foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_visit') . ' WHERE idsite = ? ORDER BY idvisit ASC', array($request->sourceIdSite)) as $visitRows) { $count += count($visitRows); $this->migrateVisits($visitRows, $request, $logActionMigration, $targetDb); @@ -74,7 +75,7 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration $visitorIds = implode(',', $visitorIds); $batchQuery = new BatchQuery(); - foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_link_visit_action') . ' WHERE idvisit in ('.$visitorIds.') ORDER BY idlink_va ASC') as $actionRows) { + foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_link_visit_action') . ' WHERE idvisit in (' . $visitorIds . ') ORDER BY idlink_va ASC') as $actionRows) { foreach ($actionRows as $row) { $oldIdLinkAction = $row['idlink_va']; $row['idvisit'] = $visitorIdMap[$row['idvisit']]; @@ -91,7 +92,7 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration } unset($actionRows); - $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion') . ' WHERE idvisit in ('.$visitorIds.')'); + $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion') . ' WHERE idvisit in (' . $visitorIds . ')'); foreach ($rows as $row) { $row['idvisit'] = $visitorIdMap[$row['idvisit']]; if (isset($row['idlink_va'])) { @@ -106,7 +107,7 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration unset($rows); - $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion_item') . ' WHERE idvisit in ('.$visitorIds.')'); + $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion_item') . ' WHERE idvisit in (' . $visitorIds . ')'); foreach ($rows as $row) { $row['idvisit'] = $visitorIdMap[$row['idvisit']]; $row['idsite'] = $request->targetIdSite; @@ -136,5 +137,4 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration unset($visitRows); } - } diff --git a/Migrations/Provider.php b/Migrations/Provider.php index 3162f13..f0b5a5d 100644 --- a/Migrations/Provider.php +++ b/Migrations/Provider.php @@ -1,4 +1,5 @@ isPluginActivated('CustomDimensions') - && $pluginManager->isPluginLoaded('CustomDimensions')) { + if ( + $pluginManager->isPluginActivated('CustomDimensions') + && $pluginManager->isPluginLoaded('CustomDimensions') + ) { $migrations[] = new CustomDimensionMigration(); } diff --git a/Migrations/Request.php b/Migrations/Request.php index a5bd5a7..bf7f267 100644 --- a/Migrations/Request.php +++ b/Migrations/Request.php @@ -1,4 +1,5 @@ 'Download Software', 'match' => 'url', 'pattern' => 'download', 'patternType' => 'contains', 'revenue' => 0.10), @@ -64,7 +65,7 @@ public function setUp(): void $this->trackSecondVisit(); // make sure to archive data - Request::processRequest('API.get',array( + Request::processRequest('API.get', array( 'period' => 'year', 'date' => '2013-01-23', 'idSite' => $this->idSite @@ -74,9 +75,9 @@ public function setUp(): void private function setUpCustomDimensions() { $configuration = new Configuration(); - $configuration->configureNewDimension($this->idSite, 'MyName1', CustomDimensions::SCOPE_VISIT, 1, $active = true, $extractions = array(), $caseSensitive = true); - $configuration->configureNewDimension($this->idSite, 'MyName2', CustomDimensions::SCOPE_ACTION, 1, $active = true, $extractions = array(), $caseSensitive = true); - $configuration->configureNewDimension($this->idSite, 'MyName3', CustomDimensions::SCOPE_ACTION, 2, $active = false, $extractions = array(), $caseSensitive = true); + $configuration->configureNewDimension($this->idSite, 'MyName1', CustomDimensions::SCOPE_VISIT, 1, $active = true, $extractions = array(), $caseSensitive = true); + $configuration->configureNewDimension($this->idSite, 'MyName2', CustomDimensions::SCOPE_ACTION, 1, $active = true, $extractions = array(), $caseSensitive = true); + $configuration->configureNewDimension($this->idSite, 'MyName3', CustomDimensions::SCOPE_ACTION, 2, $active = false, $extractions = array(), $caseSensitive = true); } private function setUpSegments() @@ -109,7 +110,7 @@ public static function copyTableStructure(TargetDb $targetDb) $row = Db::fetchRow('SHOW CREATE TABLE `' . $table . '`'); $sql = $row['Create Table']; - $sql = str_replace('`'.$table.'`', '`'.$tablePrefixed.'`', $sql); + $sql = str_replace('`' . $table . '`', '`' . $tablePrefixed . '`', $sql); $targetDb->getDb()->query($sql); } } @@ -190,4 +191,4 @@ protected function trackSecondVisit() $t->addEcommerceItem($sku = 'SKU_ID2', $name = 'A durable item', $category = 'Best seller', $price = 321); self::checkResponse($t->doTrackEcommerceCartUpdate($grandTotal = 33 * 77)); } -} \ No newline at end of file +} diff --git a/tests/Integration/Migrations/LogActionMigrationTest.php b/tests/Integration/Migrations/LogActionMigrationTest.php index 96f10c2..030feab 100644 --- a/tests/Integration/Migrations/LogActionMigrationTest.php +++ b/tests/Integration/Migrations/LogActionMigrationTest.php @@ -1,4 +1,5 @@ migration->migrateAction($id3, $this->targetDb); $this->assertEquals(3, $targetId3); } - } diff --git a/tests/Integration/TargetDbTest.php b/tests/Integration/TargetDbTest.php index 37c9188..d40002f 100644 --- a/tests/Integration/TargetDbTest.php +++ b/tests/Integration/TargetDbTest.php @@ -1,4 +1,5 @@ '4', 'url' => 'https://www.foobaz.com'), ), $urls); } - } diff --git a/tests/System/Commands/MigrateTest.php b/tests/System/Commands/MigrateTest.php index 1e9e70a..6d3ccf9 100644 --- a/tests/System/Commands/MigrateTest.php +++ b/tests/System/Commands/MigrateTest.php @@ -1,4 +1,5 @@