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 setConsent to properly use the update gtag command #8243

Merged
merged 9 commits into from
May 15, 2024

Conversation

DellaBitta
Copy link
Contributor

@DellaBitta DellaBitta commented May 13, 2024

Discussion

We wrap the gtag function internally and pass setConsent calls through the wrapper function, which in turn calls the gtag SDK. While we invoke the wrapped function with parameters identical to those listed in the gtag functions, we were dropping the update string on the floor inside the wrapped function. Additionally, we were feeding the consentSettings as the second parameter to gag, which should have been the update string parameter.

This ended up clobbering the data, and not send update commands properly.

This change fixes our wrappedGtag func impl to properly pass update to gtag, along with the provided ConsentSettings argument.

Closes #8210

Testing

  • Added a higher level of fidelity to our unit tests by testing the values set in window['dataLayer'].
  • CI passes.

API Changes

N/A

Include the consent string ('update') in gtagCore calls that update consent.
Copy link

changeset-bot bot commented May 13, 2024

🦋 Changeset detected

Latest commit: 5e07a73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/analytics Patch
@firebase/analytics-compat Patch
firebase Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented May 13, 2024

Changeset File Check ✅

  • No modified packages are missing from the changeset file.
  • No changeset formatting errors detected.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented May 13, 2024

Size Report 1

Affected Products

  • @firebase/analytics

    TypeBase (9cbb184)Merge (f7c9e13)Diff
    browser21.8 kB21.8 kB+19 B (+0.1%)
    esm526.9 kB27.0 kB+41 B (+0.2%)
    main28.4 kB28.4 kB+41 B (+0.1%)
    module21.8 kB21.8 kB+19 B (+0.1%)
  • @firebase/firestore

    TypeBase (9cbb184)Merge (f7c9e13)Diff
    esm5363 kB363 kB-2 B (-0.0%)
    main582 kB582 kB-11 B (-0.0%)
  • bundle

    TypeBase (9cbb184)Merge (f7c9e13)Diff
    analytics (logEvent)44.5 kB44.5 kB-5 B (-0.0%)
  • firebase

    TypeBase (9cbb184)Merge (f7c9e13)Diff
    firebase-analytics-compat.js26.5 kB26.5 kB-3 B (-0.0%)
    firebase-analytics.js29.7 kB29.7 kB-5 B (-0.0%)
    firebase-compat.js786 kB786 kB-2 B (-0.0%)
    firebase-firestore-compat.js341 kB341 kB+1 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/XTSHuCcn6V.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented May 13, 2024

Size Analysis Report 1

Affected Products

  • @firebase/analytics

    • getAnalytics

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.6 kB10.6 kB-5 B (-0.0%)
      size-with-ext-deps44.4 kB44.4 kB-5 B (-0.0%)
    • getGoogleAnalyticsClientId

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.5 kB10.5 kB-5 B (-0.0%)
      size-with-ext-deps37.2 kB37.2 kB-5 B (-0.0%)
    • initializeAnalytics

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.5 kB10.5 kB-5 B (-0.0%)
      size-with-ext-deps37.5 kB37.5 kB-5 B (-0.0%)
    • isSupported

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.4 kB10.4 kB-5 B (-0.0%)
      size-with-ext-deps37.1 kB37.1 kB-5 B (-0.0%)
    • logEvent

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.2 kB10.2 kB-5 B (-0.0%)
      size-with-ext-deps37.0 kB37.0 kB-5 B (-0.0%)
    • setAnalyticsCollectionEnabled

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.4 kB10.4 kB-5 B (-0.0%)
      size-with-ext-deps37.1 kB37.1 kB-5 B (-0.0%)
    • setConsent

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.4 kB10.4 kB-5 B (-0.0%)
      size-with-ext-deps37.1 kB37.1 kB-5 B (-0.0%)
    • setCurrentScreen

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.5 kB10.5 kB-5 B (-0.0%)
      size-with-ext-deps37.2 kB37.2 kB-5 B (-0.0%)
    • setDefaultEventParameters

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.4 kB10.4 kB-5 B (-0.0%)
      size-with-ext-deps37.1 kB37.1 kB-5 B (-0.0%)
    • setUserId

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.5 kB10.5 kB-5 B (-0.0%)
      size-with-ext-deps37.2 kB37.2 kB-5 B (-0.0%)
    • setUserProperties

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.6 kB10.5 kB-5 B (-0.0%)
      size-with-ext-deps37.3 kB37.3 kB-5 B (-0.0%)
    • settings

      Size

      TypeBase (9cbb184)Merge (f7c9e13)Diff
      size10.4 kB10.4 kB-5 B (-0.0%)
      size-with-ext-deps37.1 kB37.1 kB-5 B (-0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/r6uUBd6dZl.html

@DellaBitta DellaBitta requested a review from hsubox76 May 13, 2024 18:06
@DellaBitta DellaBitta marked this pull request as ready for review May 13, 2024 18:06
@DellaBitta DellaBitta requested review from a team as code owners May 13, 2024 18:06
@DellaBitta DellaBitta requested a review from hsubox76 May 15, 2024 15:00
@firebase firebase locked and limited conversation to collaborators Jun 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analytics Consent not sent on update
3 participants