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.

Sign up for GitHub

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

Jump to bottom

Fix setConsent to properly use the update gtag command #8243

Merged
merged 9 commits into from May 15, 2024
Merged

Fix setConsent to properly use the update gtag command #8243

merged 9 commits into from May 15, 2024

Conversation

DellaBitta
Copy link
Contributor

@DellaBitta DellaBitta commented May 13, 2024 •
edited

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

@DellaBitta
Ensure 'update' string reaches gtagCore.
a10daf1 
Include the consent string ('update') in gtagCore calls that update consent.
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 13, 2024 •
edited

🦋 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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 13, 2024 •
edited

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 •
edited

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 •
edited

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
@DellaBitta DellaBitta merged commit f66769c into master May 15, 2024
36 checks passed
@DellaBitta DellaBitta deleted the ddb-analytics-set-consent branch May 15, 2024 18:11
@DellaBitta DellaBitta mentioned this pull request May 15, 2024
Closed
@google-oss-bot google-oss-bot mentioned this pull request May 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsubox76 hsubox76 hsubox76 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Analytics Consent not sent on update
3 participants
@DellaBitta @google-oss-bot @hsubox76