From e7a994e09c949aadcf66e700dcca858038e1f4ea Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 4 Jun 2018 15:04:40 +0100 Subject: [PATCH 01/15] use mixpanel.people.set --- assets/js/lib/analytics.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/assets/js/lib/analytics.js b/assets/js/lib/analytics.js index aa879303ab..47e1185f37 100644 --- a/assets/js/lib/analytics.js +++ b/assets/js/lib/analytics.js @@ -2,14 +2,17 @@ 0)))}}var d=a;"undefined"!==typeof f?d=a[f]=[]:f="mixpanel";d.people=d.people||[];d.toString=function(b){var a="mixpanel";"mixpanel"!==f&&(a+="."+f);b||(a+=" (stub)");return a};d.people.toString=function(){return d.toString(1)+".people (stub)"};k="disable time_event track track_pageview track_links track_forms register register_once alias unregister identify name_tag set_config reset people.set people.set_once people.increment people.append people.union people.track_charge people.clear_charges people.delete_user".split(" "); for(h=0;h Date: Mon, 11 Jun 2018 13:28:32 +0100 Subject: [PATCH 02/15] analytics : use a unique public id --- assets/js/lib/analytics.js | 15 ++++++++++++-- lib/API/JSON/v1/Services.php | 9 +++++++++ lib/Analytics/Analytics.php | 32 ++++++++++++++++++++++++++++++ lib/Twig/Analytics.php | 10 ++++++++++ tests/unit/Services/BridgeTest.php | 4 +++- views/layout.html | 2 ++ 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/assets/js/lib/analytics.js b/assets/js/lib/analytics.js index 47e1185f37..c90d84419e 100644 --- a/assets/js/lib/analytics.js +++ b/assets/js/lib/analytics.js @@ -7,11 +7,22 @@ window.mixpanelTrackingId = "172e1ec7e7e6300e41defee3548dcf42"; if (mailpoet_analytics_enabled) { - mixpanel.init(window.mixpanelTrackingId); + mixpanel.init(window.mixpanelTrackingId, { + loaded: function(mixpanel) { + // used in lib/Analytics/Analytics.php + document.cookie = "mixpanel_distinct_id=" + mixpanel.get_distinct_id(); + } + }); + mixpanel.register({'Platform': 'Plugin'}); + if(window.mailpoet_analytics_new_public_id === true) { + mixpanel.alias(window.mailpoet_analytics_public_id); + } else { + mixpanel.identify(window.mailpoet_analytics_public_id); + } + if (mailpoet_analytics_data != null) { - //TODO mixpanel.identify(userId); mixpanel.people.set(mailpoet_analytics_data); } diff --git a/lib/API/JSON/v1/Services.php b/lib/API/JSON/v1/Services.php index fa80efcf29..610ad40ae0 100644 --- a/lib/API/JSON/v1/Services.php +++ b/lib/API/JSON/v1/Services.php @@ -6,6 +6,7 @@ use MailPoet\API\JSON\Endpoint as APIEndpoint; use MailPoet\API\JSON\Error as APIError; use MailPoet\Config\AccessControl; use MailPoet\Config\Installer; +use MailPoet\Models\Setting; use MailPoet\Services\Bridge; use MailPoet\WP\DateTime; @@ -53,6 +54,10 @@ class Services extends APIEndpoint { ); } + if(!empty($result['data']['public_id'])) { + Analytics::setPublicId($result['data']['public_id']); + } + if($success_message) { return $this->successResponse(array('message' => $success_message)); } @@ -106,6 +111,10 @@ class Services extends APIEndpoint { ); } + if(!empty($result['data']['public_id'])) { + Analytics::setPublicId($result['data']['public_id']); + } + if($success_message) { return $this->successResponse( array('message' => $success_message), diff --git a/lib/Analytics/Analytics.php b/lib/Analytics/Analytics.php index 60b4e2cea9..f4162f0886 100644 --- a/lib/Analytics/Analytics.php +++ b/lib/Analytics/Analytics.php @@ -34,6 +34,38 @@ class Analytics { return !empty($analytics_settings['enabled']) === true; } + static function setPublicId($new_public_id) { + $current_public_id = Setting::getValue('public_id'); + if($current_public_id !== $new_public_id) { + Setting::setValue('public_id', $new_public_id); + Setting::setValue('new_public_id', 'true'); + } + } + + /** @return string */ + function getPublicId() { + $public_id = Setting::getValue('public_id'); + // if we didn't get the user public_id from the shop yet : we create one based on mixpanel distinct_id + if(empty($public_id) && !empty($_COOKIE['mixpanel_distinct_id'])) { + // the public id has to be diffent that mixpanel_distinct_id in order to be used on different browser + $mixpanel_distinct_id = md5($_COOKIE['mixpanel_distinct_id']); + Setting::setValue('public_id', $mixpanel_distinct_id); + Setting::setValue('new_public_id', 'true'); + return $mixpanel_distinct_id; + } + return $public_id; + } + + /** @return boolean */ + function isPublicIdNew() { + $new_public_id = Setting::getValue('new_public_id'); + if($new_public_id === 'true') { + Setting::setValue('new_public_id', 'false'); + return true; + } + return false; + } + private function shouldSend() { if(!$this->isEnabled()) { return false; diff --git a/lib/Twig/Analytics.php b/lib/Twig/Analytics.php index 5e5694ee0b..e8e2270842 100644 --- a/lib/Twig/Analytics.php +++ b/lib/Twig/Analytics.php @@ -21,6 +21,16 @@ class Analytics extends \Twig_Extension { array($analytics, 'isEnabled'), array('is_safe' => array('all')) ), + new \Twig_SimpleFunction( + 'get_analytics_public_id', + array($analytics, 'getPublicId'), + array('is_safe' => array('all')) + ), + new \Twig_SimpleFunction( + 'is_analytics_public_id_new', + array($analytics, 'isPublicIdNew'), + array('is_safe' => array('all')) + ) ); } } diff --git a/tests/unit/Services/BridgeTest.php b/tests/unit/Services/BridgeTest.php index ec24f6705b..075cc31eb0 100644 --- a/tests/unit/Services/BridgeTest.php +++ b/tests/unit/Services/BridgeTest.php @@ -70,6 +70,7 @@ class BridgeTest extends \MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); + expect($result['data']['public_id'])->notEmpty(); } function testItChecksAlreadyUsed() { @@ -147,6 +148,7 @@ class BridgeTest extends \MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); + expect($result['data']['public_id'])->notEmpty(); } function testItReturnsErrorStateOnEmptyAPIResponseCodeDuringPremiumCheck() { @@ -314,4 +316,4 @@ class BridgeTest extends \MailPoetTest { WPHelper::releaseAllFunctions(); \ORM::raw_execute('TRUNCATE ' . Setting::$_table); } -} \ No newline at end of file +} diff --git a/views/layout.html b/views/layout.html index 4a71fc7833..ad2c2269f3 100644 --- a/views/layout.html +++ b/views/layout.html @@ -48,6 +48,8 @@ jQuery('.toplevel_page_mailpoet-newsletters.menu-top-last') var mailpoet_premium_version = <%= json_encode(mailpoet_premium_version()) %>; var mailpoet_analytics_enabled = <%= is_analytics_enabled() | json_encode %>; var mailpoet_analytics_data = <%= json_encode(get_analytics_data()) %>; + var mailpoet_analytics_public_id = <%= json_encode(get_analytics_public_id()) %>; + var mailpoet_analytics_new_public_id = <%= is_analytics_public_id_new() | json_encode %>; // RFC 5322 standard; http://emailregex.com/ combined with https://google.github.io/closure-library/api/goog.format.EmailAddress.html#isValid var mailpoet_email_regex = /(?=^[+a-zA-Z0-9_.!#$%&'*\/=?^`{|}~-]+@([a-zA-Z0-9-]+\.)+[a-zA-Z0-9]{2,63}$)(?=^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,})))/; From 709182589336f9d636a905ed19b666c41073e2c0 Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 11 Jun 2018 13:33:13 +0100 Subject: [PATCH 03/15] production mixpanel key --- assets/js/lib/analytics.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/assets/js/lib/analytics.js b/assets/js/lib/analytics.js index c90d84419e..3bd50fb5d6 100644 --- a/assets/js/lib/analytics.js +++ b/assets/js/lib/analytics.js @@ -2,8 +2,7 @@ 0)))}}var d=a;"undefined"!==typeof f?d=a[f]=[]:f="mixpanel";d.people=d.people||[];d.toString=function(b){var a="mixpanel";"mixpanel"!==f&&(a+="."+f);b||(a+=" (stub)");return a};d.people.toString=function(){return d.toString(1)+".people (stub)"};k="disable time_event track track_pageview track_links track_forms register register_once alias unregister identify name_tag set_config reset people.set people.set_once people.increment people.append people.union people.track_charge people.clear_charges people.delete_user".split(" "); for(h=0;h Date: Mon, 11 Jun 2018 18:10:56 +0100 Subject: [PATCH 04/15] rm non implemented tests --- tests/unit/Services/BridgeTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/Services/BridgeTest.php b/tests/unit/Services/BridgeTest.php index 075cc31eb0..10f21a8632 100644 --- a/tests/unit/Services/BridgeTest.php +++ b/tests/unit/Services/BridgeTest.php @@ -70,7 +70,7 @@ class BridgeTest extends \MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); - expect($result['data']['public_id'])->notEmpty(); + // expect($result['data']['public_id'])->notEmpty(); TO IMPLEMENT } function testItChecksAlreadyUsed() { @@ -148,7 +148,7 @@ class BridgeTest extends \MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); - expect($result['data']['public_id'])->notEmpty(); + // expect($result['data']['public_id'])->notEmpty(); TO IMPLEMENT } function testItReturnsErrorStateOnEmptyAPIResponseCodeDuringPremiumCheck() { From f2baf1c9966095b2248c0e540a532195bd5edc7d Mon Sep 17 00:00:00 2001 From: qfrery Date: Thu, 21 Jun 2018 17:43:18 +0200 Subject: [PATCH 05/15] rm commented tests --- tests/unit/Services/BridgeTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/Services/BridgeTest.php b/tests/unit/Services/BridgeTest.php index 10f21a8632..68a4f20b05 100644 --- a/tests/unit/Services/BridgeTest.php +++ b/tests/unit/Services/BridgeTest.php @@ -70,7 +70,6 @@ class BridgeTest extends \MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); - // expect($result['data']['public_id'])->notEmpty(); TO IMPLEMENT } function testItChecksAlreadyUsed() { @@ -148,7 +147,6 @@ class BridgeTest extends \MailPoetTest { expect($result)->notEmpty(); expect($result['state'])->equals(Bridge::KEY_EXPIRING); expect($result['data']['expire_at'])->notEmpty(); - // expect($result['data']['public_id'])->notEmpty(); TO IMPLEMENT } function testItReturnsErrorStateOnEmptyAPIResponseCodeDuringPremiumCheck() { From 6505871eac5a8179a0b540ac9e82261c2aceeacb Mon Sep 17 00:00:00 2001 From: qfrery Date: Fri, 13 Jul 2018 16:07:56 +0100 Subject: [PATCH 06/15] add comment to isPublicIdNew function --- lib/Analytics/Analytics.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Analytics/Analytics.php b/lib/Analytics/Analytics.php index f4162f0886..81b79ae15e 100644 --- a/lib/Analytics/Analytics.php +++ b/lib/Analytics/Analytics.php @@ -56,7 +56,10 @@ class Analytics { return $public_id; } - /** @return boolean */ + /** + * Returns true if a the public_id was added and update new_public_id to false + * @return boolean + */ function isPublicIdNew() { $new_public_id = Setting::getValue('new_public_id'); if($new_public_id === 'true') { From 411fe307d123082ec6866c3eb3dd4b6977d7f2ef Mon Sep 17 00:00:00 2001 From: qfrery Date: Sun, 15 Jul 2018 22:19:51 +0100 Subject: [PATCH 07/15] add tests --- tests/unit/API/JSON/v1/ServicesTest.php | 29 +++++++++++++++++++++++++ tests/unit/Analytics/AnalyticsTest.php | 20 ++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/unit/API/JSON/v1/ServicesTest.php b/tests/unit/API/JSON/v1/ServicesTest.php index c96e63f0ed..93bcce45ac 100644 --- a/tests/unit/API/JSON/v1/ServicesTest.php +++ b/tests/unit/API/JSON/v1/ServicesTest.php @@ -6,6 +6,7 @@ use Codeception\Stub\Expected; use MailPoet\API\JSON\v1\Services; use MailPoet\API\JSON\Response as APIResponse; use MailPoet\Config\Installer; +use MailPoet\Models\Setting; use MailPoet\Services\Bridge; class ServicesTest extends \MailPoetTest { @@ -263,4 +264,32 @@ class ServicesTest extends \MailPoetTest { expect($response->status)->equals(APIResponse::STATUS_NOT_FOUND); expect($response->errors[0]['message'])->equals('test'); } + + function testItRespondsContainsPublicIdForMSS() { + $this->services_endpoint->bridge = Stub::make( + new Bridge(), + array( + 'checkMSSKey' => array('state' => Bridge::KEY_VALID), + 'storeMSSKeyAndState' => Expected::once() + ), + $this + ); + $response = $this->services_endpoint->checkMSSKey($this->data); + expect($response->data['public_id'])->notEmpty(); + expect($response->data['public_id'])->equals(Setting::getValue('public_id');); + } + + function testItRespondsContainsPublicIdForPremium() { + $this->services_endpoint->bridge = Stub::make( + new Bridge(), + array( + 'checkPremiumKey' => array('state' => Bridge::KEY_VALID), + 'storeMSSKeyAndState' => Expected::once() + ), + $this + ); + $response = $this->services_endpoint->checkMSSKey($this->data); + expect($response->data['public_id'])->notEmpty(); + expect($response->data['public_id'])->equals(Setting::getValue('public_id');); + } } diff --git a/tests/unit/Analytics/AnalyticsTest.php b/tests/unit/Analytics/AnalyticsTest.php index 35b0ea8e05..03e366e1d2 100644 --- a/tests/unit/Analytics/AnalyticsTest.php +++ b/tests/unit/Analytics/AnalyticsTest.php @@ -102,4 +102,22 @@ class AnalyticsTest extends \MailPoetTest { expect($analytics->generateAnalytics())->equals($data); } -} \ No newline at end of file + function testSetPublicId() { + $analytics = new Analytics(new Reporter()); + $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; + + Setting::setValue('new_public_id', 'true'); + $analytics->setPublicId($fakePublicId); + expect(Setting::getValue('public_id'))->equals($fakePublicId); + // When we update public_id it's marked as new + expect(Setting::getValue('new_public_id'))->equals('true'); + expect(Setting::isPublicIdNew())->true(); + + $analytics->setPublicId($fakePublicId); + expect(Setting::getValue('public_id'))->equals($fakePublicId); + // We tried to update public_id with the same value, so it's not marked as new + expect(Setting::getValue('new_public_id'))->equals('false'); + expect(Setting::isPublicIdNew())->false(); + } + +} From e523fa055ec62b2586d39eeef89a18e63399b7d9 Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 16 Jul 2018 10:32:06 +0100 Subject: [PATCH 08/15] fix errors --- tests/unit/API/JSON/v1/ServicesTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/API/JSON/v1/ServicesTest.php b/tests/unit/API/JSON/v1/ServicesTest.php index 93bcce45ac..44d10ac726 100644 --- a/tests/unit/API/JSON/v1/ServicesTest.php +++ b/tests/unit/API/JSON/v1/ServicesTest.php @@ -276,7 +276,7 @@ class ServicesTest extends \MailPoetTest { ); $response = $this->services_endpoint->checkMSSKey($this->data); expect($response->data['public_id'])->notEmpty(); - expect($response->data['public_id'])->equals(Setting::getValue('public_id');); + expect($response->data['public_id'])->equals(Setting::getValue('public_id')); } function testItRespondsContainsPublicIdForPremium() { @@ -290,6 +290,6 @@ class ServicesTest extends \MailPoetTest { ); $response = $this->services_endpoint->checkMSSKey($this->data); expect($response->data['public_id'])->notEmpty(); - expect($response->data['public_id'])->equals(Setting::getValue('public_id');); + expect($response->data['public_id'])->equals(Setting::getValue('public_id')); } } From 8f05d885670e67c7838f0abd048214f4914a0f18 Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 16 Jul 2018 10:41:42 +0100 Subject: [PATCH 09/15] split public id tests in two --- tests/unit/Analytics/AnalyticsTest.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/unit/Analytics/AnalyticsTest.php b/tests/unit/Analytics/AnalyticsTest.php index 03e366e1d2..49c78a0e14 100644 --- a/tests/unit/Analytics/AnalyticsTest.php +++ b/tests/unit/Analytics/AnalyticsTest.php @@ -106,18 +106,26 @@ class AnalyticsTest extends \MailPoetTest { $analytics = new Analytics(new Reporter()); $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; - Setting::setValue('new_public_id', 'true'); $analytics->setPublicId($fakePublicId); expect(Setting::getValue('public_id'))->equals($fakePublicId); + } + + function testIsPublicIdNew() { + $analytics = new Analytics(new Reporter()); + $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; + + Setting::setValue('new_public_id', 'false'); + $analytics->setPublicId($fakePublicId); // When we update public_id it's marked as new expect(Setting::getValue('new_public_id'))->equals('true'); expect(Setting::isPublicIdNew())->true(); + expect(Setting::getValue('new_public_id'))->equals('false'); $analytics->setPublicId($fakePublicId); - expect(Setting::getValue('public_id'))->equals($fakePublicId); // We tried to update public_id with the same value, so it's not marked as new expect(Setting::getValue('new_public_id'))->equals('false'); expect(Setting::isPublicIdNew())->false(); + expect(Setting::getValue('new_public_id'))->equals('false'); } } From 2f83a74b95aec513b11ee9a63eb258cf8691056c Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 16 Jul 2018 10:44:13 +0100 Subject: [PATCH 10/15] fix errors --- tests/unit/Analytics/AnalyticsTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/Analytics/AnalyticsTest.php b/tests/unit/Analytics/AnalyticsTest.php index 49c78a0e14..10aba20dd9 100644 --- a/tests/unit/Analytics/AnalyticsTest.php +++ b/tests/unit/Analytics/AnalyticsTest.php @@ -107,25 +107,25 @@ class AnalyticsTest extends \MailPoetTest { $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; $analytics->setPublicId($fakePublicId); - expect(Setting::getValue('public_id'))->equals($fakePublicId); + expect($analytics->getValue('public_id'))->equals($fakePublicId); } function testIsPublicIdNew() { $analytics = new Analytics(new Reporter()); $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; - Setting::setValue('new_public_id', 'false'); + $analytics->setValue('new_public_id', 'false'); $analytics->setPublicId($fakePublicId); // When we update public_id it's marked as new - expect(Setting::getValue('new_public_id'))->equals('true'); - expect(Setting::isPublicIdNew())->true(); - expect(Setting::getValue('new_public_id'))->equals('false'); + expect($analytics->getValue('new_public_id'))->equals('true'); + expect($analytics->isPublicIdNew())->true(); + expect($analytics->getValue('new_public_id'))->equals('false'); $analytics->setPublicId($fakePublicId); // We tried to update public_id with the same value, so it's not marked as new - expect(Setting::getValue('new_public_id'))->equals('false'); - expect(Setting::isPublicIdNew())->false(); - expect(Setting::getValue('new_public_id'))->equals('false'); + expect($analytics->getValue('new_public_id'))->equals('false'); + expect($analytics->isPublicIdNew())->false(); + expect($analytics->getValue('new_public_id'))->equals('false'); } } From 184f38f18629854dc29e480802e22a319dc8e9a6 Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 16 Jul 2018 11:36:54 +0100 Subject: [PATCH 11/15] fix tests --- tests/unit/API/JSON/v1/ServicesTest.php | 7 +++---- tests/unit/Analytics/AnalyticsTest.php | 13 +++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/API/JSON/v1/ServicesTest.php b/tests/unit/API/JSON/v1/ServicesTest.php index 44d10ac726..bcd8b9e8c3 100644 --- a/tests/unit/API/JSON/v1/ServicesTest.php +++ b/tests/unit/API/JSON/v1/ServicesTest.php @@ -6,7 +6,6 @@ use Codeception\Stub\Expected; use MailPoet\API\JSON\v1\Services; use MailPoet\API\JSON\Response as APIResponse; use MailPoet\Config\Installer; -use MailPoet\Models\Setting; use MailPoet\Services\Bridge; class ServicesTest extends \MailPoetTest { @@ -275,8 +274,8 @@ class ServicesTest extends \MailPoetTest { $this ); $response = $this->services_endpoint->checkMSSKey($this->data); + expect(isset($response->data['public_id']))->true(); expect($response->data['public_id'])->notEmpty(); - expect($response->data['public_id'])->equals(Setting::getValue('public_id')); } function testItRespondsContainsPublicIdForPremium() { @@ -288,8 +287,8 @@ class ServicesTest extends \MailPoetTest { ), $this ); - $response = $this->services_endpoint->checkMSSKey($this->data); + $response = $this->services_endpoint->checkPremiumKey($this->data); + expect(isset($response->data['public_id']))->true(); expect($response->data['public_id'])->notEmpty(); - expect($response->data['public_id'])->equals(Setting::getValue('public_id')); } } diff --git a/tests/unit/Analytics/AnalyticsTest.php b/tests/unit/Analytics/AnalyticsTest.php index 10aba20dd9..8b5570c5c1 100644 --- a/tests/unit/Analytics/AnalyticsTest.php +++ b/tests/unit/Analytics/AnalyticsTest.php @@ -107,25 +107,26 @@ class AnalyticsTest extends \MailPoetTest { $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; $analytics->setPublicId($fakePublicId); - expect($analytics->getValue('public_id'))->equals($fakePublicId); + expect(Setting::getValue('public_id'))->equals($fakePublicId); } function testIsPublicIdNew() { $analytics = new Analytics(new Reporter()); $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; - $analytics->setValue('new_public_id', 'false'); + Setting::setValue('public_id', 'old-fake-public-id'); + Setting::setValue('new_public_id', 'false'); $analytics->setPublicId($fakePublicId); // When we update public_id it's marked as new - expect($analytics->getValue('new_public_id'))->equals('true'); + expect(Setting::getValue('new_public_id'))->equals('true'); expect($analytics->isPublicIdNew())->true(); - expect($analytics->getValue('new_public_id'))->equals('false'); + expect(Setting::getValue('new_public_id'))->equals('false'); $analytics->setPublicId($fakePublicId); // We tried to update public_id with the same value, so it's not marked as new - expect($analytics->getValue('new_public_id'))->equals('false'); + expect(Setting::getValue('new_public_id'))->equals('false'); expect($analytics->isPublicIdNew())->false(); - expect($analytics->getValue('new_public_id'))->equals('false'); + expect(Setting::getValue('new_public_id'))->equals('false'); } } From c368b664a189474b4344ad4665e58cbc287d7eda Mon Sep 17 00:00:00 2001 From: qfrery Date: Mon, 16 Jul 2018 16:58:42 +0100 Subject: [PATCH 12/15] force user data to be resent on public_id change --- lib/Analytics/Analytics.php | 2 ++ tests/unit/Analytics/AnalyticsTest.php | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/lib/Analytics/Analytics.php b/lib/Analytics/Analytics.php index 81b79ae15e..d190197022 100644 --- a/lib/Analytics/Analytics.php +++ b/lib/Analytics/Analytics.php @@ -39,6 +39,8 @@ class Analytics { if($current_public_id !== $new_public_id) { Setting::setValue('public_id', $new_public_id); Setting::setValue('new_public_id', 'true'); + // Force user data to be resent + Setting::deleteValue(Analytics::SETTINGS_LAST_SENT_KEY); } } diff --git a/tests/unit/Analytics/AnalyticsTest.php b/tests/unit/Analytics/AnalyticsTest.php index 8b5570c5c1..cf2aea49ca 100644 --- a/tests/unit/Analytics/AnalyticsTest.php +++ b/tests/unit/Analytics/AnalyticsTest.php @@ -106,8 +106,14 @@ class AnalyticsTest extends \MailPoetTest { $analytics = new Analytics(new Reporter()); $fakePublicId = 'alk-ded-egrg-zaz-fvf-rtr-zdef'; + Setting::setValue('public_id', 'old-fake-public-id'); + Setting::setValue(Analytics::SETTINGS_LAST_SENT_KEY, Carbon::now()); + $analytics->setPublicId($fakePublicId); + expect(Setting::getValue('public_id'))->equals($fakePublicId); + expect(Setting::getValue('new_public_id'))->equals('true'); + expect(Setting::getValue(Analytics::SETTINGS_LAST_SENT_KEY, null))->null(); } function testIsPublicIdNew() { @@ -116,6 +122,7 @@ class AnalyticsTest extends \MailPoetTest { Setting::setValue('public_id', 'old-fake-public-id'); Setting::setValue('new_public_id', 'false'); + $analytics->setPublicId($fakePublicId); // When we update public_id it's marked as new expect(Setting::getValue('new_public_id'))->equals('true'); From 42f91f506926b8762bb09ab0ff416120d6fd9a03 Mon Sep 17 00:00:00 2001 From: qfrery Date: Tue, 17 Jul 2018 13:03:50 +0100 Subject: [PATCH 13/15] fix failing bridge tests --- lib/API/JSON/v1/Services.php | 1 + tests/unit/API/JSON/v1/ServicesTest.php | 69 ++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/lib/API/JSON/v1/Services.php b/lib/API/JSON/v1/Services.php index 32f6ba1cc1..b4ba166fa3 100644 --- a/lib/API/JSON/v1/Services.php +++ b/lib/API/JSON/v1/Services.php @@ -2,6 +2,7 @@ namespace MailPoet\API\JSON\v1; +use MailPoet\Analytics\Analytics; use MailPoet\API\JSON\Endpoint as APIEndpoint; use MailPoet\API\JSON\Error as APIError; use MailPoet\Config\AccessControl; diff --git a/tests/unit/API/JSON/v1/ServicesTest.php b/tests/unit/API/JSON/v1/ServicesTest.php index bcd8b9e8c3..f6a37fd188 100644 --- a/tests/unit/API/JSON/v1/ServicesTest.php +++ b/tests/unit/API/JSON/v1/ServicesTest.php @@ -6,6 +6,7 @@ use Codeception\Stub\Expected; use MailPoet\API\JSON\v1\Services; use MailPoet\API\JSON\Response as APIResponse; use MailPoet\Config\Installer; +use MailPoet\Models\Setting; use MailPoet\Services\Bridge; class ServicesTest extends \MailPoetTest { @@ -264,31 +265,83 @@ class ServicesTest extends \MailPoetTest { expect($response->errors[0]['message'])->equals('test'); } - function testItRespondsContainsPublicIdForMSS() { + function testItRespondsWithPublicIdForMSS() { + $fake_public_id = 'a-fake-public_id'; + Setting::deleteValue('public_id'); + Setting::deleteValue('new_public_id'); + $this->services_endpoint->bridge = Stub::make( new Bridge(), array( - 'checkMSSKey' => array('state' => Bridge::KEY_VALID), + 'checkMSSKey' => array( + 'state' => Bridge::KEY_VALID, + 'data' => array( 'public_id' => $fake_public_id ) + ), 'storeMSSKeyAndState' => Expected::once() ), $this ); $response = $this->services_endpoint->checkMSSKey($this->data); - expect(isset($response->data['public_id']))->true(); - expect($response->data['public_id'])->notEmpty(); + + expect(Setting::getValue('public_id'))->equals($fake_public_id); + expect(Setting::getValue('new_public_id'))->equals('true'); } - function testItRespondsContainsPublicIdForPremium() { + function testItRespondsWithoutPublicIdForMSS() { + Setting::deleteValue('public_id'); + Setting::deleteValue('new_public_id'); + $this->services_endpoint->bridge = Stub::make( new Bridge(), array( - 'checkPremiumKey' => array('state' => Bridge::KEY_VALID), + 'checkMSSKey' => array( 'state' => Bridge::KEY_VALID ), 'storeMSSKeyAndState' => Expected::once() ), $this ); + $response = $this->services_endpoint->checkMSSKey($this->data); + + expect(Setting::getValue('public_id', null))->null(); + expect(Setting::getValue('new_public_id', null))->null(); + } + + function testItRespondsWithPublicIdForPremium() { + $fake_public_id = 'another-fake-public_id'; + Setting::deleteValue('public_id'); + Setting::deleteValue('new_public_id'); + + $this->services_endpoint->bridge = Stub::make( + new Bridge(), + array( + 'checkPremiumKey' => array( + 'state' => Bridge::KEY_VALID, + 'data' => array( 'public_id' => $fake_public_id ) + ), + 'storePremiumKeyAndState' => Expected::once() + ), + $this + ); $response = $this->services_endpoint->checkPremiumKey($this->data); - expect(isset($response->data['public_id']))->true(); - expect($response->data['public_id'])->notEmpty(); + + expect(Setting::getValue('public_id'))->equals($fake_public_id); + expect(Setting::getValue('new_public_id'))->equals('true'); + } + + function testItRespondsWithoutPublicIdForPremium() { + Setting::deleteValue('public_id'); + Setting::deleteValue('new_public_id'); + + $this->services_endpoint->bridge = Stub::make( + new Bridge(), + array( + 'checkPremiumKey' => array('state' => Bridge::KEY_VALID), + 'storePremiumKeyAndState' => Expected::once() + ), + $this + ); + $response = $this->services_endpoint->checkPremiumKey($this->data); + + expect(Setting::getValue('public_id', null))->null(); + expect(Setting::getValue('new_public_id', null))->null(); } } From 5818db1a239909bd0bfd28813cb63c63370156cd Mon Sep 17 00:00:00 2001 From: qfrery Date: Tue, 17 Jul 2018 16:04:38 +0100 Subject: [PATCH 14/15] don't identify mixpanel users if public_id is null --- assets/js/lib/analytics.js | 17 ++++++++++------- lib/Analytics/Analytics.php | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/assets/js/lib/analytics.js b/assets/js/lib/analytics.js index 3bd50fb5d6..c41375da22 100644 --- a/assets/js/lib/analytics.js +++ b/assets/js/lib/analytics.js @@ -15,14 +15,17 @@ if (mailpoet_analytics_enabled) { mixpanel.register({'Platform': 'Plugin'}); - if(window.mailpoet_analytics_new_public_id === true) { - mixpanel.alias(window.mailpoet_analytics_public_id); - } else { - mixpanel.identify(window.mailpoet_analytics_public_id); - } + console.log(window.mailpoet_analytics_public_id); + if(typeof window.mailpoet_analytics_public_id === 'string' && window.mailpoet_analytics_public_id.length > 0) { + if(window.mailpoet_analytics_new_public_id === true) { + mixpanel.alias(window.mailpoet_analytics_public_id); + } else { + mixpanel.identify(window.mailpoet_analytics_public_id); + } - if (mailpoet_analytics_data != null) { - mixpanel.people.set(mailpoet_analytics_data); + if (mailpoet_analytics_data != null) { + mixpanel.people.set(mailpoet_analytics_data); + } } } diff --git a/lib/Analytics/Analytics.php b/lib/Analytics/Analytics.php index d190197022..10d7f42b9f 100644 --- a/lib/Analytics/Analytics.php +++ b/lib/Analytics/Analytics.php @@ -46,7 +46,7 @@ class Analytics { /** @return string */ function getPublicId() { - $public_id = Setting::getValue('public_id'); + $public_id = Setting::getValue('public_id', ''); // if we didn't get the user public_id from the shop yet : we create one based on mixpanel distinct_id if(empty($public_id) && !empty($_COOKIE['mixpanel_distinct_id'])) { // the public id has to be diffent that mixpanel_distinct_id in order to be used on different browser From 9dc0e61603a56331520d79d488170eaf7af15b01 Mon Sep 17 00:00:00 2001 From: qfrery Date: Tue, 17 Jul 2018 17:18:16 +0100 Subject: [PATCH 15/15] remove console.log and sent user data even if no public_id is set --- assets/js/lib/analytics.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/assets/js/lib/analytics.js b/assets/js/lib/analytics.js index c41375da22..8b6fa8fd75 100644 --- a/assets/js/lib/analytics.js +++ b/assets/js/lib/analytics.js @@ -15,17 +15,16 @@ if (mailpoet_analytics_enabled) { mixpanel.register({'Platform': 'Plugin'}); - console.log(window.mailpoet_analytics_public_id); if(typeof window.mailpoet_analytics_public_id === 'string' && window.mailpoet_analytics_public_id.length > 0) { if(window.mailpoet_analytics_new_public_id === true) { mixpanel.alias(window.mailpoet_analytics_public_id); } else { mixpanel.identify(window.mailpoet_analytics_public_id); } + } - if (mailpoet_analytics_data != null) { - mixpanel.people.set(mailpoet_analytics_data); - } + if (mailpoet_analytics_data != null) { + mixpanel.people.set(mailpoet_analytics_data); } }