FBE: Use of recommended delete connection endpoint over delete permission endpoint #2844
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mshymon
previously requested changes
Jan 17, 2025
|
@vinkmeta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
atuld123
force-pushed
the
delete_fbe_connection
branch
from
c54485d to
c258b97
Compare
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
added 3 commits
February 2, 2025 12:02
1) Earlier Delete Permission User API was only cleaning Meta connection assets from WooCommerce database and leaving asset related data enabled at Meta surfaces. 2) Managed Connection was another option provided to uninstall this assets from Meta surface; but if user clicked Disconnect connection earlier than that then there is no way for user to connect those asset at Meta surface and install them Solution: 1) Meta has new endpoint Delete FBE connection; this new endpoint uninstall the asset from Meta surface as well as removed its permissions too. I have replaced old fbe Delete Permission User API with new Delete FBE Connection API. 2) Removed Managed connection UI button for uninstallation of FBE from Meta surface. As new Delete Connection endpoint handles the same
amended code review changes
atuld123
force-pushed
the
delete_fbe_connection
branch
from
c62fdc9 to
9d2f397
Compare
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
atuld123
requested review from
halilozanakgul
and removed request for
vinkmeta
- added execution permission which was reverted in earlier commit - removed unused managed url public function
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
facebook-github-bot
pushed a commit
that referenced
this pull request
Feb 4, 2025
…sion 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="1022" alt="Screenshot 2025-02-01 at 18 51 41" src="https://github.com/user-attachments/assets/55cb6a86-9454-4809-8d94-7b29931c6fad" /> <img width="803" alt="image" src="https://github.com/user-attachments/assets/45e7ebc8-4a74-4589-a9e3-0e4fff1f4f20"> <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) Differential Revision: D68972713 Pulled By: atuld123
facebook-github-bot
force-pushed
the
delete_fbe_connection
branch
from
1abeebb to
e55355e
Compare
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
This pull request was exported from Phabricator. Differential Revision: D68972713 |
atuld123
force-pushed
the
delete_fbe_connection
branch
from
e55355e to
1abeebb
Compare
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@atuld123 has updated the pull request. You must reimport the pull request before landing. |
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
iodic
approved these changes
Feb 5, 2025
|
@atuld123 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
halilozanakgul
approved these changes
Feb 5, 2025
devbodaghe
approved these changes
Feb 5, 2025
devbodaghe
approved these changes
Feb 5, 2025
vahidkay-meta
added a commit
that referenced
this pull request
Feb 6, 2025
* 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">  <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]>
vahidkay-meta
pushed a commit
that referenced
this pull request
Feb 11, 2025
…sion endpoint (#2844) Summary: **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. **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">  <img width="797" alt="image" src="https://github.com/user-attachments/assets/f4d215b0-0959-4ee8-8347-16c64afe3460"> 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 - **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 parent 4be2987 author github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> 1738833933 +0000 committer Vahid Kaykhaei <[email protected]> 1739267285 +0100 Start `release/3.3.3`. Start `release/3.3.1`. Updated the plugin version to 3.3.3 Updated the stable tag to 3.3.3 Fixed the version nr in fb-for-woo.php Fixed a review comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
Problem:
Solution:
Screenshots:
Before:
After:
Detailed test instructions:
./vendor/bin/phpunit --filter test_delete_mbe_connection_deletes_user_permission_requestnpm run test:php./vendor/bin/phpcsChangelog 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)