Report classes should not be instantiated directly #7821
Labels
c: Platform
For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.
Task
Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone
In Controllers we often instantiate a report directly such as in https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/VisitorInterest/Controller.php#L22-L25 , also related reports are currently instantiated directly eg in https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/Actions/Reports/GetPageTitles.php#L83-L86 . For example
new GetNumberOfVisitsPerVisitDuration()
.This is not good as another plugin could overwrite this report instance and for example provide a different name or display it in a different way.
Instead a factory should be used at all times. Eg
Report::factory('VisitorInterest', 'getNumberOfVisitsPerVisitDuration');
.This could be also done via DI but for this we'd need to change
Plugin\Manager::find(Multi)Components
.Using the factory will be required for #7131 and #4734 . The
Report::factory($module, $action, $idsite|$type)
(orReport::builder()
) will instantiate a report and based on the type of the site (website, mobileapp, ...) not return the report instance or change properties of the report instance (eg overwrite the report name). Instead of using one method we might want to create eg aReportRepository
and aReportBuilder
as reports itself should be not aware of Sites and Types.I do not really care how it is solved. It will be just important that we create report instances via one method that can decide whether we're allowed to return the report and can change the report instance (eg
$report->setName('MobileAppName');
). For now we do not care aboutidSites
andtypes
in this issue. It is only a preparing so that we can refactor later. Therefore we could just go withfactory($module, $action) for now
The text was updated successfully, but these errors were encountered: