Skip to content

Loading…

Metadata API: Support sub tables calls #2742

Closed
mattab opened this Issue · 24 comments

4 participants

@mattab
Piwik Open Source Analytics 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
@mattab
Piwik Open Source Analytics 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
Piwik Open Source Analytics member

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

@BeezyT
Piwik Open Source Analytics member

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

@mattab
Piwik Open Source Analytics 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
Piwik Open Source Analytics member

@tsteur do you mind checking the comment #2742

@tsteur
Piwik Open Source Analytics member

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

@diosmosis
Piwik Open Source Analytics member

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

@diosmosis
Piwik Open Source Analytics 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
Piwik Open Source Analytics 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
Piwik Open Source Analytics member

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

@diosmosis
Piwik Open Source Analytics 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
Piwik Open Source Analytics 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
Piwik Open Source Analytics 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
Piwik Open Source Analytics member

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

@tsteur
Piwik Open Source Analytics 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
Piwik Open Source Analytics 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
Piwik Open Source Analytics member

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

@tsteur
Piwik Open Source Analytics 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
Piwik Open Source Analytics 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
Piwik Open Source Analytics 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
Piwik Open Source Analytics member

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

@diosmosis
Piwik Open Source Analytics member

@tsteur should be working now.

@tsteur
Piwik Open Source Analytics member

Works! Thanks

@diosmosis
Piwik Open Source Analytics member

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

@mattab mattab added this to the 1.12.x - Piwik 1.12.x milestone
@diosmosis diosmosis was assigned by mattab
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.