Skip to content

Commit ba3943f

Browse files
Adds PHPStan, [no_release] (#234)
* Adds PHPStan, [no_release] * changed to level 1 * changed to level 0 * changed to level 1 * fixes error * Fixes * phpcs * changed name * Fix * Removed unwanted file * Fixed * changed test * Fixed docs * removed redundant logic * Added test cases --------- Co-authored-by: Lachlan Reynolds <lachlan.reynolds@innocraft.com>
1 parent 34c93f0 commit ba3943f

11 files changed

Lines changed: 341 additions & 5 deletions

File tree

.git-hooks-matomo/pre-push

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#!/bin/bash
2+
3+
# This hook is called with the following parameters:
4+
#
5+
# $1 -- Name of the remote to which the push is being done
6+
# $2 -- URL to which the push is being done
7+
#
8+
# If pushing without using a named remote those arguments will be equal.
9+
#
10+
# Information about the commits which are being pushed is supplied as lines to
11+
# the standard input in the form:
12+
#
13+
# <local ref> <local oid> <remote ref> <remote oid>
14+
15+
16+
17+
### Check we're running in the context of a plugin and get helpful dir variables ###
18+
19+
REPO_DIR="$(git rev-parse --show-toplevel)"
20+
echo "Running pre-commit hook in repo: $REPO_DIR"
21+
22+
if [[ "$REPO_DIR" =~ /plugins/(.*) ]]; then
23+
PLUGIN_PATH="plugins/${BASH_REMATCH[1]}/"
24+
else
25+
echo "Not a plugin, not running any further checks"
26+
exit 1
27+
fi
28+
MATOMO_DIR=$(echo "$REPO_DIR" | sed -E 's|/plugins/.*$||')
29+
30+
31+
32+
### Figure out how to run PHPStan - ddev or not. ###
33+
34+
COMMAND=""
35+
# Use local PHP if setup
36+
if command -v php >/dev/null 2>&1; then
37+
if [ -f "${MATOMO_DIR}/vendor/bin/phpstan" ]; then
38+
COMMAND="${MATOMO_DIR}/vendor/bin/phpstan"
39+
PLUGIN_PATH=''
40+
fi
41+
elif command -v ddev >/dev/null 2>&1; then
42+
# Use ddev if setup (overridding local setup)
43+
if [ -d "$MATOMO_DIR/.ddev" ]; then
44+
cd "$MATOMO_DIR" || exit 1
45+
if ddev status 2>&1 > /dev/null; then
46+
COMMAND="ddev exec phpstan"
47+
fi
48+
fi
49+
fi
50+
# If no command, exit
51+
if [[ -z "$COMMAND" ]]; then
52+
echo "No way to run phpstan found."
53+
exit 1
54+
fi
55+
56+
57+
58+
# Basic setup
59+
cd "$REPO_DIR"
60+
STATUS=0
61+
62+
63+
64+
65+
### Run PHPStan on newly created files. ###
66+
67+
PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon
68+
MAIN_BRANCH='5.x-dev'
69+
if [[ -f "$PHPSTAN_CREATED_CONFIG" ]]; then
70+
CHANGED_FILES=$(git diff --name-only ${MAIN_BRANCH} --diff-filter=A | grep '\.php$' || true)
71+
if [ -z "$CHANGED_FILES" ]; then
72+
echo "No created PHP files"
73+
else
74+
echo "Running PHPstan at a very high level on new files"
75+
CHANGED_FILES=`echo "$CHANGED_FILES" | sed -e 's/^\(.*\)$/"\1"/' | xargs -I{} echo "${PLUGIN_PATH}{}"`
76+
echo "$CHANGED_FILES" | xargs $COMMAND analyse -c ${PLUGIN_PATH}${PHPSTAN_CREATED_CONFIG} || STATUS=1
77+
fi
78+
fi
79+
80+
81+
82+
### Run PHPStan on modified files. ###
83+
PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon
84+
if [[ -f "$PHPSTAN_MODIFIED_CONFIG" ]]; then
85+
CHANGED_FILES=$(git diff --name-only ${MAIN_BRANCH} --diff-filter=CM | grep '\.php$' || true)
86+
if [ -z "$CHANGED_FILES" ]; then
87+
echo "No changed PHP files"
88+
else
89+
echo "Running PHPstan on modified files"
90+
CHANGED_FILES=`echo "$CHANGED_FILES" | sed -e 's/^\(.*\)$/"\1"/' | xargs -I{} echo "${PLUGIN_PATH}{}"`
91+
echo "$CHANGED_FILES" | xargs $COMMAND analyse -c ${PLUGIN_PATH}${PHPSTAN_MODIFIED_CONFIG} || STATUS=1
92+
fi
93+
fi
94+
95+
# Don't bother running the full check, as we check changes files already, and
96+
# can assume that the unchanged files don't need rechecking.
97+
#
98+
# Github will check this anyway.
99+
#
100+
# PHPSTAN_BASE_CONFIG=phpstan.neon
101+
# if [[ -f "$PHPSTAN_BASE_CONFIG" ]]; then
102+
# echo "Running PHPstan at a base level on all plugin files"
103+
# $COMMAND analyse -c ${PLUGIN_PATH}/${PHPSTAN_BASE_CONFIG} || STATUS=1
104+
# fi
105+
106+
exit $STATUS

.github/workflows/phpstan.yml

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
name: PHPStan check
2+
3+
on: pull_request
4+
5+
permissions:
6+
actions: read
7+
checks: read
8+
contents: read
9+
deployments: none
10+
issues: read
11+
packages: none
12+
pull-requests: read
13+
repository-projects: none
14+
security-events: none
15+
statuses: read
16+
17+
env:
18+
PLUGIN_NAME: CustomAlerts
19+
20+
jobs:
21+
phpstan:
22+
name: PHPStan
23+
runs-on: ubuntu-latest
24+
steps:
25+
- uses: actions/checkout@v4
26+
with:
27+
lfs: false
28+
persist-credentials: false
29+
- name: Setup PHP
30+
uses: shivammathur/setup-php@v2
31+
with:
32+
php-version: '7.2'
33+
34+
- name: Check out github-action-tests repository
35+
uses: actions/checkout@v4
36+
with:
37+
repository: matomo-org/github-action-tests
38+
ref: main
39+
path: github-action-tests
40+
41+
- name: checkout matomo for plugin builds
42+
shell: bash
43+
run: ${{ github.workspace }}/github-action-tests/scripts/bash/checkout_matomo.sh
44+
env:
45+
PLUGIN_NAME: ${{ env.PLUGIN_NAME }}
46+
WORKSPACE: ${{ github.workspace }}
47+
ACTION_PATH: ${{ github.workspace }}/github-action-tests
48+
MATOMO_TEST_TARGET: maximum_supported_matomo
49+
50+
- name: prepare setup
51+
shell: bash
52+
run: |
53+
cd ${{ github.workspace }}/matomo
54+
echo -e "composer install"
55+
composer install --ignore-platform-reqs
56+
57+
- name: checkout additional plugins
58+
if: ${{ env.DEPENDENT_PLUGINS != '' }}
59+
shell: bash
60+
working-directory: ${{ github.workspace }}/matomo
61+
run: ${{ github.workspace }}/github-action-tests/scripts/bash/checkout_dependent_plugins.sh
62+
63+
env:
64+
GITHUB_USER_TOKEN: ${{ secrets.TESTS_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
65+
66+
- name: "Restore result cache"
67+
uses: actions/cache/restore@v4
68+
with:
69+
path: /tmp/phpstan # same as in phpstan.neon
70+
key: "phpstan-result-cache-${{ github.run_id }}"
71+
restore-keys: |
72+
phpstan-result-cache-
73+
74+
- name: PHPStan whole repo
75+
id: phpstan-all
76+
run: cd ${{ github.workspace }}/matomo && composer run phpstan -- -vvv -c plugins/${{ env.PLUGIN_NAME }}/phpstan.neon
77+
78+
- name: "Save result cache"
79+
uses: actions/cache/save@v4
80+
if: ${{ !cancelled() }}
81+
with:
82+
path: /tmp/phpstan # same as in phpstan.neon
83+
key: "phpstan-result-cache-${{ github.run_id }}"

API.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public function deleteAlert($idAlert)
271271
/**
272272
* Get triggered alerts.
273273
*
274-
* @param int[] idSites
274+
* @param int[] $idSites
275275
*
276276
* @return array
277277
*/

Controller.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public function historyTriggeredAlerts()
118118

119119
$idSites = $this->getSiteIdsHavingAccess();
120120
$alerts = API::getInstance()->getTriggeredAlerts($idSites);
121-
array_slice($alerts, 0, 100);
121+
$alerts = array_slice($alerts, 0, 100);
122122
$alerts = array_reverse($alerts);
123123

124124
$view->alertsFormatted = $this->formatAlerts($alerts, 'html_extended');
@@ -280,7 +280,7 @@ private function addBasicCreateAndEditVariables($view, $alert)
280280
$view->supportsSMS = $this->supportsPlugin('MobileMessaging');
281281
$supportsSlack = $this->supportsPlugin('Slack');
282282
$isSlackOAuthTokenAdded = false;
283-
if ($supportsSlack) {
283+
if ($supportsSlack && class_exists(\Piwik\Plugins\Slack\SystemSettings::class)) {
284284
$slackSettings = StaticContainer::get(\Piwik\Plugins\Slack\SystemSettings::class);
285285
$isSlackOAuthTokenAdded = !empty($slackSettings->slackOauthToken->getValue());
286286
}

CustomAlerts.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public function registerEvents()
4141
'Request.dispatch' => 'checkControllerPermission',
4242
'Translate.getClientSideTranslationKeys' => 'getClientSideTranslationKeys',
4343
'UsersManager.deleteUser' => 'deleteAlertsForLogin',
44+
'UsersManager.removeSiteAccess' => 'removeAlertsForUser',
4445
'SitesManager.deleteSite.end' => 'deleteAlertsForSite',
4546
'Db.getTablesInstalled' => 'getTablesInstalled',
4647
'ScheduledTasks.execute' => 'startingScheduledTask',
@@ -295,4 +296,36 @@ public function validateReportParameters($parameters, $alertMedium)
295296
throw new \Exception(Piwik::translate('CustomAlerts_InvalidPhoneNumberReportParameter'));
296297
}
297298
}
299+
300+
301+
/**
302+
* Remove alerts associated with user
303+
*
304+
* If an alert is related to multiple sites that aren't in idSites, we
305+
* won't delete the alert, just remove the alert_site link and triggers
306+
*
307+
* @param string $userLogin Username of user who had access removed
308+
* @param array<string> $idSites List of website IDs
309+
* @return void
310+
*/
311+
public function removeAlertsForUser($userLogin, $idSites): void
312+
{
313+
if (empty($idSites) || empty($userLogin)) {
314+
return;
315+
}
316+
317+
$model = $this->getModel();
318+
$alerts = $model->getAlerts($idSites, $userLogin);
319+
320+
foreach ($alerts as $alert) {
321+
$alertId = $alert['idalert'];
322+
$alertSites = $alert['id_sites'];
323+
if (empty(array_diff($alertSites, $idSites))) {
324+
$model->deleteAlert($alertId);
325+
} else {
326+
$model->deleteTriggeredAlertsForUserAndSites($alertId, $idSites, $userLogin);
327+
$model->deleteAlertSitesForSites($alertId, $idSites);
328+
}
329+
}
330+
}
298331
}

Model.php

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public function getAlerts($idSites, $login = false)
112112

113113
/**
114114
* @param $idSites
115-
* @return array
115+
* @return string
116116
*/
117117
protected function getInnerSiteQuery($idSites)
118118
{
@@ -141,7 +141,7 @@ private function completeAlerts($alerts)
141141
private function getDefinedSiteIds($idAlert)
142142
{
143143
$sql = "SELECT idsite FROM " . Common::prefixTable('alert_site') . " WHERE idalert = ?";
144-
$sites = Db::fetchAll($sql, $idAlert, \PDO::FETCH_COLUMN);
144+
$sites = Db::fetchAll($sql, $idAlert);
145145

146146
$idSites = array();
147147
foreach ($sites as $site) {
@@ -453,4 +453,26 @@ public function deleteTriggeredAlertsForSite($idSite)
453453
{
454454
$this->getDb()->query("DELETE FROM " . Common::prefixTable("alert_triggered") . " WHERE idsite = ?", $idSite);
455455
}
456+
457+
public function deleteTriggeredAlertsForUserAndSites($idAlert, $idSites, $login)
458+
{
459+
$db = $this->getDb();
460+
$placeholders = Common::getSqlStringFieldsArray($idSites);
461+
$bind = array_merge(array($idAlert, $login), $idSites);
462+
$db->query(
463+
"DELETE FROM " . Common::prefixTable("alert_triggered") . " WHERE idalert = ? AND login = ? AND idsite in (" . $placeholders . " )",
464+
$bind
465+
);
466+
}
467+
468+
public function deleteAlertSitesForSites($idAlert, $idSites)
469+
{
470+
$db = $this->getDb();
471+
$placeholders = Common::getSqlStringFieldsArray($idSites);
472+
$bind = array_merge(array($idAlert), $idSites);
473+
$db->query(
474+
"DELETE FROM " . Common::prefixTable("alert_site") . " WHERE idalert = ? AND idsite in (" . $placeholders . " )",
475+
$bind
476+
);
477+
}
456478
}

phpstan.neon

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
parameters:
2+
level: 1
3+
phpVersion: 70200
4+
tmpDir: /tmp/phpstan/CustomAlerts/main
5+
paths:
6+
- .
7+
excludePaths:
8+
- tests/*
9+
- github-action-tests
10+
bootstrapFiles:
11+
- ../../bootstrap-phpstan.php
12+
universalObjectCratesClasses:
13+
- Piwik\Config
14+
- Piwik\View
15+
- Piwik\ViewDataTable\Config
16+
scanDirectories:
17+
# ../../ does not actually seem to give us anything
18+
# that ../plugins/ does not, but including it for
19+
# completeness. It does not seem to slow down performance.
20+
- .
21+

phpstan/phpstan.created.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
includes:
2+
- ../phpstan.neon
3+
parameters:
4+
level: 5
5+
tmpDir: /tmp/phpstan/CustomAlerts/created

phpstan/phpstan.modified.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
includes:
2+
- ../phpstan.neon
3+
parameters:
4+
level: 5
5+
tmpDir: /tmp/phpstan/CustomAlerts/modified

tests/Integration/CustomAlertsTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,26 @@ public function test_deleteAlertsForLogin()
200200
$this->assertEquals('Initial6', $alerts[2]['name']);
201201
}
202202

203+
public function test_removeSiteAccessEventRemovesAlertsForSitesAndLogin()
204+
{
205+
$this->createAlert('DeleteMe', array(), array(1), 'userLogin');
206+
207+
$this->assertCount(1, $this->model->getAlerts(array(1), 'userLogin'));
208+
Piwik::postEvent('UsersManager.removeSiteAccess', array('userLogin', array(1)));
209+
$this->assertCount(0, $this->model->getAlerts(array(1), 'userLogin'));
210+
}
211+
212+
public function test_removeSiteAccessEventRemovesSiteAndTriggersButKeepsAlert()
213+
{
214+
$alert = $this->createAlert('KeepMe', array(), array(1, 2), 'userLogin');
215+
$this->model->triggerAlert($alert['idalert'], 1, 99, 48, Date::now()->getDatetime());
216+
217+
Piwik::postEvent('UsersManager.removeSiteAccess', array('userLogin', array(1)));
218+
219+
$this->assertNotNull($this->model->getAlert($alert['idalert']));
220+
$this->assertCount(0, $this->model->getTriggeredAlerts(array(1), 'userLogin'));
221+
}
222+
203223
public function testStartingScheduledTask()
204224
{
205225
$this->checkOptionStringValue(true);

0 commit comments

Comments
 (0)