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 connected accounts in tracks from the backend. #2198

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

mikkamp
Copy link
Contributor

@mikkamp mikkamp commented Jan 15, 2024

Changes proposed in this Pull Request:

This PR adds any connected accounts to tracks which we send from the backend. The accounts are only included if they are connected (non-zero).

Detailed test instructions:

There aren't a lot of tracks being sent from the backend to test with, so I added the following line to ConnectionTest::render_admin_page:

do_action( 'woocommerce_gla_url_switch_success', [] );
  1. Make the change (suggested above) to add the manual track
  2. Browse a site with both MC and Ads accounts connected
  3. Turn on browser developer tools and browse to the hidden connection page: https://domain.test/wp-admin/admin.php?page=connection-test-admin-page
  4. In the browser developer tools filter network requests to pixel.wp.com
  5. Look for a request for t.gif and check the payload
    image
  6. Confirm that gla_version, gla_ads_id, gla_merchant_id all reflect the right values

Changelog entry

  • Add - Include connected accounts in tracks from the backend.

@mikkamp mikkamp self-assigned this Jan 15, 2024
@github-actions github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Jan 15, 2024
@mikkamp mikkamp requested a review from a team January 15, 2024 16:46
Copy link

codecov bot commented Jan 15, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a91fa6c) 59.4% compared to head (d82b62b) 59.5%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             develop   #2198     +/-   ##
===========================================
+ Coverage       59.4%   59.5%   +0.1%     
- Complexity      4127    4129      +2     
===========================================
  Files            453     453             
  Lines          16478   16482      +4     
===========================================
+ Hits            9788    9807     +19     
+ Misses          6690    6675     -15     
Flag Coverage Δ
php-unit-tests 59.5% <50.0%> (+0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/Tracking/Tracks.php 69.2% <50.0%> (-8.5%) ⬇️

... and 2 files with indirect coverage changes

Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

Hey @mikkamp, thanks for the tracks adjustment.

Confirmed that gla_version, gla_ads_id, and gla_merchant_id values are all included in the payload and values are correct so LGTM ✅

@mikkamp mikkamp merged commit 1b7abc7 into develop Jan 16, 2024
12 of 13 checks passed
@mikkamp mikkamp deleted the add/connected-accounts-backend-tracks branch January 16, 2024 08:16
@jorgemd24 jorgemd24 mentioned this pull request Jan 30, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants