search

pay-rails / pay

Payments for Ruby on Rails apps
https://github.com/pay-rails/pay
MIT License
1.98k stars 316 forks source link

Explicitly require ostruct gem in webhook model #1081

Closed jjatinggoyal closed 2 months ago

jjatinggoyal commented 2 months ago

Pull Request

Summary: Running into Error: NameError: uninitialized constant Pay::Webhook::OpenStruct error when Pay::Webhooks::ProcessJob is running for paddle_billing

Description: I'm integrating paddle with pay gem into my application, and have configured multiple webhooks for syncing subscriptions.

However, I'm running into the following error in Pay::Webhooks::ProcessJob:

Error: NameError: uninitialized constant Pay::Webhook::OpenStruct
/usr/local/bundle/ruby/3.3.0/gems/pay-8.0.0/app/models/pay/webhook.rb:35:in `to_recursive_ostruct'
/usr/local/bundle/ruby/3.3.0/gems/pay-8.0.0/app/models/pay/webhook.rb:21:in `rehydrated_event'
/usr/local/bundle/ruby/3.3.0/gems/pay-8.0.0/app/models/pay/webhook.rb:8:in `process!'
/usr/local/bundle/ruby/3.3.0/gems/pay-8.0.0/lib/pay/webhooks/process_job.rb:5:in `perform'

To fix the issue, I have explicitly added require "ostruct" to the Pay::Webhook model, and added a test case for paddle_billing for the same model.

Additionally, added the require "ostruct" line to Pay::Braintree::Subscription model (the only other place where OpenStruct is used) as well proactively.

Testing: Tested the fixes in this PR on my staging server, the webhook job passes now.

Screenshots (if applicable):

Screenshot 2024-10-01 at 12 50 26 AM

The first (failed) run in above screenshot is with pay-8.0.0, while the second successful run is with the fixes in this PR.

Additional Notes: My setup is:

excid3 commented 2 months ago

We may want to just get rid of our OpenStruct since we don't use it much.

It's moving out of Ruby core to a bundled gem in 3.5: https://bugs.ruby-lang.org/issues/20309

Could possibly replace it with Data or something Rails provides.

jjatinggoyal commented 2 months ago

Just read up on Data class, look like it requires you to declare the class attributes beforehand, which doesn't fit our use case here one-to-one.

As for rails, maybe ActiveSupport::HashWithIndifferentAccess could be used, but the access pattern is different than ostruct. Do you have anything specific in mind?

excid3 commented 2 months ago

Might not be worth messing with yet.

We should also add ostruct as a dependency in the gemspec as it will be removed from Ruby in 3.5.

jjatinggoyal commented 2 months ago

Cool, let me add ostruct as a dependency. Won't mention a min version, hope that is fine.

jjatinggoyal commented 2 months ago

@excid3 does this PR look good to you? Can we merge it?

excid3 commented 2 months ago

I figured out a way to remove OpenStruct and use a feature built-in to Rails instead.