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 support for decoration in base field handling #3765

Merged
merged 12 commits into from
Apr 7, 2025
58 changes: 28 additions & 30 deletions lib/avo/fields/base_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,19 @@ def initialize(id, **args, &block)
@summarizable = args[:summarizable] || false
@nullable = args[:nullable] || false
@null_values = args[:null_values] || [nil, ""]
@format_using = args[:format_using] || nil
@update_using = args[:update_using] || nil
@format_using = args[:format_using]
@update_using = args[:update_using]
@decorate = args[:decorate]
@placeholder = args[:placeholder]
@autocomplete = args[:autocomplete] || nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the || nil? I thought they defaulted to nil.

Copy link
Contributor

Choose a reason for hiding this comment

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

{}[:autocomplete] already returns nil

That is, accessing a non-existent key in a hash yields nil by default.
So if args doesn't contain :autocomplete, then args[:autocomplete] is already nil.
The expression args[:autocomplete] || nil is therefore equivalent to nil || nil, which is redundant.

@help = args[:help] || nil
@default = args[:default] || nil
@autocomplete = args[:autocomplete]
@help = args[:help]
@default = args[:default]
@visible = args[:visible]
@as_avatar = args[:as_avatar] || false
@html = args[:html] || nil
@view = Avo::ViewInquirer.new(args[:view]) || nil
@value = args[:value] || nil
@stacked = args[:stacked] || nil
@html = args[:html]
@view = Avo::ViewInquirer.new(args[:view])
@value = args[:value]
@stacked = args[:stacked]
@for_presentation_only = args[:for_presentation_only] || false
@resource = args[:resource]
@action = args[:action]
Expand Down Expand Up @@ -154,45 +155,42 @@ def placeholder
def value(property = nil)
return @value if @value.present?

property ||= @for_attribute || id
property ||= @for_attribute || @id

# Get record value
final_value = record.send(property) if is_model?(record) && record.respond_to?(property)
final_value = @record.send(property) if is_model?(@record) && @record.respond_to?(property)

# On new views and actions modals we need to prefill the fields with the default value if value is nil
if final_value.nil? && should_fill_with_default_value? && default.present?
if final_value.nil? && should_fill_with_default_value? && @default.present?
final_value = computed_default_value
end

# Run computable callback block if present
if computable && block.present?
final_value = execute_block
if computable && @block.present?
final_value = execute_context(@block)
end

# Run the value through resolver if present
if format_using.present?
final_value = Avo::ExecutionContext.new(
target: format_using,
value: final_value,
record: record,
resource: resource,
view: view,
field: self,
include: self.class.included_modules
).handle
if @format_using.present?
final_value = execute_context(@format_using, value: final_value)
end

if @decorate.present? && @view.display?
final_value = execute_context(@decorate, value: final_value)
end

final_value
end

def execute_block
def execute_context(target, **extra_args)
Avo::ExecutionContext.new(
target: block,
record: record,
resource: resource,
view: view,
target:,
record: @record,
resource: @resource,
view: @view,
field: self,
include: self.class.included_modules
include: self.class.included_modules,
**extra_args
).handle
end

Expand Down
2 changes: 1 addition & 1 deletion lib/avo/fields/heading_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def id
end

def value
block.present? ? execute_block : @label
@block.present? ? execute_context(@block) : @label
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/dummy/app/avo/resources/city.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def tool_fields

link_to resource.record.name, path, data: data
end
field :population, as: :number, filterable: true
field :population, as: :number, filterable: true, decorate: -> { number_with_delimiter(value, delimiter: ".") }
field :is_capital, as: :boolean, filterable: true
field :features, as: :key_value
field :image_url, as: :external_image
Expand Down
15 changes: 15 additions & 0 deletions spec/features/avo/text_field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,19 @@
end
end
end

describe "decorate" do
let!(:city) { create :city, population: 18000 }

it "only on display" do
visit avo.edit_resources_city_path(city)
expect(find_by_id("city_population").value).to eq("18000")

visit avo.resources_city_path(city)
expect(show_field_value(id: :population)).to eq "18.000"

visit avo.resources_cities_path
expect(index_field_value(id: :population, record_id: city.to_param)).to eq "18.000"
end
end
end
8 changes: 4 additions & 4 deletions spec/system/avo/multiple_actions_flux_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
visit "/admin/resources/cities"

expect(page).to have_text "Spec City"
expect(page).to have_text "123456"
expect(page).to have_text "123.456"

find(:css, 'input[type="checkbox"][data-action="input->item-selector#toggle input->item-select-all#selectRow"]', match: :first).set(true)

Expand All @@ -31,16 +31,16 @@

expect(page).to have_text "IS CAPITAL"
expect(page).to have_text "Spec City"
expect(page).to have_text "123456"
expect(page).to have_text "123.456"

fill_in "fields_population", with: "654321"

run_action

expect(page).to have_text "City updated!"
expect(page).to have_text "Spec City"
expect(page).not_to have_text "123456"
expect(page).to have_text "654321"
expect(page).not_to have_text "123.456"
expect(page).to have_text "654.321"
end
end
end
Expand Down
Loading