woocommerce / facebook-for-woocommerce

A first-party extension plugin built for WooCommerce. Development is managed by Ventures.
https://woocommerce.com/products/facebook/
GNU General Public License v2.0
211 stars 140 forks source link

Use get_title() for content_name to match catalog name. #2781

Closed rawdreeg closed 3 months ago

rawdreeg commented 3 months ago

Changes proposed in this Pull Request:

We received a report ( #2774 ) pointing out inconsistencies in populating the content_name property in events we send to Meta, sometimes using get_title() vs get_name(). I have looked into this and found that get_title() applies the woocommerce_product_title filter to the product name prop. This may result in different values being sent to Facebook. I also find that we use get_title for the catalog data, so I propose we use the get_title method

Closes #2774.

Detailed test instructions:

  1. Use the woocommerce_product_title filter. E.g.:
function custom_woocommerce_product_title_prefix( $title, $product ) {
    // Add the prefix "Special: " to the product title
    $title = 'Special: ' . $title;

    return $title;
}
add_filter( 'woocommerce_product_title', 'custom_woocommerce_product_title_prefix', 10, 2 );
  1. Using the FB pixel helper browser extension, visit a product and confirm that the ViewContent event fires and all the details are confirmed.
  2. Repeat this with the addToCart, Checkout and Purchase Events

Additional details:

Changelog entry

Update - Use get_title() for content_name to match catalog name.

rawdreeg commented 3 months ago

Thanks @nima-karimi for the review.

Dekadinious commented 3 months ago

Not to be a party pooper here, but given that this is in a WooCommerce context and that WooCommerce also has filters for $product->get_name(); one would think that using that everywhere would be more future proof as WooCommerce might move products to custom tables just as they did with orders (HPOS).

Using native getters and setters on objects is the preferable way to code for WooCommerce anyway, so people using the post title filter might be considered to be "doing_it_wrong".

rawdreeg commented 3 months ago

Thanks @Dekadinious,

I picked get_title() over get_name(). This is used for the catalogue product as well: https://github.com/woocommerce/facebook-for-woocommerce/blob/df03a97e449b242c1ef59ddc70eac2de966cc3e8/includes/fbproduct.php#L657

This aligns the events sent with whatever name is set to Catalogue, reducing the risk of mismatch.

Dekadinious commented 3 months ago

Sorry, had a brain fart there for a second. Move on, nothing to see here! :)