[Spree Upgrade][BackOffice] De-deface payment method new and adapt to spree 2 (1 broken test)

[luisramos0]

Main config menu is showing up on the right side instead of the list of hubs that can be associated with the payment method.
payment_method_new is defaced. Needs to be de-defaced (in master first).

Errors in features/admin/payment_method_spec.rb:

12) 
    As a Super Admin
    I want to be able to set a distributor on each payment method
 updating a payment method
      Failure/Error: uncheck "payment_method_distributor_ids_#{@distributors[0].id}"
      
      Capybara::ElementNotFound:
        Unable to find visible checkbox "payment_method_distributor_ids_96" that is not disabled
      # ./spec/features/admin/payment_method_spec.rb:81:in `block (2 levels) in <top (required)>'

  13) 
    As a Super Admin
    I want to be able to set a distributor on each payment method
 creating a payment method assigning a distributor to the payment method
      Failure/Error: check "payment_method_distributor_ids_#{@distributors[0].id}"
      
      Capybara::ElementNotFound:
        Unable to find visible checkbox "payment_method_distributor_ids_99" that is not disabled
      # ./spec/features/admin/payment_method_spec.rb:24:in `block (3 levels) in <top (required)>'

  14) 
    As a Super Admin
    I want to be able to set a distributor on each payment method
 as an enterprise user creates payment methods
      Failure/Error: check "payment_method_distributor_ids_#{distributor1.id}"
      
      Capybara::ElementNotFound:
        Unable to find visible checkbox "payment_method_distributor_ids_116" that is not disabled
      # ./spec/features/admin/payment_method_spec.rb:147:in `block (3 levels) in <top (required)>'

[luisramos0]

only one broken spec currently:

 10) 
    As a Super Admin
    I want to be able to set a distributor on each payment method
 updating a payment method
      Failure/Error: object.tag_list.join(",")
      
      ActionView::Template::Error:
        undefined method `association_class' for nil:NilClass
      # ./app/serializers/api/admin/payment_method/base_serializer.rb:6:in `tag_list'
      # (eval):6:in `_fast_attributes'
      # ./app/serializers/api/admin/payment_method_serializer.rb:4:in `serializable_hash'
      # ./app/helpers/admin/injection_helper.rb:123:in `admin_inject_json_ams'
      # ./app/helpers/admin/injection_helper.rb:27:in `admin_inject_payment_method'
      # ./app/views/spree/admin/payment_methods/_form.html.haml:1:in `_88e07dd962413d2b71422422a11bad15'
      # ./app/views/spree/admin/payment_methods/edit.html.haml:13:in `block in _5842be528d28f13d89e71454a874e6ff'
      # ./app/views/spree/admin/payment_methods/edit.html.haml:11:in `_5842be528d28f13d89e71454a874e6ff'
      # ./lib/spree/core/controller_helpers/respond_with_decorator.rb:15:in `call'
      # ./lib/spree/core/controller_helpers/respond_with_decorator.rb:15:in `respond_with'
      # ./lib/open_food_network/rack_request_blocker.rb:37:in `call'
      # ------------------
      # --- Caused by: ---
      # Capybara::ElementNotFound:
      #   Unable to find css ".flash"
      #   ./spec/support/request/web_helper.rb:79:in `flash_message'

[luisramos0]

So, after the de-deface in #3156 has been merged to 2-0-stable the payment methods page is working fine. The error is still the same and the problem is related to the mix of the paypalexpress method and the tag list attributes on payment_method.
Looks like it's related to the tricky #2780 issue.
I tried a similar approach, and a few variations, but it didn't work...

I tried to move/copy these acts_as_taggable and tag_list attributes:

openfoodnetwork/app/models/spree/payment_method_decorator.rb

Lines 7 to 11 in 832dca9

	acts_as_taggable
	has_many :credit_cards, class_name: "Spree::CreditCard" # from Spree v.2.3.0 d470b31798f37
	attr_accessible :tag_list

to a new Spree::PaymentMethod::Check.class_eval and also to a Spree::Gateway::PayPalExpress.class_eval, non worked...

Needs further investigation.

[mkllnk]

The remaining broken spec is:

  1) 
    As a Super Admin
    I want to be able to set a distributor on each payment method
 updating a payment method
     Failure/Error: object.tag_list.join(",")
     
     ActionView::Template::Error:
       undefined method `association_class' for nil:NilClass
     # ./app/serializers/api/admin/payment_method/base_serializer.rb:6:in `tag_list'
     # (eval):6:in `_fast_attributes'
     # ./app/serializers/api/admin/payment_method_serializer.rb:4:in `serializable_hash'
     # ./app/helpers/admin/injection_helper.rb:123:in `admin_inject_json_ams'
     # ./app/helpers/admin/injection_helper.rb:27:in `admin_inject_payment_method'
     # ./app/views/spree/admin/payment_methods/_form.html.haml:1:in `_05304cbab6c41251ca99d461ec59c608'
     # ./app/views/spree/admin/payment_methods/edit.html.haml:13:in `block in _dcfec91afc5506c656d719ff659da51f'
     # ./app/views/spree/admin/payment_methods/edit.html.haml:11:in `_dcfec91afc5506c656d719ff659da51f'
     # ./lib/spree/core/controller_helpers/respond_with_decorator.rb:15:in `call'
     # ./lib/spree/core/controller_helpers/respond_with_decorator.rb:15:in `respond_with'
     # ./lib/open_food_network/rack_request_blocker.rb:37:in `call'
     # ------------------
     # --- Caused by: ---
     # Capybara::ElementNotFound:
     #   Unable to find css ".flash"
     #   ./spec/support/request/web_helper.rb:79:in `flash_message'

rspec ./spec/features/admin/payment_method_spec.rb:72

[mkllnk]

Someone once had a similar problem, but their solution doesn't work for us: https://stackoverflow.com/questions/20045932/undefined-method-association-class-for-nilnilclass

[mkllnk]

Other people having this issue: rails/rails#3847
Seems like it's the order in which classes are loaded. PayPalExpress could be loaded early before our decorator is loaded. It is not aware of our decorator code.

> ap Spree::PaymentMethod.reflections.keys
[
    [0] :calculator,
    [1] :distributors,
    [2] :taggings,
    [3] :base_tags,
    [4] :tag_taggings,
    [5] :tags,
    [6] :credit_cards
]

> ap Spree::Gateway::PayPalExpress.reflections.keys
[
    [0] :calculator,
    [1] :provider,
    [2] :distributors
]

[mkllnk]

The console output above is from within the test server ran by the feature spec. If I run these commands within the spec, there is more visible and tag_list can be called:

> ap Spree::PaymentMethod.reflections.keys
[
    [0] :calculator,
    [1] :distributors,
    [2] :taggings,
    [3] :base_tags,
    [4] :tag_taggings,
    [5] :tags,
    [6] :credit_cards
]
> ap Spree::Gateway::PayPalExpress.reflections.keys
[
    [0] :calculator,
    [1] :provider,
    [2] :distributors,
    [3] :tag_taggings,
    [4] :tags
]
> pm.tag_list
=> []

I also wanted to find out in which order the code is loaded. I placed a put statement in the paypal express code and one in our decorator. Running rails console or server in development env:

"loading decorator"
"loading paypal"
"loading decorator"

But when run in test env:

"loading decorator"
"loading paypal"

In both cases our override is ignored though:

ap Spree::Gateway::PayPalExpress.reflections.keys
[
    [0] :calculator,
    [1] :provider,
    [2] :distributors
]

[mkllnk]

I tried going back to activerecord 3.2.22.2, but that didn't change anything. Running out of ideas...

[mkllnk]

Okay, adding acts_as_taggable to the original declaration of Spree::PaymentMethod in the Spree code makes that part of the spec pass. It's not a viable solution unless we find out that there is no other way. I'll try to find a reliable way of decorating the class. Any suggestions welcome.

[mkllnk]

Just to visualise the inheritance:

Gateway::PayPalExpress < Gateway < PaymentMethod < ActiveRecord::Base

We do a class_eval on Gateway and PaymentMethod. PaymentMethod works, but Gateway and PayPalExpress don't know about it. The load sequence is this:

1. "loading Gateway"
2. "loading PaymentMethod"
3. "loading PaymentMethod decorator"
4. "loading Gateway decorator"

And we end up with:

> Spree::PaymentMethod.new.tag_list
=> []

> Spree::Gateway.new.tag_list
NoMethodError: undefined method `association_class' for nil:NilClass

> Spree::Gateway::PayPalExpress.new.tag_list
NoMethodError: undefined method `association_class' for nil:NilClass

So the core problem is that Gateway doesn't inherit PaymentMethod's ActiveRecord associations. We can probably ignore PayPalExpress for now. Our spec code for this issue can be simplified to this:

expect(Spree::Gateway.new.tag_list).to eq []

[luisramos0]

great investigation @mkllnk
if you have a green spec in some way, that is progress :-D

Did you see the original issue? the is is on StripeConnect another child of Gateway #2781

And btw, this problem is also happening in:

• spec/features/admin/subscriptions_spec.rb:159
• spec/features/admin/subscriptions_spec.rb:313
• spec/features/admin/subscriptions_spec.rb:398
  I'll create an issue for these 3 soon. Can be seen in this build for now (job 2):
  https://semaphoreci.com/openfoodfoundation/openfoodnetwork-2/branches/pull-request-3317/builds/1

[mkllnk]

I finally made some progress here and can make that part of the spec pass without changing original Spree code, just loading decorators at the right time. This has been so difficult, because we are dealing with two issues here:

1. Our Rails version has issue has_many: undefined method `association_class' for nil:NilClass rails/rails#3847 which has never been solved. Sometimes ActiveRecord associations are not passed on to inheriting classes and nobody knows why. We can work around this by decorating Gateway with acts_as_taggable as well.
2. The acts_as_taggable code defines a class method to remember that the code is already loaded. So calling it twice doesn't declare associations twice. Unfortunately, in our case, that class method is inherited while the associations are not inherited. So including acts_as_taggable on Gateway doesn't do much if it has already been called within PaymentMethod. But if we call it within Gateway before calling it in PaymentMethod, it succeeds.

[luisramos0]

well done. amazing work @mkllnk

[luisramos0]

I just confirmed that "Main config menu is showing up on the right side" is fixed! 👍