-
Notifications
You must be signed in to change notification settings - Fork 105
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
Fixed nil error for @event instance variable #2
Conversation
…re to ignore files made by rubymine IDE.
…info that test webhooks fail. Removed application_controller, it's no longer needed. Updated Gemfile.lock to reference bumped version.
@barancw Thank you for the work you put into this pull request. I really appreciate it. I have a few concerns and ideas which I will address as soon as I'm able. |
No problem @invisiblefunnel, Keep me in the loop. I'm using the code from this pull in my app now. I'm interested in making sure this gets rolled forward so I can use future versions of this gem. I think this is a great effort you've made here and it makes dealing with the hooks very simple. |
I haven't been able to recreate this behavior. It is working properly in the specs and in the apps I've deployed.
It was just a convenient place to put it when I was sketching out the gem. As your pull request demonstrates, it could instead be in the webhook controller. More likely I'll move it into the event action, or push it down into lib.
See my comment on #1 about this. I don't know why you were seeing this warning. In your pull request, you are subclassing the parent app's I really like your additions to the README about the test webhooks button and using the dashboard to test manually. Would you create a separate pull request for those? I don't think the screenshot is necessary. I'd like the README to match the most recent release (0.3.0) so there's no need to update it to include the I'll need more information or a failing test case to know whether the behavior you saw was a bug in stripe_event. Thanks again for using the gem and for your feedback/contributions. I'll leave this pull request open, in case you want to investigate some more. Best, |
I'm running into similar errors as @barancw mentioned. Specifically, I'm getting:
and the CSRF warning as well. Full Stacktrace I'll test this patch next and report my results. |
This patch fixed the application issues I described. +1 |
@invisiblefunnel - Sorry for lumping everything together. I'm a little green when it comes to github. I thought it would freeze the commit that I made the pull request out of. I can see that it tries to lump every commit I make. I understand now why they suggest a separate branch per pull request. I'm going to see if I can dig a bit further. Overriding the application controller from your gem seems to sometimes work and sometimes not work. I'm guessing that they are making updates to rails that is causing errors with this behavior. It makes sense to me that a gem would not be allowed to modify my app's application controller. I think what you may have been thinking is that the gem itself has a local version of it's own application controller. Right now, all of this is unconfirmed but I'll keep digging and come up with a good answer. |
Just found out that it is Bundler's job to include the gems. This means that we may be on the same rails version but if we have different versions of Bundler running this may be the source of the different behavior. I'm running Bundler version 1.1.5. I had to upgrade rather recently because of some issues with capistrano calling a bundle install. I'm watching through the railscasts on initialization: http://railscasts.com/episodes/299-rails-initialization-walkthrough. It is a pro episode though so if you don't have the subscription, you can't view it. I'm going to keep digging and see if I can get a better answer on what bundler will do with an application controller override. |
I'm using Rails 3.2.8, Bundler 1.1.5 on ruby-1.9.3-p194. |
I noticed that the code that you had put into your application_controller.rb file was never executed in my application. This is where the @event variable was set. After moving this method into your webhook_controller.rb, everything worked fine.
I'm not sure if there was a reason why this ended up in the application_controller in the first place? I'm not that familiar with overriding the application controller.
The other issue that I ran into was the "WARNING: Can't verify CSRF token authenticity." This will throw a 500 error on it's own. Stripe will never send this correctly. I added in a workaround for this that I found for this so that it will be ignored in this controller.
Last thing I did was bump the version number for you by .1 so you can re-create the gem.
I left the application_controller.rb file in the repository for you to check out, but I think it's now safe to remove. Let me know if anything I did here doesn't make sense.
Cheers,
Chris