-
-
Notifications
You must be signed in to change notification settings - Fork 730
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
Don't show stock for on-demand inventory items #13064
Conversation
We changed to tracking stock of on-demand items to be able to place backorders. This is mostly hidden in the app but was still visible on the inventory page. Now we are hiding that here, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix, but waiting to merge sounds like a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Hey @mkllnk, I've set up 4 variants on the inventory page: Before this PR, these look like this: As expected by this PR, after staging, the variant on position i) does not display stock: Checking the stock by un-ticking the on demand checkbox temporarily displays the stock: So, it all seems to work as expected. From the discussion above, I'm unsure whether it is a good idea to merge this now. |
The other issue has still not been filed. So maybe it's not actually a problem at the moment. After this pull request, we can still check stock on the products page by unchecking the on-demand box as done when testing here. So it's not a blocker. I'll merge. |
What? Why?
It seems that we have a bug around subscriptions and negative stock levels. It has not been filed on Github yet but is discussed on Slack:
While the details are not all confirmed yet, one confusing part is that on-demand products show negative stock levels on the inventory page. The UI was assuming that on-demand products don't have a stock level set. But we are tracking stock levels since Track (negative) stock for on-demand products and overrides #12726.
I'm a bit hesitant to present this fix because it could make investigation of the actual bug more difficult by hiding the stock level. Ideally, we would solve the subscription bug first but we don't have a confirmed description yet. So we may wait merging this until we started work on the subscription issue.
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates