New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add index_errors association matcher #1089

Closed

brendanthomas1 wants to merge 4 commits into thoughtbot:master from brendanthomas1:add-index-errors-association-matcher

Contributor

brendanthomas1 commented Mar 8, 2018

Hello! Our team was adding index_errors to a couple has_many associations and we noticed that there's no matcher for that yet. It looked pretty similar to autosave since it's just a boolean, so I pretty much copied that 😄


          add index_errors association matcher

52b5d3e

houndci-bot reviewed

View reviewed changes

spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb Outdated

+                      end
+                      message = 'Expected Parent to have a has_many association called children (children should have index_errors set to true)'
+                      expect {

houndci-bot Mar 8, 2018

Avoid using {...} for multi-line blocks.

spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb Outdated

+                        has_many :children, autosave: false
+                      end
+                      message = 'Expected Parent to have a has_many association called children (children should have index_errors set to true)'

houndci-bot Mar 8, 2018

Line is too long. [130/80]

spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb Outdated

+                      expect(Parent.new).to have_many(:children).index_errors(true)
+                    end
+                    it 'rejects an association with a non-matching :index_errors option and returns the correct message' do

houndci-bot Mar 8, 2018

Line is too long. [109/80]

lib/shoulda/matchers/active_record/association_matcher.rb Outdated

+                          if option_verifier.correct_for_boolean?(:index_errors, options[:index_errors])
+                            true
+                          else
+                            @missing = "#{name} should have index_errors set to #{options[:index_errors]}"

houndci-bot Mar 8, 2018

Line is too long. [92/80]

lib/shoulda/matchers/active_record/association_matcher.rb Outdated

@@ @@ -1257,6 +1281,19 @@ def autosave_correct? @@
                         end
                       end
+                      def index_errors_correct?
+                        if options.key?(:index_errors)
+                          if option_verifier.correct_for_boolean?(:index_errors, options[:index_errors])

houndci-bot Mar 8, 2018

Line is too long. [90/80]

lib/shoulda/matchers/active_record/association_matcher.rb Outdated

@@ @@ -524,6 +524,24 @@ def belong_to(name) @@
                     #       should have_many(:games).autosave(true)
                     #     end
                     #
+                    # ##### index_errors
+                    #
+                    # Use `index_errors` to assert that the `:index_errors` option was specified.

houndci-bot Mar 8, 2018

Line is too long. [83/80]


          fix hound warnings

0ab8115

houndci-bot reviewed

View reviewed changes

lib/shoulda/matchers/active_record/association_matcher.rb Outdated

+                      def index_errors_correct?
+                        if options.key?(:index_errors)
+                          if option_verifier.correct_for_boolean?(:index_errors,
+                                                                  options[:index_errors])

houndci-bot Mar 8, 2018

Use one level of indentation for parameters following the first line of a multi-line method call.

lib/shoulda/matchers/active_record/association_matcher.rb

@@ @@ -1257,6 +1282,21 @@ def autosave_correct? @@
                         end
                       end
+                      def index_errors_correct?

houndci-bot Mar 8, 2018

Method has too many lines. [12/10]

houndci-bot reviewed

View reviewed changes

lib/shoulda/matchers/active_record/association_matcher.rb Outdated

+                      def index_errors_correct?
+                        if options.key?(:index_errors)
+                          if option_verifier.correct_for_boolean?(
+                              :index_errors, options[:index_errors]

houndci-bot Mar 9, 2018

Use one level of indentation for parameters following the first line of a multi-line method call.
Indent the first parameter one step more than the start of the previous line.

lib/shoulda/matchers/active_record/association_matcher.rb

@@ @@ -1257,6 +1282,22 @@ def autosave_correct? @@
                         end
                       end
+                      def index_errors_correct?

houndci-bot Mar 9, 2018

Method has too many lines. [13/10]

Contributor Author

brendanthomas1 Mar 9, 2018 •

edited

Loading

This is down to 10 lines now 🙂


          only run specs for >= rails 5

4d1fbae

brendanthomas1 force-pushed the add-index-errors-association-matcher branch from f8abd89 to 4d1fbae Compare

March 9, 2018 18:15


          fix method to long hound warning in index_errors_correct

aa4aacc

Collaborator

mcmire commented Apr 4, 2018

I've never seen the index_errors option before. What does it do?

Contributor Author

brendanthomas1 commented Apr 4, 2018

You can use it with a has_many to determine which nested object as an error

# Old style

guitar.errors["tuning_pegs.pitch"] = ["is not a number"]

# New style (if defined globally, or set in has_many_relationship)

guitar.errors["tuning_pegs[1].pitch"] = ["is not a number"]

The Rails PR is here, and it's in the changelog, but it hasn't made it into the Rails docs yet for some reason 🤷‍♂️

Collaborator

mcmire commented Apr 4, 2018

Oh very interesting, okay. Thanks for the clarification!

mcmire added the ⏱ PR: In Queue for Merging label

Collaborator

mcmire commented Apr 4, 2018

Thanks! I've tweaked this a bit for style and merged this as 795ca68.

mcmire closed this

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

Labels

⏱ PR: In Queue for Merging