Skip to content

Conditional feed upload scheduling#2964

Closed
nrostrow-meta wants to merge 22 commits intomainfrom
conditional_feed_upload_scheduling
Closed

Conditional feed upload scheduling#2964
nrostrow-meta wants to merge 22 commits intomainfrom
conditional_feed_upload_scheduling

Conversation

@nrostrow-meta
Copy link
Copy Markdown
Contributor

Description

This PR makes the following changes:

  • Updates AbstractFeed.php to only have feed uploads that extend it scheduled/regenerated if CPI ID is set for the instance as CPI ID is required for the API call to GraphCommercePartnerIntegrationFileUpdatePost
    • This ensures that the feed uploads will only be run after onboarding is complete as CPI ID is set after onboarding
    • should_skip_feed can be overwritten for future feeds that don't hit the GraphCommercePartnerIntegrationFileUpdatePost endpoint and have different requirements
  • Adds calls to logExceptionImmediatelyToMeta and logTelemetryToMeta throughout the feed upload flow to persist logs to Meta
  • Creates AbstractFeedTest.php to add some unit tests for the AbstractFeed.php logic

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Test instructions

Verified that the ratings and reviews feed upload works as expected after the changes

  1. Pushed changes to my lightsail instance
  2. Triggered the ratings and reviews feed upload by running the pending wc_facebook_regenerate_feed_ratings_and_reviews action
    Screenshot 2025-03-24 at 11 23 44 AM
    Screenshot 2025-03-24 at 11 23 55 AM
  3. Verified that there was a log for the API call to GraphCommercePartnerIntegrationFileUpdatePost for the feed upload
    Screenshot 2025-03-24 at 11 25 22 AM
  4. Verified that there was a successful feed upload session
    Screenshot 2025-03-24 at 11 28 24 AM

Verified that the ratings and reviews feed upload is not run if CPI ID isn't set for the instance

  1. Locally modified should_skip_feed to always return true
  2. Pushed changes to my lightsail instance
  3. Tried to trigger the ratings and reviews feed upload by running the pending wc_facebook_regenerate_feed_ratings_and_reviews action
  4. Verified that there were no logs for an API call to GraphCommercePartnerIntegrationFileUpdatePost and that there was no feed upload session created

Verified that the added logging calls work as expected

  1. Locally modified start of each try block associated with the catch blocks that persists log to Meta to throw PluginException
  2. Pushed changes to my lightsail instance
  3. Verified that logs were persisted to Meta as expected
    Screenshot 2025-03-24 at 12 35 05 PM
    Screenshot 2025-03-24 at 12 42 48 PM
    Screenshot 2025-03-24 at 12 43 40 PM
    Screenshot 2025-03-24 at 1 01 46 PM
    Screenshot 2025-03-24 at 1 06 32 PM

Verified that all tests in AbstractFeedTest.php pass as expected

  1. Triggered all tests in AbstractFeedTest.php to run through PHPStorm
  2. Verified that all the tests passed
    Screenshot 2025-03-24 at 12 25 23 PM

Checklist

  • I followed general Pull Request best practices. Meta employees to follow this wiki
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests and all the new and existing unit tests pass locally with my changes
  • I have completed dogfooding and QA testing, or I have conducted thorough due diligence to ensure that it does not break existing functionality.

Changelog entry

Adding logging to feed upload flow and preventing feed uploads from being scheduled/regenerated until after CPI ID is set on the instance

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@nrostrow-meta merged this pull request in 41fae1d.

@sol-loup sol-loup mentioned this pull request Mar 25, 2025
7 tasks
SayanPandey pushed a commit to SayanPandey/facebook-for-woocommerce that referenced this pull request Apr 1, 2025
Summary:
## Description

This PR makes the following changes:
- Updates AbstractFeed.php to only have feed uploads that extend it scheduled/regenerated if CPI ID is set for the instance as CPI ID is required for the API call to GraphCommercePartnerIntegrationFileUpdatePost
   - This ensures that the feed uploads will only be run after onboarding is complete as CPI ID is set after onboarding
   - should_skip_feed can be overwritten for future feeds that don't hit the GraphCommercePartnerIntegrationFileUpdatePost endpoint and have different requirements
- Adds calls to logExceptionImmediatelyToMeta and logTelemetryToMeta throughout the feed upload flow to persist logs to Meta
- Creates AbstractFeedTest.php to add some unit tests for the AbstractFeed.php logic

### Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)

## Test instructions
Verified that the ratings and reviews feed upload works as expected after the changes
1. Pushed changes to my lightsail instance
2. Triggered the ratings and reviews feed upload by running the pending wc_facebook_regenerate_feed_ratings_and_reviews action
![Screenshot 2025-03-24 at 11 23 44 AM](https://github.com/user-attachments/assets/63b1ddec-f68b-4068-9702-1ae23b29d3e6)
![Screenshot 2025-03-24 at 11 23 55 AM](https://github.com/user-attachments/assets/b9143c40-67e9-4677-b6d0-2b870bbcebe7)
3. Verified that there was a log for the API call to GraphCommercePartnerIntegrationFileUpdatePost for the feed upload
![Screenshot 2025-03-24 at 11 25 22 AM](https://github.com/user-attachments/assets/5645a9e2-0fcb-4f5f-ac33-0a651d7c57ec)
4. Verified that there was a successful feed upload session
![Screenshot 2025-03-24 at 11 28 24 AM](https://github.com/user-attachments/assets/f11960f7-69b4-4632-bcb5-b2f2097713c9)

Verified that the ratings and reviews feed upload is not run if CPI ID isn't set for the instance
1. Locally modified should_skip_feed to always return true
2. Pushed changes to my lightsail instance
3. Tried to trigger the ratings and reviews feed upload by running the pending wc_facebook_regenerate_feed_ratings_and_reviews action
4. Verified that there were no logs for an API call to GraphCommercePartnerIntegrationFileUpdatePost and that there was no feed upload session created

Verified that the added logging calls work as expected
1. Locally modified start of each try block associated with the catch blocks that persists log to Meta to throw PluginException
2. Pushed changes to my lightsail instance
3. Verified that logs were persisted to Meta as expected
![Screenshot 2025-03-24 at 12 35 05 PM](https://github.com/user-attachments/assets/045151ab-2c5e-4442-86da-dd92521719d7)
![Screenshot 2025-03-24 at 12 42 48 PM](https://github.com/user-attachments/assets/cd27ea99-0dac-4f78-a9eb-154eef686f70)
![Screenshot 2025-03-24 at 12 43 40 PM](https://github.com/user-attachments/assets/a3ac0903-0492-4b37-8eae-038b88be5714)
![Screenshot 2025-03-24 at 1 01 46 PM](https://github.com/user-attachments/assets/3b753626-07c0-4b07-a1df-30c32edaea49)
![Screenshot 2025-03-24 at 1 06 32 PM](https://github.com/user-attachments/assets/bd1a5084-790e-4cfc-b92b-b8d7d1b7255a)

Verified that all tests in AbstractFeedTest.php pass as expected
1. Triggered all tests in AbstractFeedTest.php to run through PHPStorm
2. Verified that all the tests passed
![Screenshot 2025-03-24 at 12 25 23 PM](https://github.com/user-attachments/assets/134fd549-e5a5-42ba-ab33-d68fc6bf53f1)

## Checklist

- [x] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc))
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added tests and all the new and existing unit tests pass locally with my changes
- [x] I have completed dogfooding and QA testing, or I have conducted thorough due diligence to ensure that it does not break existing functionality.

## Changelog entry

Adding logging to feed upload flow and preventing feed uploads from being scheduled/regenerated until after CPI ID is set on the instance

Pull Request resolved: facebook#2964

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: www / Diff Version V2
https://internalfb.com/intern/testinfra/testrun/2251800088604063
MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: bloks / Diff Version V2
https://internalfb.com/intern/testinfra/testrun/9007199329833336

Reviewed By: carterbuce, sol-loup, ajello-meta

Differential Revision: D71760562

Pulled By: nrostrow-meta

fbshipit-source-id: 640b609f6b9e584f3c3f20157723f7f8ecd38db8
mradmeta pushed a commit that referenced this pull request Apr 14, 2025
Summary:
## Description

This PR makes the following changes:
- Updates AbstractFeed.php to only have feed uploads that extend it scheduled/regenerated if CPI ID is set for the instance as CPI ID is required for the API call to GraphCommercePartnerIntegrationFileUpdatePost
   - This ensures that the feed uploads will only be run after onboarding is complete as CPI ID is set after onboarding
   - should_skip_feed can be overwritten for future feeds that don't hit the GraphCommercePartnerIntegrationFileUpdatePost endpoint and have different requirements
- Adds calls to logExceptionImmediatelyToMeta and logTelemetryToMeta throughout the feed upload flow to persist logs to Meta
- Creates AbstractFeedTest.php to add some unit tests for the AbstractFeed.php logic

### Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)

## Test instructions
Verified that the ratings and reviews feed upload works as expected after the changes
1. Pushed changes to my lightsail instance
2. Triggered the ratings and reviews feed upload by running the pending wc_facebook_regenerate_feed_ratings_and_reviews action
![Screenshot 2025-03-24 at 11 23 44 AM](https://github.com/user-attachments/assets/63b1ddec-f68b-4068-9702-1ae23b29d3e6)
![Screenshot 2025-03-24 at 11 23 55 AM](https://github.com/user-attachments/assets/b9143c40-67e9-4677-b6d0-2b870bbcebe7)
3. Verified that there was a log for the API call to GraphCommercePartnerIntegrationFileUpdatePost for the feed upload
![Screenshot 2025-03-24 at 11 25 22 AM](https://github.com/user-attachments/assets/5645a9e2-0fcb-4f5f-ac33-0a651d7c57ec)
4. Verified that there was a successful feed upload session
![Screenshot 2025-03-24 at 11 28 24 AM](https://github.com/user-attachments/assets/f11960f7-69b4-4632-bcb5-b2f2097713c9)

Verified that the ratings and reviews feed upload is not run if CPI ID isn't set for the instance
1. Locally modified should_skip_feed to always return true
2. Pushed changes to my lightsail instance
3. Tried to trigger the ratings and reviews feed upload by running the pending wc_facebook_regenerate_feed_ratings_and_reviews action
4. Verified that there were no logs for an API call to GraphCommercePartnerIntegrationFileUpdatePost and that there was no feed upload session created

Verified that the added logging calls work as expected
1. Locally modified start of each try block associated with the catch blocks that persists log to Meta to throw PluginException
2. Pushed changes to my lightsail instance
3. Verified that logs were persisted to Meta as expected
![Screenshot 2025-03-24 at 12 35 05 PM](https://github.com/user-attachments/assets/045151ab-2c5e-4442-86da-dd92521719d7)
![Screenshot 2025-03-24 at 12 42 48 PM](https://github.com/user-attachments/assets/cd27ea99-0dac-4f78-a9eb-154eef686f70)
![Screenshot 2025-03-24 at 12 43 40 PM](https://github.com/user-attachments/assets/a3ac0903-0492-4b37-8eae-038b88be5714)
![Screenshot 2025-03-24 at 1 01 46 PM](https://github.com/user-attachments/assets/3b753626-07c0-4b07-a1df-30c32edaea49)
![Screenshot 2025-03-24 at 1 06 32 PM](https://github.com/user-attachments/assets/bd1a5084-790e-4cfc-b92b-b8d7d1b7255a)

Verified that all tests in AbstractFeedTest.php pass as expected
1. Triggered all tests in AbstractFeedTest.php to run through PHPStorm
2. Verified that all the tests passed
![Screenshot 2025-03-24 at 12 25 23 PM](https://github.com/user-attachments/assets/134fd549-e5a5-42ba-ab33-d68fc6bf53f1)

## Checklist

- [x] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc))
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added tests and all the new and existing unit tests pass locally with my changes
- [x] I have completed dogfooding and QA testing, or I have conducted thorough due diligence to ensure that it does not break existing functionality.

## Changelog entry

Adding logging to feed upload flow and preventing feed uploads from being scheduled/regenerated until after CPI ID is set on the instance

Pull Request resolved: #2964

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: www / Diff Version V2
https://internalfb.com/intern/testinfra/testrun/2251800088604063
MFTRunTestsScript Run / Test Suite: sa_checkout / Test Collection: bloks / Diff Version V2
https://internalfb.com/intern/testinfra/testrun/9007199329833336

Reviewed By: carterbuce, sol-loup, ajello-meta

Differential Revision: D71760562

Pulled By: nrostrow-meta

fbshipit-source-id: 640b609f6b9e584f3c3f20157723f7f8ecd38db8
@carterbuce carterbuce mentioned this pull request May 29, 2025
@tzahgr tzahgr mentioned this pull request Jun 4, 2025
facebook-github-bot pushed a commit that referenced this pull request Jun 4, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants