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

Jump to bottom

Site Search report does not display tooltips with metrics documentation #8257

Closed
mattab opened this issue Jul 1, 2015 · 2 comments
Closed

Site Search report does not display tooltips with metrics documentation #8257

mattab opened this issue Jul 1, 2015 · 2 comments
Assignees
@barbushin
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Usability For issues that let users achieve a defined goal more effectively or efficiently.
Milestone
2.14.1

Comments

@mattab
Copy link
Member

mattab commented Jul 1, 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)
@mattab mattab added Bug For errors / faults / flaws / inconsistencies etc. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. labels Jul 1, 2015
@mattab mattab added this to the 2.14.1 milestone Jul 1, 2015
@barbushin barbushin self-assigned this Jul 15, 2015
@barbushin
Copy link
Contributor

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

mattab commented Jul 16, 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 👍

@barbushin barbushin mentioned this issue Jul 16, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@barbushin barbushin

Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Usability For issues that let users achieve a defined goal more effectively or efficiently.
Projects
None yet
Milestone
2.14.1
Development

No branches or pull requests
2 participants
@mattab @barbushin