Skip to content
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

ActionView::Template::Error in spree/admin/payments#new #2655

Closed
mkllnk opened this issue Sep 7, 2018 · 5 comments
Closed

ActionView::Template::Error in spree/admin/payments#new #2655

mkllnk opened this issue Sep 7, 2018 · 5 comments
Assignees
Labels
bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. bugsnag

Comments

@mkllnk
Copy link
Member

mkllnk commented Sep 7, 2018

Error in OpenFoodNetwork

ActionView::Template::Error in spree/admin/payments#new
undefined method `amount' for nil:NilClass

View on Bugsnag

Stacktrace

app/models/spree/calculator/flat_percent_item_total_decorator.rb:10 - map
app/models/spree/calculator/flat_percent_item_total_decorator.rb:10 - compute
app/serializers/api/payment_method_serializer.rb:6 - price
app/helpers/admin/injection_helper.rb:128 - admin_inject_json_ams_array
app/views/spree/admin/payments/_form.html.erb:2 - _68f38d8d3853ce3de2471e009762bf0d

View full stacktrace

Created by Maikel via Bugsnag

@mkllnk mkllnk added the bugsnag label Sep 7, 2018
@mkllnk
Copy link
Member Author

mkllnk commented Sep 7, 2018

An enterprise in Australia can't create payments via the admin interface. The above error is raised when visiting https://openfoodnetwork.org.au/admin/orders/R513858701/payments/new. Somehow it seems possible that Spree::Order.line_items returns [nil] (at least one nil in an array).

This doesn't affect all enterprises. We haven't found out yet which circumstances trigger this.

@mkllnk mkllnk added the bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. label Sep 7, 2018
@mkllnk
Copy link
Member Author

mkllnk commented Sep 7, 2018

My investigation so far: The root cause is that the current_order is not passed to the calculators. A buggy implementation then thinks that nil is a line item and tries to call amount. There are several things to fix here:

  • Write a test that exposes this bug.
  • Run git bisect to find which commit broke it. Probably Stripe work as it moved some of this code around.
  • Get rid of app/models/spree/calculator_decorator.rb. It contains only one method line_items_for which is buggy. A better home for that method would be a service or lib class.
  • Use that new method in OpenFoodNetwork::Calculator::Weight which has its own (currently better) implementation.
  • Pass on current_order as option in app/views/spree/admin/payments/_form.html.erb:2.

Bonus points:

  • Load the payment method data via the API in a JSON request instead of using ams.

The quick fix is:

diff --git a/app/views/spree/admin/payments/_form.html.erb b/app/views/spree/admin/payments/_form.html.erb
index e46b2febc..e075d1288 100644
--- a/app/views/spree/admin/payments/_form.html.erb
+++ b/app/views/spree/admin/payments/_form.html.erb
@@ -1,5 +1,5 @@
 <%= admin_inject_json "admin.payments", "currentOrderNumber", @order.number %>
-<%= admin_inject_json_ams_array "admin.payments", "paymentMethods", @payment_methods, Api::PaymentMethodSerializer %>
+<%= admin_inject_json_ams_array "admin.payments", "paymentMethods", @payment_methods, Api::PaymentMethodSerializer, current_order: @order %>
 
 <div data-hook="admin_payment_form_fields" class="row">
   <div class="alpha three columns">

@mkllnk mkllnk self-assigned this Sep 11, 2018
@mkllnk
Copy link
Member Author

mkllnk commented Sep 11, 2018

8c6b8d5 is the first bad commit for this particular bug. Before that commit, the bug was only in Stripe payments.

@sauloperez
Copy link
Contributor

Could you work on it? Do you need any help?

@mkllnk
Copy link
Member Author

mkllnk commented Sep 13, 2018

Yes, I ran out of time, but created the pull request now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. bugsnag
Projects
None yet
Development

No branches or pull requests

2 participants