Skip to content

fix: do not send consent removal event #1768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 26, 2021
Merged

Conversation

olizilla
Copy link
Member

@olizilla olizilla commented Apr 26, 2021

  • Saying no to analytics explicitly calls Countly.opt_out. Without this removing all consents triggers a "consent removed" event to be tracked which is not what we want.
  • Removing all individual consents explicitly calls Countly.opt_out
  • Granting consent for 1 or more analytic types opts back in to analytics by calling Countly.opt_in.
  • Saying yes to analytics explicitly calls Countly.opt_in, for completeness.

ref: #1041 (comment)

License: MIT
Signed-off-by: Oli Evans [email protected]

@olizilla
Copy link
Member Author

I'm updating the test to verify this change

@olizilla olizilla force-pushed the dont-send-consent-removal-event branch from 045322b to 318b563 Compare April 26, 2021 09:56
@olizilla
Copy link
Member Author

I need to figure out if this plays nicely with ipfs-desktop

@olizilla olizilla force-pushed the dont-send-consent-removal-event branch from 318b563 to 8fdd80a Compare April 26, 2021 10:04
@olizilla olizilla temporarily deployed to Deploy April 26, 2021 10:08 Inactive
- Saying no to analytics explicitly calls `Countly.opt_out`. Without this removing all consents triggers a "consent removed" event to be tracked which is not what we want.
- Removing all individual consents explicitly calls `Countly.opt_out`
- Granting consent for 1 or more analytic types opts back in to analytics by calling `Countly.opt_in`.
- Saying yes to analytics explicitly calls `Countly.opt_in`, for completeness.

ref: #1041 (comment)

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@olizilla olizilla force-pushed the dont-send-consent-removal-event branch from 8fdd80a to fca81e6 Compare April 26, 2021 10:19
@olizilla olizilla temporarily deployed to Deploy April 26, 2021 10:24 Inactive
@olizilla olizilla requested a review from lidel April 26, 2021 13:18
@olizilla olizilla changed the title fix: do no send consent removal event fix: do not send consent removal event Apr 26, 2021
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM, confirmed the countly is no longer pinged. Thank you @olizilla ❤️

If you want to test against Desktop do it from PR branch of ipfs/ipfs-desktop#1807 to ensure the latest electron is used.

Anyway, merging, if there is additional work for Desktop, can be a new PR.

@lidel lidel merged commit 6c3e672 into main Apr 26, 2021
@lidel lidel deleted the dont-send-consent-removal-event branch April 26, 2021 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants