Item count required when track inventory and propagate variants flags are disabled

aitbw commented 5 years ago

Steps to reproduce

Set config.track_inventory_levels to false
Create a new stock location with the Propagate all variants flag disabled
Go to the Product Stock tab for any product
Select a stock location and leave the Count on hand input empty
Click the + (Create) icon

Expected behavior

Selected stock location should be added to the product

Actual behavior

The Count on hand input requires a number to complete the action

System configuration

Solidus version: Latest master revision (ceae80f)

Extensions in use:

solidus_auth_devise

I think this issue is up for debate since I can't think of a real-life scenario where you would need more than one Stock Location when you're not tracking inventory levels :thinking: but I'm willing to hear opinions on the matter.

jacobherrington commented 5 years ago

I think that might be an implementation detail best left up to the stores themselves, nevertheless, it is obnoxious that we allow the user to get to a point in which a non-present count on hand value stops them.

What do you think is the best UX?

aitbw commented 5 years ago

I think that might be an implementation detail best left up to the stores themselves

+1, I think the same

What do you think is the best UX?

Based on the work done in #2960 and #2977, the obvious choice would be to disable said input when the Track inventory levels and Propagate all variants flags are disabled. Nevertheless, if you're not tracking inventory levels (let's say you have a store that only sells digital products), I don't see why you'd need more than one stock location.

kennyadsl commented 5 years ago

Yes, it's not ideal.

I also can't figure out a scenario when this makes sense but I did see stores that needed crazy customizations so I expect someone could use this. An example could be a store that has a lot of inventory and does not care about tracking since they always has the product available. They could have 2 stock locations and ship products from the closest one (with a custom made Stock Splitter) to save on shipment costs. Could this make sense?

My first thought for a possibile solution is:

defaulting the Count On Hand value to 0 when track inventory is false (or even always?)
hiding/removing the Count On Hand input (which is useless) when track inventory is false
hiding/removing the Back Orderable checkbox when track inventory is false (which also does not make sense to be editable since when inventory is not tracked I don't see a reason to use it)

I think this would also fix an issue when you enter a number in the Count On Hand and click on + (Create). The UI allow you to change the form fields until you reload the page:

After click `+ (Create)` before reloading the page:

After click `+ (Create)` after reloading the page:

aitbw commented 5 years ago

An example could be a store that has a lot of inventory and does not care about tracking since they always has the product available. They could have 2 stock locations and ship products from the closest one (with a custom made Stock Splitter) to save on shipment costs. Could this make sense?

Yeah @kennyadsl, that makes sense —it's an edge case but seems reasonable.

defaulting the Count On Hand value to 0 when track inventory is false (or even always?)

+1 to always use 0 as the default value

hiding/removing the Count On Hand input (which is useless) when track inventory is false

+1

hiding/removing the Back Orderable checkbox when track inventory is false (which also does not make sense to be editable since when inventory is not tracked I don't see a reason to use it)

+1

That said, I was wondering if maybe there's something we should do with the Restock Inventory and Check Stock on Transfer flags as well since they seem so closely linked with the Track inventory levels one.

kennyadsl commented 5 years ago

That said, I was wondering if maybe there's something we should do with the Restock Inventory and Check Stock on Transfer flags as well since they seem so closely linked with the Track inventory levels one.

Like disable them when Track inventory levels is not checked?

I think it's a good idea but I'm not 100% sure this is reflected in code everywhere. We should check better before changing that and this could be done/discussed into another PR/Issue maybe?

aitbw commented 5 years ago

Sounds good to me @kennyadsl

solidusio / solidus