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.

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

Can't be used without Rails and with ActiveModel but not ActiveRecord? #1553

Open
dgmstuart opened this issue Apr 16, 2023 · 1 comment
Open

Can't be used without Rails and with ActiveModel but not ActiveRecord? #1553

dgmstuart opened this issue Apr 16, 2023 · 1 comment

Comments

@dgmstuart
Copy link
Contributor

As I interpret the documentation, it seems like it should be possible to use shoulda-matchers without rails, with ActiveModel and not ActiveRecord?

I'm trying to use validates_presence_of in an isolated spec of an object which includes ActiveModel.

This fails with uninitialized constant ActiveRecord.

This seems to be because the matcher depends on RailsShim: shoulda/matchers/active_model/validate_presence_of_matcher.rb#L385

...which does a type check of attributes against ::ActiveRecord::Type::Serialized.
shoulda/matchers/rails_shim.rb#L65

It seems like this might be fixable with a check like Object.const_defined('::ActiveRecord::Type::Serialized'), but I'm not sure if I understand enough what RailsShim is to understand if this is a good idea.
dgmstuart added a commit to dgmstuart/swing-out-london that referenced this issue Apr 16, 2023
@dgmstuart
Event create and edit use form objects
e05aed7 
I'd like to decouple the form more from the model in order to provide a
better UX with form fields which don't exist on the model.

A first step is getting the existing functionality working with form
objects.

Note that on Edit we print the dates rather than returning the date
array from the model.

get rid of usage of date array - this hasn't actually been what we use
for making the listings for a while now.

Building the URL separately based on @event is a tradeoff: without this
we need to somehow get the event ID into the form so that `form_with`
can do its thing and infer which URL to use, but I couldn't find an
approach for that which works well:

  - adding an extra initialisation parameter breaks the validation specs
    which are expecting to be able to `build` an activemodel instance
    with FactoryBot
  - we could add ID as an attribute and set it in the validation error
    case on update, but that feels awkward.

Note that we need to monkeypatch Shoulda Matchers to run it without
ActiveRecord. This is NOT a "responsible" way of monkeypatching, but I
couldn't get it to work and all the examples I can find online are about
monkeypatching instance methods not class methods. Raised an issue on
the Shoulda Matchers repo here:

  thoughtbot/shoulda-matchers#1553
dgmstuart added a commit to dgmstuart/swing-out-london that referenced this issue Apr 16, 2023
@dgmstuart
Event forms use form objects
93a5682 
I'd like to decouple the form more from the model in order to provide a
better UX with form fields which don't exist on the model.

A first step is getting the existing functionality working with form
objects.

Note that on Edit we print the dates rather than returning the date
array from the model.

Get rid of usage of date_array - this hasn't actually been what we use
for making the listings for a while now, but it's still been used in
parallel - I guess because I wasn't sure about the SwingDates approach
(which I still think is kind of batshit crazy and a bad idea, but it
does mostly work)

Building the URL separately based on @event is a tradeoff: without this
we need to somehow get the event ID into the form so that `form_with`
can do its thing and infer which URL to use, but I couldn't find an
approach for that which works well:

  - adding an extra initialisation parameter breaks the validation specs
    which are expecting to be able to `build` an activemodel instance
    with FactoryBot
  - we could add ID as an attribute and set it in the validation error
    case on update, but that feels awkward.

Note that we need to monkeypatch Shoulda Matchers to run it without
ActiveRecord. This is NOT a "responsible" way of monkeypatching, but I
couldn't get it to work and all the examples I can find online are about
monkeypatching instance methods not class methods. Raised an issue on
the Shoulda Matchers repo here:

  thoughtbot/shoulda-matchers#1553
dgmstuart added a commit to dgmstuart/swing-out-london that referenced this issue Apr 16, 2023
@dgmstuart
Refactor: Event forms use form objects
c24f371 
I'd like to decouple the form more from the model in order to provide a
better UX with form fields which don't exist on the model.

A first step is getting the existing functionality working with form
objects.

Note that on Edit we print the dates rather than returning the date
array from the model.

Get rid of usage of date_array - this hasn't actually been what we use
for making the listings for a while now, but it's still been used in
parallel - I guess because I wasn't sure about the SwingDates approach
(which I still think is kind of batshit crazy and a bad idea, but it
does mostly work)

Building the URL separately based on @event is a tradeoff: without this
we need to somehow get the event ID into the form so that `form_with`
can do its thing and infer which URL to use, but I couldn't find an
approach for that which works well:

  - adding an extra initialisation parameter breaks the validation specs
    which are expecting to be able to `build` an activemodel instance
    with FactoryBot
  - we could add ID as an attribute and set it in the validation error
    case on update, but that feels awkward.

Note that we need to monkeypatch Shoulda Matchers to run it without
ActiveRecord. This is NOT a "responsible" way of monkeypatching, but I
couldn't get it to work and all the examples I can find online are about
monkeypatching instance methods not class methods. Raised an issue on
the Shoulda Matchers repo here:

  thoughtbot/shoulda-matchers#1553
@stonefield
Copy link
Contributor

stonefield commented Nov 9, 2023 •
edited

I tried to write a spec for @dgmstuart suggested implementation, without succeeding.
I conclude that it is impossible to test for this as long as we do not want to destroy the internal constant mappings.
However, I think the suggested implementation should be done without a spec (since it is not possible).
It will at least allow us which is only using ActiveModel to use matchers.

So please replace the line shoulda/matchers/rails_shim.rb#L65 with:

if Object.const_defined?('ActiveRecord::Type::Serialized') && attribute_type.is_a?(::ActiveRecord::Type::Serialized)

@stonefield stonefield mentioned this issue Nov 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@stonefield @dgmstuart