Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an API for plugins to display (multiple) reports in a page #7822

Closed
tsteur opened this issue May 4, 2015 · 9 comments
Closed

Provide an API for plugins to display (multiple) reports in a page #7822

tsteur opened this issue May 4, 2015 · 9 comments
Assignees
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@tsteur
Copy link
Member

tsteur commented May 4, 2015

Background: In #7131 and #4734 we want to allow plugins to define a new type (eg Website, Mobile App, Server, ...). Types will be able to rename and to remove/add reports.

Problem 1: We have many pages in Piwik that display multiple reports in one page. They usually define the name of the report hard coded in the twig template such as in https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/VisitorInterest/templates/index.twig#L3 . This is not good as the name could have been changed by a plugin from eg Visits by days since last visit to Sessions by days since last session

Problem 2: All reports that should be displayed are hard coded in twig templates and controller eg https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/VisitorInterest/templates/index.twig#L3 and https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/VisitorInterest/Controller.php#L21-L26 . If some reports are disabled by a type, they would be still rendered and displayed. Instead only the actual enabled reports should be displayed.

Problem 3: Links to such a page are hard coded in the Menu class like this: https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/VisitFrequency/Menu.php#L17 There is no way for us to find out whether all reports within that page are disabled or not. Eg if all reports within that page are disabled, we should not display this link in the menu. There would be a way for a plugin to disable that menu item by using the translation key of the menu name but this is no good. It is hard to find the translation key to use and the key could change; and another plugin does not know which plugins are displayed in that page (this can change over time) etc.

In some cases a report class defines a $menuTitle causing it to be displayed in the menu. When such a report is disabled by a type, the menuTitle will not appear in the reporting menu which is good.

Goals:

  • When displaying a report name in a page, use the $name of the $report instance
  • Display only reports in a page that are actually enabled
  • If no report within a page is enabled/displayed, do not show the link in the reporting menu
  • Make it easy for plugin developers to define where a report should be displayed
  • When creating a report, a developer should not need to know anything about controller or view
  • We want to get rid of Menu::configureReportingMenu. The reporting menu should be built based on top of the API output of whatever composes those pages
  • We need to get rid of hard coded reporting menu links like this https://github.com/piwik/piwik/blob/2.13.0-rc3/plugins/Actions/Menu.php#L23-L26
  • Types will be later able to rename a page, (maybe even change pages such as layout etc)

This will also help us make report pages look and feel more "similar".

Out of scope:

  • We do not need to allow plugins to move a report from one page into another page (not for now, probably needed later)

There are many ways to solve this. Ideally, we would generate the pages etc based on the reporting metadata. It could be as well a new API. This API / layout information could be interesting for Piwik Mobile as well.

As we want to render everything in the frontend anyway (long term), we could fetch the reports metadata (layout) via ajax and render the layouts (not the report itself) + reporting menu via angularjs.

Pages might be the wrong word, on a phone it is eg a "Screen". What would be a good name for eg an API method API.getReportPages? Or do we have to adjust API.getReportMetadata (might be hard since pages != report)

Maybe we can decouple pages and reports. Eg a report says it belongs to a category Visits, subcategory Devices. A subcategory devices decides what layout to use (and maybe also adds a report?) ... maybe rather not.

Ideas?

@tsteur tsteur added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. labels May 4, 2015
@tsteur tsteur changed the title Provide an API for plugins to display multiple reports in one page Provide an API for plugins to display (multiple) reports in a page May 4, 2015
@tsteur
Copy link
Member Author

tsteur commented May 7, 2015

In Piwik we have the following layouts in report pages:

  • One report only on the entire page (eg Pages)
  • One report normal width (eg Custom Variables)
    • Maybe we could have one layout with one column, the browser decided automatically depending on the number of columns etc how to display it (whether it takes full width or not).
  • Wide evolution graph, followed by 2 column sparklines (eg Visitors Overview) optionally followed by 2 column reports (eg Visitors Engagement)
  • Reports in 2 columns (eg Visitors => Software), each column width is 50%
  • ReportsByDimension: (eg Events)
  • Special case Goals overview

To be considered

  • What about ReportsByDimension? This is kinda like a "page" in a "page".
  • One of my first thoughts was kinda like being able to specify a layout like '33-33-33', similar to dashboard . But isn't this hard coded for browsers etc? It would not be really helpful for the mobile app. So maybe a report would only say I belong to Visitors->Devices. The browser decides how to display. Eg if there is only one report, we display it in one column. If there are multiple reports, depending on how many columns they have we display them in 2 column (on big screens maybe 3 columns?). TBD: How to render overview pages with evolution (and Visits->Engagement). We cannot really detect this automatically. Also how would we define the order of categories/subcategories? Maybe, if we have classes for this, could we as well define a default layout there such as "overview" but we will also generate it automatically for easy API?

Rough idea, very experimental, possibly doesn't work

  • We create classes for all categories
  • We introduce subcategories
  • A report belongs to a category, optionally a subcateogry. If no subcategory is put the report - maybe - into a subcategory "overview".
  • We need to expose subcategory and order (it is there indirectly as it is ordered I think) of a report to report metadata
  • Any report can define a new subcategory - maybe just be defining a string or new Subcateogry('test').
  • If there is a class for a subcategory, we pick it up. A subcategory can define a layout to use to force a certain layout eg what Visits->Engagement does with Evolution, Sparklines, ... It can also define a order.
  • To simplify the "overview" type page any subcategory can define an evolution and/or sparklines view
  • Reports returned in reportMetadata need to be ordered by order of Category, Subcategory so we display them correct in the menu. Subcategories defined by a 'string' will be added to the end Randomly
  • So a report specifies only a category and subcategory. Easy!
  • Works for mobile
  • Will allow us later to rename reports etc. when having classes and so on
  • Problem reports by dimension! (should be maybe rather requested as a widget but then we do not know whether it contains any report or not) ---> maybe we need to use post events on top for such special cases?
  • Problem a report cannot be in two pages!
  • Problem we'd need the possibility to add widgets as well to a category/subcategory
    Note for myself: If no subcategory defined, goes into an overview page maybe

@tsteur tsteur added this to the 2.14.0 milestone May 8, 2015
@tsteur tsteur self-assigned this May 8, 2015
@tsteur
Copy link
Member Author

tsteur commented May 8, 2015

@mattab I'm a little confused here. While working on a proof of concept I noticed that a report belongs to a category, for example the VisitorLog belongs to the category "Live!". In the UI it is listed under "Visitors" though and the category is pretty much ignored except for widgets. I was wondering why we do have categories? Was it only for the list of widgets? And why is the list of widgets completely different to the menu? Aren't reports harder to find this way? Was it imaginable that we do use the same structure in WidgetsList as in the menu?

If we have a category for the widgetsList, how would we name the "category" (eg Visitors) and "subcategory" (eg Engagement) for the menu?

Note to myself: We might as well want to refactor the widgets API and make it similar to a Report to have one class per widget eg Widgets/GetDonateForm.php. This would allow us to show widgets within any page. But then Types/Properties would need the possibility to remove them again -> makes everything more complicated. Maybe we don't want to allow to add them as pages but change this API though. It would allow us in the future to be more flexible and to treat reports / widgets similar. Also we could allow types to disable any report or widget via "$module.$action"

@tsteur
Copy link
Member Author

tsteur commented May 12, 2015

While working on #7861 I noticed that all reports, that define a menu title (and therefore have a reporting page), also define a widgets. So I'm thinking changing this into

Provide an API for plugins to display (multiple) WIDGETS in a page

This might simplify a lot of things.

Can anyone think of a use case when we want to put a report on a page but not as widget?

Also View\ReportsByDimension might be actually something like View\GroupedWidgets? Need to check. Basically a report doesn't have a category etc which ReportsByDimension needs, only the widget that a report creates has a category.

@tsteur
Copy link
Member Author

tsteur commented May 17, 2015

Some more thoughts on what we want to achieve etc

  • Simple report creation for developers
  • Simple visualization creation for developers
  • Create easily 1 or many "ReportViews" where a "ReportView" connect one report with one visualization
  • Easily put a ReportView on any page. (by defining a category/subcategory)
  • The API output has to be generic so that other consumers can render this as well
    • Problem our visualization ids such as graphEvolution will be hard to be handled by other clients (eg Piwik Mobile)
  • URL's to render the report as HTML and to get the processed report should be in the API output (maybe also imagegraphUrl and pdf report url and export links?)
  • What about Manage Goals?!? To add something like this we'd have to create a fake report with a Visualization that renders a controller... we need an event...
  • MenuReporting will become obsolete
  • When we built the visualization API, the Report class did not exist yet. Maybe we can simplify some things? And move some properties from Visualization config/requestConfig to Report config maybe? Eg relatedReports is present in both
  • Rename ViewDataTable => ReportView?
  • Rename Visualization => ReportVisualization?
  • ViewDataTable / ReportView should only contain a few methods like, const ID, requestReport() and render()
  • ReportView => ReportViewConfig? It should not render! only ViewDataTable/Visualization should render, ReportViewConfig is confusing since we also have ViewDataTableConfig (which would be then ReportViewConfig as well if we rename ViewDataTable => ReportView)
  • Many things are based on $controllerAction which we no longer have, maybe instead we pass the Report?
  • Category / Subcategory needs a module/action problem!

@mattab
Copy link
Member

mattab commented May 20, 2015

Good to see this progress towards simpler & meaningful core APIs, and a better generic analytics platform.

Looking forward to seeing where it will take us!

@tsteur
Copy link
Member Author

tsteur commented Jul 20, 2015

FYI: I spent one or two days trying to fix this screenshot test but give up: http://builds-artifacts.piwik.org/ui-tests.7822_4/14275.7/UIIntegrationTest_visitors_engagement

It seems related to triggering the archiving in UiTestFixture vs before in the UI. I tried for hours (almost a day) to create a new OmniFixture.sql that fixes it but it doesn't work, it makes it rather worse and shows many zeros. I have no clue why and what is going on there... Background: In the UiTestFixture we generate all dashboards, to do this we need all widgets, to do this some archived data needs to be requested. This means while generating the fixture we already generate the archives.

The UI itself works and all the code etc but as the archiving is triggered earlier different numbers are shown.

@mattab
Copy link
Member

mattab commented Jul 22, 2015

Sorry to hear the troubles with UITestFixture and OmniFixture.sql... is there something we could investigate to unblock you on this? or do you have a plan regarding this UI test UIIntegrationTest_visitors_engagement?

btw in the Processed screenshot is looks like "actions by returning visits" and "actions per returning visits" are switched, ie. the metrics should be switched so they match the label. It was correct in Expected screenshot.

@tsteur
Copy link
Member Author

tsteur commented Jul 22, 2015

I worked around it.

btw in the Processed screenshot is looks like "actions by returning visits" and "actions per returning visits" are switched,

Cheers, yes they are switched

@tsteur
Copy link
Member Author

tsteur commented Mar 9, 2016

This is pretty much done for now and I will close it for now. We need to kinda re-visit this topic over time again and will create issues when needed

@tsteur tsteur closed this as completed Mar 9, 2016
@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

No branches or pull requests

2 participants