From f8ac65b0e7324a48d12337f05c697bd50393c8c5 Mon Sep 17 00:00:00 2001 From: Igor Fedoronchuk Date: Thu, 1 Oct 2026 15:13:12 +0200 Subject: [PATCH] Ask the model for its DOM name instead of approximating it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #53, from the same review. #53 fixed how a *label* becomes a CSS class; these are the two remaining derivations of the same kind, where a *model* becomes one. Arbre builds a record's DOM name from `record.class.model_name.singular` (html/tag.rb:164-168) and ActiveAdmin builds the index table id from `active_admin_config.resource_name.plural` (index_as_table.rb:240) — both of which underscore a namespace. The gem used `gsub(' ', '_').downcase` plus an unconditional singularize/pluralize, which leaves the `::` alone: have_attributes_table(model: 'Billing::Employee') => div.attributes_table.billing::employee have_table(resource_name: Billing::Employee) => table#index_table_billing::employees Neither is a selector. Under cuprite the first aborts the example with Ferrum::JavaScriptError, under rack_test with Nokogiri::CSS::SyntaxError — in both cases with nothing naming the cause. `table_selector` also had no class branch at all, so passing a model class was broken for every namespaced model. Verified against real rendered pages: the dummy renders `
` and ``, and the new system spec asserts both. Restoring the old derivation fails it. Note the two names have different sources: the table id follows the name a resource was *registered* under (`as: 'Business Employee'`), so a renamed resource cannot be addressed by its class. Documented at the call site rather than papered over. Also fixes six dead `have_table_cell` examples in README.md, which still showed a positional-text signature the matcher has never had — they raise TypeError or ArgumentError as written. --- CHANGELOG.md | 8 +++++++ README.md | 12 +++++----- lib/capybara/active_admin/selectors/table.rb | 8 +++++-- lib/capybara/active_admin/util.rb | 23 ++++++++++++++++---- spec/lib/selectors/attributes_table_spec.rb | 15 +++++++++++++ spec/lib/selectors/table_spec.rb | 21 ++++++++++++++++++ spec/system/namespaced_model_names_spec.rb | 22 +++++++++++++++++++ 7 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 spec/system/namespaced_model_names_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ccdcb76..036f70b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + - `attributes_table_selector`, `form_selector` and `table_selector` now ask the + model for its DOM name (`model_name.singular` / `.plural`, the same source + Arbre and ActiveAdmin use) instead of `gsub(' ', '_')`. A namespaced name + previously kept its `::` and produced `div.attributes_table.billing::employee` + — not a selector, so it aborted the example instead of failing it. + `table_selector` also accepts a model class now. + ### Changed - tested against Ruby 3.3, 3.4, 4.0 and Rails 8.0, 8.1 (EOL Rails 7.2 dropped from CI) diff --git a/README.md b/README.md index d3f3460..4f3ecbc 100644 --- a/README.md +++ b/README.md @@ -55,17 +55,17 @@ RSpec.describe 'Users', js: true do within_table_for('users') do expect(page).to have_table_row(count: 2) - expect(page).to have_table_cell('John Doe') + expect(page).to have_table_cell(text: 'John Doe') within_table_row(id: john.id) do - expect(page).to have_table_cell('John Doe', row_id: john.id) - expect(page).to have_table_cell('John Doe', row_id: john.id, col_name: 'Full Name') - expect(page).to_not have_table_cell('John Doe', row_id: john.id, col_name: 'Id') + expect(page).to have_table_cell(text: 'John Doe') + expect(page).to have_table_cell(text: 'John Doe', column: 'Full Name') + expect(page).to_not have_table_cell(text: 'John Doe', column: 'Id') end within_table_row(id: jane.id) do - expect(page).to_not have_table_cell('John Doe') - expect(page).to_not have_table_cell('John Doe', col_name: 'Full Name') + expect(page).to_not have_table_cell(text: 'John Doe') + expect(page).to_not have_table_cell(text: 'John Doe', column: 'Full Name') end end end diff --git a/lib/capybara/active_admin/selectors/table.rb b/lib/capybara/active_admin/selectors/table.rb index 67012e8..7951431 100644 --- a/lib/capybara/active_admin/selectors/table.rb +++ b/lib/capybara/active_admin/selectors/table.rb @@ -10,8 +10,12 @@ module Table def table_selector(resource_name = nil) return 'table.index_table' if resource_name.nil? - resource_name = resource_name.to_s.gsub(' ', '_').pluralize.downcase - "table#index_table_#{resource_name}" + # NOTE: ActiveAdmin builds this id from the registered resource name + # (`active_admin_config.resource_name.plural`, index_as_table.rb:240), + # which `register Model, as: 'Other Name'` detaches from the model. + # A class can only ever yield the model's own name, so a renamed + # resource has to be addressed by the name it was registered under. + "table#index_table_#{Util.parse_model_name(resource_name, singular: false)}" end # @param record_id [String, Integer, nil] record ID. diff --git a/lib/capybara/active_admin/util.rb b/lib/capybara/active_admin/util.rb index 6ca6f2f..8ed85ae 100644 --- a/lib/capybara/active_admin/util.rb +++ b/lib/capybara/active_admin/util.rb @@ -5,12 +5,27 @@ module ActiveAdmin module Util # Common pure utility functions + # Arbre derives a record's DOM name from + # `record.class.model_name.singular`, falling back to + # `name.underscore.gsub('/', '_')` (arbre html/tag.rb:164-168). Ask the + # model the same question instead of approximating it: `gsub(' ', '_')` + # leaves the `::` of a namespaced name in place, and + # `div.attributes_table.billing::employee` is not a selector -- Nokogiri + # raises `SyntaxError` on it with nothing pointing at the cause. def parse_model_name(model_name, singular: true) return if model_name.nil? - model_name = model_name.model_name.singular if model_name.is_a?(Class) - model_name = model_name.to_s.gsub(' ', '_').downcase - singular ? model_name.singularize : model_name.pluralize + if model_name.is_a?(Class) && model_name.respond_to?(:model_name) + name = model_name.model_name + return singular ? name.singular : name.plural + end + + dom_model_name(model_name.is_a?(Class) ? model_name.name : model_name, singular: singular) + end + + def dom_model_name(name, singular: true) + name = name.to_s.underscore.tr('/', '_').tr(' ', '_') + singular ? name.singularize : name.pluralize end # ActiveAdmin derives the CSS class of a row or column from the @@ -28,7 +43,7 @@ def options_with_text(text, options = {}) options.except(:exact).merge(key => text) end - module_function :parse_model_name, :css_class_for_label, :options_with_text + module_function :parse_model_name, :dom_model_name, :css_class_for_label, :options_with_text end end end diff --git a/spec/lib/selectors/attributes_table_spec.rb b/spec/lib/selectors/attributes_table_spec.rb index 301f327..fa4268b 100644 --- a/spec/lib/selectors/attributes_table_spec.rb +++ b/spec/lib/selectors/attributes_table_spec.rb @@ -5,6 +5,21 @@ Class.new { include Capybara::ActiveAdmin::Selectors::AttributesTable }.new end + describe '#attributes_table_selector' do + it 'asks a model class for its DOM name' do + expect(helper.attributes_table_selector(model: Billing::Employee)) + .to eq('div.attributes_table.billing_employee') + end + + # Arbre uses `model_name.singular` — underscore, then '/' -> '_'. The + # previous `gsub(' ', '_')` kept the '::' and Nokogiri raised on + # `div.attributes_table.billing::employee`. + it 'underscores a namespaced name given as a string' do + expect(helper.attributes_table_selector(model: 'Billing::Employee')) + .to eq('div.attributes_table.billing_employee') + end + end + describe '#attributes_row_selector' do it 'returns a generic row selector when no label given' do expect(helper.attributes_row_selector).to eq('tr.row > td') diff --git a/spec/lib/selectors/table_spec.rb b/spec/lib/selectors/table_spec.rb index 85007dc..3290986 100644 --- a/spec/lib/selectors/table_spec.rb +++ b/spec/lib/selectors/table_spec.rb @@ -42,6 +42,27 @@ end end + describe '#table_selector' do + it 'returns the generic selector when no resource name given' do + expect(helper.table_selector).to eq('table.index_table') + end + + it 'builds the id from a registered resource name' do + expect(helper.table_selector('Business Employee')).to eq('table#index_table_business_employees') + end + + # ActiveAdmin's id comes from `resource_name.plural`, which underscores the + # namespace. `gsub(' ', '_')` left the `::` in place, and Nokogiri raises + # on `table#index_table_billing::employees`. + it 'underscores a namespaced name instead of leaving ::' do + expect(helper.table_selector('Billing::Employee')).to eq('table#index_table_billing_employees') + end + + it 'asks a model class for its own plural' do + expect(helper.table_selector(Billing::Employee)).to eq('table#index_table_billing_employees') + end + end + describe '#table_cell_selector' do it 'returns generic selector when column is nil' do expect(helper.table_cell_selector).to eq('td.col') diff --git a/spec/system/namespaced_model_names_spec.rb b/spec/system/namespaced_model_names_spec.rb new file mode 100644 index 0000000..5377d55 --- /dev/null +++ b/spec/system/namespaced_model_names_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# ActiveAdmin and Arbre derive these DOM names from the model and the +# registered resource name. Asserting them against a real rendered page is the +# only way to catch a derivation that merely looks plausible: the previous one +# produced `billing::employee`, which is not a selector at all. +RSpec.describe 'DOM names of a namespaced resource', type: :feature, js: true do + let!(:record) { Billing::Employee.create!(full_name: 'John Doe', salary: 100) } + + it 'finds the attributes table from the model class and from its name' do + visit admin_business_employee_path(record.id) + + expect(page).to have_attributes_table(model: Billing::Employee) + expect(page).to have_attributes_table(model: 'Billing::Employee') + end + + it 'finds the index table by the name the resource was registered under' do + visit admin_business_employees_path + + expect(page).to have_table(resource_name: 'Business Employee') + end +end