-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix search by variant on stock items #5660
Fix search by variant on stock items #5660
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5660 +/- ##
=======================================
Coverage 88.55% 88.55%
=======================================
Files 685 685
Lines 16408 16408
=======================================
Hits 14530 14530
Misses 1878 1878 ☔ View full report in Codecov by Sentry. |
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.
I think the solution is good. For me the variants do not need to be sorted by sku
, but who am I to judge. A least the screen is usable again after we broke it in 4.2 🙈
Before we fix this bug, we should reproduce it in a feature spec. Co-authored-by: An Stewart <andrew@super.gd> Co-authored-by: Adam Mueller <adam@super.gd>
This "fixes" the variant scope issue but I'm not sure it's the solution we want. I suppose it depends if ordering by sku is important here. Co-authored-by: An Stewart <andrew@super.gd>
f8ed5b0
to
c4bd75a
Compare
Thanks, @nvandoorn! |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Summary
This branch fixes the invalid SQL error that occurs when filtering stock locations by variant (#5506). The implementation changes are still a work in progress, so this pull request is in draft, but I wanted to put this up now so other contributors can see/use the feature spec.
Checklist
The following are mandatory for all PRs:
The following are not always needed: