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
Group different writings of Instagram in reports #14436
Conversation
plugins/Referrers/API.php
Outdated
@@ -365,6 +365,13 @@ public function getSocials($idSite, $period, $date, $segment = false, $expanded | |||
|
|||
$dataTable = Archive::createDataTableFromArchive(Archiver::SOCIAL_NETWORKS_RECORD_NAME, $idSite, $period, $date, $segment, $expanded, $flat); | |||
|
|||
$dataTable->filter('GroupBy', ['label', function($value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do this only if one or the other label exists? Typically the GroupBy filter is quite slow, and checking if a label exists in the report is really fast usually.
Not too important but could be worth thinking about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the branch
plugins/Referrers/API.php
Outdated
@@ -365,6 +365,35 @@ public function getSocials($idSite, $period, $date, $segment = false, $expanded | |||
|
|||
$dataTable = Archive::createDataTableFromArchive(Archiver::SOCIAL_NETWORKS_RECORD_NAME, $idSite, $period, $date, $segment, $expanded, $flat); | |||
|
|||
$groupIntagramLabel = function($dataTable) use (&$groupIntagramLabel) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgiehl might be better to create a filter for this? would remove some complexity and be better to read etc. like $dataTable->filter('Piwik\Plugins\Referrers\DataTable\Filter\Group')
then you don't need to check if it's a map etc and in theory it would be even possible to write a unit test for the group filter but wouldn't really be needed.
fixes #14433