Skip to content

Commit

Permalink
Strip whitespace from name on existing Organisation records
Browse files Browse the repository at this point in the history
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
  • Loading branch information
floehopper committed Sep 25, 2023
1 parent d79280b commit 4bdebf6
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4bdebf6

Please sign in to comment.