-
Notifications
You must be signed in to change notification settings - Fork 143
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
Error trying to edit order #91
Comments
Hi @jalberto, please provide the information we ask for in the Contributing Guidelines. Thanks! |
Steps:
Expecting:
Happening:
Stack trace:
Gemfile:
|
Can confirm I'm also seeing this error. It appears as though the spree view Since I don't ship product in my store, I've added a view to my app with the same filename but empty inside. This has solved my particular issue. |
thanks @binaryphile for the help Any news on this error? It yet happens 2-1-stable branch |
@jalberto This is happening because |
Agreed that overriding a method using different parameters is trouble. Maybe a solution for now could be changing the signature of spree_flexi_variants/app/models/spree/order_decorator.rb Lines 36 to 42 in 2de9573
def find_line_item_by_variant(variant, ad_hoc_option_value_ids = [], product_customizations = []) No idea what those other two parameters are for though. |
No solution yet? |
@coorasse If there was a solution you'd be sure that we would post about it here :) |
@radar can you explain what are these parameters for? I can't get it really. We can try to find a solution |
@coorasse I'm not really familiar with the internals of this extension either. The general idea of what is happening there as far as I know is that you pass in the custom options with the variant in order to determine whether it's a new line item or just incrementing the quantity. To make things more stable those parameters should probably be made into an options hash so calls with just a variant work same as calls with the added options. |
Hi All, Sorry for not responding earlier. I didn't see the notification. FYI, we have been having discussions on how to deal with this type of thing in spree and extensions going forward. We don't yet have a solution, but are actively working on one. Those two parameters are key for this extension. The represent the customizations to a variant (ad hoc option types selected and/or customizations (like engraving) and they are ultimately attached to the line item. |
They are key to this extension, but they are optional... If the method was refactored so that you passed those params in as an options hash instead of setting default values of empty then you could maintain this extensions requirement of having those options, without breaking the usage of this method outside of the extension where those parameters are not being passed. Basically the solution for this is to make those params optional rather than being required, which is the case when you define them in the way that's being done. So something like:
Would prevent the error in question from being raised because the required parameter count remains 1 instead of being 3. |
I'm in total agreement Jeff. He had just asked what those were for so I was trying to convey what they were and how important they were. I'm +10000 on the suggested refactor. |
The text was updated successfully, but these errors were encountered: