Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

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

Merged
merged 8 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions composables/settings/definition.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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.privacy.label') }}</span>
</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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading