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

Add ENV to disable inventory auto-publish #5149

Merged
merged 4 commits into from
Apr 25, 2019

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Apr 24, 2019

Impact: minor
Type: feature

Issue

By default, Reaction auto-publishes inventory quantity changes to the Catalog.

In some cases, due to syncing inventory with external systems, code may want to emit "afterVariantUpdate" with field set to "inventoryInStock" but NOT have those changes auto-published to the Catalog.

Solution

AUTO_PUBLISH_INVENTORY_FIELDS=false environment variable can now be used to disable the auto-publishing. If you are editing the inventory "in stock" or "warn at" values manually in the operator UI, you'll see the "changes need publishing" and you'll have to click Publish.

Breaking changes

None. The default is true.

Testing

Without the new ENV set, change the inventory in stock for an option in the UI. Verify:

  • All inventory fields on the parent variant are immediately updated in the Products collection.
  • All inventory fields on the top-level product are immediately updated in the Products collection.
  • The corresponding Catalog item is immediately updated in the database, at all levels.

With the new ENV set, change the inventory in stock for an option in the UI. Verify:

  • All inventory fields on the parent variant ARE immediately updated in the Products collection.
  • All inventory fields on the top-level product ARE immediately updated in the Products collection.
  • The corresponding Catalog item is NOT updated.

In either case, toggling inventory management or "allow backorder" should NOT result in any updates. Those changes must be manually published with the Publish button.

into inventory plugin

Signed-off-by: Eric Dobbertin <[email protected]>
into inventory plugin

Signed-off-by: Eric Dobbertin <[email protected]>
to inventory plugin

Signed-off-by: Eric Dobbertin <[email protected]>
To turn off auto-publish of inventory quantities
and related boolean fields.

Signed-off-by: Eric Dobbertin <[email protected]>
Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

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

Verified updates function as advertised. Left a code comment

"isSoldOut"
);

publishedProductVariantFields.push(
Copy link
Member

Choose a reason for hiding this comment

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

Not clear an why the variant field are outside of the if statement above? From my understanding, it should be an all or nothing update. Meaning, all inventory fields across top level product variants and options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@willopez If I am reading the auto-publishing code correctly, then I don't see where it updates those two fields. It auto-publishes the others but not those. So I left them as always needing to be manually published.

I agree it may be best for it to work as you say, all fields, but I would treat that as a separate PR because it's a bigger change and I'm not sure what the ramifications would be.

Copy link
Member

Choose a reason for hiding this comment

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

OK, that sounds good. Lets create another issue to follow up on the other fields in the future.

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

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

LGTM

"isSoldOut"
);

publishedProductVariantFields.push(
Copy link
Member

Choose a reason for hiding this comment

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

OK, that sounds good. Lets create another issue to follow up on the other fields in the future.

@aldeed aldeed merged commit 47993ed into develop Apr 25, 2019
@aldeed aldeed deleted the feat-aldeed-disable-inventory-auto-publish branch April 25, 2019 19:27
@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