Skip to content

Accept an option 'default' to allow setting default serialized field value if null#115

Merged
AllPurposeName merged 3 commits intomasterfrom
defaultFieldOptions
Nov 16, 2018
Merged

Accept an option 'default' to allow setting default serialized field value if null#115
AllPurposeName merged 3 commits intomasterfrom
defaultFieldOptions

Conversation

@mcclayton
Copy link
Copy Markdown
Contributor

@mcclayton mcclayton commented Nov 13, 2018

Included in the PR

For fields that have a value of nil, blueprinter serializes to null by default. This PR adds an ability to add a default option to blueprinter fields which can be used as the serialized value instead of null.

Before

class UserBlueprint < Blueprinter::Base
  identifier :uuid
  field :first_name
end

UserBlueprint.render(user) renders { "id": 1, "first_name": null }

After

class UserBlueprint < Blueprinter::Base
  identifier :uuid
  field :first_name, default: "N/A"
end

UserBlueprint.render(user) renders { "id": 1, "first_name": "N/A" }

Aims to address: #114

Copy link
Copy Markdown
Contributor

@vinaya-procore vinaya-procore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


def extract(association_name, object, local_options, options={})
value = @extractor.extract(association_name, object, local_options, options)
value = @extractor.extract(association_name, object, local_options, options.except(:default))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to do except(:default)?

Copy link
Copy Markdown
Contributor Author

@mcclayton mcclayton Nov 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to except(:default) here, because otherwise, when it makes the call through to the AutoExtractor, the AutoExtractor (with these changes) would then return the default value instead of nil. Then the association extractor would not pass the return options[:default] if value.nil? step and it would go on to call options[:blueprint].prepare(value, view_name: view, local_options: local_options). You can verify that this would fail the test for default association values.

Another option (which is closer to what you are describing) would be something like this:

def extract(association_name, object, local_options, options={})
  value = @extractor.extract(association_name, object, local_options, options)
  if (options.key?(:default) || value.nil?)
    return value
  else
    view = options[:view] || :default
    options[:blueprint].prepare(value, view_name: view, local_options: local_options)
  end
end

Thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OHH I see ok nevermind. I think what you have is fine.

def extract(association_name, object, local_options, options={})
value = @extractor.extract(association_name, object, local_options, options)
value = @extractor.extract(association_name, object, local_options, options.except(:default))
return options[:default] if value.nil?
Copy link
Copy Markdown
Contributor

@philipqnguyen philipqnguyen Nov 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just let the AutoExtractor deal with the options[:default] stuff? And take it out of here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I proposed an alternative solution above that let's the AutoExtractor do all the work necessary for defaults.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind

@AllPurposeName AllPurposeName merged commit fdd92b3 into master Nov 16, 2018
@AllPurposeName AllPurposeName deleted the defaultFieldOptions branch November 16, 2018 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants