-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Prevent merging of non-frontend-viewable orders #6362
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
Conversation
non-frontend viewable orders are created by Solidus admins, and these can be unintentionally merged into a user's existing orders when they login. We want to exclude them from the list of order merge candidates Co-authored-by: Chris Todorov <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6362 +/- ##
=======================================
Coverage 89.37% 89.37%
=======================================
Files 961 961
Lines 20200 20200
=======================================
Hits 18054 18054
Misses 2146 2146 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tvdeyen
left a comment
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 am not sure about this. Consider a customer service agent creating an order for a customer (on the phone or maybe in a showroom).
If those orders now do not are listed in the customers account section they might be surprised.
What about making the order merger a configurable class and let stores decide how they want to handle this in their store?
|
The order merger is already configurable. Why are we not using it? |
|
Ok. We do, but this business logic is outside of it and just handles merging one order. This is worth refactoring |
|
Another idea: Why are we hard coding frontend_viewable into the order params in the admin orders controller. What about making this a checkbox on the admin orders screen? Then a customer agent could decide. Anyhow. This is worth a bit more discussion. |
tvdeyen
left a comment
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.
We agreed in the core team meeting on merging this and work on the topics I mentioned in subsequent PRs
Thanks
Summary
Non-frontend viewable orders are created by Solidus admins, and these can be unintentionally merged into a user's existing orders when they login.
We want to exclude them from the list of order merge candidates
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: