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

Metadata API: Support sub tables calls #2742

Closed
mattab opened this issue Oct 25, 2011 · 24 comments
Closed

Metadata API: Support sub tables calls #2742

mattab opened this issue Oct 25, 2011 · 24 comments
Assignees
Labels
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

@mattab
Copy link
Member

mattab commented Oct 25, 2011

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
Copy link
Member Author

mattab commented Dec 8, 2011

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.

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

@timo-bes
Copy link
Member

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

@timo-bes
Copy link
Member

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

@mattab
Copy link
Member Author

mattab commented Jun 18, 2012

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
Copy link
Member Author

mattab commented Jun 19, 2012

@tsteur do you mind checking the comment #2742

@tsteur
Copy link
Member

tsteur commented Jun 20, 2012

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

@diosmosis
Copy link
Member

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

@diosmosis
Copy link
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
Copy link
Member Author

mattab commented Aug 7, 2012

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
Copy link
Member

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

@diosmosis
Copy link
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
Copy link
Member Author

mattab commented Aug 11, 2012

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
Copy link
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
Copy link
Member Author

mattab commented Aug 18, 2012

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

@tsteur
Copy link
Member

tsteur commented Aug 25, 2012

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
Copy link
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
Copy link
Member

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

@tsteur
Copy link
Member

tsteur commented Aug 25, 2012

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
Copy link
Member

tsteur commented Aug 25, 2012

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
Copy link
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
Copy link
Member

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

@diosmosis
Copy link
Member

@tsteur should be working now.

@tsteur
Copy link
Member

tsteur commented Aug 26, 2012

Works! Thanks

@diosmosis
Copy link
Member

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

@mattab mattab added this to the 1.12.x - Piwik 1.12.x milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

4 participants