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

Refactor uses of spree_variants.count_on_hand #2014

Closed
oeoeaio opened this issue Dec 21, 2017 · 2 comments
Closed

Refactor uses of spree_variants.count_on_hand #2014

oeoeaio opened this issue Dec 21, 2017 · 2 comments
Assignees

Comments

@oeoeaio
Copy link
Contributor

oeoeaio commented Dec 21, 2017

VariantOverride is particularly coupled to this column, but we should be able to override the new variant.total_on_hand method instead of variant.count_on_hand to achieve the same result. This is a bandaid solution but a longer term solution to this issue will likely appear out of Networked Products discussions.

@mkllnk
Copy link
Member

mkllnk commented Jun 27, 2018

Spree::Variant decorator

scope :in_stock: Not on Spree's model. Plenty of use of method in_stock?, but no use of the scope in_stock.

VariantOverride

It has its own count_on_hand value. If this value is present, it uses it for stock management. Otherwise it expects that it's not used for stock. increment_stock! will intentionally raise an error instead of using the variant's count_on_hand value.

ScopeVariantToHub

This is a nasty one. Whenever code wants to deal with variants from a distributor's perspective, it doesn't deal with VariantOverride directly. It calls ScopeVariantToHub.scope to override some of the variant's methods to use the variant override if present. For example, it overrides count_on_hand. The old Spree code uses count_on_hand in methods like in_stock?. These are not overridden, because they just add logic to the overridden values from the variant override and everything "works". Now, Spree 2 changes that logic, not using count_on_hand and therefore getting the value from our variant override. That means that we have to override all these methods as well.

In old Spree, but not in Spree 2. We need to replace them:

Modified in Spree 2. We need to override this for VariantOverrides:

New in Spree 2:

  • total_on_hand

More uses of count_on_hand:

I made a task list of all code that references count_on_hand. We can tick them off when we create pull requests for them.

WIP feature Product Import

Code calling method count_on_hand

Code using count_on_hand in SQL

Code calling decrement! or increment!

Other code

@mkllnk
Copy link
Member

mkllnk commented Aug 24, 2018

After reviewing all uses again, I found that #2575 is the only issue left to do. That should solve all listed issues. Closing this one.

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

No branches or pull requests

3 participants