Conversation
|
@Jmencab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
fa39f9e to
d87e4c2
Compare
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
|
@Jmencab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Looking good so far! I will refrain from commenting on adding extra logging until you push the final version. :) |
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
65d88da to
a2bd728
Compare
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
Thank you! I think this is good to review now. I have also added some verification steps in the doc and a simple unit test that matches what was used in the product feed upload PR. |
|
@Jmencab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
|
@Jmencab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
| // Leave feed manger out for now as will just make example feed. Use for dogfooding. | ||
| $this->feed_manager = new WooCommerce\Facebook\Feed\FeedManager(); |
There was a problem hiding this comment.
So this won't be present in the diff but will be a requirement to be added when we implement the first feed upload?
There was a problem hiding this comment.
actually, I think we settled on creating the feed manager but not making any feeds in its constructor
| ); | ||
|
|
||
| try { | ||
| $cpi_id = \WC_Facebookcommerce::instance()->get_integration()->get_commerce_partner_integration_id(); |
There was a problem hiding this comment.
I think that this can just be replaced with: get_option( 'wc_facebook_commerce_partner_integration_id', '' )
| /* $secret = get_option( $this->feed_url_secret_option_name, '' ); | ||
| if ( ! $secret ) { | ||
| $secret = wp_hash( 'example-feed-' . time() ); | ||
| update_option( $this->feed_url_secret_option_name, $secret ); | ||
| } | ||
|
|
||
| return $secret;*/ |
There was a problem hiding this comment.
What is the plan for this?
There was a problem hiding this comment.
This snapshot is what you need for testing this. need to be able to know what the "secret" will be so you can easily hit the url yourself.
For the actual implementation, it will be the commented out code
| /** | ||
| * Schedules the recurring feed generation. | ||
| * | ||
| * This method must be implemented by the concrete feed class, usually by providing a recurring interval |
There was a problem hiding this comment.
Update comment since this logic is now defined in the base class
| * @since 3.5.0 | ||
| */ | ||
| public function regenerate_feed(): void { | ||
| // Maybe use new ( experimental ), feed generation framework. |
There was a problem hiding this comment.
I think it would be good to have a comment here explaining the difference between these two flows
| if ( self::get_feed_secret() !== Helper::get_requested_value( 'secret' ) ) { | ||
| throw new PluginException( "{$name} feed: Invalid secret provided.", 401 ); | ||
| } |
There was a problem hiding this comment.
What exactly is the secret here?
There was a problem hiding this comment.
it's a hash that gets appended to the file names. you verify the secret here to make sure it's the correct file
|
|
||
| // bail early if the file can't be read. | ||
| if ( ! is_readable( $file_path ) ) { | ||
|
|
| header( 'Content-Length:' . filesize( $file_path ) ); | ||
|
|
||
| // phpcs:ignore | ||
| $file = @fopen( $file_path, 'rb' ); |
There was a problem hiding this comment.
So if fpassthru is enabled then the file just needs to be opened?
There was a problem hiding this comment.
if ( \WC_Facebookcommerce_Utils::is_fpassthru_disabled() || ! @fpassthru( $file ) ) {
Is doing a lot... if fpassthru returns true, then it was passed to Meta, if it did not work then it needs to be streamed
| $this->data_stream_name = FeedManager::EXAMPLE; | ||
| $this->gen_feed_interval = DAY_IN_SECONDS; | ||
| $this->feed_type = 'PRODUCT_RATINGS_AND_REVIEWS'; | ||
| $this->feed_url_secret_option_name = self::OPTION_FEED_URL_SECRET . $this->data_stream_name; |
There was a problem hiding this comment.
What is the importance/use of this value?
There was a problem hiding this comment.
this value stores the secret option. the option value is used for the hash appended to the file name. the hash is used to verify they file on "handle_feed_data_request"
b5ca7ce to
0c5ba3e
Compare
|
@Jmencab has updated the pull request. You must reimport the pull request before landing. |
|
@Jmencab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: ### Changes proposed in this Pull Request: This PR builds on #2875 to setup a Ratings and Reviews feed upload in WooCommerce using the feed upload framework. RatingsAndReviewsFeed, RatingsAndReviewsFeedGenerator, and RatingsAndReviewsFeedHandler are created to handle the feed upload for Ratings and Reviews. FeedManager is updated to include the new Ratings and Reviews feed upload. The feed upload is setup to run once a week. FeedUploadUtils class was created to hold helper functions for the various feed uploads to prevent code duplication. CsvFeedFileWriter was also updated to use fputcsv so that commas in the content being written to the feed upload file doesn't mess up the parsing/ingestion. - [x] Do the changed files pass `phpcs` checks? Please remove `phpcs:ignore` comments in changed files and fix any issues, or delete if not practical. ### Detailed test instructions: 1. Once the hourly heartbeat cron job is run you'll find wc_facebook_regenerate_feed_ratings_and_reviews listed as pending in the scheduled actions UI. The action can be triggered to be run whenever you want to kick off the ratings and reviews feed upload.  2. Added some ratings and reviews to the product in my test shop.  3. Modified regenerate_feed locally for RatingsAndReviewsFeed to force the feed handler to be used to run the feed upload. Modified the title set in FeedUploadUtils::get_ratings_and_reviews_data to "Feed Handler Review" so that I could verify that the reviews were synced to Meta using the feed handler. 4. Triggered the ratings and reviews feed upload action to run. Verified that there was a log on the Meta side for the API call, a successful feed upload session created, and that the ratings and reviews data was propagated to the right table.    5. Modified regenerate_feed locally for RatingsAndReviewsFeed to force the feed generator to be used to run the feed upload. Modified the title set in FeedUploadUtils::get_ratings_and_reviews_data to "Feed Generator Review" so that I could verify that the reviews were synced to Meta using the feed generator. 6. Triggered the ratings and reviews feed upload action to run. Verified that there was a log on the Meta side for the API call, a successful feed upload session created, and that the ratings and reviews data was propagated to the right table.    7. Reverted regenerate_feed and FeedUploadUtils::get_ratings_and_reviews_data to the code found in this PR to manually test the flow without any tweaks. 8. Triggered the ratings and reviews feed upload action to run. Verified that there was a log on the Meta side for the API call, a successful feed upload session created, and that the ratings and reviews data was propagated to the right table.    Pull Request resolved: #2937 Test Plan: Imported from GitHub, without a `Test Plan:` line. **!---- (auto-generated) DO NOT EDIT OR PUT ANYTHING AFTER THIS LINE ----!** MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: bloks / Diff Version V3 https://internalfb.com/intern/testinfra/testrun/4785074874512106 MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: www / Diff Version V3 https://internalfb.com/intern/testinfra/testrun/2251800086528751 Reviewed By: carterbuce Differential Revision: D71008268 Pulled By: nrostrow-meta fbshipit-source-id: a4b4aaa7a962e73870b94ece22a5b219228454b7
Summary: ### Changes proposed in this Pull Request: This PR builds on #2875 and #2937 to setup a Promotions feed upload in WooCommerce using the feed upload framework. - [ ] Do the changed files pass `phpcs` checks? Please remove `phpcs:ignore` comments in changed files and fix any issues, or delete if not practical. ### Detailed test instructions: 1. Add coupons to test shop <img width="2053" alt="image" src="https://github.com/user-attachments/assets/2d67e804-bc44-4bfc-8445-7d67dcc1d6f5" /> 2. Ran the feed upload job. See #2937 for detailed instructions 3. Examine output feed file [promotions_feed_d5cdc5bb95ba79cb039a10968782cd78 (4).csv](https://github.com/user-attachments/files/19302224/promotions_feed_d5cdc5bb95ba79cb039a10968782cd78.4.csv) 4. Verify promotions were created on Meta: <img width="1283" alt="image" src="https://github.com/user-attachments/assets/be58af8c-5280-4d41-8bc7-18035590e7c6" /> ### Changelog entry > Added Coupon Feed Sync to Meta Pull Request resolved: #2941 Reviewed By: mradmeta, nrostrow-meta Differential Revision: D71084811 Pulled By: carterbuce fbshipit-source-id: 00d2458bcbe9e1ce1e6217a25651cbfd52733295
Summary: ### Changes proposed in this Pull Request: This PR builds on facebook#2875 and facebook#2937 to setup a Promotions feed upload in WooCommerce using the feed upload framework. - [ ] Do the changed files pass `phpcs` checks? Please remove `phpcs:ignore` comments in changed files and fix any issues, or delete if not practical. ### Detailed test instructions: 1. Add coupons to test shop <img width="2053" alt="image" src="https://github.com/user-attachments/assets/2d67e804-bc44-4bfc-8445-7d67dcc1d6f5" /> 2. Ran the feed upload job. See facebook#2937 for detailed instructions 3. Examine output feed file [promotions_feed_d5cdc5bb95ba79cb039a10968782cd78 (4).csv](https://github.com/user-attachments/files/19302224/promotions_feed_d5cdc5bb95ba79cb039a10968782cd78.4.csv) 4. Verify promotions were created on Meta: <img width="1283" alt="image" src="https://github.com/user-attachments/assets/be58af8c-5280-4d41-8bc7-18035590e7c6" /> ### Changelog entry > Added Coupon Feed Sync to Meta Pull Request resolved: facebook#2941 Reviewed By: mradmeta, nrostrow-meta Differential Revision: D71084811 Pulled By: carterbuce fbshipit-source-id: 00d2458bcbe9e1ce1e6217a25651cbfd52733295
Summary: ### Changes proposed in this Pull Request: This PR builds on #2875 and #2937 to setup a Promotions feed upload in WooCommerce using the feed upload framework. - [ ] Do the changed files pass `phpcs` checks? Please remove `phpcs:ignore` comments in changed files and fix any issues, or delete if not practical. ### Detailed test instructions: 1. Add coupons to test shop <img width="2053" alt="image" src="https://github.com/user-attachments/assets/2d67e804-bc44-4bfc-8445-7d67dcc1d6f5" /> 2. Ran the feed upload job. See #2937 for detailed instructions 3. Examine output feed file [promotions_feed_d5cdc5bb95ba79cb039a10968782cd78 (4).csv](https://github.com/user-attachments/files/19302224/promotions_feed_d5cdc5bb95ba79cb039a10968782cd78.4.csv) 4. Verify promotions were created on Meta: <img width="1283" alt="image" src="https://github.com/user-attachments/assets/be58af8c-5280-4d41-8bc7-18035590e7c6" /> ### Changelog entry > Added Coupon Feed Sync to Meta Pull Request resolved: #2941 Reviewed By: mradmeta, nrostrow-meta Differential Revision: D71084811 Pulled By: carterbuce fbshipit-source-id: 00d2458bcbe9e1ce1e6217a25651cbfd52733295
Summary: = 3.5.0 - 2025-05-28 = * Add - Create basic checkout permalink w/ products and coupon support by ajello-meta in #2887 * Add - Common Feed Upload Framework by Jmencab in #2875 * Fix - Fix bug where templates were not loading correctly by ajello-meta in #2915 * Tweak - Change MICE to use base site url instead of shop url by carterbuce in #2934 * Tweak - Improve custom checkout UI by ajello-meta in #2930 * Tweak - Make custom checkout UI mobile compatible by ajello-meta in #2942 * Fix - Update parsing for Checkout URL Product IDs by carterbuce in #2935 * Add - Implement dummy logging util by nealweiMeta in #2920 * Add - Setup cron job for batch logging with global message queue by nealweiMeta in #2924 * Add - Error log request api activate by nealweiMeta in #2933 * Add - Log locally with debug mode enabled by nealweiMeta in #2939 * Add - Ratings and reviews feed upload by nrostrow-meta in #2937 * Tweak - Feed upload skip logic and logging calls by nrostrow-meta in #2964 * Add - Add function to fetch feed upload instance by nrostrow-meta in #2970 * Tweak - Have feed uploads always use feed generator by nrostrow-meta in #2971 * Tweak - Trigger metadata feed uploads on CPI ID change (post onboarding) by nrostrow-meta in #2995 * Add - Shipping profile feed upload button by nrostrow-meta in #3140 * Add - Navigation menu feed upload logic by nrostrow-meta in #3159 * Fix - Fixing some fclose and logging gaps in the feed upload logic by nrostrow-meta in #3192 * Add - Enabling navigation menu feed upload and adding manual sync button by nrostrow-meta in #3223 * Add - Promotions feed upload by carterbuce in #2941 * Add - Plugin AJAX API Framework by sol-loup in #2928 * Tweak - Test Infrastructure Enhancement by sol-loup in #2944 * Add - Implement telemetry logs api by nealweiMeta in #2940 * Fix - Make error logging event configurable by nealweiMeta in #2954 * Add - Implement logging toggle by nealweiMeta in #2959 * Fix - auto products sync by nealweiMeta in #2978 * Tweak - Sync products with restriction by nealweiMeta in #2983 * Fix - Fix use_enhanced_onboarding for legacy connections by carterbuce in #2986 * Add - Create enhanced settings UI by ajello-meta in #2968 * Add - Create new troubleshooting drawer from legacy debug settings by ajello-meta in #2977 * Add - Add manual product and coupon sync buttons by ajello-meta in #2984 * Tweak - Make page title in enhanced settings static by ajello-meta in #2985 * Tweak - Align finalized content for logging toggle by nealweiMeta in #2992 * Tweak - Improve local log by nealweiMeta in #3009 * Fix - Fix free shipping coupon sync by carterbuce in #2993 * Tweak - Add logging for feed generation scheduling failure by carterbuce in #2994 * Tweak - Add logging in checkout for coupon code by ajello-meta in #2991 * Tweak - Clean up CSS in enhanced settings UI by ajello-meta in #2996 * Tweak - Remove the "Advertise" tab by ajello-meta in #3024 * Tweak - Sync "Usage Count" in Promos Feed by carterbuce in #3036 * Tweak - Disable mini_shops product capability for unsupported items by carterbuce in #3084 * Add - Add usage logging for enhanced settings tabs by ajello-meta in #3202 * Tweak - Remove UI of a checkbox that controls enablement of the new style feed generation by mshymon in #3056 * Fix - Fix linter errors for ./includes/fbutils.php files by ajello-meta in #3075 * Fix - Hotfix for Rollout Switches by vinkmeta in #3236 * Add - Opt out sync experience. by SayanPandey in #3220 * Fix - Added a transient flag to avoid flooding of product set api requests by vinkmeta in #3245 * Fix - Additional check for the opt-out banner by SayanPandey in #3259 * Fix - Bump up GraphAPI version to 21 by vahidkay-meta in #3219 * Fix - fix linter errors for ./class-wc-facebookcommerce.php by ajello-meta in #3255 * Fix - fix linter errors for ./facebook-commerce-events-tracker.php by ajello-meta in #3254 * Fix - fix linter errors for ./includes/Admin/Settings_Screens/Advertise.php by ajello-meta in #3237 * Fix - fix linter errors for ./includes/Admin/Settings_Screens/Product_Sync.php by ajello-meta in #3239 * Fix - fix function return typing for get_settings() by ajello-meta in #3257 * Tweak - Addition check for opt out by SayanPandey in #3259 * Tweak - Update the GraphAPI version to 21 by vahidkay-meta in #3219 * Fix - Enabled rollout switch only for plugin admins by vinkmeta in #3242 * Add - reset connection functionality by jczhuoMeta in #3262 * Fix - fixing the non static method called as static issue by SayanPandey in #3263 * Fix - Fix linter errors for ./facebook-commerce.php by ajello-meta in #3251 Pull Request resolved: #3264 Test Plan: Imported from GitHub, without a `Test Plan:` line. **!---- (auto-generated) DO NOT EDIT OR PUT ANYTHING AFTER THIS LINE ----!** MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: bloks / Diff Version V7 https://internalfb.com/intern/testinfra/testrun/10414574226197703 MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: www / Diff Version V7 https://internalfb.com/intern/testinfra/testrun/844425351364266 Reviewed By: vinkmeta, nrostrow-meta Differential Revision: D75569981 Pulled By: carterbuce fbshipit-source-id: 11bb39a9d66dfcbc80af413466feef0792d27ce4
Changes proposed in this Pull Request:
Changes in detail here: https://docs.google.com/document/d/1PaXHKK1PRVtiBEwJtH_8ZP8h0UitDCXZnpFFDWo-eik/edit?tab=t.0#heading=h.m8p14s8600vn
The main idea is to add a FeedManager singleton that will manage all the distinct Feed singletons. The Feed singletons will inherit from an AbstractFeed which will delegate the actual file creation to the Feed Handler or Feed Generator given the admin preferences. With that being said, the sequence will look similar to the existing product flow, but with the above simplifications:
Closes # .
Detailed test instructions:
Added an API test as done in this PR: #2841
Integration test not yet possible as dependent on Mice framework addition and Ent creation
Added comments for how to test/dogfood as well as included instructions in the above linked doc
Successful upload via Feed Handler:
https://www.internalfb.com/intern/cat/sessions/642042718535396/
Successful upload via Feed Generator:
https://www.internalfb.com/intern/cat/sessions/642101398529528/
See branch common_framework_test to see an example proof of concept that utilizes the base classes.
This will just include what will be checked in via diff.
Unit Tests
A. Run new tests:
./vendor/bin/phpunit --filter ApiTest
B. Run all tests:
npm run test:php
Manual Tests.
Steps were added via an internal dogfooding doc and requires access to Meta internal tools to validate feed upload sessions status.
phpcschecks? Please removephpcs:ignorecomments in changed files and fix any issues, or delete if not practical.