Skip to content

Adds PHPStan, [no_release]#234

Merged
lachiebol merged 16 commits into5.x-devfrom
add-phpstan
Feb 4, 2026
Merged

Adds PHPStan, [no_release]#234
lachiebol merged 16 commits into5.x-devfrom
add-phpstan

Conversation

@AltamashShaikh
Copy link
Contributor

Description

Adds PHPStan, [no_release]

Issue No

Steps to Replicate the Issue

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [NA] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [NA] Version bumped?
  • [NA] I have understood, reviewed, and tested all AI outputs before use
  • [NA] All AI instructions respect security, IP, and privacy rules

@AltamashShaikh AltamashShaikh requested a review from a team February 3, 2026 01:22
Copy link

@james-hill-matomo james-hill-matomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except for the extra pre-push file.

As an improvement, see Lachlan's new pre-push file, but that's a nitpick. https://github.com/innocraft/plugin-CrashAnalytics/pull/94

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate file

Copy link
Contributor Author

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left 1 comment around test, rest looks good to me

$alerts = $this->model->getTriggeredAlerts(array($this->idSite, $this->idSite2), 'userA');
$this->assertCount(3, $alerts);

$this->model->deleteTriggeredAlertsForUserAndSites($alertIdA, array($this->idSite, $this->idSite2), 'userA');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lachiebol Can we trigger the UsersManager.removeSiteAccess from here ? or add a similar unit test ?

Piwik::postEvent('UsersManager.removeSiteAccess',[[$idsites], $userLogin]);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AltamashShaikh That's been added

Copy link

@james-hill-matomo james-hill-matomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra file removed :)

@lachiebol lachiebol merged commit ba3943f into 5.x-dev Feb 4, 2026
9 checks passed
@lachiebol lachiebol deleted the add-phpstan branch February 4, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants