Add respond_to_missing? to ChargeBee::Model

[sobrinho]

That will fix an issue when calling flatten to an array of ChargeBee::Model:

resources.map(&:class).tally
#=> {ChargeBee::Transaction=>890,
 ChargeBee::Invoice=>712,
 ChargeBee::Customer=>17088,
 ChargeBee::Subscription=>14774,
 ChargeBee::PaymentSource=>534,
 ChargeBee::Card=>6942,
 ChargeBee::CreditNote=>178}

resources.flatten
#=> There's no method called to_ary [] here -- please try again.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[reagent]

I'm using this in a Rails project and have some places where I want to decorate the underlying ChargeBee::Entitlement object using delegate_missing_to, but can't due to respond_to? not being implemented correctly on ChargeBee::Model . Is it possible to get this merged into the next release?

[cb-alish]

Thanks for reaching out, @reagent! We'll analyze, test this, and explore merging possibilities. We'll keep you posted.

[cb-alish]

Hi @sobrinho & @reagent,
Could you provide more context on how you plan to utilize this function? We're concerned about potential breaking changes. Previously, result.resources.respond_to?(:object) would return false, but now it will return true. Our concern is that users already relying on this behavior might experience breaking changes in their implementations.

[sobrinho]

@cb-alish even if this would break something, this would be incorrect because that would be the correct usage:

def something
  result.resources.object if result.resources.respond_to?(:object)
end

If respond_to?(:object) returns false, it would be impossible to call this method but that's not the case when respond_to_missing? is not implemented.

Every time a def method_missing is implemented, you MUST implement def respond_to_missing? accordingly to it.

Ruby does not require you to but it is the expected implementation.

Ref:

1. https://thoughtbot.com/blog/always-define-respond-to-missing-when-overriding?utm_source=chatgpt.com
2. https://medium.com/%40imrohitkushwaha2001/unleashing-the-power-of-method-missing-in-ruby-the-hidden-gem-for-dynamic-method-handling-a8841aa77e49

That's exactly why calling flatten doesn't work without respond_to_missing:

resources.map(&:class).tally
#=> {ChargeBee::Transaction=>890,
 ChargeBee::Invoice=>712,
 ChargeBee::Customer=>17088,
 ChargeBee::Subscription=>14774,
 ChargeBee::PaymentSource=>534,
 ChargeBee::Card=>6942,
 ChargeBee::CreditNote=>178}

resources.flatten
#=> There's no method called to_ary [] here -- please try again.

[reagent]

I dug through the Ruby docs across versions for method_missing and respond_to(_missing)? and I think @sobrinho's referenced Thoughtbot post is a succinct explanation of the problem and solution.

As far back as Ruby 1.9.3 (the minimum supported version of this library), there is a note on the method_missing documentation about delegating the handling of method_missing up the ancestor chain:

If it is decided that a particular method should not be handled, then super should be called, so that ancestors can pick up the missing method.

Because of that, I'm wondering if instead of raising a NoMethodError (as it does today) Model should instead fall back to calling super:

    def method_missing(m, *args, &block)
      if(@values.has_key?(m))
          return @values[m]
      elsif(m[0,3] == "cf_") # All the custom fields start with prefix cf_. 
          return nil
      end
      super
    end

If backwards compatibility is a concern, semver would require a bump to 3.0 -- I think that's a conservative move, but maybe there are more breaking changes that the Chargebee team would want to consider before a 3.0 release? Another option would be to give it a beta suffix to require people to opt-in for pre-release testing (e.g. 3.0.0.beta1 / 3.0.0.rc1).

[sobrinho]

Ideally yes, the super would be called, but I'm being conservative here and only adding the respond_to_missing.

I can update the PR if we agree it's a better move.

[sobrinho]

By the way, I have been running a monkey patch for this in production for about a year now and it didn't break anything in our codebase.

[sobrinho]

Any updates here?

[cb-alish]

Hi @sobrinho & @reagent, sorry for the delayed response and thank you for your patience. We've included this in v2.55.0. We'll keep an eye on it for a few days due to possible breaking changes if anyone was relying on the previous respond_to behavior. Thanks again for the PR!