Skip to content

Conversation

knagode
Copy link

@knagode knagode commented Sep 11, 2025

It is often very handy to use .save(validate: false), for example when allowing users to save drafts.

I noticed (too late) that strip_attributes only strips attributes before validation. My expectation was that it would always convert '' to NULL. Because it didn’t, I ended up with many empty string values in the database, which caused unique index exceptions ('' behaves very differently from NULL when dealing with unique indexes, and I wanted to ensure that empty strings are never persisted).

I believe strip_attributes should also work when using .save(validate: false), or at least provide a configurable setting to enable this behaviour by default across all models.

Workaround: call .valid? before .save(validate: false). This works, but it doesn’t feel intuitive.

@rmm5t
Copy link
Owner

rmm5t commented Sep 11, 2025

I think I'd like to consider moving strip_attributes to use the normalizes API, which would also solve for this. See #67.

However, this will likely require Rails 8.1 (rails/rails#53887)

In the meantime, redefining strip_attributes for your app and/or performing the before_save manually like you did is a fine workaround.

If I incorporated this before_save behavior into strip_attributes to support versions prior to Rails 8.1, I'd want to make sure that the stripping normalization only occurred once per #save.

@knagode
Copy link
Author

knagode commented Sep 12, 2025

I added code to ensure that stripping normalization isn’t executed twice. I didn’t add a test, as I don’t think it’s necessary, but I’m happy to add one if you think it’s worth it before merging. I can confirm that this PR would prevent several issues on our side but I understand that not many people use .save(validate: false).

If we were already on Rails 8.1, I’d probably just replace the strip_attributes operation with Rails’ built-in normalize.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants