@mattab opened this issue on July 1st 2015

In Actions > Site Search report, when hovering on the column names: - Got: no tooltip - Expected: a tooltip with the hovered column documentation

The metrics documentation are already available in the metadata API, see for example this test file: https://github.com/piwik/piwik/blob/master/tests/PHPUnit/System/expected/test_SiteSearch_Actions.getSiteSearchCategories_firstSite_lastN__API.getProcessedReport_month.xml#L16-21

Therefore the goal of this issue is to display these docs on hover on the column (as it already does with other metricsDocumentation)

@barbushin commented on July 16th 2015

I spent whole day debugging this issue, and I can't understand next things: 1. Why SiteSearchBase::configureReportMetaData() does not depends on request siteId? https://github.com/piwik/piwik/blob/b2f5b5489ceca056217fc6663ec6c2c6b052cd42/plugins/Actions/Reports/SiteSearchBase.php#L42 That's why SiteSearchBase::configureReportMetaData() always returns null, and that's why documentation is not displayed. And if we replace if (!$this->isEnabledForIdSites($idSites, 0)) { with if (!$this->isEnabledForIdSites($idSites, Common::getRequestVar('idSite', 0, 'int'))) - it will work, we will get correct metaData for SiteSearch report. 2. If reports meta data sometime depends on siteId, then why siteId is not passed in https://github.com/piwik/piwik/blob/master/core/ViewDataTable/Config.php#L489?

I was trying to understand logics of reports configs and meta data initialization, but unsuccessfully. I see many cross-dependencies, and I don't know what is the best way to fix it.

Can somebody please consult me about it?

@mattab commented on July 16th 2015

Hi @barbushin

it will work, we will get correct metaData for SiteSearch report.

it sounds like you found the bug / it is correct fix

If reports meta data sometime depends on siteId, then why siteId is not passed in

it's probably a bug :) @barbushin please suggest the Pull request and we will review it, but likely what you think is correct solution, will be correct solution :+1:

This issue was closed on July 16th 2015
Powered by GitHub Issue Mirror