Closed
Conversation
Contributor
|
Hi @raymon1! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
from
d5dc183 to
938ec5b
Compare
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
from
938ec5b to
a291ccc
Compare
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
from
a291ccc to
37a3e7e
Compare
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Collaborator
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
2 times, most recently
from
8fb3a56 to
14f457b
Compare
6 tasks
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
from
14f457b to
1358525
Compare
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
from
1358525 to
b6c4492
Compare
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
raymon1
force-pushed
the
feature/relaxing_sync_validation
branch
from
b6c4492 to
8255399
Compare
Contributor
|
@raymon1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
facebook-github-bot
pushed a commit
that referenced
this pull request
Apr 8, 2025
Summary: ## Description in #2969 the validations were relaxed, this might cause some backend issues. In this diff, in order to sync products, we are truncating any extra characters form the title and description ### Type of change - New feature (non-breaking change which adds functionality) ## Checklist - [X] I have commented my code, particularly in hard-to-understand areas. - [X] I have confirmed that my changes do not introduce any new PHPCS warnings or errors. - [X] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc)). - [X] I have added tests (if necessary) 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. - [X] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki]([url](https://fburl.com/wiki/nhx73tgs)). ## Changelog entry Truncates extra characters from title and description Pull Request resolved: #3023 Test Plan: Create item with more than the limits of 150 character for title and more than 5000 for description Before: The title/description will be missing if both title and description are above limit, the item won't be synced After: Truncates (title/description) <img width="2056" alt="image" src="https://github.com/user-attachments/assets/4eb494c7-d070-485a-8b6d-e9ff187fedfb" /> synced <img width="2056" alt="image" src="https://github.com/user-attachments/assets/01b0a61d-3fe2-4a92-9a01-e2e263ad76a5" /> ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before ### After Reviewed By: vinkmeta, SayanPandey Differential Revision: D72567584 Pulled By: raymon1 fbshipit-source-id: 17a6b279f1a3ce5d55237b885c3d349b9b6abdfb
SayanPandey
pushed a commit
to SayanPandey/facebook-for-woocommerce
that referenced
this pull request
Apr 8, 2025
Summary: ## Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ### Type of change Please delete options that are not relevant. - [X] New feature (non-breaking change which adds functionality) | # | Field | Validation | Change | | --- | --- | --- | ---| | 1 | Description | ALL Cap | Remove | | 2 | Description | length(5000) | Remove | | 3 | Price | If missing | Remove| | 4 | Title | length(150) | Remove | | 5 | validate_product_visibility | if hidden| Keep for now (we need to sync items as hidden in case they are not visible) | | 6 | Out of stock | if woocommerce_hide_out_of_stock_items we don't sync | Removed in another PR | | 7 | product_status | ALL Cap | Keep (part of other efforts) | | 8 | validate_product_sync_field| | Keep | | 9 | validate_product_terms| Check whether the product's categories or tags (terms) exclude it from sync.| Keep | Note: I am not super sure about (5) validate_product_visibility, I have removed it, but the item is being synced as active ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before In the before nothing synced showing error in the meta_box component ### After <img width="1280" alt="image" src="https://github.com/user-attachments/assets/195d2190-cbbb-493c-aee4-94a9fb0bbaf8" /> https://www.facebook.com/commerce/catalogs/955650953442091/products As you can see, I have synced items with different validations to ensure syncing is happening ## Test instructions Create a product with a validation issue and try saving and check if it's being synced or not ## 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 Relaxing sync validations Pull Request resolved: facebook#2969 Reviewed By: vinkmeta Differential Revision: D71912613 Pulled By: raymon1 fbshipit-source-id: 8e358749664d426f809711a889a6665d257f9a4c
Closed
6 tasks
vinkmeta
pushed a commit
that referenced
this pull request
Apr 13, 2025
Summary: ## Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ### Type of change Please delete options that are not relevant. - [X] New feature (non-breaking change which adds functionality) | # | Field | Validation | Change | | --- | --- | --- | ---| | 1 | Description | ALL Cap | Remove | | 2 | Description | length(5000) | Remove | | 3 | Price | If missing | Remove| | 4 | Title | length(150) | Remove | | 5 | validate_product_visibility | if hidden| Keep for now (we need to sync items as hidden in case they are not visible) | | 6 | Out of stock | if woocommerce_hide_out_of_stock_items we don't sync | Removed in another PR | | 7 | product_status | ALL Cap | Keep (part of other efforts) | | 8 | validate_product_sync_field| | Keep | | 9 | validate_product_terms| Check whether the product's categories or tags (terms) exclude it from sync.| Keep | Note: I am not super sure about (5) validate_product_visibility, I have removed it, but the item is being synced as active ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before In the before nothing synced showing error in the meta_box component ### After <img width="1280" alt="image" src="https://github.com/user-attachments/assets/195d2190-cbbb-493c-aee4-94a9fb0bbaf8" /> https://www.facebook.com/commerce/catalogs/955650953442091/products As you can see, I have synced items with different validations to ensure syncing is happening ## Test instructions Create a product with a validation issue and try saving and check if it's being synced or not ## 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 Relaxing sync validations Pull Request resolved: #2969 Reviewed By: vinkmeta Differential Revision: D71912613 Pulled By: raymon1 fbshipit-source-id: 8e358749664d426f809711a889a6665d257f9a4c
vinkmeta
pushed a commit
that referenced
this pull request
Apr 13, 2025
Summary: ## Description in #2969 the validations were relaxed, this might cause some backend issues. In this diff, in order to sync products, we are truncating any extra characters form the title and description ### Type of change - New feature (non-breaking change which adds functionality) ## Checklist - [X] I have commented my code, particularly in hard-to-understand areas. - [X] I have confirmed that my changes do not introduce any new PHPCS warnings or errors. - [X] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc)). - [X] I have added tests (if necessary) 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. - [X] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki]([url](https://fburl.com/wiki/nhx73tgs)). ## Changelog entry Truncates extra characters from title and description Pull Request resolved: #3023 Test Plan: Create item with more than the limits of 150 character for title and more than 5000 for description Before: The title/description will be missing if both title and description are above limit, the item won't be synced After: Truncates (title/description) <img width="2056" alt="image" src="https://github.com/user-attachments/assets/4eb494c7-d070-485a-8b6d-e9ff187fedfb" /> synced <img width="2056" alt="image" src="https://github.com/user-attachments/assets/01b0a61d-3fe2-4a92-9a01-e2e263ad76a5" /> ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before ### After Reviewed By: vinkmeta, SayanPandey Differential Revision: D72567584 Pulled By: raymon1 fbshipit-source-id: 17a6b279f1a3ce5d55237b885c3d349b9b6abdfb
vahidkay-meta
pushed a commit
that referenced
this pull request
Apr 14, 2025
Summary: ## Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ### Type of change Please delete options that are not relevant. - [X] New feature (non-breaking change which adds functionality) | # | Field | Validation | Change | | --- | --- | --- | ---| | 1 | Description | ALL Cap | Remove | | 2 | Description | length(5000) | Remove | | 3 | Price | If missing | Remove| | 4 | Title | length(150) | Remove | | 5 | validate_product_visibility | if hidden| Keep for now (we need to sync items as hidden in case they are not visible) | | 6 | Out of stock | if woocommerce_hide_out_of_stock_items we don't sync | Removed in another PR | | 7 | product_status | ALL Cap | Keep (part of other efforts) | | 8 | validate_product_sync_field| | Keep | | 9 | validate_product_terms| Check whether the product's categories or tags (terms) exclude it from sync.| Keep | Note: I am not super sure about (5) validate_product_visibility, I have removed it, but the item is being synced as active ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before In the before nothing synced showing error in the meta_box component ### After <img width="1280" alt="image" src="https://github.com/user-attachments/assets/195d2190-cbbb-493c-aee4-94a9fb0bbaf8" /> https://www.facebook.com/commerce/catalogs/955650953442091/products As you can see, I have synced items with different validations to ensure syncing is happening ## Test instructions Create a product with a validation issue and try saving and check if it's being synced or not ## 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 Relaxing sync validations Pull Request resolved: #2969 Reviewed By: vinkmeta Differential Revision: D71912613 Pulled By: raymon1 fbshipit-source-id: 8e358749664d426f809711a889a6665d257f9a4c
vahidkay-meta
pushed a commit
that referenced
this pull request
Apr 14, 2025
Summary: ## Description in #2969 the validations were relaxed, this might cause some backend issues. In this diff, in order to sync products, we are truncating any extra characters form the title and description ### Type of change - New feature (non-breaking change which adds functionality) ## Checklist - [X] I have commented my code, particularly in hard-to-understand areas. - [X] I have confirmed that my changes do not introduce any new PHPCS warnings or errors. - [X] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc)). - [X] I have added tests (if necessary) 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. - [X] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki]([url](https://fburl.com/wiki/nhx73tgs)). ## Changelog entry Truncates extra characters from title and description Pull Request resolved: #3023 Test Plan: Create item with more than the limits of 150 character for title and more than 5000 for description Before: The title/description will be missing if both title and description are above limit, the item won't be synced After: Truncates (title/description) <img width="2056" alt="image" src="https://github.com/user-attachments/assets/4eb494c7-d070-485a-8b6d-e9ff187fedfb" /> synced <img width="2056" alt="image" src="https://github.com/user-attachments/assets/01b0a61d-3fe2-4a92-9a01-e2e263ad76a5" /> ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before ### After Reviewed By: vinkmeta, SayanPandey Differential Revision: D72567584 Pulled By: raymon1 fbshipit-source-id: 17a6b279f1a3ce5d55237b885c3d349b9b6abdfb
This was referenced Apr 14, 2025
Closed
mradmeta
pushed a commit
that referenced
this pull request
Apr 14, 2025
Summary: ## Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ### Type of change Please delete options that are not relevant. - [X] New feature (non-breaking change which adds functionality) | # | Field | Validation | Change | | --- | --- | --- | ---| | 1 | Description | ALL Cap | Remove | | 2 | Description | length(5000) | Remove | | 3 | Price | If missing | Remove| | 4 | Title | length(150) | Remove | | 5 | validate_product_visibility | if hidden| Keep for now (we need to sync items as hidden in case they are not visible) | | 6 | Out of stock | if woocommerce_hide_out_of_stock_items we don't sync | Removed in another PR | | 7 | product_status | ALL Cap | Keep (part of other efforts) | | 8 | validate_product_sync_field| | Keep | | 9 | validate_product_terms| Check whether the product's categories or tags (terms) exclude it from sync.| Keep | Note: I am not super sure about (5) validate_product_visibility, I have removed it, but the item is being synced as active ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before In the before nothing synced showing error in the meta_box component ### After <img width="1280" alt="image" src="https://github.com/user-attachments/assets/195d2190-cbbb-493c-aee4-94a9fb0bbaf8" /> https://www.facebook.com/commerce/catalogs/955650953442091/products As you can see, I have synced items with different validations to ensure syncing is happening ## Test instructions Create a product with a validation issue and try saving and check if it's being synced or not ## 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 Relaxing sync validations Pull Request resolved: #2969 Reviewed By: vinkmeta Differential Revision: D71912613 Pulled By: raymon1 fbshipit-source-id: 8e358749664d426f809711a889a6665d257f9a4c
mradmeta
pushed a commit
that referenced
this pull request
Apr 14, 2025
Summary: ## Description in #2969 the validations were relaxed, this might cause some backend issues. In this diff, in order to sync products, we are truncating any extra characters form the title and description ### Type of change - New feature (non-breaking change which adds functionality) ## Checklist - [X] I have commented my code, particularly in hard-to-understand areas. - [X] I have confirmed that my changes do not introduce any new PHPCS warnings or errors. - [X] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc)). - [X] I have added tests (if necessary) 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. - [X] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki]([url](https://fburl.com/wiki/nhx73tgs)). ## Changelog entry Truncates extra characters from title and description Pull Request resolved: #3023 Test Plan: Create item with more than the limits of 150 character for title and more than 5000 for description Before: The title/description will be missing if both title and description are above limit, the item won't be synced After: Truncates (title/description) <img width="2056" alt="image" src="https://github.com/user-attachments/assets/4eb494c7-d070-485a-8b6d-e9ff187fedfb" /> synced <img width="2056" alt="image" src="https://github.com/user-attachments/assets/01b0a61d-3fe2-4a92-9a01-e2e263ad76a5" /> ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before ### After Reviewed By: vinkmeta, SayanPandey Differential Revision: D72567584 Pulled By: raymon1 fbshipit-source-id: 17a6b279f1a3ce5d55237b885c3d349b9b6abdfb
rubycalling
pushed a commit
that referenced
this pull request
Apr 16, 2025
Summary: ## Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ### Type of change Please delete options that are not relevant. - [X] New feature (non-breaking change which adds functionality) | # | Field | Validation | Change | | --- | --- | --- | ---| | 1 | Description | ALL Cap | Remove | | 2 | Description | length(5000) | Remove | | 3 | Price | If missing | Remove| | 4 | Title | length(150) | Remove | | 5 | validate_product_visibility | if hidden| Keep for now (we need to sync items as hidden in case they are not visible) | | 6 | Out of stock | if woocommerce_hide_out_of_stock_items we don't sync | Removed in another PR | | 7 | product_status | ALL Cap | Keep (part of other efforts) | | 8 | validate_product_sync_field| | Keep | | 9 | validate_product_terms| Check whether the product's categories or tags (terms) exclude it from sync.| Keep | Note: I am not super sure about (5) validate_product_visibility, I have removed it, but the item is being synced as active ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before In the before nothing synced showing error in the meta_box component ### After <img width="1280" alt="image" src="https://github.com/user-attachments/assets/195d2190-cbbb-493c-aee4-94a9fb0bbaf8" /> https://www.facebook.com/commerce/catalogs/955650953442091/products As you can see, I have synced items with different validations to ensure syncing is happening ## Test instructions Create a product with a validation issue and try saving and check if it's being synced or not ## 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 Relaxing sync validations Pull Request resolved: #2969 Reviewed By: vinkmeta Differential Revision: D71912613 Pulled By: raymon1 fbshipit-source-id: 8e358749664d426f809711a889a6665d257f9a4c
rubycalling
pushed a commit
that referenced
this pull request
Apr 16, 2025
Summary: ## Description in #2969 the validations were relaxed, this might cause some backend issues. In this diff, in order to sync products, we are truncating any extra characters form the title and description ### Type of change - New feature (non-breaking change which adds functionality) ## Checklist - [X] I have commented my code, particularly in hard-to-understand areas. - [X] I have confirmed that my changes do not introduce any new PHPCS warnings or errors. - [X] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc)). - [X] I have added tests (if necessary) 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. - [X] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki]([url](https://fburl.com/wiki/nhx73tgs)). ## Changelog entry Truncates extra characters from title and description Pull Request resolved: #3023 Test Plan: Create item with more than the limits of 150 character for title and more than 5000 for description Before: The title/description will be missing if both title and description are above limit, the item won't be synced After: Truncates (title/description) <img width="2056" alt="image" src="https://github.com/user-attachments/assets/4eb494c7-d070-485a-8b6d-e9ff187fedfb" /> synced <img width="2056" alt="image" src="https://github.com/user-attachments/assets/01b0a61d-3fe2-4a92-9a01-e2e263ad76a5" /> ## Screenshots Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate ### Before ### After Reviewed By: vinkmeta, SayanPandey Differential Revision: D72567584 Pulled By: raymon1 fbshipit-source-id: 17a6b279f1a3ce5d55237b885c3d349b9b6abdfb
Merged
This was referenced Apr 18, 2025
This was referenced May 2, 2025
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.
Description
Please include a summary of the changes and the related issue.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
Type of change
Please delete options that are not relevant.
Note: I am not super sure about (5) validate_product_visibility, I have removed it, but the item is being synced as active
Screenshots
Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate
Before
In the before nothing synced showing error in the meta_box component
After
As you can see, I have synced items with different validations to ensure syncing is happening
Test instructions
Create a product with a validation issue and try saving and check if it's being synced or not
Checklist
Changelog entry
Relaxing sync validations