From 4bdebf6ededc4edeedd3a6b3778d03cc368c7fc6 Mon Sep 17 00:00:00 2001 From: James Mead Date: Mon, 25 Sep 2023 12:21:21 +0100 Subject: [PATCH] Strip whitespace from name on existing Organisation records In #2373 I noticed that the query we'd used to do this in #2351 didn't escape the regular expression properly. ActiveRecord seems to have generated the following SQL WHERE expression: WHERE (name REGEXP('^ +') OR name REGEXP(' +$') I _think_ this will only have found names with leading/trailing space characters and not leading/trailing newlines/carriage-returns/tabs. Thus the previous migration didn't do anything bad, it just may have missed _some_ records. In this new version of the migration I've constructed the query slightly differently. Firstly I've double-escaped the whitespace character in the regular expression and secondly I've used an Array Condition vs a Pure String Condition [1] to ensure argument safety. The new query generates the following SQL WHERE expression: WHERE (name REGEXP '^\\s+' OR name REGEXP '\\s+$') This has the REGEXP string argument escaped correctly. As per the migrations in #2373 I've use ActiveRecord::Persistence#update_attribute in order to skip model validation, in case some of the existing Organisation records are invalid for other reasons. As before I did consider writing the migration as a SQL UPDATE, but generating a TRIM expression that is exactly equivalent to String#strip isn't as trivial as it sounds! [1]: https://guides.rubyonrails.org/active_record_querying.html#conditions --- ...230925104849_strip_whitespace_from_organisation_name.rb | 7 +++++++ db/schema.rb | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20230925104849_strip_whitespace_from_organisation_name.rb diff --git a/db/migrate/20230925104849_strip_whitespace_from_organisation_name.rb b/db/migrate/20230925104849_strip_whitespace_from_organisation_name.rb new file mode 100644 index 000000000..3282a9485 --- /dev/null +++ b/db/migrate/20230925104849_strip_whitespace_from_organisation_name.rb @@ -0,0 +1,7 @@ +class StripWhitespaceFromOrganisationName < ActiveRecord::Migration[7.0] + def change + Organisation.where("name REGEXP ? OR name REGEXP ?", "^\\s+", "\\s+$").each do |o| + o.update_attribute(:name, o.name&.strip) # rubocop:disable Rails/SkipsModelValidations + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f67b8819f..51398648e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_25_104848) do +ActiveRecord::Schema[7.0].define(version: 2023_09_25_104849) do create_table "batch_invitation_application_permissions", id: :integer, charset: "utf8mb3", force: :cascade do |t| t.integer "batch_invitation_id", null: false t.integer "supported_permission_id", null: false