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('