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

Add dedicated integration with Product Bundles #439

Merged
merged 3 commits into from
Jun 19, 2024
Merged

Conversation

tomalec
Copy link
Member

@tomalec tomalec commented Jun 16, 2024

Changes proposed in this Pull Request:

Add dedicated integration with Product Bundles,
Use ->get_bundle_price( 'min' ); instead of ->get_price() for bundles.

Fixes #219 the simple way.

Checks:

  • Does your code follow the WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Screenshots:

image

Detailed test instructions:

  1. Install and activate the Product Bundles (PB) extension
  2. Add a new bundle product
    image
  3. Add bundled products that are priced individually
    image
  4. Publish the product bundle
  5. Open the site through https://tagassistant.google.com/
  6. Go to the product bundle page
  7. Check that view_item event is tracked with the correct bundle price
  8. Add the bundle to cart
  9. Check that add_to_cart event is tracked with the correct bundle price
  10. Uninstall the PB extension and smoke test for regression.

Additional details:

  1. I didn't add E2E tests, as PB is a paid extension, and public contributors to this repo would not be able to use it to run E2Es locally. We can try mocking PB interface, but then it's not really E2E. I'm open to better ideas :)

Changelog entry

Add - WooCommerce Product Bundles integration.

Use `->get_bundle_price( 'min' );` instead of `->get_price()` for bundles.

Fixes #219 the simples way.
@tomalec tomalec requested a review from a team June 16, 2024 18:30
@tomalec tomalec self-assigned this Jun 16, 2024
@github-actions github-actions bot added type: bug The issue/PR is a confirmed bug. changelog: fix Took care of something that wasn't working. labels Jun 16, 2024
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 @tomalec, thanks for working on the fix for bundle pricing!

Events tracked via the classic integration all look to be using the correct values for bundle prices.

There is still an issue with the Block remove_from_cart event. When we're building the product object in JS we're making use of product.prices.price and when a product is removed from cart on a block powered page that value will be the base price:

Screenshot 2024-06-18 at 13 42 53

We could instead rely on totals.line_total for the price if it is available which should give us consistent pricing across different integrations.

Screenshot 2024-06-18 at 13 50 08

I didn't add E2E tests, as PB is a paid extension, and public contributors to this repo would not be able to use it to run E2Es locally. We can try mocking PB interface, but then it's not really E2E. I'm open to better ideas :)

I don't have much else to suggest here I'm afraid. We could add a unit test in PHP for the bundled price in get_formatted_product which would be a lot easier to mock although that wouldn't cover Blocks.

includes/class-wc-abstract-google-analytics-js.php Outdated Show resolved Hide resolved
@tomalec
Copy link
Member Author

tomalec commented Jun 18, 2024

Thanks, @martynmjones, for catching that!
I applied your suggestions. Speaking of tests, as there are none for get_formatted_product so far, WDYT of adding them as a follow-up in the next porter or cooldown cycle?

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.

Thanks for the changes @tomalec!

Pricing is consistent across different events and integrations now so LGTM ✅

WDYT of adding them as a follow-up in the next porter or cooldown cycle?

Agreed! A follow-up for adding tests sounds good.

@tomalec
Copy link
Member Author

tomalec commented Jun 19, 2024

Thanks :) Created the follow-up issue to track tests #442

@tomalec tomalec merged commit e13e368 into trunk Jun 19, 2024
7 checks passed
@tomalec tomalec deleted the fix/219-bundle-price branch June 19, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue/PR is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Product Bundles total cost in Google Analytics doesn't reflect bundled items
2 participants