Skip to content

Privacy: use the wpcom API to centralize opt-out#9155

Closed
mattwiebe wants to merge 1 commit intomasterfrom
update/centralize-user-privacy
Closed

Privacy: use the wpcom API to centralize opt-out#9155
mattwiebe wants to merge 1 commit intomasterfrom
update/centralize-user-privacy

Conversation

@mattwiebe
Copy link
Copy Markdown
Contributor

@mattwiebe mattwiebe commented Mar 27, 2018

Update the UI to use the WPCom user tracking opt-out API.

Depends on #9142 and #9154 to be merged first to work.

Changes proposed in this Pull Request:

  • Centralize user tracking opt out with WPCom user setting

Testing instructions:

Toggle the setting on or off as first enabled in #8993 from the user account that connected to Jetpack and verify that it's the same while logged into WP.com as detailed in Automattic/wp-calypso#23231 - you will currently need to build Calypso locally to view the Privacy section. Setting in one place will be reflected in the other.

Update the UI to use the WPCom user tracking opt-out API.

Depends on #9142 and #9154
@mattwiebe mattwiebe added the [Status] Needs Review This PR is ready for review. label Mar 27, 2018
@mattwiebe mattwiebe added this to the GDPR Compliant milestone Mar 27, 2018
@mattwiebe mattwiebe requested a review from a team as a code owner March 27, 2018 04:20
Copy link
Copy Markdown
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, thank you, but I have left a couple of comments.

isSavingAnyOption,
} = this.props;
const isActive = ! this.props.tracking.tracks_opt_out;
const doToggle = () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick, but is there any reason to keep this function as a constant inside the render method? We could have left it a method just as togglePrivacy was before, and just passed this.doToggle to onChange.

state => ( {
settings: getSettings( state ),
tracking: getTrackingSettings( state ),
isUpdating: isUpdatingTrackingSettings( state ),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need isUpdating here? I can't find a place where we're using it.

@mattwiebe
Copy link
Copy Markdown
Contributor Author

Going to address @zinigor's feedback in #9156, which supersedes this

@mattwiebe mattwiebe closed this Mar 27, 2018
@kraftbj kraftbj deleted the update/centralize-user-privacy branch May 24, 2019 17:31
@kraftbj kraftbj removed the [Status] Needs Review This PR is ready for review. label Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants