From 8fbf16c8b455528cb536a60484bf89664b93026e Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 19 Dec 2024 12:26:31 +0000 Subject: [PATCH 01/53] Rework category test factory Ensure categories have parent associations. This change enforces parent associations for categories to support Rails 5.0's `active_record.belongs_to_required_by_default` setting. --- spec/factories/categories.rb | 6 ++++++ spec/models/category_spec.rb | 7 ++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/spec/factories/categories.rb b/spec/factories/categories.rb index 72685e174c..12449af38a 100644 --- a/spec/factories/categories.rb +++ b/spec/factories/categories.rb @@ -12,11 +12,17 @@ # FactoryBot.define do + factory :category_root, class: Category do + title { 'Root node' } + end + factory :category do title { 'Popular authorities' } description { 'The most popular authorities' } category_tag { 'popular_agency' } + parents { [association(:category_root)] } + trait :public_body do parents { [PublicBody.category_root] } end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 2b1a69aec1..c0ca3ff967 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -71,10 +71,11 @@ it 'has many parents' do parent_1 = FactoryBot.create(:category, children: [category]) parent_2 = FactoryBot.create(:category, children: [category]) + category.parents.reload expect(category.parents).to all be_a(Category) - expect(category.parents).to match_array([parent_1, parent_2]) + expect(category.parents).to include(parent_1, parent_2) expect(category.parent_relationships).to all be_a(CategoryRelationship) - expect(category.parent_relationships.count).to eq(2) + expect(category.parent_relationships.count).to eq(3) end it 'has many children' do @@ -222,7 +223,7 @@ subject { branch.root } let(:root) do - FactoryBot.create(:category, title: 'PublicBody') + FactoryBot.create(:category_root) end let(:trunk) do From 8e1c7f946c25fce7bd0a7caced1e03eb616ebae7 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 20 Dec 2024 10:56:11 +0000 Subject: [PATCH 02/53] Update category relationship association Disable parent/child relation validation. This change is needed so test examples pass after enabling support for Rails 5.0's `active_record.belongs_to_required_by_default` setting. --- app/models/category.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 6267131809..3afa4a37c7 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -21,7 +21,8 @@ class Category < ApplicationRecord has_many :parent_relationships, class_name: 'CategoryRelationship', foreign_key: 'child_id', - dependent: :destroy + dependent: :destroy, + validate: false has_many :parents, through: :parent_relationships, source: :parent @@ -29,7 +30,8 @@ class Category < ApplicationRecord has_many :child_relationships, class_name: 'CategoryRelationship', foreign_key: 'parent_id', - dependent: :destroy + dependent: :destroy, + validate: false has_many :children, through: :child_relationships, source: :child, From 5774c5fd53d5a8d23c58a346e9e0653e0ca038c7 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 19 Dec 2024 13:22:46 +0000 Subject: [PATCH 03/53] Update UserInfoRequestSendAlert test example Use test factory so the instance has belongs_to associated objects. This change is needed so this test example passes after enabling support for Rails 5.0's `active_record.belongs_to_required_by_default` setting. --- spec/models/user_info_request_sent_alert_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/models/user_info_request_sent_alert_spec.rb b/spec/models/user_info_request_sent_alert_spec.rb index 5f7487f322..92f4211a1f 100644 --- a/spec/models/user_info_request_sent_alert_spec.rb +++ b/spec/models/user_info_request_sent_alert_spec.rb @@ -33,7 +33,9 @@ end it 'should allow an alert type of "survey_1"' do - alert = UserInfoRequestSentAlert.new(alert_type: 'survey_1') + alert = FactoryBot.create( + :user_info_request_sent_alert, alert_type: 'survey_1' + ) expect(alert).to be_valid end end From 99a21ab2dea1d385a38833717b3ada43a3465016 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 19 Dec 2024 13:22:31 +0000 Subject: [PATCH 04/53] Update Comment#user association Configure Comment#user to be optional. Although we have a DB not null constraint we need to allow user to be optional as the controller calls `invalid?` and to allow the form to be rendered prior login. --- app/models/comment.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 458f26856f..efda291dcb 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -41,7 +41,8 @@ class Comment < ApplicationRecord belongs_to :user, inverse_of: :comments, - counter_cache: true + counter_cache: true, + optional: true # has to be optional for controller action to work belongs_to :info_request, inverse_of: :comments @@ -50,7 +51,6 @@ class Comment < ApplicationRecord inverse_of: :comment, dependent: :destroy - # validates_presence_of :user # breaks during construction of new ones :( validate :check_body_has_content, :check_body_uses_mixed_capitals From a17230914c874824b1f56cf739cc5a1bb9e48048 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Sun, 15 Dec 2024 23:48:37 +0000 Subject: [PATCH 05/53] Update belongs_to associations Configure optional associations. This change is needed so test examples pass after enabling support for Rails 5.0's `active_record.belongs_to_required_by_default` setting. --- app/models/alaveteli_pro/request_summary.rb | 3 ++- app/models/censor_rule.rb | 9 ++++++--- app/models/citation.rb | 4 ++-- app/models/comment.rb | 3 ++- app/models/dataset/key.rb | 2 +- app/models/dataset/value_set.rb | 2 +- app/models/draft_info_request.rb | 2 +- app/models/foi_attachment.rb | 2 +- app/models/info_request.rb | 6 ++++-- app/models/info_request_event.rb | 9 ++++++--- app/models/mail_server_log.rb | 6 ++++-- app/models/note.rb | 2 +- app/models/outgoing_message.rb | 3 ++- app/models/post_redirect.rb | 3 ++- app/models/profile_photo.rb | 3 ++- app/models/project/submission.rb | 2 +- app/models/public_body_change_request.rb | 6 ++++-- app/models/role.rb | 3 ++- app/models/track_thing.rb | 9 ++++++--- app/models/track_things_sent_email.rb | 9 ++++++--- app/models/user/sign_in.rb | 2 +- app/models/user_info_request_sent_alert.rb | 3 ++- app/models/user_message.rb | 3 ++- lib/has_tag_string/has_tag_string.rb | 2 +- lib/tasks/export.rake | 4 ++-- 25 files changed, 64 insertions(+), 38 deletions(-) diff --git a/app/models/alaveteli_pro/request_summary.rb b/app/models/alaveteli_pro/request_summary.rb index 1ac859fe76..98276a9c81 100644 --- a/app/models/alaveteli_pro/request_summary.rb +++ b/app/models/alaveteli_pro/request_summary.rb @@ -19,7 +19,8 @@ class AlaveteliPro::RequestSummary < ApplicationRecord belongs_to :summarisable, polymorphic: true belongs_to :user, - inverse_of: :request_summaries + inverse_of: :request_summaries, + optional: true has_and_belongs_to_many :request_summary_categories, class_name: "AlaveteliPro::RequestSummaryCategory", inverse_of: :request_summaries diff --git a/app/models/censor_rule.rb b/app/models/censor_rule.rb index 4548f21b2c..ae5a99306d 100644 --- a/app/models/censor_rule.rb +++ b/app/models/censor_rule.rb @@ -32,11 +32,14 @@ class CensorRule < ApplicationRecord ].freeze belongs_to :info_request, - inverse_of: :censor_rules + inverse_of: :censor_rules, + optional: true belongs_to :user, - inverse_of: :censor_rules + inverse_of: :censor_rules, + optional: true belongs_to :public_body, - inverse_of: :censor_rules + inverse_of: :censor_rules, + optional: true validate :require_valid_regexp, if: proc { |rule| rule.regexp? == true } diff --git a/app/models/citation.rb b/app/models/citation.rb index 92d5e4d837..2780fbcceb 100644 --- a/app/models/citation.rb +++ b/app/models/citation.rb @@ -27,8 +27,8 @@ class Citation < ApplicationRecord belongs_to :user, inverse_of: :citations belongs_to :citable, polymorphic: true - belongs_to :info_request, via: :citable - belongs_to :info_request_batch, via: :citable + belongs_to :info_request, via: :citable, optional: true + belongs_to :info_request_batch, via: :citable, optional: true validates :user, :citable, presence: true validates :citable_type, inclusion: { in: %w(InfoRequest InfoRequestBatch) } diff --git a/app/models/comment.rb b/app/models/comment.rb index efda291dcb..e9e5353c00 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -45,7 +45,8 @@ class Comment < ApplicationRecord optional: true # has to be optional for controller action to work belongs_to :info_request, - inverse_of: :comments + inverse_of: :comments, + optional: true has_many :info_request_events, # in practice only ever has one inverse_of: :comment, diff --git a/app/models/dataset/key.rb b/app/models/dataset/key.rb index 2d4aef091f..da775ef78b 100644 --- a/app/models/dataset/key.rb +++ b/app/models/dataset/key.rb @@ -18,7 +18,7 @@ # resource # class Dataset::Key < ApplicationRecord - belongs_to :key_set, foreign_key: 'dataset_key_set_id' + belongs_to :key_set, foreign_key: 'dataset_key_set_id', optional: true has_many :values, foreign_key: 'dataset_key_id', inverse_of: :key default_scope -> { order(:order) } diff --git a/app/models/dataset/value_set.rb b/app/models/dataset/value_set.rb index 64cdd8b718..df2d4331df 100644 --- a/app/models/dataset/value_set.rb +++ b/app/models/dataset/value_set.rb @@ -15,7 +15,7 @@ # A dataset collection of values # class Dataset::ValueSet < ApplicationRecord - belongs_to :resource, polymorphic: true + belongs_to :resource, polymorphic: true, optional: true belongs_to :key_set, foreign_key: 'dataset_key_set_id' has_many :values, foreign_key: 'dataset_value_set_id', inverse_of: :value_set diff --git a/app/models/draft_info_request.rb b/app/models/draft_info_request.rb index b5f718c3ca..5eb9584ae6 100644 --- a/app/models/draft_info_request.rb +++ b/app/models/draft_info_request.rb @@ -21,7 +21,7 @@ class DraftInfoRequest < ApplicationRecord belongs_to :user, inverse_of: :draft_info_requests - belongs_to :public_body, inverse_of: :draft_info_requests + belongs_to :public_body, inverse_of: :draft_info_requests, optional: true strip_attributes diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb index 35e4259dea..4e04877d98 100644 --- a/app/models/foi_attachment.rb +++ b/app/models/foi_attachment.rb @@ -35,7 +35,7 @@ class FoiAttachment < ApplicationRecord MissingAttachment = Class.new(StandardError) - belongs_to :incoming_message, inverse_of: :foi_attachments + belongs_to :incoming_message, inverse_of: :foi_attachments, optional: true has_one :info_request, through: :incoming_message, source: :info_request has_one :raw_email, through: :incoming_message, source: :raw_email diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 37be186146..9b4df4f9b7 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -67,7 +67,8 @@ def self.admin_title belongs_to :user, inverse_of: :info_requests, - counter_cache: true + counter_cache: true, + optional: true validate :must_be_internal_or_external @@ -75,7 +76,8 @@ def self.admin_title inverse_of: :info_requests, counter_cache: true belongs_to :info_request_batch, - inverse_of: :info_requests + inverse_of: :info_requests, + optional: true validates_presence_of :public_body, message: N_("Please select an authority") diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 1a1382ce9c..80b086cd20 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -64,11 +64,14 @@ class InfoRequestEvent < ApplicationRecord validates_presence_of :info_request belongs_to :outgoing_message, - inverse_of: :info_request_events + inverse_of: :info_request_events, + optional: true belongs_to :incoming_message, - inverse_of: :info_request_events + inverse_of: :info_request_events, + optional: true belongs_to :comment, - inverse_of: :info_request_events + inverse_of: :info_request_events, + optional: true has_one :request_classification, inverse_of: :info_request_event diff --git a/app/models/mail_server_log.rb b/app/models/mail_server_log.rb index 25d6b9c1a2..0e9ceff179 100644 --- a/app/models/mail_server_log.rb +++ b/app/models/mail_server_log.rb @@ -24,9 +24,11 @@ class MailServerLog < ApplicationRecord serialize :delivery_status, DeliveryStatusSerializer belongs_to :info_request, - inverse_of: :mail_server_logs + inverse_of: :mail_server_logs, + optional: true belongs_to :mail_server_log_done, - inverse_of: :mail_server_logs + inverse_of: :mail_server_logs, + optional: true before_create :calculate_delivery_status diff --git a/app/models/note.rb b/app/models/note.rb index 7145abf4df..2ee8933d10 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -35,7 +35,7 @@ class Note < ApplicationRecord default: Note.default_style, suffix: true - belongs_to :notable, polymorphic: true + belongs_to :notable, polymorphic: true, optional: true validates :body, presence: true, if: ->(n) { n.original_style? } validates :rich_body, presence: true, unless: ->(n) { n.original_style? } diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index dfa8e2d94a..857cc09b5f 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -56,7 +56,8 @@ class OutgoingMessage < ApplicationRecord belongs_to :incoming_message_followup, inverse_of: :outgoing_message_followups, foreign_key: 'incoming_message_followup_id', - class_name: 'IncomingMessage' + class_name: 'IncomingMessage', + optional: true has_one :user, inverse_of: :outgoing_messages, diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb index 42fafa59bc..afa9824eb7 100644 --- a/app/models/post_redirect.rb +++ b/app/models/post_redirect.rb @@ -34,7 +34,8 @@ class PostRedirect < ApplicationRecord # Optional, does a login confirm before redirect for use in email links. belongs_to :user, - inverse_of: :post_redirects + inverse_of: :post_redirects, + optional: true validates :circumstance, inclusion: CIRCUMSTANCES diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb index 0f70ad2578..2a7cde10d5 100644 --- a/app/models/profile_photo.rb +++ b/app/models/profile_photo.rb @@ -25,7 +25,8 @@ class ProfilePhoto < ApplicationRecord MAX_DRAFT = 500 # keep even pre-cropped images reasonably small belongs_to :user, - inverse_of: :profile_photo + inverse_of: :profile_photo, + optional: true validate :data_and_draft_checks diff --git a/app/models/project/submission.rb b/app/models/project/submission.rb index ab7bc88b24..51b18bd783 100644 --- a/app/models/project/submission.rb +++ b/app/models/project/submission.rb @@ -21,7 +21,7 @@ class Project::Submission < ApplicationRecord belongs_to :project belongs_to :user belongs_to :info_request - belongs_to :resource, polymorphic: true + belongs_to :resource, polymorphic: true, optional: true scope :classification, -> { where(resource_type: 'InfoRequestEvent') } scope :extraction, -> { where(resource_type: 'Dataset::ValueSet') } diff --git a/app/models/public_body_change_request.rb b/app/models/public_body_change_request.rb index 16bd3f87dc..6afab614ea 100644 --- a/app/models/public_body_change_request.rb +++ b/app/models/public_body_change_request.rb @@ -20,9 +20,11 @@ class PublicBodyChangeRequest < ApplicationRecord belongs_to :user, inverse_of: :public_body_change_requests, - counter_cache: true + counter_cache: true, + optional: true belongs_to :public_body, - inverse_of: :public_body_change_requests + inverse_of: :public_body_change_requests, + optional: true validates_presence_of :public_body_name, message: N_("Please enter the name of the authority"), diff --git a/app/models/role.rb b/app/models/role.rb index 1147dd8557..8826b5bb9b 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -19,7 +19,8 @@ class Role < ApplicationRecord inverse_of: :roles belongs_to :resource, - polymorphic: true + polymorphic: true, + optional: true validates :resource_type, inclusion: { in: Rolify.resource_types }, diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb index 43be628d40..5e99d7f0e7 100644 --- a/app/models/track_thing.rb +++ b/app/models/track_thing.rb @@ -29,16 +29,19 @@ class TrackThing < ApplicationRecord TRACK_MEDIUMS = %w(email_daily feed) belongs_to :info_request, - inverse_of: :track_things + inverse_of: :track_things, + optional: true belongs_to :public_body, - inverse_of: :track_things + inverse_of: :track_things, + optional: true belongs_to :tracking_user, class_name: 'User', inverse_of: :track_things, counter_cache: true belongs_to :tracked_user, class_name: 'User', - inverse_of: :track_things + inverse_of: :track_things, + optional: true has_many :track_things_sent_emails, inverse_of: :track_thing, dependent: :destroy diff --git a/app/models/track_things_sent_email.rb b/app/models/track_things_sent_email.rb index 18bb3514bb..0be7a17ca1 100644 --- a/app/models/track_things_sent_email.rb +++ b/app/models/track_things_sent_email.rb @@ -20,11 +20,14 @@ class TrackThingsSentEmail < ApplicationRecord belongs_to :info_request_event, - inverse_of: :track_things_sent_emails + inverse_of: :track_things_sent_emails, + optional: true belongs_to :user, - inverse_of: :track_things_sent_emails + inverse_of: :track_things_sent_emails, + optional: true belongs_to :public_body, - inverse_of: :track_things_sent_emails + inverse_of: :track_things_sent_emails, + optional: true belongs_to :track_thing, inverse_of: :track_things_sent_emails diff --git a/app/models/user/sign_in.rb b/app/models/user/sign_in.rb index 159741080b..c8079c9be6 100644 --- a/app/models/user/sign_in.rb +++ b/app/models/user/sign_in.rb @@ -15,7 +15,7 @@ class User::SignIn < ApplicationRecord default_scope { order(created_at: :desc) } - belongs_to :user, inverse_of: :sign_ins + belongs_to :user, inverse_of: :sign_ins, optional: true before_create :create? diff --git a/app/models/user_info_request_sent_alert.rb b/app/models/user_info_request_sent_alert.rb index 0143b146c6..b709d4b2d9 100644 --- a/app/models/user_info_request_sent_alert.rb +++ b/app/models/user_info_request_sent_alert.rb @@ -43,7 +43,8 @@ class UserInfoRequestSentAlert < ApplicationRecord belongs_to :info_request, inverse_of: :user_info_request_sent_alerts belongs_to :info_request_event, - inverse_of: :user_info_request_sent_alerts + inverse_of: :user_info_request_sent_alerts, + optional: true validates_inclusion_of :alert_type, in: ALERT_TYPES diff --git a/app/models/user_message.rb b/app/models/user_message.rb index bb65c330c6..b5a2fa4bfd 100644 --- a/app/models/user_message.rb +++ b/app/models/user_message.rb @@ -11,5 +11,6 @@ class UserMessage < ApplicationRecord belongs_to :user, inverse_of: :user_messages, - counter_cache: true + counter_cache: true, + optional: true end diff --git a/lib/has_tag_string/has_tag_string.rb b/lib/has_tag_string/has_tag_string.rb index 8b4e2c02b3..022198d666 100644 --- a/lib/has_tag_string/has_tag_string.rb +++ b/lib/has_tag_string/has_tag_string.rb @@ -12,7 +12,7 @@ module HasTagString class HasTagStringTag < ActiveRecord::Base # TODO: strip_attributes - belongs_to :model, polymorphic: true + belongs_to :model, polymorphic: true, optional: true validates_presence_of :name diff --git a/lib/tasks/export.rake b/lib/tasks/export.rake index 310e909a90..2d4e3cc85b 100644 --- a/lib/tasks/export.rake +++ b/lib/tasks/export.rake @@ -6,8 +6,8 @@ namespace :export do #create models to access join and translation tables class InfoRequestBatchPublicBody < ActiveRecord::Base self.table_name = "info_request_batches_public_bodies" - belongs_to :info_request_batch - belongs_to :public_body + belongs_to :info_request_batch, optional: true + belongs_to :public_body, optional: true default_scope -> { order(:info_request_batch_id, :public_body_id) } end From 7fa6af7c5771d569a6cb5ae63c4abf609cbf219d Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 19 Dec 2024 13:22:15 +0000 Subject: [PATCH 06/53] Update model error validation test examples Check errors include message for required belongs_to associations. This change ensures these test examples will pass after enabling support for Rails 5.0's `active_record.belongs_to_required_by_default` setting. --- spec/models/info_request_batch_spec.rb | 2 +- spec/models/public_body_category_link_spec.rb | 4 ++-- spec/models/widget_vote_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/info_request_batch_spec.rb b/spec/models/info_request_batch_spec.rb index 4572a83659..bad3f99a0e 100644 --- a/spec/models/info_request_batch_spec.rb +++ b/spec/models/info_request_batch_spec.rb @@ -27,7 +27,7 @@ it 'should require a user' do info_request_batch.user = nil expect(info_request_batch.valid?).to be false - expect(info_request_batch.errors.full_messages).to eq(["User can't be blank"]) + expect(info_request_batch.errors[:user]).to include("can't be blank") end it 'should require a body' do diff --git a/spec/models/public_body_category_link_spec.rb b/spec/models/public_body_category_link_spec.rb index 268e9b8399..6fc39c1bb3 100644 --- a/spec/models/public_body_category_link_spec.rb +++ b/spec/models/public_body_category_link_spec.rb @@ -26,13 +26,13 @@ it 'should be invalid without a category' do category_link = PublicBodyCategoryLink.new expect(category_link).not_to be_valid - expect(category_link.errors[:public_body_category]).to eq(["can't be blank"]) + expect(category_link.errors[:public_body_category]).to include("can't be blank") end it 'should be invalid without a heading' do category_link = PublicBodyCategoryLink.new expect(category_link).not_to be_valid - expect(category_link.errors[:public_body_heading]).to eq(["can't be blank"]) + expect(category_link.errors[:public_body_heading]).to include("can't be blank") end end diff --git a/spec/models/widget_vote_spec.rb b/spec/models/widget_vote_spec.rb index 7b22ce8b65..e3361078af 100644 --- a/spec/models/widget_vote_spec.rb +++ b/spec/models/widget_vote_spec.rb @@ -17,7 +17,7 @@ it 'requires an info request' do widget_vote = WidgetVote.new expect(widget_vote).not_to be_valid - expect(widget_vote.errors[:info_request]).to eq(["can't be blank"]) + expect(widget_vote.errors[:info_request]).to include("can't be blank") end it 'validates the cookie length' do From 6e6508e0617703fa38c206895ac2846a0918e860 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Sun, 15 Dec 2024 23:48:53 +0000 Subject: [PATCH 07/53] Allow deprecation warnings to be silenced In the test environment some `script` test examples look at output stderr. When doing Rails upgrades we sometimes see deprecation warnings on boot and these examples will fail. This change adds a new ENV which can be set to silence these warnings. --- config/environments/test.rb | 4 ++++ spec/script/handle-mail-replies_spec.rb | 6 ++++++ spec/script/load_mail_server_logs_spec.rb | 6 ++++++ spec/script/mailin_spec.rb | 6 ++++++ 4 files changed, 22 insertions(+) diff --git a/config/environments/test.rb b/config/environments/test.rb index b8109c174c..9fbc53246a 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -68,6 +68,10 @@ config.i18n.fallbacks = true config.i18n.enforce_available_locales = false + if ENV['DISABLE_DEPRECATION_WARNINGS'] + config.active_support.deprecation = :silence + end + unless ENV['RAILS_ENABLE_TEST_LOG'] config.logger = Logger.new(nil) config.log_level = :fatal diff --git a/spec/script/handle-mail-replies_spec.rb b/spec/script/handle-mail-replies_spec.rb index 5a54d232d0..5049eb71fe 100644 --- a/spec/script/handle-mail-replies_spec.rb +++ b/spec/script/handle-mail-replies_spec.rb @@ -12,6 +12,12 @@ def mail_reply_test(email_filename) end RSpec.describe "When filtering" do + around do |example| + ENV['DISABLE_DEPRECATION_WARNINGS'] = 'true' + example.call + ENV['DISABLE_DEPRECATION_WARNINGS'] = nil + end + describe "when not in test mode" do it "should not fail handling a bounce mail" do xc = ExternalCommand.new("script/handle-mail-replies", { diff --git a/spec/script/load_mail_server_logs_spec.rb b/spec/script/load_mail_server_logs_spec.rb index f315b5d8d2..a488c1fdef 100644 --- a/spec/script/load_mail_server_logs_spec.rb +++ b/spec/script/load_mail_server_logs_spec.rb @@ -2,6 +2,12 @@ require 'external_command' RSpec.describe 'when importing mail logs into the application' do + around do |example| + ENV['DISABLE_DEPRECATION_WARNINGS'] = 'true' + example.call + ENV['DISABLE_DEPRECATION_WARNINGS'] = nil + end + def load_mail_server_logs_test(log_file = nil) Dir.chdir Rails.root do ExternalCommand.new('script/load-mail-server-logs', *log_file).run diff --git a/spec/script/mailin_spec.rb b/spec/script/mailin_spec.rb index c76a11d973..8e01584576 100644 --- a/spec/script/mailin_spec.rb +++ b/spec/script/mailin_spec.rb @@ -19,6 +19,12 @@ def mailin_test(email_filename) # transaction self.use_transactional_tests = false + around do |example| + ENV['DISABLE_DEPRECATION_WARNINGS'] = 'true' + example.call + ENV['DISABLE_DEPRECATION_WARNINGS'] = nil + end + it "should not produce any output and should return a 0 code on importing a plain email" do r = mailin_test("incoming-request-empty.email") expect(r.status).to eq(0) From 735be2a02d5499480d3831697d26782c0e9f7cee Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Sun, 15 Dec 2024 23:49:25 +0000 Subject: [PATCH 08/53] Update translation test examples Ensures translation objects are correctly build with `globalized_model` association. This change is needed so test examples pass after enabling support for Rails 5.0's `active_record.belongs_to_required_by_default` setting. --- spec/models/public_body_category_spec.rb | 6 ++++-- spec/models/public_body_heading_spec.rb | 3 ++- spec/models/public_body_spec.rb | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/models/public_body_category_spec.rb b/spec/models/public_body_category_spec.rb index 29a4786a19..006a7eab91 100644 --- a/spec/models/public_body_category_spec.rb +++ b/spec/models/public_body_category_spec.rb @@ -158,8 +158,10 @@ end it 'is valid if no required attributes are assigned' do - translation = PublicBodyCategory::Translation. - new(locale: AlaveteliLocalization.default_locale) + translation = PublicBodyCategory::Translation.new( + locale: AlaveteliLocalization.default_locale, + globalized_model: PublicBodyCategory.first + ) expect(translation).to be_valid end diff --git a/spec/models/public_body_heading_spec.rb b/spec/models/public_body_heading_spec.rb index 8f6459717a..f5acda02b4 100644 --- a/spec/models/public_body_heading_spec.rb +++ b/spec/models/public_body_heading_spec.rb @@ -149,7 +149,8 @@ it 'is valid if all required attributes are assigned' do translation = PublicBodyHeading::Translation.new( - locale: AlaveteliLocalization.default_locale + locale: AlaveteliLocalization.default_locale, + globalized_model: PublicBodyHeading.first ) expect(translation).to be_valid end diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 2838d8b51a..67d956a0ad 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -2354,7 +2354,8 @@ def set_default_attributes(public_body) it 'is valid if all required attributes are assigned' do translation = PublicBody::Translation.new( - locale: AlaveteliLocalization.default_locale + locale: AlaveteliLocalization.default_locale, + globalized_model: PublicBody.first ) expect(translation).to be_valid end From d261d602dc2bcbdec1d9aa1ef59f9aabb3df4eee Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:20:45 +0000 Subject: [PATCH 09/53] Add missing new framework defaults file Added so we can review these options. --- config/initializers/new_framework_defaults_5_1.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 config/initializers/new_framework_defaults_5_1.rb diff --git a/config/initializers/new_framework_defaults_5_1.rb b/config/initializers/new_framework_defaults_5_1.rb new file mode 100644 index 0000000000..9010abd5c2 --- /dev/null +++ b/config/initializers/new_framework_defaults_5_1.rb @@ -0,0 +1,14 @@ +# Be sure to restart your server when you modify this file. +# +# This file contains migration options to ease your Rails 5.1 upgrade. +# +# Once upgraded flip defaults one by one to migrate to the new default. +# +# Read the Guide for Upgrading Ruby on Rails for more info on each option. + +# Make `form_with` generate non-remote forms. +Rails.application.config.action_view.form_with_generates_remote_forms = false + +# Unknown asset fallback will return the path passed in when the given +# asset is not present in the asset pipeline. +# Rails.application.config.assets.unknown_asset_fallback = false From 8d9df1d90661c70c1db4057ed84c33b7f11dbb76 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 20 Dec 2024 16:29:46 +0000 Subject: [PATCH 10/53] Add belongs_to validation test examples Add examples where we already validated belongs_to associations. These additional test examples will be useful after we turn on Rails 5.0's `active_record.belongs_to_required_by_default` setting as we will be able to clean up the `validates presence` calls. --- spec/models/alaveteli_pro/embargo_spec.rb | 9 ++++++--- spec/models/incoming_message_spec.rb | 16 ++++++++++++++++ spec/models/info_request_event_spec.rb | 11 +++++++++++ spec/models/outgoing_message_spec.rb | 11 +++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/spec/models/alaveteli_pro/embargo_spec.rb b/spec/models/alaveteli_pro/embargo_spec.rb index f2bd3fc01f..c6c27849af 100644 --- a/spec/models/alaveteli_pro/embargo_spec.rb +++ b/spec/models/alaveteli_pro/embargo_spec.rb @@ -15,10 +15,13 @@ require 'spec_helper' RSpec.describe AlaveteliPro::Embargo, type: :model do - let(:embargo) { FactoryBot.create(:embargo) } + subject(:embargo) { FactoryBot.create(:embargo) } - it 'belongs to an info_request' do - expect(embargo.info_request).not_to be_nil + it { is_expected.to be_valid } + + it 'requires info_request' do + embargo.info_request = nil + expect(embargo).not_to be_valid end it 'has a publish_at field' do diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index a2f217bc08..48c0c8c247 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -54,6 +54,22 @@ end end + describe 'validations' do + subject(:incoming_message) { FactoryBot.build(:incoming_message) } + + it { is_expected.to be_valid } + + it 'requires info_reqeust' do + incoming_message.info_request = nil + expect(incoming_message).not_to be_valid + end + + it 'requires raw_email' do + incoming_message.raw_email = nil + expect(incoming_message).not_to be_valid + end + end + describe '#response_event' do subject { message.response_event } diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index 95601750e6..b2ddff9a7c 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -29,6 +29,17 @@ end end + describe 'validations' do + subject(:ire) { FactoryBot.build(:info_request_event) } + + it { is_expected.to be_valid } + + it 'requires info_reqeust' do + ire.info_request = nil + expect(ire).not_to be_valid + end + end + describe "when checking for a valid state" do it 'should add an error message for described_state if it is not valid' do ire = InfoRequestEvent.new(described_state: 'nope') diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb index 86aab5fbde..b6ef121320 100644 --- a/spec/models/outgoing_message_spec.rb +++ b/spec/models/outgoing_message_spec.rb @@ -117,6 +117,17 @@ class TestError < StandardError; end end end + describe 'validations' do + subject(:outgoing_message) { FactoryBot.build(:initial_request) } + + it { is_expected.to be_valid } + + it 'requires info_reqeust' do + outgoing_message.info_request = nil + expect(outgoing_message).not_to be_valid + end + end + describe '#default_letter' do it 'reloads the default body when set after initialization' do req = FactoryBot.build(:info_request) From 36eb3c7eafcdf5ee382a46d0f6ace6da298c4182 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 19 Dec 2024 13:22:15 +0000 Subject: [PATCH 11/53] Update InfoRequest#public_body association Don't validate this association is present as we already have validation in place with a custom error message. --- app/models/info_request.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 9b4df4f9b7..daa2384465 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -74,7 +74,8 @@ def self.admin_title belongs_to :public_body, inverse_of: :info_requests, - counter_cache: true + counter_cache: true, + validate: false belongs_to :info_request_batch, inverse_of: :info_requests, optional: true From 75e0d7f5c0b6c67df0e93ed88a4dfafccab17ddd Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:21:47 +0000 Subject: [PATCH 12/53] Update to configuration defaults for Rails 5.0 --- config/application.rb | 1 + config/initializers/new_framework_defaults.rb | 20 ------------------- 2 files changed, 1 insertion(+), 20 deletions(-) delete mode 100644 config/initializers/new_framework_defaults.rb diff --git a/config/application.rb b/config/application.rb index b1dee98822..b02d1725bc 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,6 +31,7 @@ class Application < Rails::Application # # config.time_zone = "Central Time (US & Canada)" # config.eager_load_paths << Rails.root.join("extras") + config.load_defaults 5.0 config.autoloader = :zeitwerk config.active_record.legacy_connection_handling = false config.active_support.use_rfc4122_namespaced_uuids = true diff --git a/config/initializers/new_framework_defaults.rb b/config/initializers/new_framework_defaults.rb deleted file mode 100644 index a2bfed3009..0000000000 --- a/config/initializers/new_framework_defaults.rb +++ /dev/null @@ -1,20 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 5.0 upgrade. -# -# Once upgraded flip defaults one by one to migrate to the new default. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. - -# Enable per-form CSRF tokens. Previous versions had false. -Rails.application.config.action_controller.per_form_csrf_tokens = false - -# Enable origin-checking CSRF mitigation. Previous versions had false. -Rails.application.config.action_controller.forgery_protection_origin_check = false - -# Make Ruby 2.4 preserve the timezone of the receiver when calling `to_time`. -# Previous versions had false. -ActiveSupport.to_time_preserves_timezone = false - -# Require `belongs_to` associations by default. Previous versions had false. -Rails.application.config.active_record.belongs_to_required_by_default = false From 6557d6e988dfae7177e3406b6125c7cf19e5df23 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 19 Dec 2024 13:22:15 +0000 Subject: [PATCH 13/53] Remove unneeded model validations These validations are now covered by the relevant belongs_to association. --- app/models/alaveteli_pro/draft_info_request_batch.rb | 2 -- app/models/alaveteli_pro/embargo.rb | 1 - app/models/alaveteli_pro/embargo_extension.rb | 1 - app/models/alaveteli_pro/request_summary.rb | 3 +-- app/models/announcement.rb | 2 +- app/models/announcement_dismissal.rb | 2 -- app/models/citation.rb | 1 - app/models/dataset/key_set.rb | 1 - app/models/dataset/value_set.rb | 1 - app/models/draft_info_request.rb | 2 -- app/models/incoming_message.rb | 4 ---- app/models/info_request_batch.rb | 1 - app/models/info_request_event.rb | 2 -- app/models/notification.rb | 2 +- app/models/outgoing_message.rb | 1 - app/models/project/membership.rb | 2 -- app/models/project/resource.rb | 2 -- app/models/project/submission.rb | 1 - app/models/public_body_category_link.rb | 3 --- app/models/widget_vote.rb | 1 - spec/models/info_request_batch_spec.rb | 2 +- spec/models/public_body_category_link_spec.rb | 4 ++-- spec/models/widget_vote_spec.rb | 2 +- 23 files changed, 7 insertions(+), 36 deletions(-) diff --git a/app/models/alaveteli_pro/draft_info_request_batch.rb b/app/models/alaveteli_pro/draft_info_request_batch.rb index 9b5f7c7e03..3043b3b1a9 100644 --- a/app/models/alaveteli_pro/draft_info_request_batch.rb +++ b/app/models/alaveteli_pro/draft_info_request_batch.rb @@ -25,8 +25,6 @@ class AlaveteliPro::DraftInfoRequestBatch < ApplicationRecord end }, inverse_of: :draft_info_request_batches - validates_presence_of :user - after_initialize :set_default_body strip_attributes only: %i[embargo_duration] diff --git a/app/models/alaveteli_pro/embargo.rb b/app/models/alaveteli_pro/embargo.rb index 864414b343..ff064ed6e8 100644 --- a/app/models/alaveteli_pro/embargo.rb +++ b/app/models/alaveteli_pro/embargo.rb @@ -22,7 +22,6 @@ class Embargo < ApplicationRecord inverse_of: :embargoes, through: :info_request - validates_presence_of :info_request validates_presence_of :publish_at validates_inclusion_of :embargo_duration, in: ->(e) { e.allowed_durations }, diff --git a/app/models/alaveteli_pro/embargo_extension.rb b/app/models/alaveteli_pro/embargo_extension.rb index bff61e5f15..9220d60718 100644 --- a/app/models/alaveteli_pro/embargo_extension.rb +++ b/app/models/alaveteli_pro/embargo_extension.rb @@ -14,7 +14,6 @@ module AlaveteliPro class EmbargoExtension < ApplicationRecord belongs_to :embargo, inverse_of: :embargo_extensions - validates_presence_of :embargo validates_presence_of :extension_duration validates_inclusion_of :extension_duration, in: lambda { |_e| AlaveteliPro::Embargo.new.allowed_durations } diff --git a/app/models/alaveteli_pro/request_summary.rb b/app/models/alaveteli_pro/request_summary.rb index 98276a9c81..7855d58736 100644 --- a/app/models/alaveteli_pro/request_summary.rb +++ b/app/models/alaveteli_pro/request_summary.rb @@ -25,8 +25,7 @@ class AlaveteliPro::RequestSummary < ApplicationRecord class_name: "AlaveteliPro::RequestSummaryCategory", inverse_of: :request_summaries - validates_presence_of :summarisable, - :request_created_at, + validates_presence_of :request_created_at, :request_updated_at validates_uniqueness_of :summarisable_id, scope: :summarisable_type diff --git a/app/models/announcement.rb b/app/models/announcement.rb index fc1411550a..9601ec77e1 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -74,7 +74,7 @@ class Announcement < ApplicationRecord end } - validates :content, :user, + validates :content, presence: true validates :visibility, presence: true, diff --git a/app/models/announcement_dismissal.rb b/app/models/announcement_dismissal.rb index 51599ead58..aa158646b8 100644 --- a/app/models/announcement_dismissal.rb +++ b/app/models/announcement_dismissal.rb @@ -15,6 +15,4 @@ class AnnouncementDismissal < ApplicationRecord inverse_of: :dismissals belongs_to :user, inverse_of: :announcement_dismissals - - validates :announcement, :user, presence: true end diff --git a/app/models/citation.rb b/app/models/citation.rb index 2780fbcceb..67427af352 100644 --- a/app/models/citation.rb +++ b/app/models/citation.rb @@ -30,7 +30,6 @@ class Citation < ApplicationRecord belongs_to :info_request, via: :citable, optional: true belongs_to :info_request_batch, via: :citable, optional: true - validates :user, :citable, presence: true validates :citable_type, inclusion: { in: %w(InfoRequest InfoRequestBatch) } validates :source_url, length: { maximum: 255, message: _('Source URL is too long') }, diff --git a/app/models/dataset/key_set.rb b/app/models/dataset/key_set.rb index 0fbea1cdf9..940b2bc241 100644 --- a/app/models/dataset/key_set.rb +++ b/app/models/dataset/key_set.rb @@ -26,6 +26,5 @@ class Dataset::KeySet < ApplicationRecord InfoRequestBatch ].freeze - validates :resource, presence: true validates :resource_type, inclusion: { in: RESOURCE_TYPES } end diff --git a/app/models/dataset/value_set.rb b/app/models/dataset/value_set.rb index df2d4331df..d76edf97d6 100644 --- a/app/models/dataset/value_set.rb +++ b/app/models/dataset/value_set.rb @@ -27,7 +27,6 @@ class Dataset::ValueSet < ApplicationRecord FoiAttachment ].freeze - validates :key_set, :values, presence: true validates :resource_type, inclusion: { in: RESOURCE_TYPES }, if: :resource validates_associated :values validate :check_at_least_one_value_is_present diff --git a/app/models/draft_info_request.rb b/app/models/draft_info_request.rb index 5eb9584ae6..671415dafe 100644 --- a/app/models/draft_info_request.rb +++ b/app/models/draft_info_request.rb @@ -17,8 +17,6 @@ class DraftInfoRequest < ApplicationRecord include AlaveteliPro::RequestSummaries include InfoRequest::DraftTitleValidation - validates_presence_of :user - belongs_to :user, inverse_of: :draft_info_requests belongs_to :public_body, inverse_of: :draft_info_requests, optional: true diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 9024959087..129c43afa6 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -49,10 +49,6 @@ class IncomingMessage < ApplicationRecord inverse_of: :incoming_messages, counter_cache: true - validates_presence_of :info_request - - validates_presence_of :raw_email - has_many :outgoing_message_followups, inverse_of: :incoming_message_followup, foreign_key: 'incoming_message_followup_id', diff --git a/app/models/info_request_batch.rb b/app/models/info_request_batch.rb index 5f38b36176..71d055d857 100644 --- a/app/models/info_request_batch.rb +++ b/app/models/info_request_batch.rb @@ -38,7 +38,6 @@ class InfoRequestBatch < ApplicationRecord attr_accessor :ignore_existing_batch - validates_presence_of :user validates_presence_of :body validates_absence_of :existing_batch, unless: -> { ignore_existing_batch }, diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 80b086cd20..77474a4972 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -61,8 +61,6 @@ class InfoRequestEvent < ApplicationRecord belongs_to :info_request, inverse_of: :info_request_events - validates_presence_of :info_request - belongs_to :outgoing_message, inverse_of: :info_request_events, optional: true diff --git a/app/models/notification.rb b/app/models/notification.rb index e240251913..ffc2c0cf90 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -24,7 +24,7 @@ class Notification < ApplicationRecord DAILY = :daily enum frequency: [ INSTANTLY, DAILY ] - validates_presence_of :info_request_event, :user, :frequency, :send_after + validates_presence_of :frequency, :send_after before_validation :calculate_send_after diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 857cc09b5f..ab2306e5dc 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -42,7 +42,6 @@ class OutgoingMessage < ApplicationRecord attr_accessor :default_letter before_validation :cache_from_name - validates_presence_of :info_request validates_presence_of :from_name, unless: -> (m) { !m.info_request&.user } validates_inclusion_of :status, in: STATUS_TYPES validates_inclusion_of :message_type, in: MESSAGE_TYPES diff --git a/app/models/project/membership.rb b/app/models/project/membership.rb index 5f62d3f46f..dddeb4d3a0 100644 --- a/app/models/project/membership.rb +++ b/app/models/project/membership.rb @@ -19,6 +19,4 @@ class Project::Membership < ApplicationRecord belongs_to :project belongs_to :user belongs_to :role - - validates :project, :user, :role, presence: true end diff --git a/app/models/project/resource.rb b/app/models/project/resource.rb index 89fc8344d7..35e2fd1006 100644 --- a/app/models/project/resource.rb +++ b/app/models/project/resource.rb @@ -18,6 +18,4 @@ class Project::Resource < ApplicationRecord belongs_to :project belongs_to :resource, polymorphic: true - - validates :project, :resource, presence: true end diff --git a/app/models/project/submission.rb b/app/models/project/submission.rb index 51b18bd783..4084c0831f 100644 --- a/app/models/project/submission.rb +++ b/app/models/project/submission.rb @@ -31,7 +31,6 @@ class Project::Submission < ApplicationRecord Dataset::ValueSet ].freeze - validates :project, :user, :info_request, :resource, presence: true validates :resource_type, inclusion: { in: RESOURCE_TYPES } validates_associated :resource end diff --git a/app/models/public_body_category_link.rb b/app/models/public_body_category_link.rb index 8a6b799bcb..9f9ba3c846 100644 --- a/app/models/public_body_category_link.rb +++ b/app/models/public_body_category_link.rb @@ -18,9 +18,6 @@ class PublicBodyCategoryLink < ApplicationRecord belongs_to :public_body_heading, inverse_of: :public_body_category_links - validates_presence_of :public_body_category - validates_presence_of :public_body_heading - validates :category_display_order, numericality: { only_integer: true, message: 'Display order must be a number' } diff --git a/app/models/widget_vote.rb b/app/models/widget_vote.rb index 85801e3ab3..d2737ea107 100644 --- a/app/models/widget_vote.rb +++ b/app/models/widget_vote.rb @@ -14,7 +14,6 @@ class WidgetVote < ApplicationRecord belongs_to :info_request, inverse_of: :widget_votes - validates :info_request, presence: true validates :cookie, length: { is: 20 } validates :cookie, uniqueness: { scope: :info_request_id } end diff --git a/spec/models/info_request_batch_spec.rb b/spec/models/info_request_batch_spec.rb index bad3f99a0e..38fb96f3af 100644 --- a/spec/models/info_request_batch_spec.rb +++ b/spec/models/info_request_batch_spec.rb @@ -27,7 +27,7 @@ it 'should require a user' do info_request_batch.user = nil expect(info_request_batch.valid?).to be false - expect(info_request_batch.errors[:user]).to include("can't be blank") + expect(info_request_batch.errors[:user]).to include("must exist") end it 'should require a body' do diff --git a/spec/models/public_body_category_link_spec.rb b/spec/models/public_body_category_link_spec.rb index 6fc39c1bb3..23b34c2baa 100644 --- a/spec/models/public_body_category_link_spec.rb +++ b/spec/models/public_body_category_link_spec.rb @@ -26,13 +26,13 @@ it 'should be invalid without a category' do category_link = PublicBodyCategoryLink.new expect(category_link).not_to be_valid - expect(category_link.errors[:public_body_category]).to include("can't be blank") + expect(category_link.errors[:public_body_category]).to include("must exist") end it 'should be invalid without a heading' do category_link = PublicBodyCategoryLink.new expect(category_link).not_to be_valid - expect(category_link.errors[:public_body_heading]).to include("can't be blank") + expect(category_link.errors[:public_body_heading]).to include("must exist") end end diff --git a/spec/models/widget_vote_spec.rb b/spec/models/widget_vote_spec.rb index e3361078af..04dc0995ad 100644 --- a/spec/models/widget_vote_spec.rb +++ b/spec/models/widget_vote_spec.rb @@ -17,7 +17,7 @@ it 'requires an info request' do widget_vote = WidgetVote.new expect(widget_vote).not_to be_valid - expect(widget_vote.errors[:info_request]).to include("can't be blank") + expect(widget_vote.errors[:info_request]).to include("must exist") end it 'validates the cookie length' do From c4b8354567a890772e00ee0d81816952124fc5e6 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:23:03 +0000 Subject: [PATCH 14/53] Update to configuration defaults for Rails 5.1 --- config/application.rb | 2 +- config/initializers/new_framework_defaults_5_1.rb | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_5_1.rb diff --git a/config/application.rb b/config/application.rb index b02d1725bc..54097aa7f2 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,7 @@ class Application < Rails::Application # # config.time_zone = "Central Time (US & Canada)" # config.eager_load_paths << Rails.root.join("extras") - config.load_defaults 5.0 + config.load_defaults 5.1 config.autoloader = :zeitwerk config.active_record.legacy_connection_handling = false config.active_support.use_rfc4122_namespaced_uuids = true diff --git a/config/initializers/new_framework_defaults_5_1.rb b/config/initializers/new_framework_defaults_5_1.rb deleted file mode 100644 index 9010abd5c2..0000000000 --- a/config/initializers/new_framework_defaults_5_1.rb +++ /dev/null @@ -1,14 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 5.1 upgrade. -# -# Once upgraded flip defaults one by one to migrate to the new default. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. - -# Make `form_with` generate non-remote forms. -Rails.application.config.action_view.form_with_generates_remote_forms = false - -# Unknown asset fallback will return the path passed in when the given -# asset is not present in the asset pipeline. -# Rails.application.config.assets.unknown_asset_fallback = false From d89896be554dd065f048f8ce8f1b8704201924ec Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:23:31 +0000 Subject: [PATCH 15/53] Update to configuration defaults for Rails 5.2 --- config/application.rb | 2 +- .../new_framework_defaults_5_2.rb | 38 ------------------- 2 files changed, 1 insertion(+), 39 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_5_2.rb diff --git a/config/application.rb b/config/application.rb index 54097aa7f2..c8ef0cf9df 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,7 @@ class Application < Rails::Application # # config.time_zone = "Central Time (US & Canada)" # config.eager_load_paths << Rails.root.join("extras") - config.load_defaults 5.1 + config.load_defaults 5.2 config.autoloader = :zeitwerk config.active_record.legacy_connection_handling = false config.active_support.use_rfc4122_namespaced_uuids = true diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb deleted file mode 100644 index c383d072bc..0000000000 --- a/config/initializers/new_framework_defaults_5_2.rb +++ /dev/null @@ -1,38 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 5.2 upgrade. -# -# Once upgraded flip defaults one by one to migrate to the new default. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. - -# Make Active Record use stable #cache_key alongside new #cache_version method. -# This is needed for recyclable cache keys. -# Rails.application.config.active_record.cache_versioning = true - -# Use AES-256-GCM authenticated encryption for encrypted cookies. -# Also, embed cookie expiry in signed or encrypted cookies for increased security. -# -# This option is not backwards compatible with earlier Rails versions. -# It's best enabled when your entire app is migrated and stable on 5.2. -# -# Existing cookies will be converted on read then written with the new scheme. -# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true - -# Use AES-256-GCM authenticated encryption as default cipher for encrypting messages -# instead of AES-256-CBC, when use_authenticated_message_encryption is set to true. -# Rails.application.config.active_support.use_authenticated_message_encryption = true - -# Add default protection from forgery to ActionController::Base instead of in -# ApplicationController. -# Rails.application.config.action_controller.default_protect_from_forgery = true - -# Store boolean values are in sqlite3 databases as 1 and 0 instead of 't' and -# 'f' after migrating old data. -# Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true - -# Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. -# Rails.application.config.active_support.use_sha1_digests = true - -# Make `form_with` generate id attributes for any generated HTML tags. -# Rails.application.config.action_view.form_with_generates_ids = true From 19d8eaeea27929ec6a59c2e0a9ed06ae034a6174 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:24:07 +0000 Subject: [PATCH 16/53] Update to configuration defaults for Rails 6.0 --- config/application.rb | 2 +- .../new_framework_defaults_6_0.rb | 45 ------------------- 2 files changed, 1 insertion(+), 46 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_6_0.rb diff --git a/config/application.rb b/config/application.rb index c8ef0cf9df..944b78284c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,7 @@ class Application < Rails::Application # # config.time_zone = "Central Time (US & Canada)" # config.eager_load_paths << Rails.root.join("extras") - config.load_defaults 5.2 + config.load_defaults 6.0 config.autoloader = :zeitwerk config.active_record.legacy_connection_handling = false config.active_support.use_rfc4122_namespaced_uuids = true diff --git a/config/initializers/new_framework_defaults_6_0.rb b/config/initializers/new_framework_defaults_6_0.rb deleted file mode 100644 index 92240ef5f5..0000000000 --- a/config/initializers/new_framework_defaults_6_0.rb +++ /dev/null @@ -1,45 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 6.0 upgrade. -# -# Once upgraded flip defaults one by one to migrate to the new default. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. - -# Don't force requests from old versions of IE to be UTF-8 encoded. -# Rails.application.config.action_view.default_enforce_utf8 = false - -# Embed purpose and expiry metadata inside signed and encrypted -# cookies for increased security. -# -# This option is not backwards compatible with earlier Rails versions. -# It's best enabled when your entire app is migrated and stable on 6.0. -# Rails.application.config.action_dispatch.use_cookies_with_metadata = true - -# Change the return value of `ActionDispatch::Response#content_type` to Content-Type header without modification. -# Rails.application.config.action_dispatch.return_only_media_type_on_content_type = false - -# Return false instead of self when enqueuing is aborted from a callback. -# Rails.application.config.active_job.return_false_on_aborted_enqueue = true - -# Send Active Storage analysis and purge jobs to dedicated queues. -# Rails.application.config.active_storage.queues.analysis = :active_storage_analysis -# Rails.application.config.active_storage.queues.purge = :active_storage_purge - -# When assigning to a collection of attachments declared via `has_many_attached`, replace existing -# attachments instead of appending. Use #attach to add new attachments without replacing existing ones. -# Rails.application.config.active_storage.replace_on_assign_to_many = true - -# Use ActionMailer::MailDeliveryJob for sending parameterized and normal mail. -# -# The default delivery jobs (ActionMailer::Parameterized::DeliveryJob, ActionMailer::DeliveryJob), -# will be removed in Rails 6.1. This setting is not backwards compatible with earlier Rails versions. -# If you send mail in the background, job workers need to have a copy of -# MailDeliveryJob to ensure all delivery jobs are processed properly. -# Make sure your entire app is migrated and stable on 6.0 before using this setting. -# Rails.application.config.action_mailer.delivery_job = "ActionMailer::MailDeliveryJob" - -# Enable the same cache key to be reused when the object being cached of type -# `ActiveRecord::Relation` changes by moving the volatile information (max updated at and count) -# of the relation's cache key into the cache version to support recycling cache key. -# Rails.application.config.active_record.collection_cache_versioning = true From 8e9158036102ee0ef11b6130004e081e6dc875c3 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Mon, 16 Dec 2024 10:27:44 +0000 Subject: [PATCH 17/53] Remove duplicate call to create_track_for_request This is already called from an before action hook. Calling it a second time works until we enable the Rails 6.1's framework default configuration options due to Rails now tracking built associated objects resulting in two TrackThing instances, but only one being assign values for required attributes. --- app/controllers/comment_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 1d96061325..2e8ea2f79d 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -54,7 +54,6 @@ def new flash[:notice] = _("Thank you for making an annotation!") if params[:subscribe_to_request] - @track_thing = TrackThing.create_track_for_request(@info_request) @existing_track = TrackThing.find_existing(@user, @track_thing) if @user && @info_request.user == @user # don't subscribe to own request! From 14578243e76d7ab390a4001f80bcbfe9032f5f50 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Tue, 17 Dec 2024 06:17:40 +0000 Subject: [PATCH 18/53] Add comment to new framework config overrides So there is better visibility why these have been set without going into the git commit history. --- config/application.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/config/application.rb b/config/application.rb index 944b78284c..aae4ead8a3 100644 --- a/config/application.rb +++ b/config/application.rb @@ -33,9 +33,12 @@ class Application < Rails::Application # config.eager_load_paths << Rails.root.join("extras") config.load_defaults 6.0 config.autoloader = :zeitwerk - config.active_record.legacy_connection_handling = false - config.active_support.use_rfc4122_namespaced_uuids = true - config.active_storage.replace_on_assign_to_many = true + + # Enable new framework defaults configurations for later Rails versions + # preventing deprecation warnings + config.active_record.legacy_connection_handling = false # 7.0 + config.active_support.use_rfc4122_namespaced_uuids = true # 7.0 + config.active_storage.replace_on_assign_to_many = true # 7.1 # The default locale is :en and all translations from config/locales/*.rb,yml are auto loaded. # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] From 5aa91db61ea0afa3208d05ff6073243c0b497e27 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Tue, 17 Dec 2024 06:18:08 +0000 Subject: [PATCH 19/53] Disable new framework default It appears Rails 6.1's `active_record.has_many_inversing` configuration option change has introduced a regression in Rails which causes issues with our code base. We expect the follow to work, but with this configuration options this breaks due to how we associated objects via FactoryBot in our test suite: user = User.new info_request = InfoRequest.new(user: user) comment = Comment.create!(info_request: info_request, user: user) --- config/application.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/application.rb b/config/application.rb index aae4ead8a3..dba4d2e565 100644 --- a/config/application.rb +++ b/config/application.rb @@ -40,6 +40,10 @@ class Application < Rails::Application config.active_support.use_rfc4122_namespaced_uuids = true # 7.0 config.active_storage.replace_on_assign_to_many = true # 7.1 + # Disable new framework default has_many_inversing breaks some specs due to + # an apparent regression in Rails + config.active_record.has_many_inversing = false # 6.1 + # The default locale is :en and all translations from config/locales/*.rb,yml are auto loaded. # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] # config.i18n.default_locale = :de From 3261a2f40494b5e5d900b3a04ffa1037e3d847b0 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:24:37 +0000 Subject: [PATCH 20/53] Update to configuration defaults for Rails 6.1 --- config/application.rb | 2 +- .../new_framework_defaults_6_1.rb | 67 ------------------- 2 files changed, 1 insertion(+), 68 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_6_1.rb diff --git a/config/application.rb b/config/application.rb index dba4d2e565..7204d0b6eb 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,7 @@ class Application < Rails::Application # # config.time_zone = "Central Time (US & Canada)" # config.eager_load_paths << Rails.root.join("extras") - config.load_defaults 6.0 + config.load_defaults 6.1 config.autoloader = :zeitwerk # Enable new framework defaults configurations for later Rails versions diff --git a/config/initializers/new_framework_defaults_6_1.rb b/config/initializers/new_framework_defaults_6_1.rb deleted file mode 100644 index 9526b835ab..0000000000 --- a/config/initializers/new_framework_defaults_6_1.rb +++ /dev/null @@ -1,67 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 6.1 upgrade. -# -# Once upgraded flip defaults one by one to migrate to the new default. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. - -# Support for inversing belongs_to -> has_many Active Record associations. -# Rails.application.config.active_record.has_many_inversing = true - -# Track Active Storage variants in the database. -# Rails.application.config.active_storage.track_variants = true - -# Apply random variation to the delay when retrying failed jobs. -# Rails.application.config.active_job.retry_jitter = 0.15 - -# Stop executing `after_enqueue`/`after_perform` callbacks if -# `before_enqueue`/`before_perform` respectively halts with `throw :abort`. -# Rails.application.config.active_job.skip_after_callbacks_if_terminated = true - -# Specify cookies SameSite protection level: either :none, :lax, or :strict. -# -# This change is not backwards compatible with earlier Rails versions. -# It's best enabled when your entire app is migrated and stable on 6.1. -# Rails.application.config.action_dispatch.cookies_same_site_protection = :lax - -# Generate CSRF tokens that are encoded in URL-safe Base64. -# -# This change is not backwards compatible with earlier Rails versions. -# It's best enabled when your entire app is migrated and stable on 6.1. -# Rails.application.config.action_controller.urlsafe_csrf_tokens = true - -# Specify whether `ActiveSupport::TimeZone.utc_to_local` returns a time with an -# UTC offset or a UTC time. -# ActiveSupport.utc_to_local_returns_utc_offset_times = true - -# Change the default HTTP status code to `308` when redirecting non-GET/HEAD -# requests to HTTPS in `ActionDispatch::SSL` middleware. -# Rails.application.config.action_dispatch.ssl_default_redirect_status = 308 - -# Use new connection handling API. For most applications this won't have any -# effect. For applications using multiple databases, this new API provides -# support for granular connection swapping. -# Rails.application.config.active_record.legacy_connection_handling = false - -# Make `form_with` generate non-remote forms by default. -# Rails.application.config.action_view.form_with_generates_remote_forms = false - -# Set the default queue name for the analysis job to the queue adapter default. -# Rails.application.config.active_storage.queues.analysis = nil - -# Set the default queue name for the purge job to the queue adapter default. -# Rails.application.config.active_storage.queues.purge = nil - -# Set the default queue name for the incineration job to the queue adapter default. -# Rails.application.config.action_mailbox.queues.incineration = nil - -# Set the default queue name for the routing job to the queue adapter default. -# Rails.application.config.action_mailbox.queues.routing = nil - -# Set the default queue name for the mail deliver job to the queue adapter default. -# Rails.application.config.action_mailer.deliver_later_queue_name = nil - -# Generate a `Link` header that gives a hint to modern browsers about -# preloading assets when using `javascript_include_tag` and `stylesheet_link_tag`. -# Rails.application.config.action_view.preload_links_header = true From 69c46ceea10e2146f8332893ad9cdffdf21013d4 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Thu, 1 Feb 2024 10:25:13 +0000 Subject: [PATCH 21/53] Update to configuration defaults for Rails 7.0 --- config/application.rb | 4 +- .../new_framework_defaults_7_0.rb | 141 ------------------ 2 files changed, 1 insertion(+), 144 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_7_0.rb diff --git a/config/application.rb b/config/application.rb index 7204d0b6eb..408611976e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,13 +31,11 @@ class Application < Rails::Application # # config.time_zone = "Central Time (US & Canada)" # config.eager_load_paths << Rails.root.join("extras") - config.load_defaults 6.1 + config.load_defaults 7.0 config.autoloader = :zeitwerk # Enable new framework defaults configurations for later Rails versions # preventing deprecation warnings - config.active_record.legacy_connection_handling = false # 7.0 - config.active_support.use_rfc4122_namespaced_uuids = true # 7.0 config.active_storage.replace_on_assign_to_many = true # 7.1 # Disable new framework default has_many_inversing breaks some specs due to diff --git a/config/initializers/new_framework_defaults_7_0.rb b/config/initializers/new_framework_defaults_7_0.rb deleted file mode 100644 index 1cd17e47f7..0000000000 --- a/config/initializers/new_framework_defaults_7_0.rb +++ /dev/null @@ -1,141 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file eases your Rails 7.0 framework defaults upgrade. -# -# Uncomment each configuration one by one to switch to the new default. -# Once your application is ready to run with all new defaults, you can remove -# this file and set the `config.load_defaults` to `7.0`. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. -# https://guides.rubyonrails.org/upgrading_ruby_on_rails.html - -# `button_to` view helper will render `