From cd2394145943973fb9364dbe3f47cbee6d7a7bf6 Mon Sep 17 00:00:00 2001 From: Igor Fedoronchuk Date: Tue, 20 Oct 2026 09:31:01 +0200 Subject: [PATCH 1/2] wip: tighten TableForExtension readability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfinished — parked here so it stops riding along in the working tree during releases (a `gem build` from a dirty tree would publish code that is not in the tag). No behaviour change intended, only expression: * `footer_data` collapses the nil-guard and the call into one safe navigation, keeping the `defined?` memoization that caches a nil result. * `build_footer_cell_for` builds the class list declaratively with `compact_blank` + `presence` instead of a reject/join/ternary chain. * `column_key_for` returns early on the AA 4 `title_id` path and derives the AA 3 key with safe navigation. Green on both supported majors locally: 25 examples, 0 failures against activeadmin 3.5.1 and 4.0.0.beta23. --- .../table_for_extension.rb | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/activeadmin_table_footer/table_for_extension.rb b/lib/activeadmin_table_footer/table_for_extension.rb index 25956b0..8ff4898 100644 --- a/lib/activeadmin_table_footer/table_for_extension.rb +++ b/lib/activeadmin_table_footer/table_for_extension.rb @@ -10,8 +10,7 @@ def build(obj, *attrs) def footer_data return @footer_data if defined?(@footer_data) - return (@footer_data = nil) unless @footer_data_proc - @footer_data = @footer_data_proc.call(unscoped_collection_for_footer) + @footer_data = @footer_data_proc&.call(unscoped_collection_for_footer) end # The collection AA passes us is the paginated + ordered slice. Aggregates @@ -49,14 +48,15 @@ def column_has_footer?(col) def build_footer_cell_for(col) within @aatf_footer_row do column_key = column_key_for(col) - # Always include `col col-` so capybara_active_admin matchers - # (`have_table_cell(column: ...)`) work in both AA 3 and AA 4 — the - # selector convention is `td.col.col-`. - compat_classes = column_key ? "col col-#{column_key}" : "col" - classes = [col.html_class, compat_classes, ActiveadminTableFooter.footer_th_class] - .reject { |c| c.nil? || c.to_s.empty? } - .join(" ") - attrs = { class: classes.empty? ? nil : classes } + # `col` + `col-` keeps capybara_active_admin's + # `have_table_cell(column: ...)` matcher working on AA 3 and AA 4. + classes = [ + col.html_class, + "col", + ("col-#{column_key}" if column_key), + ActiveadminTableFooter.footer_th_class + ].compact_blank.join(" ").presence + attrs = { class: classes } attrs[:"data-column"] = column_key if column_key td(**attrs) do render_footer_value(col) if column_has_footer?(col) @@ -66,12 +66,8 @@ def build_footer_cell_for(col) def column_key_for(col) # AA 4 exposes Column#title_id; AA 3 does not — derive from title. - if col.respond_to?(:title_id) && col.title_id.respond_to?(:presence) - return col.title_id.presence - end - title = col.title.to_s - return nil if title.empty? - title.parameterize(separator: "_") + return col.title_id.presence if col.respond_to?(:title_id) + col.title.to_s.presence&.parameterize(separator: "_") end def ensure_tfoot! From ce96769ec1d407166be2ca38f581de87a892050e Mon Sep 17 00:00:00 2001 From: Igor Fedoronchuk Date: Tue, 20 Oct 2026 09:42:38 +0200 Subject: [PATCH 2/2] fix: keep the falsy guard in footer_data The readability pass replaced the `unless @footer_data_proc` early return with `&.`, which only skips nil. `table_for items, footer_data: false` therefore raised NoMethodError instead of rendering no footer. `false` reaches @footer_data_proc through a direct `table_for` call (the dashboard case the array specs cover) when the callback is written as `footer_data: flag && ->(c) { ... }`. The index wrapper already treats a falsy callback as "no footer" (`footer_proc && page_presenter.block`), so raising here was inconsistent as well as a regression. Guard on truthiness again, keeping the one-expression shape and the `defined?` memoization that caches a nil result. Two unit examples cover it; the `false` one fails on the previous commit with "undefined method 'call' for false". 27 examples, 0 failures on activeadmin 3.5.1 and 4.0.0.beta23. --- .../table_for_extension.rb | 5 ++++- spec/unit/table_for_extension_spec.rb | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/activeadmin_table_footer/table_for_extension.rb b/lib/activeadmin_table_footer/table_for_extension.rb index 8ff4898..fbfe188 100644 --- a/lib/activeadmin_table_footer/table_for_extension.rb +++ b/lib/activeadmin_table_footer/table_for_extension.rb @@ -8,9 +8,12 @@ def build(obj, *attrs) super(obj, *attrs, options) end + # Any falsy callback means "no footer" — `&.` would only skip nil and raise + # on `footer_data: false`, which is what `footer_data: flag && ->(c) { ... }` + # passes when the flag is off. def footer_data return @footer_data if defined?(@footer_data) - @footer_data = @footer_data_proc&.call(unscoped_collection_for_footer) + @footer_data = (@footer_data_proc.call(unscoped_collection_for_footer) if @footer_data_proc) end # The collection AA passes us is the paginated + ordered slice. Aggregates diff --git a/spec/unit/table_for_extension_spec.rb b/spec/unit/table_for_extension_spec.rb index b059498..001e1e4 100644 --- a/spec/unit/table_for_extension_spec.rb +++ b/spec/unit/table_for_extension_spec.rb @@ -12,4 +12,23 @@ expect(ActiveAdmin::Views::TableFor.instance_method(:footer_data)).to be_a(UnboundMethod) end end + + # `table_for items, footer_data: show_totals && ->(c) { ... }` passes `false` + # when the flag is off. A falsy callback means "no footer", same as the index + # wrapper already assumes (`footer_proc && page_presenter.block`). + describe "#footer_data with a falsy callback" do + let(:table) { ActiveAdmin::Views::TableFor.allocate } + + it "treats false as no callback instead of raising" do + table.instance_variable_set(:@footer_data_proc, false) + + expect(table.footer_data).to be_nil + end + + it "treats nil as no callback" do + table.instance_variable_set(:@footer_data_proc, nil) + + expect(table.footer_data).to be_nil + end + end end