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

Include Google Analytics code #451

Merged
merged 2 commits into from
Sep 3, 2021
Merged

Include Google Analytics code #451

merged 2 commits into from
Sep 3, 2021

Conversation

kalvinwang
Copy link
Contributor

@kalvinwang kalvinwang commented Sep 3, 2021

This PR adds the Google Analytics code snippet. It also creates a new environment variable, ENABLE_GOOGLE_ANALYTICS, that must be set to 'enabled' for the GA snippet to render.

  • Tested by going to Google Analytics Realtime, searching for /claimstatus, and seeing results
  • Tested environment variable locally to see if network requests appear in each case

===

Resolves #313

  • Relevant documentation (e.g. READMEs, Technical Foundation) updated
  • Issue AC are copied to this PR & are met
  • Manual Browser Testing performed (with modheader, either locally or on BrowserStack)
  • Changes are tested on Staging post-merge into main

@@ -75,6 +101,7 @@ export default function Home({
href="https://fonts.googleapis.com/css2?family=Source+Sans+Pro:ital,wght@0,400;0,700;1,400;1,700&display=swap"
rel="stylesheet"
/>
{enableGoogleAnalytics === 'enabled' && googleAnalytics}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I might be being dense here. Doesn't this expression evaluate to true/false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

😖

Copy link
Contributor

Choose a reason for hiding this comment

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

oh wow Javascript doesn't do this natively? TIL

@@ -99,6 +126,9 @@ export const getServerSideProps: GetServerSideProps = async ({ req, res, locale,
urlPrefixBpo: process.env.URL_PREFIX_BPO ?? '',
}

// Set to 'enabled' to include Google Analytics code
const ENABLE_GOOGLE_ANALYTICS = process.env.ENABLE_GOOGLE_ANALYTICS ?? ''
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to be alerted on whether this env var is missing? If so, there is code in queryApiGateway.ts that could be re-used or generalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting! I guess ideally we could be alerted if it's missing in production, and only in production. I don't think we currently have a way of detecting whether we're in production (although it's easy enough to add an env var for that), so I'll create a ticket to alert if it's missing in prod, how's that sound?

Copy link
Contributor

@rocketnova rocketnova Sep 3, 2021

Choose a reason for hiding this comment

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

I don't think we currently have a way of detecting whether we're in production

We do! We're doing that for the missing unique number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(talked to Rocket, we don't have a way in our codebase of detecting whether we're in prod)

created: #460

Comment on lines +51 to +57
gtag('config', 'UA-3419582-2', { 'anonymize_ip': true }) // www.ca.gov
gtag('config', 'UA-3419582-31', { 'anonymize_ip': true }) // edd.ca.gov`

const googleAnalytics = (
<>
{/* Global site tag (gtag.js) - Google Analytics */}
<script async src="https://www.googletagmanager.com/gtag/js?id=UA-3419582-2" />
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like there's a teeny bit of a bad code smell here with the hardcoded IDs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Is there a constants file I could move this to? It doesn't make sense for it to be an env var.

@kalvinwang kalvinwang merged commit 2059d99 into main Sep 3, 2021
@kalvinwang kalvinwang deleted the kalvin-analytics branch September 3, 2021 19:03
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.

Add Google Analytics tag
3 participants