From 4a6fa98ff71b07c98e0654fd0a0927edcb11ef2c Mon Sep 17 00:00:00 2001 From: Jonathan Foote Date: Fri, 29 Apr 2016 10:13:17 -0400 Subject: [PATCH 1/2] Adding addUserProperties() to Event object --- src/Amplitude.php | 10 +++++----- src/Event.php | 11 +++++++++++ test/AmplitudeTest.php | 21 +++++++++++++++++++++ test/EventTest.php | 25 +++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/Amplitude.php b/src/Amplitude.php index 3f47adf..aef9f8a 100644 --- a/src/Amplitude.php +++ b/src/Amplitude.php @@ -252,9 +252,7 @@ public function logEvent($eventType = '', array $eventProperties = []) $event->deviceId = $this->deviceId; } if (!empty($this->userProperties)) { - $props = !empty($event->userProperties) ? $event->userProperties : []; - $props = array_merge($props, $this->userProperties); - $event->userProperties = $props; + $event->addUserProperties($this->userProperties); $this->resetUserProperties(); } @@ -348,9 +346,11 @@ public function setDeviceId($deviceId) } /** - * Set the user properties, will be sent with the next event sent to Amplitude + * Add user properties, will be sent with the next event sent to Amplitude * - * If no events are logged, it will not get sent to Amplitude + * If user properties are added to the event directly, these will be merged onto the event's user properties. + * + * If no events are logged after this point, it will not get sent to Amplitude * * @param array $userProperties */ diff --git a/src/Event.php b/src/Event.php index 4c53f6c..38cb177 100644 --- a/src/Event.php +++ b/src/Event.php @@ -100,6 +100,17 @@ public function __construct(array $data = []) } } + /** + * Add user properties + * + * @param array $userProperties + */ + public function addUserProperties(array $userProperties) + { + $props = !empty($this->userProperties) ? $this->userProperties : []; + $this->userProperties = array_merge($props, $userProperties); + return $this; + } /** * Set a value in the event. diff --git a/test/AmplitudeTest.php b/test/AmplitudeTest.php index 1c64385..8eefd3a 100644 --- a/test/AmplitudeTest.php +++ b/test/AmplitudeTest.php @@ -314,4 +314,25 @@ public function testOptOut() ->queueEvent('Another Queued Event'); $this->assertTrue($amplitude->getOptOut()); } + + public function testAddUserProperties() + { + $userProps = ['dob' => 'tomorrow', 'gender' => 'f']; + $amplitude = new Amplitude(); + $amplitude->addUserProperties($userProps); + $this->assertSame($userProps, $amplitude->getUserProperties()); + + $userProps2 = ['dob' => 'yesterday', 'name' => 'Baby']; + $expected = [ + 'dob' => 'yesterday', + 'gender' => 'f', + 'name' => 'Baby', + ]; + $amplitude->addUserProperties($userProps2); + $this->assertSame( + $expected, + $amplitude->getUserProperties(), + 'Second call to addUserProperties should overwrite properties with same-name but keep everything else intact' + ); + } } diff --git a/test/EventTest.php b/test/EventTest.php index 1967100..f130e0a 100644 --- a/test/EventTest.php +++ b/test/EventTest.php @@ -201,4 +201,29 @@ public function testUnsetProperty() unset($event->userId); $this->assertEmpty($event->userId, 'Should unset built-in properties with magic unset'); } + + public function testAddUserProperties() + { + $userProps = ['dob' => 'tomorrow', 'gender' => 'f']; + $event = new Event(); + $event->addUserProperties($userProps); + $this->assertSame( + ['user_properties' => $userProps], + $event->toArray(), + 'Should set user properties in user_properties' + ); + + $userProps2 = ['dob' => 'yesterday', 'name' => 'Baby']; + $expected = [ + 'dob' => 'yesterday', + 'gender' => 'f', + 'name' => 'Baby', + ]; + $event->addUserProperties($userProps2); + $this->assertSame( + ['user_properties' => $expected], + $event->toArray(), + 'Second call to addUserProperties should overwrite properties with same-name but keep everything else intact' + ); + } } From 287d51a5cca7e4e213b43b4520c96ec715c926f2 Mon Sep 17 00:00:00 2001 From: Jonathan Foote Date: Fri, 29 Apr 2016 10:30:51 -0400 Subject: [PATCH 2/2] Changing behavior of addUserProperties to not overwrite existing properties --- src/Amplitude.php | 10 +++++++--- src/Event.php | 7 ++++++- test/AmplitudeTest.php | 4 ++-- test/EventTest.php | 4 ++-- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/Amplitude.php b/src/Amplitude.php index aef9f8a..e34776d 100644 --- a/src/Amplitude.php +++ b/src/Amplitude.php @@ -348,15 +348,19 @@ public function setDeviceId($deviceId) /** * Add user properties, will be sent with the next event sent to Amplitude * - * If user properties are added to the event directly, these will be merged onto the event's user properties. + * If user properties are added to the event directly, these will be added on top, so the properties set directly + * on the Event object would take precedence. * - * If no events are logged after this point, it will not get sent to Amplitude + * If this is called multiple times before an event is sent, later calls will only add to the array, it will not + * overwrite values already set if no events have been sent yet. + * + * Note that if no events are logged after this point, it will not get sent to Amplitude * * @param array $userProperties */ public function addUserProperties(array $userProperties) { - $this->userProperties = array_merge($this->userProperties, $userProperties); + $this->userProperties = $this->userProperties + $userProperties; return $this; } diff --git a/src/Event.php b/src/Event.php index 38cb177..9c2a827 100644 --- a/src/Event.php +++ b/src/Event.php @@ -103,12 +103,17 @@ public function __construct(array $data = []) /** * Add user properties * + * If called multiple times, the first time a user property is set will take precedence. + * + * If need to overwrite a property already set, you can manipulate $event->userProperties array directly + * * @param array $userProperties + * @return \Zumba\Amplitude\Event */ public function addUserProperties(array $userProperties) { $props = !empty($this->userProperties) ? $this->userProperties : []; - $this->userProperties = array_merge($props, $userProperties); + $this->userProperties = $props + $userProperties; return $this; } diff --git a/test/AmplitudeTest.php b/test/AmplitudeTest.php index 8eefd3a..281a42e 100644 --- a/test/AmplitudeTest.php +++ b/test/AmplitudeTest.php @@ -324,7 +324,7 @@ public function testAddUserProperties() $userProps2 = ['dob' => 'yesterday', 'name' => 'Baby']; $expected = [ - 'dob' => 'yesterday', + 'dob' => 'tomorrow', 'gender' => 'f', 'name' => 'Baby', ]; @@ -332,7 +332,7 @@ public function testAddUserProperties() $this->assertSame( $expected, $amplitude->getUserProperties(), - 'Second call to addUserProperties should overwrite properties with same-name but keep everything else intact' + 'Second call to addUserProperties should only add new properties, not overwrite existing' ); } } diff --git a/test/EventTest.php b/test/EventTest.php index f130e0a..3df7e82 100644 --- a/test/EventTest.php +++ b/test/EventTest.php @@ -215,7 +215,7 @@ public function testAddUserProperties() $userProps2 = ['dob' => 'yesterday', 'name' => 'Baby']; $expected = [ - 'dob' => 'yesterday', + 'dob' => 'tomorrow', 'gender' => 'f', 'name' => 'Baby', ]; @@ -223,7 +223,7 @@ public function testAddUserProperties() $this->assertSame( ['user_properties' => $expected], $event->toArray(), - 'Second call to addUserProperties should overwrite properties with same-name but keep everything else intact' + 'Second call to addUserProperties should only add new properties, not overwrite existing' ); } }