Skip to content
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.

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

Add with_primary_key to AssociationMatchers #597

Merged
merged 1 commit into from
Nov 2, 2014
Merged

Add with_primary_key to AssociationMatchers #597

merged 1 commit into from
Nov 2, 2014

Conversation

petedmarsh
Copy link
Contributor

This adds with_primary_key to AssociationMatchers so you can now test the :primary_key option of an association.

@@ -924,16 +951,30 @@ def foreign_key_exists?
!(belongs_foreign_key_missing? || has_foreign_key_missing?)
end

def primary_key_exists?
!(belongs_primary_key_missing? || has_primary_key_missing?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These methods are awkwardly named.

What if we combined the checks on macro into one method (macro_supports_primary_key?, perhaps?), then here we said macro_supports_primary_key? && !class_has_primary_key?(model_class)?

@mcmire
Copy link
Collaborator

mcmire commented Oct 9, 2014

@petedmarsh Thanks for this PR. Just had a couple of suggestions above if you don't mind.

@petedmarsh
Copy link
Contributor Author

@mcmire no problem, I should get this done this evening!
On 9 Oct 2014 08:31, "Elliot Winkler" notifications@github.com wrote:

@petedmarsh https://github.com/petedmarsh Thanks for this PR, sorry it
took me so long to review it. Just had a couple of suggestions above if you
don't mind.


Reply to this email directly or view it on GitHub
#597 (comment)
.

@petedmarsh
Copy link
Contributor Author

@mcmire I've made those changes, the methods for with_foreign_key could possibly do with the same treatment. I've left them as-is but I can change them too if you'd like :)

@mcmire
Copy link
Collaborator

mcmire commented Oct 10, 2014

@petedmarsh Ah, you were just copying with_foreign_key, that makes sense now.

Yeah if you want to open a new PR to change that too, I would gladly merge it in.

# class PersonTest < ActiveSupport::TestCase
# should belong_to(:great_country).
# with_primary_key('country_id')
# end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a newline here? (Well technically a newline but still within the comment section... if you know what I mean)

@petedmarsh
Copy link
Contributor Author

@mcmire I've made those minor changes, and I've added some documentation for has_one and has_many that I had missed - should be good to go!

@mcmire mcmire added this to the 2.7.1 milestone Oct 13, 2014
@petedmarsh
Copy link
Contributor Author

I noticed that I hadn't added an item to NEWS.md so I did that too :)

@petedmarsh
Copy link
Contributor Author

I fixed the merge conflict - it was in the NEWS.md file, should I just remove my changes there and let you guys update it post-merge?

@mcmire
Copy link
Collaborator

mcmire commented Nov 2, 2014

@petedmarsh Generally I do discourage people from adding NEWS items now, but that's fine. Sorry for the delay on this, I will merge this in now.

@mcmire mcmire merged commit 0f34231 into thoughtbot:master Nov 2, 2014
@petedmarsh
Copy link
Contributor Author

Absolutely no problem, thanks!
On 2 Nov 2014 19:40, "Elliot Winkler" notifications@github.com wrote:

Merged #597 #597.


Reply to this email directly or view it on GitHub
#597 (comment).

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Jun 12, 2015
pkgsrc change: allow build on Ruby 2.2.

# 2.8.0

### Deprecations

* `ensure_length_of` has been renamed to `validate_length_of`.
  `ensure_length_of` is deprecated and will be removed in 3.0.0.

* `set_the_flash` has been renamed to `set_flash`. `set_the_flash` is
  deprecated and will be removed in 3.0.0.

* `set_session(:foo)` is deprecated in favor of `set_session[:foo]`.
  `set_session(:foo)` will be invalid syntax in 3.0.0.

* Using `should set_session[:key].to(nil)` to assert that that a value has not
  been set is deprecated. Please use `should_not set_session[:key]` instead.
  In 3.0.0, `should set_session[:key].to(nil)` will only pass if the value is
  truly nil.

### Bug fixes

* Fix `delegate_method` so that it works again with shoulda-context. ([#591])

* Fix `validate_uniqueness_of` when used with `scoped_to` so that when one of
  the scope attributes is a polymorphic `*_type` attribute and the model has
  another validation on the same attribute, the matcher does not fail with an
  error. ([#592])

* Fix `has_many` used with `through` so that when the association does not
  exist, and the matcher fails, it does not raise an error when producing the
  failure message. ([#588])

* Fix `have_and_belong_to_many` used with `join_table` so that it does not fail
  when `foreign_key` and/or `association_foreign_key` was specified on the
  association as a symbol instead of a string. ([#584])

* Fix `allow_value` when an i18n translation key is passed to `with_message` and
  the `:against` option is used to specify an alternate attribute. A bug here
  also happened to affect `validate_confirmation_of` when an i18n translation
  key is passed to `with_message`. ([#593])

* Fix `class_name` qualifier for association matchers so that if the model being
  referenced is namespaced, the matcher will correctly resolve the class before
  checking it against the association's `class_name`. ([#537])

* Fix `validate_inclusion_of` used with `with_message` so that it fails if given
  a message that does not match the message on the validation. ([#598])

* Fix `route` matcher so that when controller and action are specified in hash
  notation (e.g. `posts#show`), route parameters such as `id` do not need to be
  specified as a string but may be specified as a number as well. ([#602])

* Fix `allow_value`, `validate_numericality_of` and `validate_inclusion_of` so
  that they handle RangeErrors emitted from ActiveRecord 4.2. These exceptions
  arise whenever we attempt to set an attribute using a value that lies outside
  the range of the column (assuming the column is an integer). RangeError is now
  treated specially, failing the test instead of bubbling up as an error.
  ([#634], [#637], [#642])

### Features

* Add ability to test `:primary_key` option on associations. ([#597])

* Add `allow_blank` qualifier to `validate_uniqueness_of` to complement
  the `allow_blank` option. ([#543])

* Change `set_session` so that #[] and #to qualifiers are optional, similar to
  `set_flash`. That is, you can now say `should set_session` to assert that any
  flash value has been set, or `should set_session.to('value')` to assert that
  any value in the session is 'value'.

* Change `set_session` so that its #to qualifier supports regexps, similar to
  `set_flash`.

* Add `with_prefix` qualifier to `delegate_method` to correspond to the `prefix`
  option for Rails's `delegate` macro. ([#622])

* Add support for Rails 4.2, especially fixing `serialize` matcher to remove
  warning about `serialized_attributes` being deprecated. ([#627])

* Update `dependent` qualifier on association matchers to support `:destroy`,
  `:delete`, `:nullify`, `:restrict`, `:restrict_with_exception`, and
  `:restrict_with_error`. You can also pass `true` or `false` to assert that
  the association has (or has not) been declared with *any* dependent option.
  ([#631])

### Improvements

* Tweak `allow_value` failure message so that it reads a bit nicer when listing
  existing errors.

[#591]: thoughtbot/shoulda-matchers#591
[#592]: thoughtbot/shoulda-matchers#592
[#588]: thoughtbot/shoulda-matchers#588
[#584]: thoughtbot/shoulda-matchers#584
[#593]: thoughtbot/shoulda-matchers#593
[#597]: thoughtbot/shoulda-matchers#597
[#537]: thoughtbot/shoulda-matchers#537
[#598]: thoughtbot/shoulda-matchers#598
[#602]: thoughtbot/shoulda-matchers#602
[#543]: thoughtbot/shoulda-matchers#543
[#622]: thoughtbot/shoulda-matchers#622
[#627]: thoughtbot/shoulda-matchers#627
[#631]: thoughtbot/shoulda-matchers#631
[#634]: thoughtbot/shoulda-matchers#634
[#637]: thoughtbot/shoulda-matchers#637
[#642]: thoughtbot/shoulda-matchers#642

# 2.7.0

### Deprecations

* `ensure_inclusion_of` has been renamed to `validate_inclusion_of`.
  `ensure_inclusion_of` is deprecated and will be removed in 3.0.0.

* `ensure_exclusion_of` has been renamed to `validate_exclusion_of`.
  `ensure_exclusion_of` is deprecated and will be removed in 3.0.0.

### Bug fixes

* Fix `delegate_method` so that it does not raise an error if the method that
  returns the delegate object is private.

* Warn when `ensure_inclusion_of` is chained with `.in_array([false, true])`
  as well as with `.in_array([true, false])`.

* Fix `set_session` so that the `to` qualifier if given nil checks that the
  session variable in question was set to nil (previously this actually did
  nothing).

* Fix `filter_param` so that it works when `config.filter_parameters` contains
  regexes.

* Fix `delegate_method` so that it can be required independent of Active
  Support.

* Fix `validate_uniqueness_of`. When used against an unpersisted record whose
  model contained a non-nullable column other than the one being validated, the
  matcher would break. Even if the test set that column to a value beforehand,
  the record had to be persisted in order for the matcher to work. Now this is
  no longer the case and the record can remain unpersisted.

* Fix `validate_absence_of`: it required that a string be passed as the
  attribute name rather than a symbol (which is the usual and documented usage).

### Improvements

* `have_and_belongs_to_many` now checks to make sure that the join table
  contains the correct columns for the left- and right-hand side of the
  association.

* Reword failure message for `delegate_method` so that it's a little more
  helpful.

### Features

* Add new matcher `define_enum_for` to test usage of the `enum` macro introduced
  in Rails 4.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants