Handling field aliases and block definitions in global conditionals

[nmdoliveira]

I'm trying to do the same as #124, but I'm unable to handle all fields.

In the case of aliased fields, the field_name that I receive in the if proc is the alias, and not the actual method to be called. It is also not present in the options argument.

Minimal example:

Blueprinter.configure do |config|
  config.if = ->(field_name, obj, options) do
    puts field_name, obj, options
    !obj.public_send(field_name).nil?
  end
end

class ContactBlueprint < Blueprinter::Base
  field :phone, name: :phone_number
end

class Contact
  attr_accessor :phone
end

contact = Contact.new
contact.phone = "123"

ContactBlueprint.render_as_hash(contact)
# phone_number
# <Contact:0x000055672d361890>
# {}
# => NoMethodError (undefined method `phone_number' for #<Contact:0x000055672d361890 @phone="123">)

In the case of fields defined in the blueprint, I also didn't find a way to detect that. For example:

class ContactSerializer < Blueprinter::Base
  field :phone_number do |contact|
    a_utility_method_that_could_return_nil(contact)
  end
end

The arguments to the proc are the same in this case.

How could these cases be handled in the global if?

[mcclayton]

Thank you so much for creating this issue, based on your findings, this does look like an apparent bug! The if proc should be receiving the actual method to be called. A PR is of course welcome, but otherwise, thank you for bringing this to our attention!

[nmdoliveira]

Thanks for your response!

I would like to help with a PR, but I'd like your opinion on what would be the best approach.

After I noticed the problem, I tried to monkeypatch a solution, but I ended up needing to pass the whole field object into the proc, instead of only its name or even the non-aliased name, because some fields aren't methods to be called on the object (like the block example).

It was something like this:

class Field
  def skip?(_field_name, object, local_options)
    if_callable && !if_callable.call(self, object, local_options) ||
      unless_callable && unless_callable.call(self, object, local_options)
  end
end

And then my config became this:

Blueprinter.configure do |config|
  config.unless = ->(field, obj, options) { field.extract(obj, options).nil? }
end

That worked, but that makes my proc dependant on internals of the Field class and is effectively extracting every field twice, so I decided to go for another approach.

I looked into BaseHelpers#object_to_hash and thought of basically two options:

1. Extract the value first and pass the extracted value along to the procs: this would add more flexibility to the global if and unless configs, and avoid extracting the values twice, but the downside would be that it wouldn't be possible to skip a field before extraction (I don't know how important this is, but still). It would look something like this:

def object_to_hash(object, view_name:, local_options:)
  result_hash = view_collection.fields_for(view_name).each_with_object({}) do |field, hash|
    value = field.extract(object, local_options)
    next if field.skip?(object, value, local_options)
    
    hash[field.name] = value
  end

And the config to skip nils:

Blueprinter.configure do |config|
  # This could probably be done in a non-breaking way
  config.unless = ->(_field_name, _obj, _options, value) { value.nil? }
end

2. Add a new configuration option, skip_nil, that would add a check after the current skip check and skip if the extracted value is nil; I think this would make sense as a configuration option, it's not uncommon to see in other similar gems, but could be confusing existing simultaneously with global if and unless configs. It would look something like this:

def object_to_hash(object, view_name:, local_options:)
  result_hash = view_collection.fields_for(view_name).each_with_object({}) do |field, hash|
    next if field.skip?(field.name, object, local_options)

    value = field.extract(object, local_options)
    next if value.nil? && Blueprinter.configuration.skip_nil
    
    hash[field.name] = value
  end

And the config to skip nils:

Blueprinter.configure do |config|
  config.skip_nil = true
end

These seemed possible, but too disruptive for me to monkeypatch. I ended up writing a transformer to remove nil values and added it to a base blueprint class, from which all of my other ones inherit.

What are your thoughts on this?

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.