Opened 2 years ago

Closed 20 months ago

Last modified 20 months ago

#2742 closed New feature (fixed)

Metadata API: Support sub tables calls

Reported by: matt Owned by: capedfuzz
Priority: major Milestone: 1.12.x - Piwik 1.12.x
Component: Core Keywords:
Cc: tsteur Sensitive: no

Description

Metadata API should have full coverage of Piwik reports. In particular, one should be able to fetch subtables for Page URLs, Page Titles, Custom Variables, Campaign Names, etc.

  • Piwik Metadata API should return which API to call when a row is clicked eg. CustomVariables.getCustomVariablesValuesFromNameId
  • Piwik metadata should support the API call CustomVariables.getCustomVariablesValuesFromNameId currently unsupported

Attachments (2)

2742.diff.tar.gz (3.1 KB) - added by capedfuzz 21 months ago.
Initial patch for this issue.
2742.diff.tar.2.gz (3.2 KB) - added by capedfuzz 21 months ago.
New patch for this issue.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 2 years ago by matt (mattab)

At the same time, we could add support needed for #2137

This is a slightly different "subtable" requirement: we need the full table expanded and truncated.

  • Support for recursive filter_truncate #2816
  • Include the full children hierarchy in the Metadata output
  • New Metadata report attribute: displayExpandedFriendly=1 for these in #2137

Then it will be possible to get eg. expanded Page URLs report in Scheduled reports (where clicking to expand is not possible)

comment:2 Changed 2 years ago by matt (mattab)

  • Type changed from Bug to New feature

comment:3 Changed 2 years ago by EZdesign (BeezyT)

(In [5722]) refs #534, #2742 metadata api extensions: more/better translations, actionToLoadSubTables

comment:4 Changed 2 years ago by EZdesign (BeezyT)

(In [5723]) refs #534, #2742 integration tests for previous commit

comment:5 Changed 22 months ago by matt (mattab)

  • Cc thomas added

As discussed during the meetup, you could call the raw API to fetch subtables.

However, maybe this is not enough: eg. Campaign name click to "Campaign keyword". Without metadata, you wouldn't know the column name. This would be a non optimal experience for mobile app users and would be confusing. Therefore we need to add the support to metadata :-)

comment:6 Changed 22 months ago by matt (mattab)

  • Cc tsteur added; thomas removed

@tsteur do you mind checking the comment http://dev.piwik.org/trac/ticket/2742#comment:5

comment:7 Changed 22 months ago by tsteur

You're right. We need to add the support to metadata cause of the data structure and so on.

Changed 21 months ago by capedfuzz (diosmosis)

Initial patch for this issue.

comment:8 Changed 21 months ago by capedfuzz (diosmosis)

I put up a patch that solves this for the CustomVariables report. Let me know what you think of the approach I took. (Not sure if the subtable metadata is correct.)

comment:9 Changed 21 months ago by matt (mattab)

  • Owner set to capedfuzz

comment:10 follow-up: Changed 21 months ago by matt (mattab)

Nice one....

Review:

  • change the parameters of getProcessedReport but you didnt change all callers, ie. ImageGraph/API
  • dimension of subtable is the custom var value: CustomVariables_ColumnCustomVariableValue
  • we should hide the isSubtableReport APIs from the listing in getReportMetadata (if not done already)
  • can you think of any issue with the approach you took?

Changed 21 months ago by capedfuzz (diosmosis)

New patch for this issue.

comment:11 in reply to: ↑ 10 Changed 21 months ago by capedfuzz (diosmosis)

I uploaded a new patch.

Replying to matt:

Nice one....

Review:

  • change the parameters of getProcessedReport but you didnt change all callers, ie. ImageGraph/API

I moved the parameter to the end of the function call. This might be annoying for future uses of it, but I think this is better.

  • dimension of subtable is the custom var value: CustomVariables_ColumnCustomVariableValue
  • we should hide the isSubtableReport APIs from the listing in getReportMetadata (if not done already)

It's hidden, which is proved by the fact that the output for the getReportMetadata/getMetadata tests does not have to change.

  • can you think of any issue with the approach you took?

No... but I'm not aware of every use case of API.getProcessedReport. W/ my changes, you can call reports that return subtable reports, and if desired, get metadata for those reports by supplying an extra parameter (showSubtableReports=1) to getReportMetadata/getMetadata.

I think it's ok to commit?

comment:12 Changed 21 months ago by matt (mattab)

Looks good!

+ public function getAnotherApiToTest()
+ {
+ if (get_class($this) == 'Test_Piwik_Integration_TwoVisitsWithCustomVariables')

In general it's bad practise to reference test names in the test, if the class changes the test won't run without us noticing. So it's better to redefine the function to do nothing in children tests, and remove the if()

  • Patch it is missing the subtable support to all other reports that have subtables: Referers and Actions APIs. You can commit add these just after if it's easier for you, but definitely important to support all :)

comment:13 Changed 20 months ago by capedfuzz (diosmosis)

  • Resolution set to fixed
  • Status changed from new to closed

(In [6800]) Fixes #2742, added support for getting subtable reports through metadata API and added necessary metadata so existing subtable reports can be obtained.

Notes:

  • Added ability to test subtable API actions.
  • Added subtable metadata to methods in Actions, CustomVariables & Referers APIs.

comment:14 Changed 20 months ago by matt (mattab)

Nothing to add except, very nice commit & new tests!

comment:15 follow-up: Changed 20 months ago by tsteur

I have implemented a first working version in Piwik Mobile and I have two questions.

  • Are there other reports beside "Custom Variables" that supports Subtables via Metadata yet?
  • When requesting the subtable report, the "imageGraphUrl" property has a value like the following:

index.php?module=API&method=ImageGraph.get&idSite=1&apiModule=CustomVariables&apiAction=getCustomVariablesValuesFromNameI&period=week&date=yesterday

When displaying this graph it always displays the following error message: "Invalid API Module and/or API Action"

I'm not sure how much work it is to add graph support for more Modules/Actions. Is it possible to disable the graphs in Piwik if it is a subtable report? Once graphs for more Modules/Actions are working we can simply enable them step by step without having to change Piwik Mobile.

comment:16 in reply to: ↑ 15 Changed 20 months ago by capedfuzz (diosmosis)

Replying to tsteur:

I have implemented a first working version in Piwik Mobile and I have two questions.

  • Are there other reports beside "Custom Variables" that supports Subtables via Metadata yet?

I added subtable metadata for the following reports in my last commit:

  • Actions.getPageUrls
  • Actions.getEntryPageUrls
  • Actions.getExitPageUrls
  • Actions.getPageTitles
  • Actions.getEntryPageTitles
  • Actions.getExitPageTitles
  • Actions.getOutlinks
  • Actions.getDownloads
  • Referers.getKeywords
  • Referers.getWebsites
  • Referers.getSearchEngines
  • Referers.getCampaigns

Metadata calls w/ subtables should work w/ these reports.

  • When requesting the subtable report, the "imageGraphUrl" property has a value like the following:

index.php?module=API&method=ImageGraph.get&idSite=1&apiModule=CustomVariables&apiAction=getCustomVariablesValuesFromNameI&period=week&date=yesterday

When displaying this graph it always displays the following error message: "Invalid API Module and/or API Action"

This specific error is because the URL is using 'getCustomVariablesValuesFromNameI' instead of 'getCustomVariablesValuesFromNameId'. I don't think this is an issue w/ Piwik, though. If I get the processed report for the subtable report, the imageGraphUrl uses the correct method. And the URL works. At least, it doesn't throw.

I did notice another bug though. The idSubtable parameter isn't set in the imageGraphUrl. I should have a fix committed soon.

I'm not sure how much work it is to add graph support for more Modules/Actions. Is it possible to disable the graphs in Piwik if it is a subtable report? Once graphs for more Modules/Actions are working we can simply enable them step by step without having to change Piwik Mobile.

comment:17 Changed 20 months ago by capedfuzz (diosmosis)

(In [6872]) Refs #2742, add idSubtable parameter to imageGraphUrl if it is present in the url.

comment:18 Changed 20 months ago by tsteur

Oh, the missing "d" was my copy/paste fail. I removed the "token_auth" url parameter before copying the link into this command and accidentally also removed the trailing "d". It's correct in MetaData API but still getting the error when trying to display the graph. I'll try it later again with your latest patch.

comment:19 follow-up: Changed 20 months ago by tsteur

Still getting the graph error even with idSubtable parameter.

The url in metadata looks like the following:

"imageGraphUrl":"index.php?module=API&method=ImageGraph.get&idSite=1&apiModule=CustomVariables&apiAction=getCustomVariablesValuesFromNameId&period=range&date=previous7&idSubtable=3"

Piwik Mobile adds a few parameters and it'll look like the following:

http://apache.piwik/index.php?module=API&method=ImageGraph.get&idSite=1&apiModule=CustomVariables&apiAction=getCustomVariablesValuesFromNameId&period=range&date=previous7&idSubtable=3&filter_sort_column=nb_visits&column=nb_visits&idSite=1&language=en&width=556&height=300&fontSize=18&showMetricTitle=0&aliasedGraph=1

The generated graph still displays the message "Invalid API Module and/or API Action". I'm not getting any error in the log. Maybe one of the additional url parameter causes the "error"? All other graphs are displayed without any issues, except graphs from subtable reports.

comment:20 in reply to: ↑ 19 Changed 20 months ago by capedfuzz (diosmosis)

Ok, I'm seeing the error now, and I think I know what the cause is. Hopefully I'll have a commit in soon.

comment:21 Changed 20 months ago by capedfuzz (diosmosis)

(In [6874]) Refs #2742, add subtable support to ImageGraph plugin.

comment:22 Changed 20 months ago by capedfuzz (diosmosis)

@tsteur should be working now.

comment:23 Changed 20 months ago by tsteur

Works! Thanks

comment:24 Changed 20 months ago by capedfuzz (diosmosis)

(In [6886]) Refs #2742, fix test issue.

Note: See TracTickets for help on using tickets.