Strip whitespace from BatchInvitationUser email & organisation_slug #2373

floehopper · 2023-09-25T10:20:23Z

I forgot to do this as part of #2371.

I've constructed the query in the migration slightly differently to similar previous migrations, because I noticed the whitespace character in the regular expression wasn't working as intended. I plan to add new migrations for the columns we've already updated using the flawed query in a separate PR.

I've had to use ActiveRecord::Persistence#update_attribute in order to skip model validation, because some of the existing BatchInvitationUser records are invalid for other reasons.

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!

In #2373 I noticed that the query we'd used to do this in #2291 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 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 User 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

In #2373 I noticed that the query we'd used to do this in #2291 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 BatchInvitationUser 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

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

Trello: https://trello.com/c/aO4nVCZT I forgot to do this as part of #2371. I've constructed the query in the migration slightly differently to similar previous migrations, because I noticed the whitespace character in the regular expression wasn't working as intended. I've had to use ActiveRecord::Persistence#update_attribute in order to skip model validation, because some of the existing BatchInvitationUser records are invalid for other reasons. 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!

In #2373 I noticed that the query we'd used to do this in #2291 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 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 User 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

In #2373 I noticed that the query we'd used to do this in #2291 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 BatchInvitationUser 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

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

In #2373 I noticed that the query we'd used to do this in #2291 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 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 User 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

In #2373 I noticed that the query we'd used to do this in #2291 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 BatchInvitationUser 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

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

floehopper changed the title ~~Strip leading & trailing whitespace from batch invitation user email & organisation_slug~~ Strip leading & trailing whitespace from BatchInvitationUser email & organisation_slug Sep 25, 2023

floehopper changed the title ~~Strip leading & trailing whitespace from BatchInvitationUser email & organisation_slug~~ Strip whitespace from BatchInvitationUser email & organisation_slug Sep 25, 2023

floehopper mentioned this pull request Sep 25, 2023

Fix for previous migrations that didn't strip whitespace for all the relevant records #2374

Merged

mike29736 self-assigned this Sep 25, 2023

mike29736 approved these changes Sep 25, 2023

View reviewed changes

floehopper added 2 commits September 25, 2023 15:34

floehopper force-pushed the strip-leading-and-trailing-whitespace-from-batch-invitation-user-email-and-organisation-slug branch from cc6b7a2 to 6977102 Compare September 25, 2023 14:34

floehopper merged commit afe562d into main Sep 25, 2023
6 checks passed

floehopper deleted the strip-leading-and-trailing-whitespace-from-batch-invitation-user-email-and-organisation-slug branch September 25, 2023 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strip whitespace from BatchInvitationUser email & organisation_slug #2373

Strip whitespace from BatchInvitationUser email & organisation_slug #2373

floehopper commented Sep 25, 2023

Strip whitespace from BatchInvitationUser email & organisation_slug #2373

Strip whitespace from BatchInvitationUser email & organisation_slug #2373

Conversation

floehopper commented Sep 25, 2023