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

sync inventory policy toggle between top-level variants and child variants #3004

Merged
merged 6 commits into from
Oct 3, 2017

Conversation

kieckhafer
Copy link
Member

@kieckhafer kieckhafer commented Oct 2, 2017

Resolves #2911

  • Introduce ReactionProduct.getSiblings() to get documents of a variants siblings, who have the same ancestor tree
  • Introduce ReactionProduct.getParent() to get document of a variants direct parent
  • Updates the way the inventory policy toggle updates.

If a variant has no child variants, the toggle functionality is unchanged.

If a variant has child variants, the toggle is disabled on the top-level variant, and inherits the following value, based on it's children:

  1. If any of the children are set to inventoryPolicy === false, meaning backorders are allowed, then the top-level variant will also be set to inventoryPolicy === false
  2. If all of the children are set to inventoryPolicy === true, then the top-level variant will also be set to inventoryPolicy === true

@kieckhafer kieckhafer changed the title sync inventory policy toggle between top-level variants and child variants [WIP] sync inventory policy toggle between top-level variants and child variants Oct 2, 2017
@rymorgan
Copy link
Contributor

rymorgan commented Oct 2, 2017

If we are showing the inventory control on the options, don't we also need to show the warn at field?
inventory-warn-at

@kieckhafer kieckhafer changed the title [WIP] sync inventory policy toggle between top-level variants and child variants sync inventory policy toggle between top-level variants and child variants Oct 2, 2017
Copy link
Collaborator

@brent-hoover brent-hoover left a comment

Choose a reason for hiding this comment

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

One lint problem and a couple of question but this seems to be working well.

@@ -195,6 +199,44 @@ const wrapComponent = (Comp) => (
Meteor.call("products/updateProductField", variant._id, "isVisible", !variant.isVisible);
}

/**
* @method updateInventoryPolicyIfChildVariants
* @description update parent inventory policy if variant has children children
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo? "children children"

const parent = ReactionProduct.getVariantParent(variant);

// If this is not a top-level variant, update top-level inventory policy as well
if (parent) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we combine this into one conditional e.g. if (parent && options && options.length) ?

@@ -6,6 +6,7 @@ import Velocity from "velocity-animate";
import "velocity-animate/velocity.ui";
import { Components } from "@reactioncommerce/reaction-components";
import { formatPriceString } from "/client/api";
import { Reaction } from "/lib/api";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Imported but not used.

@brent-hoover
Copy link
Collaborator

So is the default behavior supposed to be that if I create an option for a product it's "disabled" at the top level forever more?

@kieckhafer
Copy link
Member Author

@zenweasel

So is the default behavior supposed to be that if I create an option for a product it's "disabled" at the top level forever more?

yes

@brent-hoover brent-hoover merged commit bb183af into marketplace Oct 3, 2017
@brent-hoover brent-hoover deleted the ek-optionBackorderToggle branch October 3, 2017 23:11
@spencern spencern mentioned this pull request Oct 11, 2017
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.

3 participants