fix PaperTrail::Rails::Controller.included method

[jordan-brough]

this bombed if the controller wasn't ActionController::Base and the rails-api gem wasn't included.

it seems like there ought to be a better way of doing this in general but this at least prevents the blow up.

[batter]

So, a scenario where someone is including part of the gem in order to use ActionController::API, but not including the entire gem? I was under the impression that you needed to require that gem in order to use that class.

[jordan-brough]

No not that -- A scenario where someone is not using rails-api at all but is including PaperTrail::Rails::Controller into one of their own controllers because their own controller doesn't inherit from ActionController::Base.

My particular use case is including PaperTrail::Rails::Controller into Spree::Api::BaseController

[jordan-brough]

to be clear -- the error I was getting was:

gems/paper_trail-3.0.1/lib/paper_trail/frameworks/rails.rb:6:in `included': uninitialized constant ActionController::API (NameError)

because in this:

(base == ActionController::Base || base == ActionController::API)

the first condition was false and the second condition blew up

[batter]

Ohh... I get it. So you don't have ActionController::Base or ActionController::API defined? Are you still wanting to get these before_filter's to hook in to your controller? (I must confess I don't know how Spree works). The reason for the if statement was to prevent the hooks from being triggered in usage outside of Rails (where there is no ActionController... Sinatra for instance).

I actually was wondering if this statement made the most sense, but it was a quick fix suggested in #313 which seemed to make sense. Just wasn't sure how many other Controller types there might be out there in the wild (clearly more than I was aware of).

[jordan-brough]

I do have ActionController::Base defined, but base != ActionController::Base, because I'm including it into something that doesn't inherit from ActionController::Base. However, I don't have ActionController::API defined at all because I'm not using the rails-api gem.

It seems to me that it would make sense just to drop the "if" condition. If you're including PaperTrail::Rails::Controller into something, wouldn't you expect that thing to be a Rails controller? (Maybe I don't understand all the use cases for PaperTrail::Rails::Controller)

[jordan-brough]

@batter perhaps could we merge in this PR for now so that things aren't broken? And then consider a better solution after that?

[batter]

You may be right, within the gem, it always gets fired off from this hook.

In an ideal world, it may not make sense to even load that class (or the Sinatra one) in to memory if ActionController is not defined, however, it's hard to guarantee load order by bundler without doing some sort of trick like what is being done for ProtectedAttributes, which is ugly and hackish. Unless there's some tricks I'm not aware of.

[jordan-brough]

Do you like this fix better? jordan-brough@d93fe87

I can make a replacement PR out of that instead if so.

[batter]

I think that makes more sense, the question is can we rely on the fact that ActionController will have been required prior to loading in PaperTrail on a typical bundle.require. I'm just thinking back to #279, and the solution I had to use there. I guess if it really is loading them in in alphabetical order it shouldn't be an issue. (Plus that hook would never get fired if it wasn't already loaded at that point anyways, right?).

[jordan-brough]

Plus that hook would never get fired if it wasn't already loaded at that point anyways, right?

Yeah, that's exactly what I was thinking. I opened #352 to replace this PR.