Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default value option #352

Merged
merged 2 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

* Your contribution here.

* [#352](https://github.com/ruby-grape/grape-entity/pull/352): Add Default value option - [@ahmednaguib](https://github.com/ahmednaguib).

#### Fixes

* Your contribution here.
Expand Down
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- [Aliases](#aliases)
- [Format Before Exposing](#format-before-exposing)
- [Expose Nil](#expose-nil)
- [Default Value](#default-value)
- [Documentation](#documentation)
- [Options Hash](#options-hash)
- [Passing Additional Option To Nested Exposure](#passing-additional-option-to-nested-exposure)
Expand Down Expand Up @@ -482,6 +483,19 @@ module Entities
end
```

#### Default Value

This option can be used to provide a default value in case the return value is nil or empty.

```ruby
module Entities
class MyModel < Grape::Entity
expose :name, default: ''
expose :age, default: 60
end
end
```

#### Documentation

Expose documentation with the field. Gets bubbled up when used with Grape and various API documentation systems.
Expand Down
1 change: 1 addition & 0 deletions lib/grape_entity/entity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,7 @@ def to_xml(options = {})
merge
expose_nil
override
default
].to_set.freeze

# Merges the given options with current block options.
Expand Down
6 changes: 5 additions & 1 deletion lib/grape_entity/exposure/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def initialize(attribute, options, conditions)
key = options[:as] || attribute
@key = key.respond_to?(:to_sym) ? key.to_sym : key
@is_safe = options[:safe]
@default_value = options[:default]
@for_merge = options[:merge]
@attr_path_proc = options[:attr_path]
@documentation = options[:documentation]
Expand Down Expand Up @@ -82,7 +83,10 @@ def serializable_value(entity, options)
end

def valid_value(entity, options)
value(entity, options) if valid?(entity)
return unless valid?(entity)

output = value(entity, options)
output.blank? && @default_value.present? ? @default_value : output
Copy link

@dlinch dlinch Jul 21, 2021

Choose a reason for hiding this comment

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

I am not a maintainer, but I stumbled across this PR while I was looking for a sane default ability for entities.
blank? and present? are Rails specific, so to keep Grape framework agnostic you might want to shuffle this around. In fact, the blank? check replaces some valid options like false and [] with the default value, which I would not expect.

Suggested change
output.blank? && @default_value.present? ? @default_value : output
output.nil? ? @default_value : output

I'm not sure the .present? check bought you anything anyway if we keep it strictly to a nil check.

Copy link
Member

Choose a reason for hiding this comment

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

.blank? and .present? are part of ActiveSupport and this is included in the Grap gemspec, so it can be used without adding additional dependencies

but suggest to move the getting of the value into the same named method → value()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LeFnord not sure if I get this part:

but suggest to move the getting of the value into the same named method → value()

Copy link

Choose a reason for hiding this comment

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

@LeFnord Do you still consider the behavior of blank? to be appropriate though? If I had a boolean attribute with a default of true, then the above implementation will always return true no matter what, as:

false.blank?
# => true

It seems to me like the only sane value to override with a default would be nil

Copy link
Member

Choose a reason for hiding this comment

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

@dlinch … I'm only saying, that is no problem to use it, cause it is in place
and yeap would prefer your suggestion but with a check on blank, instead only nil

output.blank? ? @default_value.presence : output

end

def should_return_key?(options)
Expand Down
124 changes: 124 additions & 0 deletions spec/grape_entity/entity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,130 @@ def initialize(a, b, c)
end
end

context 'with :default option' do
let(:a) { nil }
let(:b) { nil }
let(:c) { 'value' }

context 'when model is a PORO' do
let(:model) { Model.new(a, b, c) }

before do
stub_const 'Model', Class.new
Model.class_eval do
attr_accessor :a, :b, :c

def initialize(a, b, c)
@a = a
@b = b
@c = c
end
end
end

context 'when default option is not provided' do
it 'exposes attributes values' do
subject.expose(:a)
subject.expose(:b)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(a: nil, b: nil, c: 'value')
end
end

context 'when default option is set' do
it 'exposes default values for attributes' do
subject.expose(:a, default: 'a')
subject.expose(:b, default: 'b')
subject.expose(:c, default: 'c')
expect(subject.represent(model).serializable_hash).to eq(a: 'a', b: 'b', c: 'value')
end
end

context 'when default option is set and block passed' do
it 'return default value if block returns nil' do
subject.expose(:a, default: 'a') do |_obj, _options|
nil
end
subject.expose(:b)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(a: 'a', b: nil, c: 'value')
end

it 'return value from block if block returns a value' do
subject.expose(:a, default: 'a') do |_obj, _options|
100
end
subject.expose(:b)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(a: 100, b: nil, c: 'value')
end
end
end

context 'when model is a hash' do
let(:model) { { a: a, b: b, c: c } }

context 'when expose_nil option is not provided' do
it 'exposes nil attributes' do
subject.expose(:a)
subject.expose(:b)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(a: nil, b: nil, c: 'value')
end
end

context 'when expose_nil option is true' do
it 'exposes nil attributes' do
subject.expose(:a, expose_nil: true)
subject.expose(:b, expose_nil: true)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(a: nil, b: nil, c: 'value')
end
end

context 'when expose_nil option is false' do
it 'does not expose nil attributes' do
subject.expose(:a, expose_nil: false)
subject.expose(:b, expose_nil: false)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(c: 'value')
end

it 'is only applied per attribute' do
subject.expose(:a, expose_nil: false)
subject.expose(:b)
subject.expose(:c)
expect(subject.represent(model).serializable_hash).to eq(b: nil, c: 'value')
end

it 'raises an error when applied to multiple attribute exposures' do
expect { subject.expose(:a, :b, :c, expose_nil: false) }.to raise_error ArgumentError
end
end
end

context 'with nested structures' do
let(:model) { { a: a, b: b, c: { d: nil, e: nil, f: { g: nil, h: nil } } } }

context 'when expose_nil option is false' do
it 'does not expose nil attributes' do
subject.expose(:a, expose_nil: false)
subject.expose(:b)
subject.expose(:c) do
subject.expose(:d, expose_nil: false)
subject.expose(:e)
subject.expose(:f) do
subject.expose(:g, expose_nil: false)
subject.expose(:h)
end
end

expect(subject.represent(model).serializable_hash).to eq(b: nil, c: { e: nil, f: { h: nil } })
end
end
end
end

context 'with a block' do
it 'errors out if called with multiple attributes' do
expect { subject.expose(:name, :email) { true } }.to raise_error ArgumentError
Expand Down