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

Should WebhooksController extend ApplicationController? #114

Open
ryanwood opened this issue Jul 6, 2018 · 1 comment
Open

Should WebhooksController extend ApplicationController? #114

ryanwood opened this issue Jul 6, 2018 · 1 comment

Comments

@ryanwood
Copy link

ryanwood commented Jul 6, 2018

I have a situation where I need to handle a particular email API error across all webhook processing.

I attempted to rescue_from to my ApplicationController but my specs weren't showing that the error was handled. I then realized that StripeEvent::WebhooksController extends ActionController::Base rather than ApplicationController. I realize that ApplicationController is a Rails 5.0+ feature whereas using ActionController::Base works for everyone.

These are the options I could think of to accomplish my goal:

  1. Have StripeEvent::WebhooksController extend ApplicationController (patch the gem)
  2. I tried to create my own WebhooksController which extended StripeEvent::WebhooksController. Then I updated the routes to just point to webhooks#event but I ended up with a RuntimeError: Circular dependency detected while autoloading constant WebhooksController error. :(
  3. The last option I'm aware of is to copy the StripeEvent::WebhooksController into a controller that I manage. That sounds horrible as I'd have to manually keep that controller in sync.

Ideally, if you didn't want to change to use ApplicationController you could take an approach similar to Devise and allow the setting of the parent controller via the config.

class DeviseController < Devise.parent_controller.constantize

https://github.com/plataformatec/devise/blob/be15116426ef8f43ee07686bcacdbe41a213e7fe/app/controllers/devise_controller.rb#L4
https://github.com/plataformatec/devise/blob/962cea2039c72a92691af734ebbd8495dd5c0501/lib/devise.rb#L230-L234

I'm certainly open if there is another simpler way to accomplish this. Thanks.

@rromanchuk
Copy link

@ryanwood went down this same route but this is not an area i want to be monkey patching, what did you end up doing?

I'm just trying to add missing context back into logs

# app/controllers/application_controller.rb
class ApplicationController < ActionController::Base
  def append_info_to_payload(payload)
    super
    payload[:service] = :stripe
   # ...truncated
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants