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

Event provider JS doesn't load on production build #8955

Closed
5 tasks
aaemnnosttv opened this issue Jul 2, 2024 · 7 comments
Closed
5 tasks

Event provider JS doesn't load on production build #8955

aaemnnosttv opened this issue Jul 2, 2024 · 7 comments
Labels
P0 High priority Team S Issues for Squad 1 Type: Bug Something isn't working

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Jul 2, 2024

Bug Description

The event provider JS loaded on the front end for its respective supported plugin integration for enhanced conversion tracking does not load when using a production build of Site Kit. It does work on development builds, but not intentionally.

Steps to reproduce

  1. Use a production build of SK
  2. Setup GA and enable enhanced conversion tracking
  3. Enable a supported plugin
  4. View frontend
  5. See 404 for event provider JS

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Event provider JS should load as expected regardless of the type of build used
  • Generated provider JS filenames should have a googlesitekit-events-provider- prefix for consistency

Implementation Brief

  • Within the Conversion Events WebPack config file at webpack/conversionEventProviders.config.js:
    • Prepend the file names in the entry object with 'googlesitekit-events-provider-', i.e 'googlesitekit-events-provider-contact-form-7': './assets/js/event-providers/contact-form-7.js',
  • For each of the event provider classes within the includes/Core/Conversion_Tracking/Conversion_Event_Providers folder, undertake the following changes:
    • Replace the prepended 'gsk-cep-' string from the handle passed to Script() with the string value prepended to filenames in the webpack config, 'googlesitekit-events-provider-'. This will ensure correct resolving of the script src by the Script class, regardless of production or development modes.
    • Leave the src entry of the Script() args unchanged.

Test Coverage

  • This feature requires no updates to existing test suites

QA Brief

  • Set up Site Kit with Analytics and Enhanced Conversion Tracking enabled.
  • Install plugins that have enhanced tracking event support (Contact Form 7, WooCommerce, for example)
  • Visit the site homepage
  • Confirm the event provider .js files now load correctly by viewing page source and searching for googlesitekit-events-provider-contact-form-7-$HASH.js

Changelog entry

  • Fix bug that prevented Event Provider JavaScript files from loading.
@aaemnnosttv aaemnnosttv added Type: Bug Something isn't working P0 High priority labels Jul 2, 2024
@binnieshah binnieshah added the Team S Issues for Squad 1 label Jul 3, 2024
@benbowler
Copy link
Collaborator

@10upsimon I actually had a similar issue on my Consent Mode refactor issue #8384 today, this is how I fixed it.

@10upsimon 10upsimon assigned 10upsimon and unassigned 10upsimon Jul 3, 2024
@tofumatt tofumatt self-assigned this Jul 3, 2024
@aaemnnosttv aaemnnosttv self-assigned this Jul 3, 2024
@tofumatt
Copy link
Collaborator

tofumatt commented Jul 3, 2024

IB ✅

@tofumatt tofumatt assigned aaemnnosttv and unassigned tofumatt and aaemnnosttv Jul 3, 2024
@aaemnnosttv
Copy link
Collaborator Author

@tofumatt @10upsimon @benbowler

  • change the logic within the register_script() method to instead use the Manifest::get() method, passing self::CONVERSION_EVENT_PROVIDER_SLUG as the parameter, to obtain the filename

This would work but isn't necessary as Script already handles the application of the file in the Manifest automatically. Ultimately, this is the root cause for the issue as you'll notice no assets touch the Manifest directly yet they all load the right URLs for the build. We simply need to update the event provider scripts to follow the same approach as every other asset rather than treating these as a special case unnecessarily.

@10upsimon
Copy link
Collaborator

Thanks @aaemnnosttv I've simplified this and handed back to you.

@aaemnnosttv
Copy link
Collaborator Author

  • For each of the event providers within the includes/Core/Conversion_Tracking/Conversion_Event_Providers folder, change the logic within the register_script() method to instead pass just the self::CONVERSION_EVENT_PROVIDER_SLUG constso as to handle correct Manifest behavior when surfacing the asset URL
  • Remove the prepended 'gsk-cep-' string from the handle passed to Script(), this will ensure correct handling of the Manifest entry in the Script class.

@10upsimon while this will have the intended outcome as far as loading the script is concerned, it's changing the handle to be generic. E.g. gsk-cep-mailchimp will become mailchimp, which as a (global) WP script handle, is not something SK should be using but this might not be obvious due to the abstraction of our Script; it does not add any kind of namespacing, etc to the given handle. With that said, we should update both the handle and the entry name (key) to reference the same value (according to the AC) since the entry name is what the resulting manifest is keyed by.

Since the entry name also defines the default resulting filename, there's no need to add the prefix to the output if it's included in the entry name.

@aaemnnosttv aaemnnosttv assigned 10upsimon and unassigned aaemnnosttv Jul 3, 2024
@10upsimon 10upsimon assigned aaemnnosttv and unassigned 10upsimon Jul 3, 2024
@aaemnnosttv
Copy link
Collaborator Author

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Jul 3, 2024
@benbowler benbowler assigned benbowler and unassigned benbowler Jul 4, 2024
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Jul 4, 2024
@mohitwp mohitwp self-assigned this Jul 5, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Jul 5, 2024

QA Update ✅

  • Tested on dev environment.
  • Verified event tracking for WooCommerce and WPForms plugin.
  • Verified now event are getting track using tester plugin.
  • Verified the event provider .js files now load correctly by viewing page source and searching for googlesitekit-events-provider-contact-form-7-$HASH.js
  • Verified JS filenames have a googlesitekit-events-provider- prefix for consistency.

JS files

image

image

image

WPFORMS

When only Analytics is connected

image

When both analytics and Ads module connected

image

image

Recording.1151.mp4

Woocommerce

image

Recording.1150.mp4

event : add to cart

When only analytics is connected

image

Both analytics and Ads module connected

image

image

event : Purchase

image

When both analytics and Ads module connected

image

image

@mohitwp mohitwp removed their assignment Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Team S Issues for Squad 1 Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants