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

Allow Variant to check stock by stock_location #3884

Conversation

MadelineCollier
Copy link
Contributor

Description
Quantifier already has the ability to restrict stock checks to certain locations. This change just exposes that ability at the Variant level. This allows applications with multiple stock locations to check variant stock for a specific location without explicitly having to instantiate a new Quantifier with the stock location.

These changes allow both Variant#total_on_hand and Variant#can_supply? to optionally take a stock_location

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Quantifier already has the ability to restrict stock checks to certain
locations. This change just exposes that ability at the Variant level.
This allows applications with multiple stock locations to check variant
stock for a specific location without explicitly having to instantiate a
new Quantifier with the stock location.
Similar to the previous change, we wanted to allow for applications to
specify the stock location when calling variant.total_on_hand.
Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

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

This is a nice additive change and most of the stores I've worked on that needed to handle multiple locations have done something like this. I like.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement. I'd wait for 3.1 before merging this though, adding this PR to the right milestone to track this. 🙏

@kennyadsl kennyadsl added this to the 3.1.0 milestone Jan 8, 2021
@kennyadsl kennyadsl added release:major Breaking change on hold until next major release changelog:solidus_core Changes to the solidus_core gem labels Jan 8, 2021
Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@MadelineCollier thank you 🎉

@kennyadsl kennyadsl merged commit 5590016 into solidusio:master Apr 30, 2021
@kennyadsl kennyadsl removed the release:major Breaking change on hold until next major release label Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_core Changes to the solidus_core gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants