Fix feed upload error "feed_column_count_mismatch" when product has multiple colors separated by a coma#2947
Closed
mshymon wants to merge 2 commits intofacebook:mainfrom
Closed
Conversation
Contributor
|
Do we properly handle comma-based escaping for all fields in the feed system? Was this the only problematic field? |
Contributor
Author
Actually, I will update this PR for other fields too as some of them are definitely vulnerable to this error. |
Contributor
|
@mshymon has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
@mshymon has updated the pull request. You must reimport the pull request before landing. |
Contributor
|
@mshymon has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
Contributor
|
@mshymon has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
tzahgr
pushed a commit
that referenced
this pull request
Mar 17, 2025
…ultiple colors separated by a coma (#2947) Summary: ### Changes proposed in this Pull Request: When rolling out product sync via Feeds, we noticed a fatal upload error for products that have value of a color to be multiple colors written with commas (e.g. color value: Yellow,Blue,Red). This case right leads to fatal error "feed_column_count_mismatch" on feed upload and the product is deleted from the catalog. The fix is simple what we already fo for other fields like `name` or `description`: wrap it up with quotes by calling `format_string_for_feed`. Applying this not only to color but some other fields too. Manually tested feed generation and feed upload success. ### Additional details: Screenshot shows multi color value for a single product written via comas that leads to feed sync issue before this fix. <img width="1857" alt="Screenshot 2025-03-14 at 17 10 11" src="https://github.com/user-attachments/assets/1fdffc98-b245-4c86-a479-1688a2bd2aa0" /> ### Detailed test instructions: 1. Create or update an existing product to have multi color value written via comas, as per screenshot. 2. Trigger feed generation and check for errors. ### Additional details: <!-- Optional. Enter a summary of all changes in this Pull Request, which will be added to the changelog if accepted. Each line should start with change type prefix`(Fix|Add|…) - `, for example: > Break - A change breaking previous API or functionality. > Add - A new feature, function or functionality was added. > Update - Big changes to something that wasn't broken. > Fix - Took care of something that wasn't working. > Tweak - Small change, that isn't actually very important. > Dev - Developer-facing only change. > Doc - Updated customer or developer facing documentation If you remove the "Changelog entry" header, the Pull Request title will be used as the changelog entry. Add the `changelog: none` label if no changelog entry is needed. --> ### Changelog entry > Fix - feed upload error "feed_column_count_mismatch" when product has multiple colors separated by a coma. Pull Request resolved: #2947 Reviewed By: vinkmeta Differential Revision: D71206033 Pulled By: mshymon fbshipit-source-id: da898f1bf0d3cb701098bb1bc72f85942d97675d
Merged
vahidkay-meta
added a commit
that referenced
this pull request
Mar 24, 2025
Add - Items batch request and response tests by @nrostrow-meta in #2917 Tweak - Always run PHP-based github workflows by @carterbuce in #2926 Fix - feed upload error "feed_column_count_mismatch" when product has multiple colors separated by a comma by @mshymon in #2947 Tweak - Updated Pull Request Template by @vinkmeta in #2948 Tweak - Enabled PHPUnit code coverage report generation by @carterbuce in #2893 Dev - Improved readability of function prepare_product() in fbproduct.php by @mshymon in #2889
SayanPandey
pushed a commit
to SayanPandey/facebook-for-woocommerce
that referenced
this pull request
Apr 1, 2025
…ultiple colors separated by a coma (facebook#2947) Summary: ### Changes proposed in this Pull Request: When rolling out product sync via Feeds, we noticed a fatal upload error for products that have value of a color to be multiple colors written with commas (e.g. color value: Yellow,Blue,Red). This case right leads to fatal error "feed_column_count_mismatch" on feed upload and the product is deleted from the catalog. The fix is simple what we already fo for other fields like `name` or `description`: wrap it up with quotes by calling `format_string_for_feed`. Applying this not only to color but some other fields too. Manually tested feed generation and feed upload success. ### Additional details: Screenshot shows multi color value for a single product written via comas that leads to feed sync issue before this fix. <img width="1857" alt="Screenshot 2025-03-14 at 17 10 11" src="https://github.com/user-attachments/assets/1fdffc98-b245-4c86-a479-1688a2bd2aa0" /> ### Detailed test instructions: 1. Create or update an existing product to have multi color value written via comas, as per screenshot. 2. Trigger feed generation and check for errors. ### Additional details: <!-- Optional. Enter a summary of all changes in this Pull Request, which will be added to the changelog if accepted. Each line should start with change type prefix`(Fix|Add|…) - `, for example: > Break - A change breaking previous API or functionality. > Add - A new feature, function or functionality was added. > Update - Big changes to something that wasn't broken. > Fix - Took care of something that wasn't working. > Tweak - Small change, that isn't actually very important. > Dev - Developer-facing only change. > Doc - Updated customer or developer facing documentation If you remove the "Changelog entry" header, the Pull Request title will be used as the changelog entry. Add the `changelog: none` label if no changelog entry is needed. --> ### Changelog entry > Fix - feed upload error "feed_column_count_mismatch" when product has multiple colors separated by a coma. Pull Request resolved: facebook#2947 Reviewed By: vinkmeta Differential Revision: D71206033 Pulled By: mshymon fbshipit-source-id: da898f1bf0d3cb701098bb1bc72f85942d97675d
This was referenced Apr 10, 2025
Closed
Closed
This was referenced Apr 18, 2025
This was referenced May 2, 2025
5 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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:
When rolling out product sync via Feeds, we noticed a fatal upload error for products that have value of a color to be multiple colors written with commas (e.g. color value: Yellow,Blue,Red). This case right leads to fatal error "feed_column_count_mismatch" on feed upload and the product is deleted from the catalog.
The fix is simple what we already fo for other fields like
nameordescription: wrap it up with quotes by callingformat_string_for_feed.Applying this not only to color but some other fields too. Manually tested feed generation and feed upload success.
Additional details:
Screenshot shows multi color value for a single product written via comas that leads to feed sync issue before this fix.
Detailed test instructions:
Additional details:
Changelog entry