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

Display a footer message below Custom Variables report which explains why some metrics are not set #9643

Merged
merged 2 commits into from Jan 27, 2016

Conversation

mattab
Copy link
Member

@mattab mattab commented Jan 27, 2016

Goal: minimise user confusion and support requests.
The message is only displayed when the report table contains Custom Variable of scope 'page'.
If the report only contains Custom Variable of scope 'visit' the message won't be shown.

Fixes #8128

… are not set

 Goal: minimise user confusion and support requests.
 The message is only displayed when the report table contains Custom Variable of scope 'page'.
 If the report only contains Custom Variable of scope 'visit' the message won't be shown.
Fixes  #8128
@mattab mattab added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jan 27, 2016
@mattab mattab added this to the 2.16.0 milestone Jan 27, 2016
Piwik::translate('General_And'),
Piwik::translate('General_ColumnNbUsers')
);
$messageStart = Piwik::translate('Note: %s metrics are available for Custom Variables of scope \'visit\' only.', $metrics);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't a translation key be specified?

@mattab
Copy link
Member Author

mattab commented Jan 27, 2016

Thanks, feedback is applied @tsteur

mattab pushed a commit that referenced this pull request Jan 27, 2016
Display a footer message below Custom Variables report which explains why some metrics  are not set
@mattab mattab merged commit f03c048 into master Jan 27, 2016
@mattab mattab deleted the 8128 branch January 27, 2016 04:59
@@ -34,5 +35,40 @@ public function configureView(ViewDataTable $view)
$view->config->addTranslation('label', Piwik::translate('CustomVariables_ColumnCustomVariableName'));
$view->requestConfig->filter_sort_column = 'nb_actions';
$view->requestConfig->filter_sort_order = 'desc';

$view->config->filters[] = function (DataTable $table) use ($view) {
if($this->isReportContainsUnsetVisitsColumns($table)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails on PHP 5.3 see eg http://builds-artifacts.piwik.org/piwik/piwik/9649/17844/PivotByDimension_pivoted_columns_report . Instead we need to use use ($view, $self)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed it was already fixed in master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants