From 4acadc9a109dd377fc20fd6e37fd925a4a0cc28b Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 8 Jan 2025 20:11:39 +0000 Subject: [PATCH 01/14] Add Zeitwerk initializer Ignore files and directories which either have unusal inflections or can't be autoloaded by Zeitwerk. In the upcoming Rails upgrades Zeitwerk will attempt to load everything in lib at boot so we need to add these exceptions. --- config/initializers/zeitwerk.rb | 30 +++++++++++++++++++ .../acts_as_xapian_generator.rb | 1 + 2 files changed, 31 insertions(+) create mode 100644 config/initializers/zeitwerk.rb diff --git a/config/initializers/zeitwerk.rb b/config/initializers/zeitwerk.rb new file mode 100644 index 0000000000..f07b3fe679 --- /dev/null +++ b/config/initializers/zeitwerk.rb @@ -0,0 +1,30 @@ +Rails.autoloaders.main.inflector.inflect( + "alaveteli_geoip" => "AlaveteliGeoIP", + "alaveteli_gettext" => "AlaveteliGetText", + "html_to_pdf_converter" => "HTMLtoPDFConverter", + "ip_rate_limiter" => "IPRateLimiter", + "pstore_database" => "PStoreDatabase", + "public_body_csv" => "PublicBodyCSV", + "world_foi_websites" => "WorldFOIWebsites" +) + +Rails.autoloaders.main.ignore( + "lib/acts_as_xapian", + "lib/confidence_intervals.rb", + "lib/configuration.rb", + "lib/core_ext", + "lib/custom_cops", + "lib/generators", + "lib/has_tag_string", + "lib/i18n_fixes.rb", + "lib/languages.rb", + "lib/mail_handler/backends/mail_extensions.rb", + "lib/no_constraint_disabling.rb", + "lib/normalize_string.rb", + "lib/quiet_opener.rb", + "lib/routing_filters.rb", + "lib/stripe_mock_patch.rb", + "lib/theme.rb", + "lib/themes", + "lib/use_spans_for_errors.rb" +) diff --git a/lib/generators/acts_as_xapian/acts_as_xapian_generator.rb b/lib/generators/acts_as_xapian/acts_as_xapian_generator.rb index 434c02cb58..6151dd84e7 100644 --- a/lib/generators/acts_as_xapian/acts_as_xapian_generator.rb +++ b/lib/generators/acts_as_xapian/acts_as_xapian_generator.rb @@ -1,3 +1,4 @@ +require 'rails/generators' require 'rails/generators/active_record/migration' class ActsAsXapianGenerator < Rails::Generators::Base From 1d7c07d03fec9b1e1f120aefd941560b7882e674 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 8 Jan 2025 16:47:01 +0000 Subject: [PATCH 02/14] Update fixture set loading This fixes an exception in Rails 7.1 when using fixture sets: NameError: uninitialized constant ActiveRecord::FixtureSet::ClassCache --- lib/no_constraint_disabling.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/no_constraint_disabling.rb b/lib/no_constraint_disabling.rb index 6e520d6fb8..fdebebfa4b 100644 --- a/lib/no_constraint_disabling.rb +++ b/lib/no_constraint_disabling.rb @@ -26,7 +26,11 @@ module ActiveRecord class FixtureSet def self.create_fixtures(fixtures_directory, fixture_set_names, class_names = {}, config = ActiveRecord::Base) fixture_set_names = Array(fixture_set_names).map(&:to_s) - class_names = ClassCache.new class_names, config + if Rails.version < '7.1.0' + class_names = ClassCache.new class_names, config + else + class_names.stringify_keys! + end # FIXME: Apparently JK uses this. connection = block_given? ? yield : ActiveRecord::Base.connection From 9d75f977aaf11b68e1104689adf4f92a049f805d Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 8 Jan 2025 18:23:26 +0000 Subject: [PATCH 03/14] Add test storage service In Rails 7.1 it will require a default ActiveStorage service to be defined. This adds a service for the test environment so test examples don't break when we bump the Rails gem. This commit ensures the tests can be run on future bump commit. The other environments will be defined when we run `rails app:update`. --- config/environments/test.rb | 3 +++ config/storage.yml-example | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/config/environments/test.rb b/config/environments/test.rb index b8109c174c..dda91abe22 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -35,6 +35,9 @@ # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false + # Store uploaded files on the local file system in a temporary directory. + config.active_storage.service = :test + config.action_mailer.perform_caching = false # Tell Action Mailer not to deliver emails to the real world. diff --git a/config/storage.yml-example b/config/storage.yml-example index 378acdbe7d..8fc506215c 100644 --- a/config/storage.yml-example +++ b/config/storage.yml-example @@ -14,6 +14,10 @@ # project: '' # bucket: '' +test: + service: Disk + root: <%= Rails.root.join('tmp/storage') %> + ## Raw Emails ## raw_emails_disk: &raw_emails_disk From ef22b7e312ad0a90ec7cb22f24d65f8545d43e09 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 8 Jan 2025 15:16:57 +0000 Subject: [PATCH 04/14] Update create notes migration When migrating a new database under Rails 7.1 this migration breaks due to a new constraint ensuring enums have a database backed column. Our `Note#style` was added in a later migration so doesn't exist yet. Recreating the Note class without the enum defined allows full migration to succeed. --- db/migrate/20220720085105_create_notes.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/db/migrate/20220720085105_create_notes.rb b/db/migrate/20220720085105_create_notes.rb index 5e50edb3b8..40629de96c 100644 --- a/db/migrate/20220720085105_create_notes.rb +++ b/db/migrate/20220720085105_create_notes.rb @@ -1,4 +1,11 @@ class CreateNotes < ActiveRecord::Migration[6.1] + # Predefine Note model to allow creation of translation table before adding + # style enum is added. This prevents migration errors since enums require + # existing database columns. + class Note < ApplicationRecord + translates :body + end + def change create_table :notes do |t| t.references :notable, polymorphic: true From 9cf59e60db72ca07ef7f34882f38575f39592bdd Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 8 Jan 2025 16:47:01 +0000 Subject: [PATCH 05/14] Update translated fields test Ensure we are matching strings with strings. In Rails 7.1 this will return an `ActionView::OutputBuffer`. --- spec/helpers/admin/translated_record_form_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/admin/translated_record_form_spec.rb b/spec/helpers/admin/translated_record_form_spec.rb index 682f61c68f..9b99fb70d2 100644 --- a/spec/helpers/admin/translated_record_form_spec.rb +++ b/spec/helpers/admin/translated_record_form_spec.rb @@ -6,7 +6,7 @@ describe '#translated_fields' do subject do - builder.translated_fields { |t| template.concat(t.text_field(:name)) } + builder.translated_fields { |t| template.concat(t.text_field(:name)) }.to_s end let(:resource) do From bf3cd27bc745f7ee342195015a00b2f02a69d153 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 9 Jan 2025 11:34:32 +0000 Subject: [PATCH 06/14] Remove undefined action in callback options Once `action_controller.raise_on_missing_callback_actions` is enabled we will see exceptions due to the `signin` action not existing. --- app/controllers/user_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 3dd0977586..4590b81f47 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -25,7 +25,7 @@ class UserController < ApplicationController # tries to sign in or sign up. There's little CSRF potential here as # these actions only sign in or up users with valid credentials. The # user_id in the session is not expected, and gives no extra privilege - skip_before_action :verify_authenticity_token, only: [:signin, :signup] + skip_before_action :verify_authenticity_token, only: [:signup] # Show page about a user def show From beef021004ec618dfe1117865dfe67cf5b5db796 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 10 Jan 2025 12:09:05 +0000 Subject: [PATCH 07/14] Update locale loading before app initialisation Sometimes we need locales to be loaded before the application has been initialised. Setting in the in `config.before_configuration` block doesn't persist correctly and also needs to be called in after initialisation. This issue becomes apparent when there is an exception at boot. The error message might call `to_sentence` [1], which will attempt to load a translation and this then error if `set_locales` hasn't been called. [1]: https://github.com/rails/rails/blob/d39db5d/activestorage/lib/active_storage/service/registry.rb#L18-L19 See #5382 --- lib/alaveteli_localization/railtie.rb | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/lib/alaveteli_localization/railtie.rb b/lib/alaveteli_localization/railtie.rb index 9fc034a1d7..f6279f52f1 100644 --- a/lib/alaveteli_localization/railtie.rb +++ b/lib/alaveteli_localization/railtie.rb @@ -21,30 +21,14 @@ class Railtie < Rails::Railtie I18n::Backend::Simple.send(:include, I18n::Backend::Fallbacks) - if Rails.version < '7.0.0' && Rails.env.development? - ## - # Ideally the following would only be called in the `after_initialize` - # hook but this leads to an error when booting Rails 6.1 in development - # mode. (As config.cache_classes = false) - # - # This due Alaveteli not yet using the new Zeitwork autoloading feature - # and Rails attempts to render a deprecation warning which happens to - # includes an I18n translation so requires the default locale to be - # setup. - # - # Once we support Zeitwork (which is needed for Rails 7) then this can - # be removed. - # - # See: https://github.com/mysociety/alaveteli/issues/5382 - # - AlaveteliLocalization.set_locales( - AlaveteliConfiguration.available_locales, - AlaveteliConfiguration.default_locale - ) - end + set_locales end config.after_initialize do + set_locales + end + + def set_locales AlaveteliLocalization.set_locales( AlaveteliConfiguration.available_locales, AlaveteliConfiguration.default_locale From b8ea3a87d585f093954867e98c92d39852da16d8 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 9 Jan 2025 16:47:16 +0000 Subject: [PATCH 08/14] Update message verify data hash Ensure data can be retrieved via symbol keys. When enabling the Rails 7.1 framework default data will be returned with string keys. --- app/models/post_redirect.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb index 472bbc85fb..84b86bb543 100644 --- a/app/models/post_redirect.rb +++ b/app/models/post_redirect.rb @@ -105,7 +105,8 @@ def local_part_uri def email_token_valid? return true unless PostRedirect.verifier.valid_message?(email_token) - data = PostRedirect.verifier.verify(email_token, purpose: circumstance) + data = PostRedirect.verifier.verify(email_token, purpose: circumstance). + symbolize_keys user.id == data[:user_id] && user.login_token == data[:login_token] end From 31644faddc64909d0c4d88e3e3822466f940c5a9 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 9 Jan 2025 16:53:13 +0000 Subject: [PATCH 09/14] Move ConfigHelper into lib So these helpers are available to files which aren't loaded via Zeitwerk such as the poller and text masker. --- config/initializers/alaveteli.rb | 1 - lib/alaveteli_mail_poller.rb | 1 + lib/alaveteli_text_masker.rb | 1 + {app/helpers => lib}/config_helper.rb | 0 lib/typeahead_search.rb | 2 -- 5 files changed, 2 insertions(+), 3 deletions(-) rename {app/helpers => lib}/config_helper.rb (100%) diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index 49b5faca9d..3dd8b8c760 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -39,7 +39,6 @@ require 'attachment_to_html' require 'health_checks' require 'mail_handler' -require 'ability' require 'normalize_string' require 'alaveteli_file_types' require 'theme' diff --git a/lib/alaveteli_mail_poller.rb b/lib/alaveteli_mail_poller.rb index 9be8745107..2992adbed4 100644 --- a/lib/alaveteli_mail_poller.rb +++ b/lib/alaveteli_mail_poller.rb @@ -1,4 +1,5 @@ require 'net/pop' +require 'config_helper' class AlaveteliMailPoller include ConfigHelper diff --git a/lib/alaveteli_text_masker.rb b/lib/alaveteli_text_masker.rb index 9da9e24896..26090bab4b 100644 --- a/lib/alaveteli_text_masker.rb +++ b/lib/alaveteli_text_masker.rb @@ -1,4 +1,5 @@ require 'tempfile' +require 'config_helper' module AlaveteliTextMasker include ConfigHelper diff --git a/app/helpers/config_helper.rb b/lib/config_helper.rb similarity index 100% rename from app/helpers/config_helper.rb rename to lib/config_helper.rb diff --git a/lib/typeahead_search.rb b/lib/typeahead_search.rb index 1d443be881..8d15784e21 100644 --- a/lib/typeahead_search.rb +++ b/lib/typeahead_search.rb @@ -1,6 +1,4 @@ class TypeaheadSearch - include ConfigHelper - attr_accessor :query, :model, :page, :per_page, :wildcard, :run_search def initialize(query, opts = {}) From 3c027ff98801e274d738061c70f40d50204fc8ed Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 9 Jan 2025 17:39:34 +0000 Subject: [PATCH 10/14] Update mailer preview_paths configuration This config has changed in Rails 7.1 to allow multiple paths to be defined. --- config/environments/development.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 46dfdb6ada..e564d070f1 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -73,9 +73,15 @@ # to make Rails upgrades easier. # ---------------------------------------------------------------- - config.action_mailer.preview_path = Rails.root.join( - 'spec', 'mailers', 'previews' - ) + if Rails.version < '7.1.0' + config.action_mailer.preview_path = Rails.root.join( + 'spec', 'mailers', 'previews' + ) + else + config.action_mailer.preview_paths = [ + Rails.root.join('spec', 'mailers', 'previews') + ] + end # Set LOG_LEVEL in the environment to a valid log level to temporarily run the # application with a non-default setting. From b0b50a2f4794b4af7cc8ed0234ed5081e7ea8112 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 10 Jan 2025 08:45:07 +0000 Subject: [PATCH 11/14] Rewrite query using Ruby infinity range Fixes `PG::SyntaxError` exception when upgrading to Rails 7.2. --- app/models/info_request.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 37be186146..8859fc71b1 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -519,14 +519,13 @@ def self.download_zip_dir end def self.reject_incoming_at_mta(options) - query = InfoRequest.where(["updated_at < (now() - - interval ?) - AND allow_new_responses_from = 'nobody' - AND rejected_incoming_count >= ? - AND reject_incoming_at_mta = ? - AND url_title <> 'holding_pen'", - "#{options[:age_in_months]} months", - options[:rejection_threshold], false]) + query = InfoRequest.where( + updated_at: ...options[:age_in_months].months.ago, + rejected_incoming_count: options[:rejection_threshold].., + allow_new_responses_from: 'nobody', + reject_incoming_at_mta: false + ).where.not(url_title: 'holding_pen') + yield query.pluck(:id) if block_given? if options[:dryrun] From 66fff236bc5673a04602f8495d620bcfb3f9aa09 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 10 Jan 2025 08:46:33 +0000 Subject: [PATCH 12/14] Configure test ActiveJob queue adapter In Rails 8 you have to set `active_job.queue_adapter`. This change sets it back to the default value in Rails 7.x. --- config/environments/test.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/environments/test.rb b/config/environments/test.rb index dda91abe22..35fe0e8f3b 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -86,4 +86,6 @@ exception_recipients: AlaveteliConfiguration.exception_notifications_to } end + + config.active_job.queue_adapter = :test end From 6b80efd1431d0580333858ebc848fe5a13d6e431 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 10 Jan 2025 15:04:23 +0000 Subject: [PATCH 13/14] Remove useless routing option Rails 8 adds additional route validation so this raise an exception. --- config/routes.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 14a77c88c6..363023e7fd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -684,8 +684,7 @@ def matches?(request) post 'redeliver', :on => :member end resource :incoming_messages, - :controller => 'admin_incoming_message', - :only => [:bulk_destroy] do + :controller => 'admin_incoming_message' do post 'bulk_destroy' end end From d1e152ea11fa681dd405e1ae3b2dd3b670db931c Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 10 Jan 2025 12:26:59 +0000 Subject: [PATCH 14/14] Update enum declaration Switch to positional arguments instead of keyword arguments. This fixes deprecation warning when upgrading to Rails 7.2. --- app/models/notification.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index e240251913..65397d5055 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -22,7 +22,7 @@ class Notification < ApplicationRecord INSTANTLY = :instantly DAILY = :daily - enum frequency: [ INSTANTLY, DAILY ] + enum :frequency, [ INSTANTLY, DAILY ] validates_presence_of :info_request_event, :user, :frequency, :send_after