-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Open
Labels
EnhancementFor new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.c: PlatformFor Matomo platform changes that aren't impacting any of our APIs but improve the core itself.For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.
Description
Needed for #7131 and #4734. Requires #7821 and #7822.
A "type" could be a plugin that defines a type "Website" or "MobileApp". Different types need different reports and different naming of reports.
Requirements
- A type needs to be able to rename the name of a report
- A type needs to be able to rename the widget title of a report
- A type needs to be able to rename the menu title of a report
- A type needs to be able to disable a report that would be usually enabled (eg a report that is not needed for MobileApp type)
- A type needs to be able to enable a report that would be usually disabled (eg a new report specifically for MobileApp type)
- In some cases we need the possibility to access all reports (eg when creating a new report via
console generate:report
) - Does it actually make more sense in general to let types enable/disable dimensions instead of reports? Or both needed long term? Eg in theory there could be two different reports based on one dimension. Disabling only one dimension would make sure all later added reports (eg by another plugin) would be disabled as well. This can be wanted, but maybe not always.
To be considered
- Currently, all report instances are cached as it is quite expensive to create new report instances (currently). So probably a solution should allow us to still cache reporting instances (or make creating report instances fast)
- Some reports contain titles like "Visits by days since last visit". Here we should find a solution to use
%s
eg%s by days since last %s
if possible. This would lower the amount of reports having to rename a lot. This won't be easy and needs to be considered when working on this. I couldn't think of a nice solution so far. If it is not possible then okay. Maybe it gets more clear once we resolved the possibility to rename metrics in Add possibility to rename metrics based on the type of the site #7824 - From this point on we will allow only reports that are defined via a
Report
class. - In the future we want to allow types to disable a report based on a disabled dimension, just FYI. Eg a type could disable a dimension "Page URL" and then the report "getPageUrls" would be disabled.
- Sometimes we handle in the context of multiple idSites and therefore different types. Eg
getReportMetadata($idSites)
. In this case we might need to fallback to generic wording and do not apply the rename (or we apply a generic rename on top). We might deprecate$idSites
in the future. - Currently, eg the
WidgetsList
class usesReports::getAllReports()
to build a list of widgets. To do this it needs eg the name of the report which can be renamed by a type.WidgetsList
is in core. If we rename the Reports only inAPI.getProcessedReport/getReportMetadata
we could never get the renamed report name inWidgetsList
as the core does not know anything about APIs and other plugins. Therefore we might need to apply some renaming inReport::getAllReports()
and some ingetProcessedReport
(eg only metrics). Another solution would be to moveWidgetsList
to a plugin so it would know about APIs and would be able to access the metadata but Widgets are not only reports, it can be anything and it is kinda a core feature. Another reason why Widgets are Core: A report (which is a core feature) currently can add a widget. If Widgets were entirely done via a plugin, report would not be able to know anything about it. So moving widgets to a plugin also effects reports etc. - A type might want to change related reports
- How will performance be when we have like 200 or 300 reports (in case someone has installed many "types" / custom reports / etc)
- How will one later know which reports to disable? Requesting
getReportMetadata
and manually decide which reports are relevant is possible but quite a bit of work. Maybe we can make this somehow easier to get module/action for the reports to disable. Maybe a UI builder in a next step? - Will it be actually easy to change the order of a report and their widgets? Eg in Mobile apps other reports might be more important and one wants to list different reports on a page earlier. This is not important for now but will be at some point and maybe it helps to keep this in mind when looking for a solution.
- We need to make sure Reports do not translate anything directly. Currently they do eg
$this->name = Piwik::translate('MyKEy')
. They should not do that. Otherwise renaming can become difficult and we need the actual translation key for other things (eg it will allow us to cache report instances, maybe even serialize them). This will be a must do when working on reports. Instead we should translate where needed. This will also improve performance. - Similar to a new
WigdetConfig
we might want to introduce aReportConfig
and astatic configure(ReportConfig $config)
method.
Questions / TBD
- Can we use the same name for "Report Name", "Report Widget Title" and "Report Menu Title"? I know we cannot really but maybe we can somehow
Possible implementations
Here are some of my initial thoughts re possible implementations.
- Post an event
Piwik::postEvent('Reports.getAllReports', array($idSite))
when initialising the report instances, iterate over all instances and rename the report if needed. We'd need to post theidSite
as well, otherwise we cannot find the correct type. This solution would work but might be a bit slow (as we need to post many events under circumstances and we will be able to cache report instances only per site (a "Decorator" could fix that)) - In base report method
getMenuTitle
,getName
, ... we could check whether there is something to rename but we do not have the currentidSite
there. So we do not know with what to replace. Also plugins could overwrite that method and that could lever out that behaviour. Also wouldn't work when$this->menuTitle
or$this->name
is used directly within the class (easily fixable). Whenever someone uses eg$report->getName()
or$report->getMenuTitle()
, rename the title if needed. That could work as we usually have theidSite
at this point, but we'd have to do it whenever we use those methods no matter where. Error-prone... A "decorator" could help but still error-prone. Another problem: InReport::getAllReports()
the reports are sorted by category, this can lead to issues if we allow to rename categories as well as they would not be sorted by there actual renamed names but by their original names.- Pass
$idSite
toReport::getAllReports()
and initialise menu title based on that. This might work. We'd also have to do this forReport::factory()
. This is similar as1)
just that we do not post an event and couple report and title a bit more. Not 100% sure whether we always have the$idSite
but probably. - Whenever we output something (UI, Scheduled Reports, ...) always use
API.getProcessedReport()
, hook to that event, and rename any report name if needed. This might work, but it'll work only there. If something doesn't usegetProcessedReport
we have a problem. Also we have again a problem if reports are eg sorted by category. Renaming them afterwards would lead to a wrong sort order (same problem as in 4.). Edit: I think it is wrong that reports are sorted ingetAllReports
so this could be an acceptable solution. - We will have a
Type
class which uses generic words by default. Eg "Property" instead of "Website". When creating a report instance we require aReportBuilder
eg via DI and set the current type. From there we create a report. AReportRepository
might handle multiple reports. A report can create more Metrics by using eg aMetricsRepository
andMetricsBuilder
. A report can access the current type and do eg$this->name = Piwik::translate('MyKey', $this->type->getNamePlural());
or$this->name = Piwik::translate('MyKey', $this->visitsMetric->getNamePlural());
. As a report can currently define its metrics like$this->metrics = array('nb_visits', 'nb_visitoors')
we could maybe keep it this way and resolve the metrics into classes in the background (maybe).
- Solution 1 and 4 seems to work, an implementation of 4) is available in https://github.com/piwik/piwik/compare/poc_manage_apps (not finished). A specific report is always based on
module
,action
andidSite
. Should also work for removing report(s) and forWidgets::getAllWidgets
. - We will go most likely with 6.
Ideally, we would maybe use solution 5) as we always have an idsite there etc. I need to implement a POC. I think it will be problematic when rendering a report as it requires a report instance and not an array as returned byprocessed report / report metadata
. I think we might actually need to render reports in the browser and only consume data viaAPI.getProcessedReport
etc.- If we would render reports entirely in the browser we could go with 5) I think (ScheduledReports etc already uses metadata API)
- In theory the report should not know anything about a site / idsite. It should rather get the
$type
passed. - In theory it is easy. We have a ReportBuilder that creates a report instance, configures it or passes a
$type
to it and everything is easy. Problem are translations if we use report names like%s (Visits) by days since last %s (visit)
What a report is or what a report can do, what they are used for, ...
- A report has a name
- A report belongs to a report category
- A report consists of dimensions and widgets
- A report can define a new widget
- A report can be added/displayed on a page
- A report defines how the report should be displayed (eg defaultSortOrder, whether search is enabled, which columns should be displayed, ...)
- A report has related reports
Metadata
Metadata
Assignees
Labels
EnhancementFor new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.c: PlatformFor Matomo platform changes that aren't impacting any of our APIs but improve the core itself.For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.