diff --git a/Gemfile b/Gemfile index f55ec04..b667cf6 100644 --- a/Gemfile +++ b/Gemfile @@ -62,6 +62,13 @@ group :development, :test do gem 'shoulda-matchers' end +group :test do + # For system specs (spec/system). selenium-webdriver >= 4.6 manages the + # chromedriver binary itself (Selenium Manager) - no separate webdriver gem needed. + gem 'capybara' + gem 'selenium-webdriver' +end + gem 'pundit', '~> 2.2' gem 'kaminari', '~> 1.2' diff --git a/Gemfile.lock b/Gemfile.lock index 418e2d9..609e992 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -122,6 +122,15 @@ GEM bootsnap (1.18.4) msgpack (~> 1.2) builder (3.3.0) + capybara (3.40.0) + addressable + matrix + mini_mime (>= 0.1.3) + nokogiri (~> 1.11) + rack (>= 1.6.0) + rack-test (>= 0.6.3) + regexp_parser (>= 1.5, < 3.0) + xpath (~> 3.2) concurrent-ruby (1.3.4) connection_pool (2.4.1) crass (1.0.6) @@ -226,6 +235,7 @@ GEM net-pop net-smtp marcel (1.0.4) + matrix (0.4.3) mime-types (3.5.2) mime-types-data (~> 3.2015) mime-types-data (3.2024.0820) @@ -399,6 +409,12 @@ GEM hashie securerandom (0.3.1) selectize-rails (0.12.6) + selenium-webdriver (4.46.0) + base64 (~> 0.2) + logger (~> 1.4) + rexml (~> 3.2, >= 3.2.5) + rubyzip (>= 1.2.2, < 4.0) + websocket (~> 1.0) shoulda-matchers (6.4.0) activesupport (>= 5.2.0) sidekiq (7.3.8) @@ -436,9 +452,12 @@ GEM uuid (2.3.9) macaddr (~> 1.0) webrick (1.8.1) + websocket (1.2.11) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) + xpath (3.2.0) + nokogiri (~> 1.8) zeitwerk (2.6.17) PLATFORMS @@ -468,6 +487,7 @@ DEPENDENCIES aws-sdk-rails (~> 5.0) aws-sdk-ses (~> 1.0) bootsnap + capybara csv debug elasticsearch (~> 8) @@ -493,6 +513,7 @@ DEPENDENCIES rubocop-rspec rubocop-rspec_rails searchkick + selenium-webdriver shoulda-matchers sidekiq (>= 7.2.2, < 8) sidekiq-cron (~> 2.4.0) diff --git a/app/fields/has_many_through_field.rb b/app/fields/has_many_through_field.rb index c4a106d..af7f36f 100644 --- a/app/fields/has_many_through_field.rb +++ b/app/fields/has_many_through_field.rb @@ -8,21 +8,19 @@ def to_s end def associated_resource_options - # is_entities = resource['_index']&.include?('entities') - # where = {} - # order = {} - # where[options[:type]] = options[:type] if options[:type] - # order[options[:order_by]] = :acs if options[:order_by] - # associated_class.search('*', load: false, order:, where:).map do |resource| - # if options[:verbose_option] && is_entities - # ["#{resource.e_type.titleize} #{resource.legacy_pk}: #{resource.clean_label}", resource.id] - # else - # [resource.clean_label, resource.id] - # end - # end where = {} where[:e_type] = options[:type] if options[:type] - associated_class.search('*', load: false, order: { e_type: :asc }, where:).map do |resource| + + # Sorting here in Ruby, rather than passing `order:` to .search, is deliberate: + # an ES-level sort requires the field to have a keyword/sortable mapping, which + # e_type doesn't (Searchable, app/models/concerns/searchable.rb, never declares + # one) - that was raising a Searchkick::InvalidQueryError for every letter's + # entity picker (order: { e_type: :asc } was hardcoded here regardless of the + # order_by option below actually being requested). + results = associated_class.search('*', load: false, where:) + results = results.sort_by {|resource| resource.public_send(options[:order_by]).to_s } if options[:order_by] + + results.map do |resource| if options[:verbose_option] && resource['_index'].include?('entities') ["#{resource.e_type.titleize} #{resource.legacy_pk}: #{resource.clean_label}", resource.id] else diff --git a/app/views/admin/letters/_form.html.erb b/app/views/admin/letters/_form.html.erb index 766388b..a9ba023 100644 --- a/app/views/admin/letters/_form.html.erb +++ b/app/views/admin/letters/_form.html.erb @@ -188,10 +188,6 @@ and renders all form fields for a resource's editable attributes. } defer(() => { - let authenticity_token = document.querySelector("[name='authenticity_token']").value; - let headers = { - "content-type": "application/x-www-form-urlencoded" - }; // let letter = "<%= page.resource.id %>" const input = document.createElement("input"); input.type = "hidden"; diff --git a/config/environments/production.rb b/config/environments/production.rb index 63270ae..19fefdf 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -63,12 +63,12 @@ # Set this to true and configure the email server for immediate delivery to raise delivery errors. # config.action_mailer.raise_delivery_errors = false - # Send mail through SES (see app/lib/ses_delivery_method.rb and - # config/initializers/action_mailer_ses.rb), authenticating via the instance/task's - # IAM role through the AWS SDK's standard credential chain - no explicit AWS - # credentials configured here. + # Send mail through SES. Registration (and the region setting) lives in + # config/initializers/action_mailer_ses.rb / app/lib/ses_delivery_method.rb, not + # here - see the comment there for why. Authenticates via the instance/task's IAM + # role through the AWS SDK's standard credential chain, no explicit AWS credentials + # configured here. config.action_mailer.delivery_method = :ses - config.action_mailer.ses_settings = { region: ENV.fetch('AWS_REGION', 'us-east-1') } # Enable locale fallbacks for I18n (makes lookups for any locale fall back to # the I18n.default_locale when a translation cannot be found). diff --git a/config/initializers/action_mailer_ses.rb b/config/initializers/action_mailer_ses.rb index 81bed49..78968a6 100644 --- a/config/initializers/action_mailer_ses.rb +++ b/config/initializers/action_mailer_ses.rb @@ -4,4 +4,10 @@ # before Zeitwerk has the app/lib root set up. require Rails.root.join('app/lib/ses_delivery_method') -ActionMailer::Base.add_delivery_method :ses, SesDeliveryMethod +# Passed here, as add_delivery_method's default_options, rather than via +# config.action_mailer.ses_settings in config/environments/*.rb: referencing +# ActionMailer::Base below is what triggers its first load in this app, and Rails +# applies config.action_mailer.* settings via a load hook that fires on that same +# first load - so a `ses_settings=` assigned in production.rb would run before this +# add_delivery_method call has defined that setter, raising NoMethodError. +ActionMailer::Base.add_delivery_method :ses, SesDeliveryMethod, region: ENV.fetch('AWS_REGION', 'us-east-1') diff --git a/spec/dashboards/about_page_dashboard_spec.rb b/spec/dashboards/about_page_dashboard_spec.rb new file mode 100644 index 0000000..31004b5 --- /dev/null +++ b/spec/dashboards/about_page_dashboard_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AboutPageDashboard do + it_behaves_like 'a dashboard with display_resource', + factory: :about_page, + expected: ->(about_page) { "AboutPage ##{about_page.title}" } +end diff --git a/spec/dashboards/faq_dashboard_spec.rb b/spec/dashboards/faq_dashboard_spec.rb new file mode 100644 index 0000000..815c1e2 --- /dev/null +++ b/spec/dashboards/faq_dashboard_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe FaqDashboard do + it_behaves_like 'a dashboard with display_resource', + factory: :faq, + expected: ->(faq) { "Faq ##{faq.question}" } +end diff --git a/spec/dashboards/letter_dashboard_spec.rb b/spec/dashboards/letter_dashboard_spec.rb new file mode 100644 index 0000000..0eece31 --- /dev/null +++ b/spec/dashboards/letter_dashboard_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe LetterDashboard do + it_behaves_like 'a dashboard with display_resource', + factory: :letter, + expected: ->(letter) { "Letter ##{letter.legacy_pk}" } + + describe '#permitted_attributes' do + it 'adds start_date on top of the default FORM_ATTRIBUTES-derived list' do + # FORM_ATTRIBUTES is %i[entities content] - `entities` (a has-many field) + # permits as {entity_ids: []}, `content` permits as itself, and this override + # adds start_date and content again (already present via FORM_ATTRIBUTES, so + # a harmless duplicate rather than a second distinct attribute). + expect(described_class.new.permitted_attributes).to eq( + [{ entity_ids: [] }, :content, :start_date, :content] + ) + end + end +end diff --git a/spec/dashboards/letter_owner_dashboard_spec.rb b/spec/dashboards/letter_owner_dashboard_spec.rb new file mode 100644 index 0000000..b16c6c5 --- /dev/null +++ b/spec/dashboards/letter_owner_dashboard_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe LetterOwnerDashboard do + it_behaves_like 'a dashboard with display_resource', + factory: :letter_owner, + expected: ->(letter_owner) { letter_owner.label } +end diff --git a/spec/dashboards/letter_publisher_dashboard_spec.rb b/spec/dashboards/letter_publisher_dashboard_spec.rb new file mode 100644 index 0000000..8283bde --- /dev/null +++ b/spec/dashboards/letter_publisher_dashboard_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe LetterPublisherDashboard do + it_behaves_like 'a dashboard with display_resource', + factory: :letter_publisher, + expected: ->(letter_publisher) { letter_publisher.label } +end diff --git a/spec/dashboards/repository_dashboard_spec.rb b/spec/dashboards/repository_dashboard_spec.rb new file mode 100644 index 0000000..1b291ac --- /dev/null +++ b/spec/dashboards/repository_dashboard_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe RepositoryDashboard do + it_behaves_like 'a dashboard with display_resource', + factory: :repository, + expected: ->(repository) { "Repository #{repository.label}" } +end diff --git a/spec/fields/contenteditable_field_spec.rb b/spec/fields/contenteditable_field_spec.rb new file mode 100644 index 0000000..fc1da38 --- /dev/null +++ b/spec/fields/contenteditable_field_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'administrate/field/base' + +RSpec.describe ContenteditableField do + describe '#to_s' do + it 'returns the raw data unchanged, with no sanitization' do + html = '

raw html

' + field = described_class.new(:content, html, nil) + + expect(field.to_s).to eq(html) + end + end +end diff --git a/spec/fields/has_many_through_field_spec.rb b/spec/fields/has_many_through_field_spec.rb new file mode 100644 index 0000000..6d8a20a --- /dev/null +++ b/spec/fields/has_many_through_field_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'rails_helper' +# In a full app boot Administrate's engine loads its own Field classes before any +# app/fields/*.rb file is ever referenced, but running this spec in isolation can +# autoload HasManyThroughField (which inherits from Administrate::Field::HasMany) +# before that happens. +require 'administrate/field/has_many' + +RSpec.describe HasManyThroughField do + let(:letter) { Letter.new } + + def field_for(options) + described_class.new(:entities, nil, nil, options.merge(resource: letter)) + end + + describe '#to_s' do + it 'returns the raw data' do + field = described_class.new(:entities, %w[a b], nil, resource: letter) + expect(field.to_s).to eq(%w[a b]) + end + end + + describe '#associated_resource_options' do + it 'scopes the search to options[:type] when given' do + field = field_for(type: 'person') + allow(Entity).to receive(:search).and_return([]) + + field.associated_resource_options + + expect(Entity).to have_received(:search).with('*', load: false, where: { e_type: 'person' }) + end + + it 'does not scope the search when no type option is given' do + field = field_for({}) + allow(Entity).to receive(:search).and_return([]) + + field.associated_resource_options + + expect(Entity).to have_received(:search).with('*', load: false, where: {}) + end + + it 'does not ask Elasticsearch to sort - e_type has no sortable mapping' do + # Regression test: this field used to hardcode order: { e_type: :asc } in the + # Searchkick query regardless of any order_by option, which raised + # Searchkick::InvalidQueryError for every letter's entity picker (e_type isn't + # mapped as sortable - see app/models/concerns/searchable.rb). Sorting now + # happens in Ruby, after the results come back - see the next example. + field = field_for(order_by: 'e_type') + allow(Entity).to receive(:search).and_return([]) + + field.associated_resource_options + + expect(Entity).to have_received(:search).with('*', load: false, where: {}) + end + + it 'sorts the results in Ruby by options[:order_by] when given' do + person = instance_double(Entity, e_type: 'person', clean_label: 'B Person', id: '1') + allow(person).to receive(:[]).with('_index').and_return('beckett_entities_test') + place = instance_double(Entity, e_type: 'place', clean_label: 'A Place', id: '2') + allow(place).to receive(:[]).with('_index').and_return('beckett_entities_test') + field = field_for(order_by: 'e_type') + allow(Entity).to receive(:search).and_return([person, place]) + + # "person" sorts before "place" alphabetically (e < l at the second character) + expect(field.associated_resource_options).to eq([['B Person', '1'], ['A Place', '2']]) + end + + it 'formats verbose entity results as "Type PK: Label"' do + result = instance_double(Entity, e_type: 'person', legacy_pk: 42, clean_label: 'Beckett, Samuel', id: 'abc') + allow(result).to receive(:[]).with('_index').and_return('beckett_entities_test') + field = field_for(verbose_option: true) + allow(Entity).to receive(:search).and_return([result]) + + expect(field.associated_resource_options).to eq([['Person 42: Beckett, Samuel', 'abc']]) + end + + it 'formats non-verbose results as [label, id]' do + result = instance_double(Entity, clean_label: 'Paris', id: 'xyz') + allow(result).to receive(:[]).with('_index').and_return('beckett_places_test') + field = field_for({}) + allow(Entity).to receive(:search).and_return([result]) + + expect(field.associated_resource_options).to eq([%w[Paris xyz]]) + end + + it 'formats verbose results as [label, id] when the result is not from the entities index' do + result = instance_double(Entity, clean_label: 'Some Repository', id: 'def') + allow(result).to receive(:[]).with('_index').and_return('beckett_repositories_test') + field = field_for(verbose_option: true) + allow(Entity).to receive(:search).and_return([result]) + + expect(field.associated_resource_options).to eq([['Some Repository', 'def']]) + end + end +end diff --git a/spec/fields/rich_text_field_spec.rb b/spec/fields/rich_text_field_spec.rb new file mode 100644 index 0000000..0f353ed --- /dev/null +++ b/spec/fields/rich_text_field_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'administrate/field/base' + +RSpec.describe RichTextField do + describe '#to_s' do + it 'permits the allowed tags and attributes' do + html = '

Bold text

' + field = described_class.new(:description, html, nil) + + expect(field.to_s).to eq(html) + end + + it 'strips the script tag itself (its text content survives as inert text, same as Rails::Html::Sanitizer)' do + field = described_class.new(:description, '

safe

', nil) + + result = field.to_s + + expect(result).not_to include('click me

', nil) + + result = field.to_s + + expect(result).not_to include('onclick') + expect(result).not_to include('alert(1)') + expect(result).to include('click me') + end + + it 'strips disallowed tags but keeps their text content' do + field = described_class.new(:description, 'plain text', nil) + + result = field.to_s + + expect(result).not_to include('click', nil) + + result = field.to_s + + expect(result).not_to include('javascript:') + end + end +end diff --git a/spec/requests/admin/about_pages_spec.rb b/spec/requests/admin/about_pages_spec.rb new file mode 100644 index 0000000..716a164 --- /dev/null +++ b/spec/requests/admin/about_pages_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::AboutPages' do + describe 'GET index' do + it 'requires authentication' do + get admin_about_pages_path + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + create(:about_page) + get admin_about_pages_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET show' do + it 'renders successfully when authenticated' do + about_page = create(:about_page) + get admin_about_page_path(about_page), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET edit' do + it 'renders successfully when authenticated' do + about_page = create(:about_page) + get edit_admin_about_page_path(about_page), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe '#accessible_action?' do + # Admin::AboutPagesController#accessible_action? is meant to hide the "new" and + # "destroy" links/buttons (its guard checks `name.to_s` against + # %w[destroy add new create]). In practice it doesn't work: Administrate's views + # call accessible_action?(resource, action) - resource first - but the override's + # parameters are (name, resource = resource_class), so `name` is bound to the + # resource object, not the action name, and `name.to_s` never matches the exclude + # list. Both links render anyway. This documents the actual behavior as a known + # bug rather than silently patching it or asserting the intended-but-wrong behavior. + it 'still renders the "new" link on the index page' do + get admin_about_pages_path, headers: admin_auth_headers + expect(response.body).to include(%(href="#{new_admin_about_page_path}")) + end + + it 'still renders the "destroy" link on the show page' do + about_page = create(:about_page) + get admin_about_page_path(about_page), headers: admin_auth_headers + expect(response.body).to include(%(href="#{admin_about_page_path(about_page)}")) + .and include('button--danger') + end + end +end diff --git a/spec/requests/admin/big_sams_spec.rb b/spec/requests/admin/big_sams_spec.rb new file mode 100644 index 0000000..0f093ea --- /dev/null +++ b/spec/requests/admin/big_sams_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'rails_helper' + +# BigSam#load_letters (app/models/big_sam.rb) destroys the record immediately after +# processing its upload (see LoadBigSamJob#perform), so a persisted BigSam essentially +# never exists to view/edit in practice - only index (which works with zero records) and +# new are meaningfully testable here. +RSpec.describe 'Admin::BigSams' do + describe 'GET index' do + it 'requires authentication' do + get admin_big_sams_path + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + get admin_big_sams_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET new' do + it 'renders successfully when authenticated' do + get new_admin_big_sam_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/requests/admin/entities_spec.rb b/spec/requests/admin/entities_spec.rb new file mode 100644 index 0000000..73d8baf --- /dev/null +++ b/spec/requests/admin/entities_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::Entities' do + describe 'GET index' do + it 'requires authentication' do + get admin_entities_path + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + create(:entity) + get admin_entities_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + + it 'excludes entities with a dash-only or comma-only label' do + # Entity#remove_blank_values (a before_save callback) normalizes a genuinely + # blank label to nil before it ever reaches the DB - and label is NOT NULL - so + # only the dash/comma placeholder values are reachable to test here. Building + + # saving directly (rather than the :entity factory's create strategy) skips the + # factory's own after(:create) hook, which randomly wraps label in tags to + # fuzz other specs - that would make the exact-string match below flaky. + # + # Each is looked up by its own unique legacy_pk (searchable), rather than + # scanning the shared, unpaginated index page, since the label exclusion filter + # (in the controller) applies after search regardless - this keeps the + # assertion independent of how much other data/pagination exists from other + # specs sharing this DB. + # + # e_type is pinned to 'place' deliberately: Entity#concat_label (another + # before_save callback) rewrites label entirely for e_type 'attendance' or + # 'person', which would silently clobber the exact label this test depends on + # if the factory's random e_type happened to land on either. + good = build(:entity, label: 'A Real Label', e_type: 'place').tap {|e| e.save!(validate: false) } + dash = build(:entity, label: '-', e_type: 'place').tap {|e| e.save!(validate: false) } + comma = build(:entity, label: ', ', e_type: 'place').tap {|e| e.save!(validate: false) } + + get admin_entities_path(search: good.legacy_pk), headers: admin_auth_headers + expect(response.body).to include(admin_entity_path(good)) + + get admin_entities_path(search: dash.legacy_pk), headers: admin_auth_headers + expect(response.body).not_to include(admin_entity_path(dash)) + + get admin_entities_path(search: comma.legacy_pk), headers: admin_auth_headers + expect(response.body).not_to include(admin_entity_path(comma)) + end + end + + describe 'GET show' do + it 'renders successfully when authenticated' do + entity = create(:entity) + get admin_entity_path(entity), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET new' do + it 'renders successfully when authenticated' do + get new_admin_entity_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + + it 'pre-populates e_type from the type param' do + get new_admin_entity_path(type: 'place'), headers: admin_auth_headers + expect(response.body).to include('name="entity[e_type]" value=place') + end + end + + describe 'GET edit' do + it 'renders successfully when authenticated' do + entity = create(:entity) + get edit_admin_entity_path(entity), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/requests/admin/faqs_spec.rb b/spec/requests/admin/faqs_spec.rb new file mode 100644 index 0000000..949fa40 --- /dev/null +++ b/spec/requests/admin/faqs_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::Faqs' do + it_behaves_like 'a routed admin dashboard', + factory: :faq, + index_helper: :admin_faqs_path, + show_helper: :admin_faq_path, + new_helper: :new_admin_faq_path, + edit_helper: :edit_admin_faq_path +end diff --git a/spec/requests/admin/languages_spec.rb b/spec/requests/admin/languages_spec.rb new file mode 100644 index 0000000..6c201ef --- /dev/null +++ b/spec/requests/admin/languages_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'rails_helper' + +# Only `show` is routed for languages (config/routes.rb) - no index/new/edit to cover. +RSpec.describe 'Admin::Languages' do + describe 'GET show' do + it 'requires authentication' do + language = create(:language) + get admin_language_path(language) + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + language = create(:language) + get admin_language_path(language), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/requests/admin/letters_spec.rb b/spec/requests/admin/letters_spec.rb new file mode 100644 index 0000000..e6cb6e9 --- /dev/null +++ b/spec/requests/admin/letters_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::Letters' do + # LetterDashboard::SHOW_PAGE_ATTRIBUTES includes :published, but ATTRIBUTE_TYPES has + # no :published entry. This breaks the show page directly (show.html.erb iterates + # SHOW_PAGE_ATTRIBUTES), and also breaks index: Administrate's own + # sanitized_order_params helper (called from the custom _collection.html.erb, used + # for both index and show) always calls dashboard#item_associations, which + # unconditionally reads show_page_attributes regardless of which page is asking. + def published_not_in_attribute_types + 'known bug: LetterDashboard::SHOW_PAGE_ATTRIBUTES includes :published, which has no ' \ + 'entry in ATTRIBUTE_TYPES - raises RuntimeError via Administrate::BaseDashboard#' \ + 'attribute_type_for, reached from show.html.erb directly and from index.html.erb via ' \ + 'sanitized_order_params -> #item_associations (always uses show_page_attributes)' + end + + describe 'GET index' do + it 'requires authentication' do + get admin_letters_path + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + skip(published_not_in_attribute_types) + create(:letter) + get admin_letters_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + + it 'includes letters within the given start_date/end_date range' do + skip(published_not_in_attribute_types) + in_range = create(:letter, date: DateTime.new(1960, 6, 1), code: 'IN-RANGE') + get admin_letters_path(start_date: '1960-01-01', end_date: '1960-12-31'), headers: admin_auth_headers + expect(response.body).to include("Letter ##{in_range.legacy_pk}") + end + + it 'excludes dated letters outside the given start_date/end_date range' do + skip(published_not_in_attribute_types) + out_of_range = create(:letter, date: DateTime.new(1900, 1, 1), code: 'OUT-OF-RANGE') + get admin_letters_path(start_date: '1960-01-01', end_date: '1960-12-31'), headers: admin_auth_headers + expect(response.body).not_to include("Letter ##{out_of_range.legacy_pk}") + end + + it 'always includes undated letters, regardless of the date range' do + skip(published_not_in_attribute_types) + undated = create(:letter, date: nil, code: 'UNDATED') + get admin_letters_path(start_date: '1960-01-01', end_date: '1960-12-31'), headers: admin_auth_headers + expect(response.body).to include("Letter ##{undated.legacy_pk}") + end + + it 'restores start_date/end_date from the referer when the request omits them' do + skip(published_not_in_attribute_types) + in_range = create(:letter, date: DateTime.new(1960, 6, 1), code: 'IN-RANGE') + out_of_range = create(:letter, date: DateTime.new(1900, 1, 1), code: 'OUT-OF-RANGE') + referer = "#{admin_letters_url}?start_date=1960-01-01&end_date=1960-12-31" + + get admin_letters_path, headers: admin_auth_headers.merge('HTTP_REFERER' => referer) + + expect(response.body).to include("Letter ##{in_range.legacy_pk}") + expect(response.body).not_to include("Letter ##{out_of_range.legacy_pk}") + end + + it 'ignores the referer when it is not an admin/letters URL' do + skip(published_not_in_attribute_types) + out_of_range = create(:letter, date: DateTime.new(1900, 1, 1), code: 'OUT-OF-RANGE') + referer = "#{admin_entities_url}?start_date=1960-01-01&end_date=1960-12-31" + + get admin_letters_path, headers: admin_auth_headers.merge('HTTP_REFERER' => referer) + + expect(response.body).to include("Letter ##{out_of_range.legacy_pk}") + end + end + + describe 'GET show' do + it 'renders successfully when authenticated' do + skip(published_not_in_attribute_types) + letter = create(:letter) + get admin_letter_path(letter), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET new' do + it 'renders successfully when authenticated' do + get new_admin_letter_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET edit' do + it 'renders successfully when authenticated' do + letter = create(:letter) + get edit_admin_letter_path(letter), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'PATCH update' do + # LetterDashboard::FORM_ATTRIBUTES is %i[entities content] (+ start_date, per + # #permitted_attributes) - `content` is the only plain-text field actually + # permitted through strong params here. + it 'redirects to the resource by default' do + letter = create(:letter) + patch admin_letter_path(letter), params: { letter: { content: 'Updated content' } }, + headers: admin_auth_headers + expect(response).to redirect_to(admin_letter_path(letter)) + expect(letter.reload.content).to eq('Updated content') + end + + it 'does not redirect when redirect=no, but still persists the update' do + letter = create(:letter) + patch admin_letter_path(letter), + params: { letter: { content: 'Updated content' }, redirect: 'no' }, + headers: admin_auth_headers + expect(response).not_to have_http_status(:redirect) + expect(letter.reload.content).to eq('Updated content') + end + end +end diff --git a/spec/requests/admin/media_spec.rb b/spec/requests/admin/media_spec.rb new file mode 100644 index 0000000..164afbd --- /dev/null +++ b/spec/requests/admin/media_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::Media' do + describe 'GET index' do + it 'requires authentication' do + get admin_media_path + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + create(:medium) + get admin_media_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET show' do + it 'renders successfully when authenticated' do + pending( + 'known bug: admin/media/show.html.erb:28 calls accessible_action?(:edit) with one ' \ + 'argument before the correct two-argument call - Administrate::ApplicationHelper#' \ + 'accessible_action? requires (target, action_name), so this raises ArgumentError ' \ + "before the &&'d correct call, or page.resource.url (line 33, Medium has no #url " \ + 'method), is ever reached' + ) + medium = create(:medium) + get admin_medium_path(medium), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET new' do + it 'renders successfully when authenticated' do + get new_admin_medium_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET edit' do + it 'renders successfully when authenticated' do + medium = create(:medium) + get edit_admin_medium_path(medium), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/requests/admin/mentions_spec.rb b/spec/requests/admin/mentions_spec.rb new file mode 100644 index 0000000..d071911 --- /dev/null +++ b/spec/requests/admin/mentions_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::Mentions' do + describe 'GET index' do + it 'requires authentication' do + get admin_mentions_path + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + create(:mention) + get admin_mentions_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET show' do + it 'renders successfully when authenticated' do + mention = create(:mention) + get admin_mention_path(mention), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET new' do + it 'renders successfully when authenticated' do + pending( + 'known bug: admin/mentions/_form.html.erb:36 iterates page.attributes(...) as a ' \ + 'flat list of fields, but it actually yields [title, attributes] pairs (see ' \ + 'admin/letters/_form.html.erb for the correct nested form), so `attribute.html_class` ' \ + 'raises NoMethodError on the Array' + ) + get new_admin_mention_path, headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET edit' do + it 'renders successfully when authenticated' do + pending( + 'known bug: admin/mentions/edit.html.erb:37 calls ' \ + 'accessible_action?(:show) with one argument before the correct two-argument call - ' \ + 'Administrate::ApplicationHelper#accessible_action? requires (target, action_name), so ' \ + 'this raises ArgumentError before the &&\'d correct call is ever reached' + ) + mention = create(:mention) + get edit_admin_mention_path(mention), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/requests/admin/repositories_spec.rb b/spec/requests/admin/repositories_spec.rb new file mode 100644 index 0000000..c5e9ebe --- /dev/null +++ b/spec/requests/admin/repositories_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::Repositories' do + it_behaves_like 'a routed admin dashboard', + factory: :repository, + index_helper: :admin_repositories_path, + show_helper: :admin_repository_path, + new_helper: :new_admin_repository_path, + edit_helper: :edit_admin_repository_path +end diff --git a/spec/support/admin_auth_helper.rb b/spec/support/admin_auth_helper.rb new file mode 100644 index 0000000..217ee24 --- /dev/null +++ b/spec/support/admin_auth_helper.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Admin::ApplicationController gates every admin route behind HTTP Basic Auth, +# hardcoded to test/test in the test environment (see +# app/controllers/admin/application_controller.rb). +module AdminAuthHelper + def admin_auth_headers + { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('test', 'test') } + end +end + +RSpec.configure do |config| + config.include AdminAuthHelper, type: :request +end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb new file mode 100644 index 0000000..dfddde6 --- /dev/null +++ b/spec/support/capybara.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'selenium/webdriver' + +# Mirrors Administrate's own spec/support/webdrivers.rb (thoughtbot/administrate) - +# selenium-webdriver >= 4.6 manages the chromedriver binary itself, no separate +# webdriver-manager gem needed. +Capybara.register_driver :headless_chrome do |app| + options = Selenium::WebDriver::Chrome::Options.new + options.add_argument('--headless=new') + options.add_argument('--window-size=1680,1050') + options.add_argument('--disable-gpu') + options.add_argument('--disable-dev-shm-usage') + + Capybara::Selenium::Driver.new(app, browser: :chrome, options:) +end + +Capybara.javascript_driver = :headless_chrome + +RSpec.configure do |config| + config.before(:each, type: :system) do + driven_by :rack_test + end + + config.before(:each, :js, type: :system) do + driven_by Capybara.javascript_driver + end +end diff --git a/spec/support/shared_examples/dashboard_display_resource.rb b/spec/support/shared_examples/dashboard_display_resource.rb new file mode 100644 index 0000000..66ec176 --- /dev/null +++ b/spec/support/shared_examples/dashboard_display_resource.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# Covers the common `display_resource` override pattern used by most dashboards +# (app/dashboards/*.rb): a simple, resource-specific string formatter with no other +# logic worth a bespoke spec. +# +# Required parameters: +# factory: the FactoryBot factory to build a resource with +# expected: ->(resource) { "expected display string" } +RSpec.shared_examples 'a dashboard with display_resource' do |factory:, expected:| + describe '#display_resource' do + it 'formats the resource for display' do + resource = create(factory) + + expect(described_class.new.display_resource(resource)).to eq(expected.call(resource)) + end + end +end diff --git a/spec/support/shared_examples/routed_admin_dashboard.rb b/spec/support/shared_examples/routed_admin_dashboard.rb new file mode 100644 index 0000000..66b61b7 --- /dev/null +++ b/spec/support/shared_examples/routed_admin_dashboard.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +# Baseline smoke coverage for a routed Administrate dashboard: confirms index/show/new/edit +# all render successfully when authenticated, and that none of them are reachable without +# auth. This exists to catch broad Administrate API breakage (helper methods, template +# changes) across every live admin resource with minimal duplication - see +# spec/requests/admin/*_spec.rb for usage, and add resource-specific specs alongside for any +# custom controller/dashboard behavior. +# +# Route helpers are passed as symbols, not lambdas/procs: procs built at the +# `it_behaves_like` call site capture the wrong `self` (the example group class, not the +# running example instance), so calling them would raise NoMethodError. Calling them via +# `send` from inside this shared example's own `it` blocks resolves correctly instead. +# +# Required parameters: +# factory: the FactoryBot factory to build a resource with +# index_helper: :admin_xyzs_path +# show_helper: :admin_xyz_path +# new_helper: :new_admin_xyz_path +# edit_helper: :edit_admin_xyz_path +RSpec.shared_examples 'a routed admin dashboard' do |factory:, index_helper:, show_helper:, new_helper:, edit_helper:| + describe 'GET index' do + it 'requires authentication' do + get send(index_helper) + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + create(factory) + get send(index_helper), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET show' do + it 'requires authentication' do + resource = create(factory) + get send(show_helper, resource) + expect(response).to have_http_status(:unauthorized) + end + + it 'renders successfully when authenticated' do + resource = create(factory) + get send(show_helper, resource), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET new' do + it 'renders successfully when authenticated' do + get send(new_helper), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end + + describe 'GET edit' do + it 'renders successfully when authenticated' do + resource = create(factory) + get send(edit_helper, resource), headers: admin_auth_headers + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/system/admin/letters_spec.rb b/spec/system/admin/letters_spec.rb new file mode 100644 index 0000000..03b77fd --- /dev/null +++ b/spec/system/admin/letters_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'rails_helper' + +# app/views/admin/letters/_form.html.erb has no visible submit button - saving +# entirely depends on the jQuery + selectize widget's auto-submit-on-change behavior, +# loaded via the custom app/views/administrate/application/_javascript.html.erb. That +# makes it the single highest-risk piece of UI for an Administrate upgrade to silently +# break (jquery-ujs -> Turbo/Stimulus, changed asset bundling in 1.0) - and the only +# way to actually verify it is a real browser, since no request spec executes JS. +RSpec.describe 'Admin::Letters entity picker', :js do + def visit_with_basic_auth(path) + server = Capybara.current_session.server + visit "http://test:test@#{server.host}:#{server.port}#{path}" + end + + def selectize + "jQuery('.field-unit--has-many-through-field select')[0].selectize" + end + + it 'initializes jQuery and the selectize widget on the entities field' do + letter = create(:letter) + + visit_with_basic_auth(edit_admin_letter_path(letter)) + + expect(page.execute_script('return window.jQuery !== undefined')).to be true + expect(page).to have_css('.selectize-control') + expect(page.execute_script("return #{selectize} !== undefined")).to be true + end + + it 'adds an entity and persists it, without a visible submit button' do + letter = create(:letter) + entity = create(:person_entity, label: 'Zzz System Spec Findable Entity') + Entity.reindex + + visit_with_basic_auth(edit_admin_letter_path(letter)) + expect(page).to have_css('.selectize-control') + + page.execute_script("#{selectize}.addItem('#{entity.id}')") + + expect(page).to have_css(".item[data-value='#{entity.id}']") + expect(letter.reload.entities).to include(entity) + end + + it 'removes an entity after confirming, and persists the removal' do + letter = create(:letter) + entity = create(:person_entity, label: 'Zzz System Spec Removable Entity') + letter.entities << entity + letter.save! + Entity.reindex + + visit_with_basic_auth(edit_admin_letter_path(letter)) + expect(page).to have_css(".item[data-value='#{entity.id}']") + + find(".item[data-value='#{entity.id}']").click + accept_confirm(/remove this entity/i) do + find('.selectize-input input', visible: :all).send_keys(:backspace) + end + + expect(page).to have_no_css(".item[data-value='#{entity.id}']") + expect(letter.reload.entities).not_to include(entity) + end + + it 'does not remove the entity when the confirmation is dismissed' do + letter = create(:letter) + entity = create(:person_entity, label: 'Zzz System Spec Kept Entity') + letter.entities << entity + letter.save! + Entity.reindex + + visit_with_basic_auth(edit_admin_letter_path(letter)) + expect(page).to have_css(".item[data-value='#{entity.id}']") + + find(".item[data-value='#{entity.id}']").click + dismiss_confirm(/remove this entity/i) do + find('.selectize-input input', visible: :all).send_keys(:backspace) + end + + expect(page).to have_css(".item[data-value='#{entity.id}']") + expect(letter.reload.entities).to include(entity) + end +end