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(glean): add user opt-out setting #70

Merged
merged 8 commits into from Oct 25, 2023
2 changes: 2 additions & 0 deletions composables/settings/definition.ts
Expand Up @@ -8,6 +8,7 @@ export type OldFontSize = 'xs' | 'sm' | 'md' | 'lg' | 'xl'
export type ColorMode = 'light' | 'dark' | 'system'

export interface PreferencesSettings {
allowGlean: boolean
hideAltIndicatorOnPosts: boolean
hideBoostCount: boolean
hideReplyCount: boolean
Expand Down Expand Up @@ -60,6 +61,7 @@ export function getDefaultLanguage(languages: string[]) {
}

export const DEFAULT__PREFERENCES_SETTINGS: PreferencesSettings = {
allowGlean: true,
hideAltIndicatorOnPosts: false,
hideBoostCount: false,
hideReplyCount: false,
Expand Down
2 changes: 1 addition & 1 deletion composables/settings/storage.ts
Expand Up @@ -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
Copy link
Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

}
6 changes: 6 additions & 0 deletions locales/en.json
Expand Up @@ -544,6 +544,12 @@
"zen_mode": "Zen mode",
"zen_mode_description": "Hide asides unless the mouse cursor is over them. Also hide some elements from the timeline."
},
"privacy": {
"data_collection": "Data collection and use",
"label": "Privacy",
"opt_out_description": "Allow technical, interaction, campaign and referral data to be sent to Mozilla. With this data you will have a personalized experience.",
"opt_out_title": "Help improve Mozilla Social"
},
"profile": {
"appearance": {
"bio": "Bio",
Expand Down
9 changes: 6 additions & 3 deletions modules/glean/runtime/glean-plugin.client.ts
Expand Up @@ -11,10 +11,13 @@ export default defineNuxtPlugin((nuxtApp) => {
log.info('Glean: App mounted, start initing glean')

const GLEAN_APP_ID = 'mozilla-social-web'
const devMode = useAppConfig().env === ('dev' || 'canary' || 'preview')
const uploadEnabled = devMode // this will eventually be a setting that the user can toggle
const env = useAppConfig().env
const devMode = env === ('dev' || 'canary' || 'preview')
const userSettings = useUserSettings()
const allowGlean = getPreferences(userSettings.value, 'allowGlean')
const uploadEnabled = devMode && allowGlean

Choose a reason for hiding this comment

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

Should this be an OR instead of AND?

Copy link
Author

Choose a reason for hiding this comment

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

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


Glean.initialize(GLEAN_APP_ID, uploadEnabled, {})
Glean.initialize(GLEAN_APP_ID, uploadEnabled, { channel: env })
Comment on lines +14 to +20
Copy link
Author

Choose a reason for hiding this comment

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

  1. Updated uploadEnabled to check for allowGlean user setting
  2. 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)

userAgent.set(navigator.userAgent)

// Debugging
Expand Down
9 changes: 9 additions & 0 deletions pages/settings.vue
Expand Up @@ -12,6 +12,7 @@ useHydratedHead({
const route = useRoute()

const isRootPath = computedEager(() => route.name === 'settings')
const devMode = useAppConfig().env === ('dev' || 'canary' || 'preview')
</script>

<template>
Expand Down Expand Up @@ -63,6 +64,14 @@ const isRootPath = computedEager(() => route.name === 'settings')
to="/settings/preferences"
:match="$route.path.startsWith('/settings/preferences/')"
/>
<SettingsItem
v-if="devMode"
command
icon="i-ri-lock-line"
:text="isHydrated ? $t('settings.privacy.label') : ''"
to="/settings/privacy"
:match="$route.path.startsWith('/settings/privacy/')"
/>
<SettingsItem
command
icon="i-ri:information-line"
Expand Down
52 changes: 52 additions & 0 deletions pages/settings/privacy/index.vue
@@ -0,0 +1,52 @@
<script setup lang="ts">
import Glean from '@mozilla/glean/web'
import { mastodonAccountHandle, mastodonAccountId } from '~/telemetry/generated/identifiers'
import { userAgent } from '~~/telemetry/generated/identifiers'

const user = currentUser.value

const { t } = useI18n()

useHydratedHead({
title: () => `${t('settings.privacy.label')} | ${t('nav.settings')}`,
})
const userSettings = useUserSettings()

function toggleOptOut() {
const allowGlean = togglePreferences('allowGlean')
Copy link
Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

Glean.setUploadEnabled(allowGlean)

// reset identifiers if user opts back in
if (user && allowGlean) {
mastodonAccountHandle.set(user.account.acct)
mastodonAccountId.set(user.account.id)
userAgent.set(navigator.userAgent)
}
Comment on lines +19 to +24
Copy link
Author

Choose a reason for hiding this comment

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

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

}
</script>

<template>
<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>

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

Good catch! fixed

</div>
</template>
<div p6 flex="~ col gap6">
<h2 text-xl>
{{ $t('settings.privacy.data_collection') }}
</h2>
<div>
<h3 text-lg>
{{ $t('settings.privacy.opt_out_title') }}
</h3>
<SettingsToggleItem
:checked="getPreferences(userSettings, 'allowGlean')"
@click="toggleOptOut()"
>
{{ $t('settings.privacy.opt_out_description') }}
</SettingsToggleItem>
</div>
</div>
</MainContent>
</template>
4 changes: 2 additions & 2 deletions telemetry/generated/ui.ts
Expand Up @@ -7,8 +7,8 @@
import EventMetricType from "@mozilla/glean/private/metrics/event";

/**
* Event triggered when a user taps/clicks on a UI element, triggering a change
* in app state.
* Event triggered when a user taps/clicks on a UI element, triggering a change in
* app state.
*
* Generated from `ui.engagement`.
*/
Expand Down
2 changes: 1 addition & 1 deletion telemetry/generated/web.ts
Expand Up @@ -4,8 +4,8 @@

// AUTOGENERATED BY glean_parser v8.1.1. DO NOT EDIT. DO NOT COMMIT.

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

Choose a reason for hiding this comment

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

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...


/**
* Event triggered when a user clicks a link on a web page.
Expand Down