Add without_scopes to enum matcher.
#1453
Conversation
|
I am sure this could use some work, particularly around the message wording. But if you are open to this let me know and I can clean it up. |
|
I am also unsure how to structure this so it only tests for rails 6 on this test. I couldn't find examples of that (based on grepping the words |
|
Hi, @dewyze, I hope you are doing fine. I think this PR can help you: https://github.com/thoughtbot/shoulda-matchers/pull/1429/files As you can see there was a method to check if the rails version supported active storage before running the tests. You can do something similar. Or maybe this PR can help you: https://github.com/thoughtbot/shoulda-matchers/pull/1428/files. Take a closer look on the spec/support/unit/helpers/rails_versions.rb. I think you will need a similar method to check if Rails is greater than 5.2. As you can see the tests are failing when running with this version. |
|
@vsppedro tests are passing, and I added a test that was missing. I had a hard time figuring out the cleanest way to handle the messages. I am also not sure if there needs to be updates with the messages. Both the instance methods and class scopes can have prefixes or suffixes, so it did seem to make sense to include that messaging in both. Additionally, I felt torn about the Nevertheless, best I can tell this includes all use cases and does work. Let me know if there is any cleanup you'd like to see. |
|
Hi, @dewyze, I hope you're doing fine. In case you don't have the time to continue working on this PR, I can help. What do you think? |
dewyze
force-pushed
the
add_without_scopes_to_enum_matcher
branch
from
16abb59 to
60d77be
Compare
|
@vsppedro sorry about the long silence! I fixed the rubocop errors, but the 2.6/2.7 errors were too old to see. If you can run the workflows I can take a look at the errors and get them fixed up. |
|
No Problem. 👍🏽 |
|
@vsppedro the issue was that I was adding the scopes parameter in the builder, but not checking the rails version, so it would fail on rails 5 versions. I updated it. |
| _suffix: suffix, | ||
| } | ||
|
|
||
| if rails_version =~ '~> 6.0' |
There was a problem hiding this comment.
Hi @dewyze, I'm wondering if this should be rails_version >= '6.0' instead? I would imagine Rails 7 has the same change as Rails 6 did.
There was a problem hiding this comment.
@mcmire annoyingly, in rails 7, they removed the leading underscore. https://api.rubyonrails.org/classes/ActiveRecord/Enum.html
So this won't work for rails 7, but there are no appraisals yet, so i wasn't going to worry about that.
|
Thank you for your contribution! |
|
What do you think about squashing the commits into one? |
Rails 6 added the `_scopes: false` option when defining enums. This will
prevent rails from creating class scopes for each of the values.
Previously, shoulda-matchers checked for the presence of singleton
methods as a way to check that the corresponding enum methods have been
defined. In rails 6, if someone were using `_scopes: false` the matcher
will fail.
Instead, we should check on the presence of the instance methods that
are generated: `#{value}!` and `#{value}?` as the default checker for
the enum values generating methods.
We can then check on the scopes/singleton methods generated when using
the new `without_scopes` matcher.
dewyze
force-pushed
the
add_without_scopes_to_enum_matcher
branch
from
121bcbe to
7c559d9
Compare
|
Thank you! ❤️ |
Rails 6 added the
_scopes: falseoption when defining enums. This willprevent rails from creating class scopes for each of the values.
Previously, shoulda-matchers checked for the presence of singleton
methods as a way to check that the corresponding enum methods have been
defined. In rails 6, if someone were using
_scopes: falsethe matcherwill fail.
Instead, we should check on the presence of the instance methods that
are generated:
#{value}!and#{value}?as the default checker forthe enum values generating methods.
We can then check on the scopes/singleton methods generated when using
the new
without_scopesmatcher.