Add normalize matcher

[stephannv]

Why

Rails 7.1 adds normalizes API.

• ActiveRecord::Base::normalizes API.
• Docs

I think would be nice to shoulda-matchers to have a matchers to easily test the new normalizes API, similar to allow_value/allow_values matchers. I'm using on my project and this:

it "normalizes email" do
  user = User.new(email: " \n myEmail@here.com \n\n")
  user.normalize_attribute(:email)
  expect(user.email).to eq "myemail@here.com"
end

Will become this:

it { is_expected.to normalize(:email).from(" \n myEmail@here.com \n\n").to("myemail@here.com") }

What do you think about? This PR makes sense?

Some considerations:

• I'm not sure about the API normalize(attr), from(value) and to(value), I'm open to change.
• normalizes accepts apply_to_nil option, but I think this should be tested in its own spec, like

  shoulda normalize(:name).from(nil).to("Untitled")

  Another option would be:

  shoulda normalize(:name).from(" jane doe ").to("Jane Doe").normalizing_nil_to("Untitled")

• I had to add Rails 7.1 to Appraisals to be able to test. I'm not sure how to restrict this tests to run only in rails_7_1. Other questions: Do we need to flag this matcher to run only in Rails 7.1 projects? Do we need to add this info to the docs?
• I think this is my first PR to shoulda-matchers, so, feedback are welcome.
• This PR is a work in progress, I opened it to receive feedbacks, so some non-related tests can be broken, or rubocop can be failing.

[mcmire]

Hey @stephannv, I think this is a great idea! I would probably suggest that we extract adding Rails 7.1 to another PR as it may cause CI failures. As for whether we should handle the apply_to_nil option by adding another method or not, I think we should keep it simple and stick to from and to like you've done here.

One thing about this is that we might want to implement a negative version of the matcher. Every matcher can be executed via should_not, and that may match or may not match, just as with should. We might want to have another failure message in that case.

Other than that this looks great!

[stephannv]

Done @mcmire . Can you take a look again?

I tried to create another PR focused on adding Rails 7.1 to pipeline but I faced some issues, I will give another try later.

[Sub-Xaero]

Any updates on this PR?
Happy to help out if more needs doing to take it over the line.

[stephannv]

Any updates on this PR? Happy to help out if more needs doing to take it over the line.

@Sub-Xaero I was waiting 7.1 release to update this PR, but I think I don't need to wait.

[stephannv]

@Sub-Xaero I remember now, this PR is waiting shoulda-matchers to support Rails 7.1. If you want to help, I think you can open a PR with necessary changes to prepare shoulda-matchers for Rails 7.1

[stephannv]

Hi @matsales28 , are you the new code owner for this PR? I rebased it with main, I think it's ready for review now. Your feedback will be very much appreciated.

[matsales28]

Hi @matsales28 , are you the new code owner for this PR? I rebased it with main, I think it's ready for review now. Your feedback will be very much appreciated.

Yes, I'll look at this PR next week. I'm pretty busy at the moment with all the holidays.

[matsales28]

Overall it looks good to me. Left some small suggestions but once adjusted we're ready to merge. One thing that could make it even better is when we're dealing with several attributes, it would be awesome if we could check all of them and then provide an error message listing all the attributes that didn't pass the normalization. Right now, the way it's set up, only the first one that doesn't meet the criteria shows up in the error message. We can make this on an next PR.

[stephannv]

Overall it looks good to me. Left some small suggestions but once adjusted we're ready to merge. One thing that could make it even better is when we're dealing with several attributes, it would be awesome if we could check all of them and then provide an error message listing all the attributes that didn't pass the normalization. Right now, the way it's set up, only the first one that doesn't meet the criteria shows up in the error message. We can make this on an next PR.

Yeah @matsales28 , I think it would be a great improvement.

Thanks for the feedback, I think I addressed everything. Let me know if I'm missing something.

[stephannv]

Sorry for that @matsales28 , I think it's working now.