@mattab opened this Issue on October 25th 2011 Member

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
@diosmosis commented on August 6th 2012 Member

Attachment: Initial patch for this issue.
2742.diff.tar.gz

@diosmosis commented on August 9th 2012 Member

Attachment: New patch for this issue.
2742.diff.tar.2.gz

@mattab commented on December 8th 2011 Member

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)

@BeezyT commented on January 27th 2012 Member

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

@BeezyT commented on January 27th 2012 Member

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

@mattab commented on June 18th 2012 Member

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 :-)

@mattab commented on June 19th 2012 Member

@tsteur do you mind checking the comment #2742

@tsteur commented on June 20th 2012 Member

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

@diosmosis commented on August 6th 2012 Member

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.)

@mattab commented on August 7th 2012 Member

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?
@diosmosis commented on August 9th 2012 Member

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?

@mattab commented on August 11th 2012 Member

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 :)
@diosmosis commented on August 17th 2012 Member

(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.
@mattab commented on August 18th 2012 Member

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

@tsteur commented on August 25th 2012 Member

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.

@diosmosis commented on August 25th 2012 Member

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.

@diosmosis commented on August 25th 2012 Member

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

@tsteur commented on August 25th 2012 Member

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.

@tsteur commented on August 25th 2012 Member

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.

@diosmosis commented on August 25th 2012 Member

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

@diosmosis commented on August 26th 2012 Member

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

@diosmosis commented on August 26th 2012 Member

@tsteur should be working now.

@tsteur commented on August 26th 2012 Member

Works! Thanks

@diosmosis commented on August 30th 2012 Member

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

This Issue was closed on August 30th 2012
Powered by GitHub Issue Mirror