diff --git a/Gemfile b/Gemfile index af589cd..6becdec 100644 --- a/Gemfile +++ b/Gemfile @@ -15,6 +15,8 @@ end gem "madmin" +gem "ostruct" # madmin builds every resource attribute with `OpenStruct.new` but never requires "ostruct" + gem "pg", ">= 0.18", "< 2.0" gem "puma", "~> 6.0" gem "sass-rails" diff --git a/Gemfile.lock b/Gemfile.lock index d8f8329..9b73517 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,7 +96,7 @@ GEM activesupport (>= 6.1) i18n (1.15.2) concurrent-ruby (~> 1.0) - json (2.6.0) + json (2.21.1) language_server-protocol (3.17.0.6) lint_roller (1.1.0) listen (3.7.0) @@ -135,6 +135,7 @@ GEM nokogiri (1.19.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) + ostruct (0.6.3) pagy (4.11.0) parallel (2.1.0) parser (3.3.12.0) @@ -267,6 +268,7 @@ DEPENDENCIES listen (~> 3.2) madmin minitest (~> 5.0) + ostruct pg (>= 0.18, < 2.0) puma (~> 6.0) rails (~> 7.0.0) diff --git a/Gemfile.next.lock b/Gemfile.next.lock index 6d118eb..076a06b 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -111,7 +111,7 @@ GEM prism (>= 1.3.0) rdoc (>= 4.0.0) reline (>= 0.4.2) - json (2.6.0) + json (2.21.1) language_server-protocol (3.17.0.6) lint_roller (1.1.0) listen (3.7.0) @@ -149,6 +149,7 @@ GEM nokogiri (1.19.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) + ostruct (0.6.3) pagy (4.11.0) parallel (2.1.0) parser (3.3.12.0) @@ -304,6 +305,7 @@ DEPENDENCIES listen (~> 3.2) madmin minitest (~> 5.0) + ostruct pg (>= 0.18, < 2.0) puma (~> 6.0) rails (~> 7.1.0) diff --git a/test/integration/madmin_test.rb b/test/integration/madmin_test.rb index 167701e..9f7f219 100644 --- a/test/integration/madmin_test.rb +++ b/test/integration/madmin_test.rb @@ -2,8 +2,12 @@ # Smoke tests for the madmin admin panel. Madmin generates its controllers and # renders its views out of the gem, so an upgrade can break the admin without -# touching a single file in this repo. These tests only assert that each screen -# renders, which is what we need in order to notice. +# touching a single file in this repo. +# +# Each screen test asserts on rendered content, not just the status code. A 200 +# with a blank body is still a broken admin, and madmin swallows attribute +# resolution errors (see `test_every_resource_resolves_its_attributes`), so the +# body is the only place some failures show up. class MadminTest < ActionDispatch::IntegrationTest def setup @report = Report.create!(report: file_fixture_body) @@ -28,6 +32,29 @@ def test_dashboard_renders get_as_admin "/madmin" assert_equal 200, status + # The navigation renders one link per registered resource, and building those + # links is what forced madmin to resolve every resource's attributes. + assert_select "a", text: "Reports" + assert_select "a", text: "AnalyzedFiles" + end + + # madmin resolves each `attribute` in a resource against the model, and wraps + # that in a bare `rescue` that reports any failure as a missing attribute + # (madmin-1.2.5/lib/madmin/resource.rb:39). A NameError on OpenStruct came out + # of it as "Madmin couldn't find attribute or association 'name'", which is + # what took the whole admin down in production while every screen test passed + # against a bundle where something else happened to require "ostruct". + # + # Asserting resolution directly gives that failure a name, and covers + # resources added later without a screen test of their own. + def test_every_resource_resolves_its_attributes + assert_predicate Madmin.resources, :any?, "expected madmin to have resources registered" + + Madmin.resources.each do |resource| + attributes = resource.attributes + + assert_predicate attributes, :any?, "expected #{resource} to resolve attributes" + end end # With nothing configured the panel has to refuse everyone. Comparing unset @@ -53,6 +80,7 @@ def test_reports_screens_render get_as_admin path assert_equal 200, status, "expected #{path} to render" + assert_select "h1", text: /Report/, message: "expected #{path} to render a heading" end end @@ -64,9 +92,24 @@ def test_analyzed_files_screens_render get_as_admin path assert_equal 200, status, "expected #{path} to render" + assert_select "h1", text: /AnalyzedFile/, message: "expected #{path} to render a heading" end end + # The index and show screens have to render the record's own attributes, which + # is where a resource that resolved but rendered nothing would show up. + def test_screens_render_the_records_attributes + get_as_admin "/madmin/analyzed_files" + + assert_includes response.body, @analyzed_file.name + assert_includes response.body, "140.36" + + get_as_admin "/madmin/analyzed_files/#{@analyzed_file.id}" + + assert_includes response.body, @analyzed_file.name + assert_includes response.body, "140.36" + end + def test_updating_a_report_through_the_admin patch "/madmin/reports/#{@report.id}", params: { report: { compare: true } },