Skip to content
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

fix: webui update metrics to opt-out by default (part of 2074) #2084

Merged
merged 13 commits into from
Jan 27, 2023

Conversation

0xDanomite
Copy link
Contributor

@0xDanomite 0xDanomite commented Jan 18, 2023

fixes: ipfs/ipfs-desktop#2370
fixes: #2085

  • Updates banner per discussion (here) and enables metrics on app load

@0xDanomite 0xDanomite requested a review from a team as a code owner January 18, 2023 22:31
@0xDanomite 0xDanomite temporarily deployed to Deploy January 18, 2023 22:36 — with GitHub Actions Inactive
@0xDanomite 0xDanomite temporarily deployed to Deploy January 18, 2023 23:08 — with GitHub Actions Inactive
@@ -14,7 +14,7 @@
},
"customApiConfig": "Custom JSON configuration",
"AskToEnable": {
"label": "Help improve this app by sending anonymous usage data."
"label": "We have recently updated our telemetry policy to switch to opt-out metrics. You previously had telemetry collection disabled. You can disable telemetry collection (again) on the settings page.",
Copy link
Member

Choose a reason for hiding this comment

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

@juliaxbow for visibility
cc @tinytb @lidel

src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
src/status/StatusPage.js Outdated Show resolved Hide resolved
src/status/StatusPage.js Outdated Show resolved Hide resolved
@0xDanomite 0xDanomite temporarily deployed to Deploy January 19, 2023 03:13 — with GitHub Actions Inactive
@0xDanomite 0xDanomite temporarily deployed to Deploy January 19, 2023 03:37 — with GitHub Actions Inactive
@0xDanomite 0xDanomite temporarily deployed to Deploy January 19, 2023 17:13 — with GitHub Actions Inactive
@0xDanomite 0xDanomite force-pushed the fix/webui-metrics-opt-out/2074 branch from 129afdd to 9d0b85c Compare January 19, 2023 18:40
@0xDanomite 0xDanomite temporarily deployed to Deploy January 19, 2023 18:44 — with GitHub Actions Inactive
Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

some naming confusion, and targeting of specific usecases with the tests.

src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Show resolved Hide resolved
Comment on lines 323 to 325
// Don't track clicks or links as it can include full url.
// Countly.q.push(['track_clicks'])
// Countly.q.push(['track_links'])
Copy link
Member

Choose a reason for hiding this comment

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

just a note that we can filter out URLs that have CIDs (for any future readers)

src/bundles/analytics.test.js Outdated Show resolved Hide resolved
src/bundles/analytics.test.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
@0xDanomite 0xDanomite temporarily deployed to Deploy January 20, 2023 08:53 — with GitHub Actions Inactive
@0xDanomite 0xDanomite temporarily deployed to Deploy January 20, 2023 20:08 — with GitHub Actions Inactive
src/bundles/analytics.js Outdated Show resolved Hide resolved
src/bundles/analytics.js Show resolved Hide resolved
src/bundles/analytics.js Show resolved Hide resolved
src/bundles/analytics.js Show resolved Hide resolved
src/bundles/analytics.test.js Show resolved Hide resolved
src/bundles/analytics.test.js Show resolved Hide resolved
src/bundles/analytics.test.js Show resolved Hide resolved
src/bundles/analytics.test.js Outdated Show resolved Hide resolved
src/bundles/analytics.test.js Outdated Show resolved Hide resolved
@0xDanomite 0xDanomite temporarily deployed to Deploy January 21, 2023 00:58 — with GitHub Actions Inactive
src/bundles/analytics.js Show resolved Hide resolved
src/bundles/analytics.js Outdated Show resolved Hide resolved
@0xDanomite 0xDanomite temporarily deployed to Deploy January 21, 2023 01:18 — with GitHub Actions Inactive
@0xDanomite 0xDanomite temporarily deployed to Deploy January 21, 2023 01:32 — with GitHub Actions Inactive
Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

I'm good to merge this if we get approval from Nishant or Lidel as well. A new webui release should wait on new privacy policy though.

Copy link
Contributor

@whizzzkid whizzzkid left a comment

Choose a reason for hiding this comment

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

A simple question, lgtm otherwise.

@SgtPooki we should host PRs on fleek, like public-gateway-checker does. We already host this on https://webui.ipfs.io/ this way we can test PR specific changes too.

Edit: #2078 I think this issue describes something similar.

Comment on lines 409 to +410
const lastDisabledAt = (consent.length === 0) ? Date.now() : state.lastDisabledAt
return { ...state, lastDisabledAt, consent }
const didOptOutCompletely = consent.length === 0
Copy link
Contributor

Choose a reason for hiding this comment

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

you can also do:

const didOptOutCompletely = consent.length === 0
const lastDisabledAt = didOptOutCompletely ? Date.now() : state.lastDisabledAt 

This clears up the intentions, but another question that arises is this value will change every time if consent is not granted, do we want that?

@SgtPooki
Copy link
Member

@SgtPooki we should host PRs on fleek, like public-gateway-checker does. We already host this on https://webui.ipfs.io/ this way we can test PR specific changes too.

yes we should

@SgtPooki SgtPooki merged commit cac7663 into main Jan 27, 2023
@SgtPooki SgtPooki deleted the fix/webui-metrics-opt-out/2074 branch January 27, 2023 00:13
ipfs-gui-bot pushed a commit that referenced this pull request Jan 27, 2023
## [2.22.0](v2.21.0...v2.22.0) (2023-01-27)

 CID `bafybeifeqt7mvxaniphyu2i3qhovjaf3sayooxbh5enfdqtiehxjv2ldte`

 ---

### Features

* different countly keys for kubo and webui.ipfs.io deployments ([#2081](#2081)) ([2e766fa](2e766fa))

### Bug Fixes

* import status window UX ([#2075](#2075)) ([4104cc6](4104cc6))
* webui update metrics to opt-out by default (part of 2074) ([#2084](#2084)) ([cac7663](cac7663))

### Trivial Changes

* pull new translations ([#2076](#2076)) ([ad9e4ff](ad9e4ff))
@ipfs-gui-bot
Copy link
Collaborator

🎉 This PR is included in version 2.22.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to opt-out metrics Switch to opt-out metrics
4 participants