Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: race condition by awaiting each publish catalog function by type… #5151

Merged
merged 1 commit into from
Apr 26, 2019

Conversation

nnnnat
Copy link
Contributor

@nnnnat nnnnat commented Apr 26, 2019

Resolves
Impact: critical
Type: bugfix

Issue

publishProductToCatalog function by type would be called synchronously, this would cause partial/incomplete Catalog publish if any of the functions by type were trying to do async operations.

Solution

Await the custom function by type calls for the publishProductToCatalog function.

Breaking changes

N/A

Testing

  1. As an admin "create" and "publish" a new Product.
  2. As an Admin "edit" an existing Product and "publish"

@@ -252,10 +252,11 @@ export default async function createCatalogProduct(product, context) {
const catalogProduct = await xformProduct({ collections, product, shop, variants });

// Apply custom transformations from plugins.
getFunctionsOfType("publishProductToCatalog").forEach((customPublishFunc) => {
for (const customPublishFn of getFunctionsOfType("publishProductToCatalog")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aldeed you mentioned writing up a few test for this bug, what should I test for here? I'm looking to make sure each customPublishFn has been called before we return out of this func?

@nnnnat nnnnat force-pushed the fix-nnnnat-awaiting-funcbytype branch from 90d2c50 to 19c4e68 Compare April 26, 2019 14:30
… call.

Signed-off-by: Nat Hamilton <info@nathamilton.com>
@nnnnat
Copy link
Contributor Author

nnnnat commented Apr 26, 2019

I'm going to merge this PR and follow up with another PR with test cases #5152.

@nnnnat nnnnat merged commit cc7af6a into develop Apr 26, 2019
@nnnnat nnnnat deleted the fix-nnnnat-awaiting-funcbytype branch April 26, 2019 16:56
@jeffcorpuz jeffcorpuz mentioned this pull request Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants