Chimpy includes ControllerHelpers::Order helper everywhere

[bryanmtl]

https://github.com/DynamoMTL/spree_chimpy/blob/master/lib/spree/chimpy/controller_filters.rb#L8

Which causes set_current_order to be called before each request.

@joshnuss @braidn Is this necessary?

[braidn]

@bryanmtl I can see if we can get around it. I am sure we pull it in just to have access to set the current_order using the current_order method. If it comes down to it, we can recreate a simple-r method within the controller

[braidn]

@bryanmtl is set_current_order being called twice due to the inclusion of the Order controller helper? I ask only because this helper is included in the BaseController which means it would be called anyway every request, no?

[bryanmtl]

I'm not sure if it's being called twice, but it's called before every request and that doesn't seem right to me. It should only be called in controllers that actually require the current order.

[braidn]

@bryanmtl So the only way I can think of doing this is to move a simple order setter into this controller. At the above WIP I played with popping the callback out of the Callbacks array however, this wouldn't work because it would always skip the set_current_order from firing unless the mailchimp params existed. I am coming up with nothing besides a small amount of code duplication to associate the params with the current order in the spree chimpy extension.

@joshnuss any ideas or @DanielWright ?

[joshnuss]

@braidn Your refactoring looks good

Think the reason it was be called too many times was because we were checking if params were in the session, and doing an update to the order for no reason. Now it looks way better

[braidn]

@joshnuss just wondering how you are testing this? The anonymous controller that Rspec fires up is showing :set_current_order in the array returned from filtering the callbacks here. This means that the set_current_method will in fact be called in the test. However, maybe not in the running code?

[braidn]

Plus, if I remove the include line from this module, run the tests, then there is still a set_current_order callback in the list. So the controller is picking it up from somewhere else?

[braidn]

Merging this for now as a refactoring.