fix(glean): add user opt-out setting #70

wtfluckey · 2023-10-24T23:53:03Z

Goal

Add a new privacy setting so that users can opt out of analytics tracking
MOSOWEB-42

To Do:

Add new Settings -> Privacy link & index page (show only in devMode)
Add allowGlean to PreferencesSettings (true by default)
Update Glean initialization to respect the allowGlean user setting
Verify that logging out persists the setting (as long as you don't clear local storage)

Implementation Decisions

I did not do the toggle switch that is designed in the ticket/figma. For the sake of time and consistency, I went with the existing checkbox pattern that Settings -> Preferences uses

Screenshots/Gifs

add channel

wtfluckey · 2023-10-24T23:54:30Z

composables/settings/storage.ts

@@ -43,5 +43,5 @@ export function getPreferences<T extends keyof PreferencesSettings>(userSettings

 export function togglePreferences(key: keyof PreferencesSettings) {
  const flag = usePreferences(key)
-  flag.value = !flag.value
+  return flag.value = !flag.value


Added this explicit return so that I could use it in the toggleOptOut() function later, will call it out

Returning a variable assignment expression is a new one to me. Buut if it works, it works 👍

I'm such a rubyist at heart. In Ruby, every method (function) returns the evaluated result of the last line that is executed. And it was my first modern language, so I often forget that javascript doesn't do what I think it should do sometimes, but really I'm just remembering back to my ruby days.

wtfluckey · 2023-10-24T23:58:01Z

modules/glean/runtime/glean-plugin.client.ts

+    const env = useAppConfig().env
+    const devMode = env === ('dev' || 'canary' || 'preview')
+    const userSettings = useUserSettings()
+    const allowGlean = getPreferences(userSettings.value, 'allowGlean')
+    const uploadEnabled = devMode && allowGlean

-    Glean.initialize(GLEAN_APP_ID, uploadEnabled, {})
+    Glean.initialize(GLEAN_APP_ID, uploadEnabled, { channel: env })


Updated uploadEnabled to check for allowGlean user setting

Pass through env as the channel when initializing Glean - this is just a little bit more of identifying information (see appChannel in the glean initializing docs)

wtfluckey · 2023-10-24T23:58:36Z

pages/settings/privacy/index.vue

+  // reset identifiers if user opts back in
+  if (user && allowGlean) {
+    mastodonAccountHandle.set(user.account.acct)
+    mastodonAccountId.set(user.account.id)
+    userAgent.set(navigator.userAgent)
+  }


Needed to add this because these identifiers were not getting reset when the user would opt out and then opt back in again.

wtfluckey · 2023-10-24T23:59:44Z

pages/settings/privacy/index.vue

+const userSettings = useUserSettings()
+
+function toggleOptOut() {
+  const allowGlean = togglePreferences('allowGlean')


This is where I'm using that explicit return from togglePreferences. For some reason calling getPreferences after doing the toggle wasn't returning the correct value and I didn't have the time or mental space to explore why

Just wondering, did you try calling getPreferences before doing the toggle?

yes! and it frustratingly returns the same value both before the toggle and after the toggle.

wtfluckey · 2023-10-25T00:00:24Z

telemetry/generated/web.ts

 import StringMetricType from "@mozilla/glean/private/metrics/string";
+import EventMetricType from "@mozilla/glean/private/metrics/event";


I swear these two generated files keep flipping back and forth even though they are supposed to be being ignored by eslint. A problem for another day...

jpezninjo

I checked out the branch and tried to break it but could not 😞 . Nice work!

anthony-liddle · 2023-10-25T17:34:07Z

modules/glean/runtime/glean-plugin.client.ts

+    const devMode = env === ('dev' || 'canary' || 'preview')
+    const userSettings = useUserSettings()
+    const allowGlean = getPreferences(userSettings.value, 'allowGlean')
+    const uploadEnabled = devMode && allowGlean


Should this be an OR instead of AND?

Right now, no - keeping the uploadEnabled for dev only until we are ready to start collecting events in production

anthony-liddle · 2023-10-25T17:44:13Z

pages/settings/privacy/index.vue

+  <MainContent back-on-small-screen no-beta-label>
+    <template #title>
+      <div text-lg font-bold flex items-center gap-2 @click="$scrollToTop">
+        <span>{{ $t('settings.interface.label') }}</span>


This translation string should settings.privacy.label (Right now it renders as Interface)

Good catch! fixed

anthony-liddle

I'm noticing a weird bug where you can't navigate to this page directly. I'm not sure if it should block this from releasing since it'll be hidden behind a flag, but it's strange.

To reproduce:

Navigate to /settings/privacy
Refresh page
~~Profit~~ Error

UPDATE
Determined the cause. Looks like there's a server error due to calling the window object

ReferenceError: window is not defined

So we may want to use the onMounted callback for this

wtfluckey · 2023-10-25T18:27:52Z

@anthony-liddle re: the refresh bug - I've done some investigating and it's the Glean.setUploadEnabled(allowGlean) line that's causing this issue. I looked into onMounted, but couldn't get it working quickly and through more digging, I found that this error is bubbling up from Glean itself. So now I'm not sure if this is something we are causing or if this is a bug in glean. But it's line 187 in the screenshot below that's causing the issue.

wtfluckey requested a review from a team as a code owner October 24, 2023 23:53

wtfluckey added 5 commits October 24, 2023 19:53

fix(glean): regen files

f65d511

fix(definitions): add allowGlean to PreferencesSettings

c8681c1

fix(locale): localize

af02f86

fix(settings): add new privacy settings link & page

0652ddb

fix(glean): use allowGlean setting for uploadEnabled

2b848b8

add channel

wtfluckey force-pushed the fix/user-setting-glean branch from 9c8b594 to 2b848b8 Compare October 24, 2023 23:53

wtfluckey commented Oct 24, 2023

View reviewed changes

wtfluckey commented Oct 25, 2023

View reviewed changes

jpezninjo approved these changes Oct 25, 2023

View reviewed changes

fix(settings): don't show privacy link unless dev mode

692f38e

anthony-liddle reviewed Oct 25, 2023

View reviewed changes

fix(privacy index): use correct localization string

db908d2

anthony-liddle approved these changes Oct 25, 2023

View reviewed changes

Merge branch 'main' into fix/user-setting-glean

a4ba26a

wtfluckey merged commit 0e9a393 into main Oct 25, 2023
3 checks passed

wtfluckey deleted the fix/user-setting-glean branch October 25, 2023 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(glean): add user opt-out setting #70

fix(glean): add user opt-out setting #70

wtfluckey commented Oct 24, 2023 •

edited

wtfluckey Oct 24, 2023

jpezninjo Oct 25, 2023

wtfluckey Oct 25, 2023

wtfluckey Oct 24, 2023

wtfluckey Oct 24, 2023

wtfluckey Oct 24, 2023

jpezninjo Oct 25, 2023

wtfluckey Oct 25, 2023

wtfluckey Oct 25, 2023

jpezninjo left a comment

anthony-liddle Oct 25, 2023

wtfluckey Oct 25, 2023

anthony-liddle Oct 25, 2023

wtfluckey Oct 25, 2023

anthony-liddle left a comment •

edited

wtfluckey commented Oct 25, 2023

		import StringMetricType from "@mozilla/glean/private/metrics/string";
		import EventMetricType from "@mozilla/glean/private/metrics/event";

fix(glean): add user opt-out setting #70

fix(glean): add user opt-out setting #70

Conversation

wtfluckey commented Oct 24, 2023 • edited

Goal

To Do:

Implementation Decisions

Screenshots/Gifs

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpezninjo left a 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

anthony-liddle left a comment • edited

Choose a reason for hiding this comment

wtfluckey commented Oct 25, 2023

wtfluckey commented Oct 24, 2023 •

edited

anthony-liddle left a comment •

edited