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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extra product/item data for hooks #324

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

migueldamota
Copy link

馃憢 Hi there,

with this PR I want to add product/item data to some hooks for WooCommerce. We store meta data in cart items and order items and with the current implementation we can't access it. So with this we are able to get specific data about the product which aren't defined in the product or their variations.

If you have any questions or need some examples of how we use it, don't hesitate to ask.

Have a great day!

@migueldamota
Copy link
Author

Hello again,

I also improved the check for productdata.quantity to handle null values because isNaN(null) isn't true, I don't know why. Anyways, I hope this is good for you.

@duracelltomi
Copy link
Owner

Hi,

I am not sure this is the best way to achieve what you want.
With your changes, everything in the $cart_item_data and $order_item variable could be processed by GA4 and sent as custom dimensions increasing the size of each event hit without any specific benefit.

What I could better imagine is the following:

  • deprecate the currently available gtm4wp_eec_product_array WP filter
  • add a new parameter to gtm4wp_woocommerce_process_product() to pass data that is not directly meant to be added into the items array of the ecommerce key in the data layer
  • implement a new filter in gtm4wp_woocommerce_process_product() that will act almost like the deprecated one but it will also get this new function parameter so that users like you can add additional data without overwhelming the items array with unnecessary data

What do you think?

@migueldamota
Copy link
Author

Hi @duracelltomi.

Oh, I haven't noticed that, sorry.

The new filter makes more sense. Thank you 馃檹 Will you implement this or should I update the PR?

@duracelltomi
Copy link
Owner

If you can implement this with the new approach, I would be very happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants