Expose Brand & MPN to Woocommerce UI #2842

devbodaghe · 2024-12-18T19:29:23Z

Changes proposed in this Pull Request:

This PR introduces a new feature to the Facebook WooCommerce Plugin, allowing users to add a brand field to their products. With this update, we can seamlessly synchronize the brand field with the Facebook Commerce Manager platform.

mshymon · 2025-01-10T14:54:12Z

@devbodaghe, is the screenshot in the description missing MPN field?
Also, it is easier to compare the change if you include before / after screenshots and potentially highlight area with the new elements in red.

Also, any particular reason why Brand goes between image URL and Price?

mshymon · 2025-01-10T14:56:05Z

includes/fbproduct.php

+	}
+
+	public function set_fb_mpn( $fb_mpn ) {
+		$fb_brand  = stripslashes(


bug

Suggested change

$fb_brand = stripslashes(

$fb_mpn = stripslashes(

mshymon · 2025-01-10T15:00:52Z

includes/fbproduct.php

+		// Check if the brand is already set
+		if ( $this->fb_brand ) {
+			$fb_brand = $this->fb_brand;
+		}
+
+		// If not set, try to get brand from post meta
+		if ( empty( $fb_brand ) ) {
+			$fb_brand = get_post_meta(
+				$this->id,
+				self::FB_BRAND,
+				true
+			);
+		}


Why do we need to keep variable $this->fb_brand, why not always read it from the DB via get_post_meta() when we need the value? Maintaining an intermediate variable in sync can lead to bugs.

You make a good point and I have updated this without the need for this value.

mshymon · 2025-01-10T15:04:27Z

includes/fbproduct.php

+			}
+		}
+
+		return WC_Facebookcommerce_Utils::clean_string( $fb_brand );


If we call WC_Facebookcommerce_Utils::clean_string at the end, can we remove previous such calls in the code of this method?

Great point

mshymon · 2025-01-10T15:07:02Z

includes/fbproduct.php

@@ -845,7 +944,11 @@ function( $key ) {
 		$matched_attributes = array_filter(
 			$all_attributes,
 			function( $attribute ) use ( $sanitized_keys ) {
-				return in_array( $attribute['key'], $sanitized_keys );
+				// Check if $attribute is an array and has the 'key' index
+				if (is_array($attribute) && isset($attribute['key'])) {


Formatting, check also through the code

Suggested change

if (is_array($attribute) && isset($attribute['key'])) {

if ( is_array( $attribute ) && isset( $attribute['key'] ) ) {

devbodaghe · 2025-01-10T19:00:08Z

@devbodaghe, is the screenshot in the description missing MPN field? Also, it is easier to compare the change if you include before / after screenshots and potentially highlight area with the new elements in red.

Also, any particular reason why Brand goes between image URL and Price?

It's updated now

mshymon · 2025-01-13T15:28:47Z

includes/fbproduct.php

@@ -881,6 +949,11 @@ public function prepare_variants_for_item( &$product_data ) {
 		// For each product field type, pull the single variant
 		foreach ( $variant_names as $original_variant_name ) {

+			// Ensure that the attribute exists before accessing it
+			if ( ! isset( $attributes[ $original_variant_name ] ) ) {


nit

Suggested change

if ( ! isset( $attributes[ $original_variant_name ] ) ) {

if ( !isset( $attributes[ $original_variant_name ] ) ) {

mshymon · 2025-01-13T17:01:40Z

includes/fbproduct.php

@@ -845,7 +909,11 @@ function( $key ) {
 		$matched_attributes = array_filter(
 			$all_attributes,
 			function( $attribute ) use ( $sanitized_keys ) {
-				return in_array( $attribute['key'], $sanitized_keys );
+				// Check if $attribute is an array and has the 'key' index


Comment is saying the same think as the code. Remove or add value why we do this check (not what we do)

vinkmeta · 2025-01-20T16:27:37Z

bin/fb_google_category_to_attribute_mapping.json

It is not a good idea to modify this json as its a standard static file that is not supposed to be modified. The alternative approach is to filter Brand when surfacing GPC attributes so that it never get surfaced and you can introduce a new field called 'Brand' that will eventually use the existing brand field's values.

devbodaghe · 2025-01-20T22:23:41Z

includes/Products/FBCategories.php

@@ -22,6 +22,8 @@
 */
 class FBCategories {

+	private $keys_to_exclude = ['brand' => true];


Added associative array here to avoid looping over regular array

vinkmeta · 2025-01-20T22:24:58Z

includes/Products/FBCategories.php

@@ -200,6 +209,7 @@ public function get_categories() {
 	 */
 	protected function get_attribute_field_by_hash( $hash ) {
 		$fields_data = $this->get_raw_attributes_fields_data();
+		// error_log('fields_data' . print_r($fields_data, true));


Lets remove this error log

vinkmeta · 2025-01-20T22:25:24Z

includes/fbproduct.php

@@ -89,6 +91,7 @@ class WC_Facebook_Product {
 	 */
 	public $fb_visibility;

+


Lets remove this extra line

vinkmeta · 2025-01-21T02:40:20Z

I manually tested this PR locally and discovered a critical bug: If a value for the Brand attribute is already saved and I then upgrade the plugin version, the Brand value appears empty. To fix this, you may need to fetch the Brand value while rendering this attribute in the Facebook.

facebook-github-bot · 2025-01-27T18:09:28Z

@devbodaghe has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-01-28T17:14:35Z

@devbodaghe has updated the pull request. You must reimport the pull request before landing.

facebook-commerce.php

facebook-github-bot · 2025-02-05T12:34:06Z

@devbodaghe has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-02-05T12:52:23Z

@devbodaghe has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-02-05T12:52:58Z

@devbodaghe has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-02-06T09:13:41Z

@devbodaghe merged this pull request in f7a6c3b.

* Update php-unit-tests.yml to install SVN (#2859) * Fix: get_global_unique_id function compatibility check (#2856) Summary: Fixed a bug that arose from our reliance on the plugin version compatibility tag for the public function get_global_unique_id. This function was actually introduced in WooCommerce version 9.2, but was incorrectly tagged as being available from version 9.1 onwards. To ensure correct functionality, we have replaced the use of Compatibility::is_wc_version_gte() with method_exists(), which checks for the availability of specific public methods rather than relying on version numbers Differential Revision: D68424348 * Update catalog link to navigate to correct tab in Commerce Manager (#2848) * Update catalog link to navigate to correct tab in Commerce Manager Co-authored-by: Marian Shymon <[email protected]> --------- Co-authored-by: David Evbodaghe <[email protected]> Co-authored-by: Marian Shymon <[email protected]> * Fixed product url links (#2852) * Fix - Removed the "Sell on Instagram" checkbox from UX and cleaned up the backend (#2854) * Removed the Sell on Instagram UX and cleane dup the backend. * Update php-unit-tests.yml to install SVN (#2859) --------- Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: Halil Ozan Akgül <[email protected]> * feat: Add custom fields to product data structure (#2836) * feat: Add custom fields to product data structure --------- Co-authored-by: David Evbodaghe <[email protected]> * Fixed Category specific attributes related issues (#2860) * Merging Trunk into Develop branch (#2850) * Start `release/3.3.1`. * Updated the version number to 3.3.1 * Release 3.3.2 (#2861) * Update php-unit-tests.yml to install SVN (#2859) * Fix: get_global_unique_id function compatibility check (#2856) Summary: Fixed a bug that arose from our reliance on the plugin version compatibility tag for the public function get_global_unique_id. This function was actually introduced in WooCommerce version 9.2, but was incorrectly tagged as being available from version 9.1 onwards. To ensure correct functionality, we have replaced the use of Compatibility::is_wc_version_gte() with method_exists(), which checks for the availability of specific public methods rather than relying on version numbers Differential Revision: D68424348 * Update catalog link to navigate to correct tab in Commerce Manager (#2848) * Update catalog link to navigate to correct tab in Commerce Manager Co-authored-by: Marian Shymon <[email protected]> --------- Co-authored-by: David Evbodaghe <[email protected]> Co-authored-by: Marian Shymon <[email protected]> * Fixed product url links (#2852) * Fix - Removed the "Sell on Instagram" checkbox from UX and cleaned up the backend (#2854) * Removed the Sell on Instagram UX and cleane dup the backend. * Update php-unit-tests.yml to install SVN (#2859) --------- Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: Halil Ozan Akgül <[email protected]> * feat: Add custom fields to product data structure (#2836) * feat: Add custom fields to product data structure --------- Co-authored-by: David Evbodaghe <[email protected]> * Fixed Category specific attributes related issues (#2860) * Start `release/3.3.2`. * updated the changelog and stable version * Updated the current version * Updated the WC supported version to 9.6 * Updated the stable version --------- Co-authored-by: Halil Ozan Akgül <[email protected]> Co-authored-by: vinkmeta <[email protected]> Co-authored-by: David <[email protected]> Co-authored-by: David Evbodaghe <[email protected]> Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Vahid Kaykhaei <[email protected]> Co-authored-by: vahidkay-meta <[email protected]> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Halil Ozan Akgül <[email protected]> Co-authored-by: vinkmeta <[email protected]> Co-authored-by: David <[email protected]> Co-authored-by: David Evbodaghe <[email protected]> Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: Marian Shymon <[email protected]> * fbshipit-source-id: bbb876a59dc530f663c5972aee5f492130b356d5 * Fixed the formatting so it matches the internal repo * fbshipit-source-id: 01c48061456e * sync'ing the internal repo with GH main * Expose Brand & MPN to Woocommerce UI (#2842) Summary: Changes proposed in this Pull Request: This PR introduces a new feature to the Facebook WooCommerce Plugin, allowing users to add a brand field to their products. With this update, we can seamlessly synchronize the brand field with the Facebook Commerce Manager platform. <img width="679" alt="Screenshot 2025-01-10 at 18 57 18" src="https://github.com/user-attachments/assets/9d941a1f-48cb-4106-9672-1620aa926be6" /> <img width="527" alt="Screenshot 2024-10-30 at 18 08 36" src="https://github.com/user-attachments/assets/d0cf9ae0-293b-4685-a033-643dc6214a1f"> Pull Request resolved: #2842 Reviewed By: vinkmeta Differential Revision: D68720240 Pulled By: devbodaghe fbshipit-source-id: 0112d2063d9d9358c9f08efde51b6bf5d04b2d59 * FBE: Use of recommended delete connection endpoint over delete permission endpoint (#2844) Summary: ### Changes proposed in this Pull Request: **Problem:** 1. The Delete Permission User API only removed Meta connection assets from the WooCommerce database, leaving asset-related data enabled on Meta surfaces. 2. If a user disconnected the connection before uninstalling assets from Meta surfaces using Managed Connection; the UI for Managed Connection was also removed, making it difficult for users to uninstall this feature from Meta surface. **Solution:** 1. Replaced the Delete Permission User API with the recommended Delete FBE Connection endpoint, which uninstalls assets from Meta surfaces and removes their permissions. 2. Removed the Managed Connection UI button for uninstalling FBE from Meta surfaces, as the Delete Connection endpoint now handles this functionality. ### Screenshots: **Before:** <img width="784" alt="image" src="https://github.com/user-attachments/assets/76bf6364-2af8-4657-a043-8e79aaff99c6"> **After:** <img width="803" alt="image" src="https://github.com/user-attachments/assets/45e7ebc8-4a74-4589-a9e3-0e4fff1f4f20"> ![Screenshot 2025-02-04 at 15 29 54](https://github.com/user-attachments/assets/77e02cba-ca89-42f9-a016-85c50e4b02fc) <img width="797" alt="image" src="https://github.com/user-attachments/assets/f4d215b0-0959-4ee8-8347-16c64afe3460"> ### Detailed test instructions: 1. Run new tests: `./vendor/bin/phpunit --filter test_delete_mbe_connection_deletes_user_permission_request` 2. Run all tests : `npm run test:php` 3. Lint: `./vendor/bin/phpcs` 4. Manual testing: I have tested new Disconnect UI flow; it uninstall FBE connection from WooCommerce as well as from Meta surface ### Changelog entry - **Removed:** Delete Permission User API - **Added:** Delete FBE Connection endpoint to uninstall assets from Meta surfaces and remove permissions - **Removed:** Managed Connection UI button for uninstalling FBE from Meta surfaces (now handled by Delete Connection endpoint) Pull Request resolved: #2844 Reviewed By: halilozanakgul Differential Revision: D68972713 Pulled By: atuld123 fbshipit-source-id: b19ee8351b4852c69192449f9af2644960a55307 * Start `release/3.3.3`. * Updated the plugin version to 3.3.3 * Updated the stable tag to 3.3.3 --------- Co-authored-by: Halil Ozan Akgül <[email protected]> Co-authored-by: vinkmeta <[email protected]> Co-authored-by: David <[email protected]> Co-authored-by: David Evbodaghe <[email protected]> Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: Marian Shymon <[email protected]> Co-authored-by: vahidkay-meta <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Facebook Community Bot <[email protected]> Co-authored-by: Vahid Kaykhaei <[email protected]> Co-authored-by: Atul Dusane <[email protected]>

mshymon requested changes Jan 10, 2025

View reviewed changes

devbodaghe marked this pull request as ready for review January 10, 2025 19:14

mshymon approved these changes Jan 13, 2025

View reviewed changes

facebook-github-bot added the CLA Signed label Jan 15, 2025

vinkmeta self-requested a review January 20, 2025 13:42

vinkmeta reviewed Jan 20, 2025

View reviewed changes

devbodaghe force-pushed the add_product_brand_sync_to_meta branch from e7b6071 to 1165e4d Compare January 20, 2025 21:46

devbodaghe commented Jan 20, 2025

View reviewed changes

vinkmeta reviewed Jan 20, 2025

View reviewed changes

devbodaghe force-pushed the add_product_brand_sync_to_meta branch from 7eb65de to c0e2de4 Compare January 20, 2025 22:54

vinkmeta approved these changes Jan 31, 2025

View reviewed changes

vahidkay-meta approved these changes Feb 5, 2025

View reviewed changes

facebook-commerce.php Show resolved Hide resolved

David Evbodaghe added 5 commits February 5, 2025 12:50

Expose Brand & MPN to Woocommerce UI

81a135f

Expose Brand & MPN to Woocommerce UI

5ef9060

Expose Brand & MPN to Woocommerce UI

9c08af9

Expose Brand & MPN to Woocommerce UI

c43a62c

Expose Brand & MPN to Woocommerce UI

6beabd5

devbodaghe force-pushed the add_product_brand_sync_to_meta branch from 6025cb9 to 6beabd5 Compare February 5, 2025 12:52

facebook-github-bot closed this in f7a6c3b Feb 6, 2025

facebook-github-bot added the Merged label Feb 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose Brand & MPN to Woocommerce UI #2842

Expose Brand & MPN to Woocommerce UI #2842

devbodaghe commented Dec 18, 2024 •

edited

Loading

mshymon commented Jan 10, 2025

mshymon Jan 10, 2025

mshymon Jan 10, 2025

devbodaghe Jan 10, 2025

mshymon Jan 10, 2025

devbodaghe Jan 10, 2025

mshymon Jan 10, 2025

devbodaghe commented Jan 10, 2025

mshymon Jan 13, 2025

mshymon Jan 13, 2025

vinkmeta Jan 20, 2025

devbodaghe Jan 20, 2025 •

edited

Loading

vinkmeta Jan 20, 2025

vinkmeta Jan 20, 2025

vinkmeta commented Jan 21, 2025

facebook-github-bot commented Jan 27, 2025

facebook-github-bot commented Jan 28, 2025

facebook-github-bot commented Feb 5, 2025

facebook-github-bot commented Feb 5, 2025

facebook-github-bot commented Feb 5, 2025

facebook-github-bot commented Feb 6, 2025

	if (is_array($attribute) && isset($attribute['key'])) {
	if ( is_array( $attribute ) && isset( $attribute['key'] ) ) {

	if ( ! isset( $attributes[ $original_variant_name ] ) ) {
	if ( !isset( $attributes[ $original_variant_name ] ) ) {

		@@ -89,6 +91,7 @@ class WC_Facebook_Product {
		*/
		public $fb_visibility;

Expose Brand & MPN to Woocommerce UI #2842

Expose Brand & MPN to Woocommerce UI #2842

Conversation

devbodaghe commented Dec 18, 2024 • edited Loading

mshymon commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devbodaghe commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devbodaghe Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinkmeta commented Jan 21, 2025

facebook-github-bot commented Jan 27, 2025

facebook-github-bot commented Jan 28, 2025

facebook-github-bot commented Feb 5, 2025

facebook-github-bot commented Feb 5, 2025

facebook-github-bot commented Feb 5, 2025

facebook-github-bot commented Feb 6, 2025

devbodaghe commented Dec 18, 2024 •

edited

Loading

devbodaghe Jan 20, 2025 •

edited

Loading