Implements "Social network" as a new referrer type #12993

sgiehl · 2018-05-26T13:03:29Z

tsteur · 2018-05-27T02:01:36Z

FYI: We'll need to check and adjust some plugins as well (not part of this PR) like Users Flow, Attribution, Funnels, ...

I presume people will see afterwards a drop in websites referrer types? I wonder if we need to mention in a blog post and explain they are now attributed differently?

sgiehl · 2018-05-27T19:14:01Z

I presume people will see afterwards a drop in websites referrer types?

I'm wondering if it maybe would make sense to automatically add an annotation to referrer type report on the day of update to point that out?

tsteur · 2018-05-27T20:25:08Z

Could do it maybe for a week after the update or so?

sgiehl · 2018-06-19T20:37:27Z

I've updated the code according to the review comments.

For dates before this goes into effect, I see that 'Social Networks' isn't added to the "Referrer Types" report:

That's not easily doable without reprocessing the reports, as the old reports for referrer type did not include that metric. Instead the social networks were included in the websites metric.

diosmosis · 2018-06-20T01:49:04Z

That's not easily doable without reprocessing the reports, as the old reports for referrer type did not include that metric. Instead the social networks were included in the websites metric.

I thought you could do the same thing for when there is no social data in Referrers.getSocials in Referrers.getReferrerType. Ie, if $idSubtable == REFERRER_TYPE_SOCIAL and there is no data returned by the getSocials method, then build the datatable from the getWebsites() result. I guess you would also have to remove social URLs from the getWebsites() result if $idSubtable == REFERRER_TYPE_WEBSITE. I guess this isn't doable? Of course, I haven't thought it through to the point that I know it'll work.

sgiehl · 2018-07-09T10:07:42Z

I thought you could do the same thing for when there is no social data in Referrers.getSocials in Referrers.getReferrerType. Ie, if $idSubtable == REFERRER_TYPE_SOCIAL and there is no data returned by the getSocials method, then build the datatable from the getWebsites() result. I guess you would also have to remove social URLs from the getWebsites() result if $idSubtable == REFERRER_TYPE_WEBSITE. I guess this isn't doable? Of course, I haven't thought it through to the point that I know it'll work.

Sorry for the delay. The referrer type numbers shown in that table are stored in an extra report. Splitting up the number for websites and socials for older data is not easy. We would need to query the reports for websites and split it up to websites and socials and then calculate all the numbers based on those resulting reports. I think that might be getting very slow when e.g. viewing row evolution. Also those referrer type reports are calculated as goal reports. I guess recalculating those would only be possible by reprocessing the logs.
Also the resulting numbers might differ if e.g. a website has more referrers than the report limit allows. While the calculated type report would show the correct number, the websites reports would only contain the limited dataset...

diosmosis

Ok, thought it might be too difficult to be worth it.

Will leave final review/merge to @mattab / @tsteur

tsteur · 2018-07-09T21:51:09Z

plugins/Referrers/API.php

+
+    private function completeSocialTablesWithOldReports($dataTable, $idSite, $period, $date, $segment, $expanded, $flat)
+    {
+        return $this->combineDataTables($dataTable, function() use ($idSite, $period, $date, $segment, $expanded, $flat) {


Maybe we could just add a simple comment to state that we may want to remove this in a year or more likely two years as it then not really won't be needed anymore? With a link to this issue...

tsteur · 2018-07-09T21:55:09Z

plugins/Referrers/API.php

+    /**
+     * @param DataTable $dataTable
+     */
+    protected function filterWebsitesForSocials($dataTable, $idSite, $period, $date, $segment, $expanded, $flat)


can we make those methods private or are there tests for it?

tsteur · 2018-07-09T21:56:25Z

plugins/Referrers/API.php

+     */
+    protected function filterWebsitesForSocials($dataTable, $idSite, $period, $date, $segment, $expanded, $flat)
+    {
+        $dataTable->filter('ColumnCallbackDeleteRow', array('label', function ($url) {


Might be good to check performance at some stage with a profiler on real data ideally just to see... especially re the groupby and mergeSubtables but also everything else like those filters.

tsteur · 2018-07-09T22:10:10Z

plugins/Referrers/API.php

+            $isMap = true;
+            $dataTables = $dataTable->getDataTables();
+        } else {
+            $dataTables = [$dataTable];


I wonder if the code could be simplified there eg just if (!$dataTable->getRowCount()) return $callbackForAdditionalData();? Bit hard to understand currently with if's in top and bottom

tsteur · 2018-07-09T22:14:13Z

Left a few minor comments... we will then definitely need to check which plugins need updating. I think eg funnels, multi attribution ...

sgiehl · 2018-07-16T11:08:28Z

@tsteur I've improved the code according to your suggestions. I've not yet done a profiling and don't know when I will find some time to do that properly.
But actually the filters aren't new. Those filters were used before to generate the social networks reports on the fly as well...

tsteur · 2018-07-16T20:19:19Z

Should be all fine then @sgiehl if they were all there before. If there are new ones, be good to have a look just to be sure.

* Implements new referrer type for social networks * removes footer message from social report * Show social network referrers in Transitions * update / improve tests * Improves API code * use transient cache instead of static properties * remove debugging statement * updates changelog * cs * Update expected test files. * Update expected screenshots

sgiehl added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label May 26, 2018

sgiehl added this to the 3.6.0 milestone May 26, 2018

sgiehl self-assigned this May 26, 2018

sgiehl force-pushed the socialreports branch from 454148c to c3a54b6 Compare May 26, 2018 16:55

sgiehl force-pushed the socialreports branch 3 times, most recently from 21696ee to 69aeb7b Compare May 31, 2018 20:25

sgiehl force-pushed the socialreports branch 19 times, most recently from 75cca51 to aa3c60a Compare June 17, 2018 18:28

sgiehl added the Needs Review PRs that need a code review label Jun 17, 2018

diosmosis approved these changes Jul 9, 2018

View reviewed changes

tsteur reviewed Jul 9, 2018

View reviewed changes

sgiehl force-pushed the socialreports branch from 2b6a571 to 4a7cab7 Compare July 16, 2018 10:56

sgiehl force-pushed the socialreports branch from 4a7cab7 to 0f211bd Compare July 17, 2018 19:35

sgiehl added 9 commits July 22, 2018 22:28

Implements new referrer type for social networks

71e8ee3

removes footer message from social report

525de15

Show social network referrers in Transitions

914803c

update / improve tests

2fe1626

Improves API code

4fa28f3

use transient cache instead of static properties

a9db238

remove debugging statement

ddfa36b

updates changelog

607a0cd

cs

9cac594

sgiehl force-pushed the socialreports branch from 0f211bd to 9cac594 Compare July 22, 2018 20:34

diosmosis added 3 commits July 22, 2018 19:39

Merge branch '3.x-dev' into socialreports

5045f15

Update expected test files.

f632c7f

Update expected screenshots

2d5843d

diosmosis merged commit 2b33801 into 3.x-dev Jul 23, 2018

diosmosis deleted the socialreports branch July 23, 2018 21:23

mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Aug 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements "Social network" as a new referrer type #12993

Implements "Social network" as a new referrer type #12993

sgiehl commented May 26, 2018 •

edited

tsteur commented May 27, 2018

sgiehl commented May 27, 2018

tsteur commented May 27, 2018

sgiehl commented Jun 19, 2018

diosmosis commented Jun 20, 2018

sgiehl commented Jul 9, 2018

diosmosis left a comment

tsteur Jul 9, 2018

tsteur Jul 9, 2018

tsteur Jul 9, 2018 •

edited

tsteur Jul 9, 2018

tsteur commented Jul 9, 2018

sgiehl commented Jul 16, 2018

tsteur commented Jul 16, 2018

Implements "Social network" as a new referrer type #12993

Implements "Social network" as a new referrer type #12993

Conversation

sgiehl commented May 26, 2018 • edited

tsteur commented May 27, 2018

sgiehl commented May 27, 2018

tsteur commented May 27, 2018

sgiehl commented Jun 19, 2018

diosmosis commented Jun 20, 2018

sgiehl commented Jul 9, 2018

diosmosis left a comment

Choose a reason for hiding this comment

tsteur Jul 9, 2018

Choose a reason for hiding this comment

tsteur Jul 9, 2018

Choose a reason for hiding this comment

tsteur Jul 9, 2018 • edited

Choose a reason for hiding this comment

tsteur Jul 9, 2018

Choose a reason for hiding this comment

tsteur commented Jul 9, 2018

sgiehl commented Jul 16, 2018

tsteur commented Jul 16, 2018

sgiehl commented May 26, 2018 •

edited

tsteur Jul 9, 2018 •

edited