Add/user tracking option api #9003

enejb · 2018-03-07T17:39:03Z

Changes proposed in this Pull Request:

Add an interface class to update if a user should be tracked.
Sync the data to .com so that we can stop tracking the users there as well when dey navigate around the site.

Testing instructions:

Do the tests pass?

Proposed changelog entry for your changes:

Changelog entry

Added ability to disable user events tracking.

… the default value. Still need sync tests for this.

eliorivero · 2018-03-09T13:21:17Z

_inc/lib/class.jetpack-user-event-tracking.php

+
+	static private function set( $user_id, $value ) {
+		self::$_cache[ $user_id ] = (bool) $value;
+		update_user_meta( $user_id, self::KEY, $value );


Please update this to return the update_user_meta and also return the self::set in disable and enable.

This was done to be able to return errors in case it doesn't work as expected.

eliorivero · 2018-03-09T13:58:47Z

_inc/lib/class.jetpack-user-event-tracking.php

+			$user_tracking = self::default_value();
+		}
+		self::$_cache[ $user_id ] = (bool) $user_tracking;
+		return (bool) $user_tracking();


Should be $user_tracking.

eliorivero · 2018-03-09T14:05:48Z

_inc/lib/class.jetpack-user-event-tracking.php

+class Jetpack_User_Event_Tracking {
+
+	private static $_cache = array();
+	const KEY = 'jetpack_event_tracking';


This is not PHP 5.2 safe.

or by using the const keyword outside a class definition as of PHP 5.3.0

http://php.net/language.constants.syntax

… safe

eliorivero · 2018-03-09T14:28:31Z

_inc/lib/class.jetpack-user-event-tracking.php

+
+	static private function set( $user_id, $value ) {
+		self::$_cache[ $user_id ] = (bool) $value;
+		return update_user_meta( $user_id, self::$KEY, $value );


Why are you casting this as boolean when you save it to the cache, but not when you update the user meta data?

Because I want to be able to distinguish between a set value (0,1) vs the default value (false)

I see, thanks!

eliorivero · 2018-03-09T14:31:55Z

_inc/lib/class.jetpack-user-event-tracking.php

+		 *
+		 * @since 6.0.0
+		 *
+		 * @param bool Default to false. (user tracking enabled)


Isn't the default true?

Update this line to

* @param bool $enable_tracking Whether to enable user tracking. Defaults to true, which allows user tracking.

eliorivero · 2018-03-09T14:35:38Z

tests/php.multisite.xml

@@ -14,6 +14,7 @@
            <file>php/test_class.jetpack-client-server.php</file>
            <file>php/test_class.jetpack-xmlrpc-server.php</file>
            <file>php/test_class.jetpack-heartbeat.php</file>
+			<file>tests/php/test_class.jetpack-user-event-tracking.php</file>


Check indentation.

…er level

bperson · 2018-03-13T16:27:48Z

👋 not sure how the sync to WP.com works: can it potentially be both ways? ( If someone sets it on WP.com via Calypso, have it be synced in Jetpack? )

For now I've been implementing the Calypso stuff using D9412-code as the backend.

mattwiebe · 2018-03-15T21:02:17Z

_inc/lib/class.jetpack-user-event-tracking.php

+
+class Jetpack_User_Event_Tracking {
+
+	private static $_cache = array();


Drive-by comment since I'm looking at this in concert with tracking on the Calypso side: what's the rationale behind having a local cache for this user meta? The user meta functions already leverage WP's built-in wp_cache_* functions, which also persist in memory for a session like this is doing.

The rational was that is would be faster. But I see your point and would be happy to remove it.

…cache

enejb · 2018-03-15T21:34:12Z

Hi @bperson
We talked about this on our meet up when we were implementing this. The conclusion was that things get pretty complicated quickly and that each the user should be responsible for update the setting on each of the sites.

If someone sets it on WP.com via Calypso, have it be synced in Jetpack?

Right now this wouldn't be the case that things get synced. We do however send this data to .com when the user sets it on the Jetpack side so that we can stop tracking the user's actions on a per site basis.

For example currently we also do not sync the user's display name across the different Jetpack sites.

dereksmart

Looks great!

eliorivero

Great work, merging.

* Add new settings route for /privacy * add basic option for toggling, connected to setting * Add smooth scroll when user clicks Privacy link so they're taken up to where the navigation bar is so they don't miss the content and think nothing happened * Add PropTypes and declare props. Combine selectors imported from the same path. Introduce ModuleSettingsForm to use getOptionValue and isSavingAnyOption so ththe toggle now works. Simplify mapStateToProps and mapDispatchToProps. Simplify other minor code. * Combine sentence to save option with an existing one to avoid duplicating code. * Add link to privacy policy plus placeholder text * Add link to privacy blog * Update code to work with PR #9003 so we save this setting by user * Update privacy texts and links.

This reverts commit c9b754c.

* Revert "Add/user tracking option api (#9003)" This reverts commit c9b754c. * remove obsolete settings value for jetpack_event_tracking

@tiagonoronha

* Changelog 6.0: create base for changelog. * Add #8938 to changelog * Add #8962 to changelog * Add #8974 to changelog * Add #8975 to changelog * Add #8978 to changelog * Add #8867 to changelog * Add #8937 to changelog * Add #8961 to changelog * Add #8855 to changelog * Add #8944 to changelog * Add #8973 to changelog * Add #8977 to changelog * Add #8979 to changelog * Add #8980 to changelog * Add #8982 to changelog * Add #8983 to changelog * Add #8984 to changelog * Add #8986 to changelog * Add #9005 to changelog * Add #9010 to changelog * Add #9012 to changelog * Add #9021 to changelog * Add #9022 to changelog * Add #9056 to changelog * Add #9061 to changelog * Add #9079 to changelog * Add #9080 to changelog * Add #9088 to changelog * Add #9096 to changelog * Add #9097 to changelog * Add #9100 to changelog * Add #9107 to changelog * Add #8969 to changelog * Add #8993 to changelog * Add #9003 to changelog * Add #9031 to changelog * Add #8945 to changelog * Add #9052 to changelog * Add #9058 to changelog * Add #9066 to changelog * Add #9076 to changelog * Add #9053 to changelog * Add #9108 to changelog * Add #9135 to changelog * Add #9148 to changelog * Add #9125 to changelog * Add #9137 to changelog * Added testing instructions for 6.0. * Added IS testing instructions, huge props to @tiagonoronha. * Added #8498 to changelog. * Added #8954 to changelog. * Added #8985 to changelog. * add #9027 * add #9112 to changelog * add #9136 to changelog * add #9102 to changelog * add #9093 to changelog * add #9062 to changelog * add #9172 to changelog

enejb requested review from lezama and eliorivero March 7, 2018 17:39

enejb requested a review from a team as a code owner March 7, 2018 17:39

jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Focus] GDPR labels Mar 7, 2018

jeherve added this to the GDPR Compliant milestone Mar 7, 2018

ghost assigned lezama Mar 9, 2018

Add User Event tracking Class interface

ca84ad0

lezama force-pushed the add/user-tracking-option-api branch from df4c193 to 1dccdce Compare March 9, 2018 00:22

Added syncing of user tracking value as it changes as well as syncing…

7cf4d35

… the default value. Still need sync tests for this.

enejb force-pushed the add/user-tracking-option-api branch from 1dccdce to 7cf4d35 Compare March 9, 2018 13:11

ghost assigned enejb Mar 9, 2018

eliorivero reviewed Mar 9, 2018

View reviewed changes

Add return to enable and disable methods.

d8cbde6

This was done to be able to return errors in case it doesn't work as expected.

eliorivero reviewed Mar 9, 2018

View reviewed changes

Fixes typo

5083991

ghost removed the [Status] In Progress label Mar 9, 2018

eliorivero reviewed Mar 9, 2018

View reviewed changes

Update to use private statis instead of const since it is not PHP 5.2…

8632994

… safe

eliorivero added a commit that referenced this pull request Mar 9, 2018

Update code to work with PR #9003 so we save this setting by user

d0f9f78

eliorivero reviewed Mar 9, 2018

View reviewed changes

dereksmart mentioned this pull request Mar 9, 2018

Add new settings route and page for Privacy settings #8993

Merged

Fix static key and only send the new user data if it is set on the us…

445566c

…er level

enejb force-pushed the add/user-tracking-option-api branch from 55d15a2 to 445566c Compare March 9, 2018 23:24

enejb added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Mar 9, 2018

mattwiebe reviewed Mar 15, 2018

View reviewed changes

enejb added 3 commits March 15, 2018 14:23

Remove private cache since the values are already stored in build in …

54c100d

…cache

Update the multisite tests file

751568c

Fix the tests

8a86e95

jaswrks mentioned this pull request Mar 19, 2018

Implement setOptOut cookie command #9085

Closed

dereksmart added the [Pri] BLOCKER label Mar 23, 2018

dereksmart approved these changes Mar 23, 2018

View reviewed changes

dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Mar 23, 2018

eliorivero approved these changes Mar 23, 2018

View reviewed changes

eliorivero merged commit c9b754c into master Mar 23, 2018

matticbot added the [Status] Needs Changelog label Mar 23, 2018

ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 23, 2018

eliorivero deleted the add/user-tracking-option-api branch March 23, 2018 19:52

eliorivero added a commit that referenced this pull request Mar 23, 2018

Update code to work with PR #9003 so we save this setting by user

1366fea

oskosk modified the milestones: GDPR Compliant, 6.0 Mar 26, 2018

oskosk added [Status] Has Changelog and removed [Status] Needs Changelog labels Mar 26, 2018

oskosk added a commit that referenced this pull request Mar 26, 2018

Add #9003 to changelog

eb4de82

dereksmart added a commit that referenced this pull request Mar 27, 2018

Revert "Add/user tracking option api (#9003)"

9709cc4

This reverts commit c9b754c.

dereksmart mentioned this pull request Mar 27, 2018

Clean up first pass at user tracks API #9164

Merged

zinigor pushed a commit that referenced this pull request Mar 27, 2018

Clean up first pass at user tracks API (#9164)

33b2f8d

* Revert "Add/user tracking option api (#9003)" This reverts commit c9b754c. * remove obsolete settings value for jetpack_event_tracking

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add/user tracking option api #9003

Add/user tracking option api #9003

enejb commented Mar 7, 2018 •

edited by oskosk

Loading

eliorivero Mar 9, 2018

eliorivero Mar 9, 2018

eliorivero Mar 9, 2018

eliorivero Mar 9, 2018

enejb Mar 9, 2018

eliorivero Mar 9, 2018

eliorivero Mar 9, 2018 •

edited

Loading

eliorivero Mar 9, 2018

bperson commented Mar 13, 2018

mattwiebe Mar 15, 2018

enejb Mar 15, 2018

enejb commented Mar 15, 2018

dereksmart left a comment

eliorivero left a comment


		class Jetpack_User_Event_Tracking {

		private static $_cache = array();

Add/user tracking option api #9003

Add/user tracking option api #9003

Conversation

enejb commented Mar 7, 2018 • edited by oskosk Loading

Changes proposed in this Pull Request:

Testing instructions:

Proposed changelog entry for your changes:

Changelog entry

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eliorivero Mar 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bperson commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enejb commented Mar 15, 2018

dereksmart left a comment

Choose a reason for hiding this comment

eliorivero left a comment

Choose a reason for hiding this comment

enejb commented Mar 7, 2018 •

edited by oskosk

Loading

eliorivero Mar 9, 2018 •

edited

Loading