contentful-management.rb icon indicating copy to clipboard operation
contentful-management.rb copied to clipboard

Add optional chaining to fix NoMethodError

Open fdocr opened this issue 1 year ago • 5 comments

Hi,

For some reason during my use of the gem I ran into this issue when trying to print out some entries while debugging. I can't seem to find an easy way to add spec coverage here but this solves a NoMethodError when properties are nil.

NoMethodError: undefined method `empty?' for nil
from /Users/XXXXXX/.rvm/gems/ruby-3.3.0/gems/contentful-management-3.10.0/lib/contentful/management/resource.rb:96:in `inspect'

Feel free to push specs for coverage or point me out how to do so if needed.

Thanks!

fdocr avatar May 10 '24 14:05 fdocr

Hi @fdocr, could you please share the code sample that raised the above error?

rubydog avatar May 22 '24 20:05 rubydog

Sure, this is one example that raises the error on the current 3.10.0 version:

# Initial setup
client = Contentful::Management::Client.new(ENV["CONTENTFUL_TOKEN"], dynamic_entries: { ENV["CONTENTFUL_SPACE"] => "staging" })
environment = client.environments(ENV["CONTENTFUL_SPACE"]).find("staging")

# Sample one content type (issue happens with different content types not only the first one)
content_type = environment.content_types.all.first

# User specific query
user_id = "XXXXXXX"
entries = environment.entries.all("content_type" => content_type.id, 'sys.createdBy.sys.id' => user_id)

# Failure
entries.properties[:items].each { |entry| puts "INSPECTED ENTRY: #{entry.inspect}" }

With those few lines on a Rails console I see the following error:

NoMethodError: undefined method `empty?' for nil
from /Users/XXXXXX/.rvm/gems/ruby-3.3.0/gems/contentful-management-3.10.0/lib/contentful/management/resource.rb:96:in `inspect'

fdocr avatar May 22 '24 22:05 fdocr

looks like a linting issue

image

@rubydog any thoughts on this? I'm not familiar enough with ruby to know

mgoudy91 avatar May 23 '24 17:05 mgoudy91

@fdocr

properties&.empty? will actually do the opposite of what we intend to do. When properties are nil, properties&.empty? returns nil, which causes the else part of the if statement to execute. This is not the intended behavior.

rubydog avatar May 27 '24 05:05 rubydog

That makes sense @rubydog. I didn't think that through since it fixed the NoMethodError on nil for me. I replaced it with:

properties_info = properties.nil? || properties.empty? ? '' : " @properties=#{properties.inspect}"

Rubocop seems to be okay with this. Please let me know if you have any alternatives for me to use instead. Thanks!

fdocr avatar May 27 '24 14:05 fdocr