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

cast_type use in rails_shim causes a problem with the serialize matcher - Rails 5 #913

Closed
adammcfadden opened this issue Mar 9, 2016 · 28 comments
Milestone

Comments

@adammcfadden
Copy link

I have seen this issue in Rails 5, and shoulda-matchers 3.1.1 (along with pulling shoulda-matchers directly from master).

IntakeRequest should serialize :payload class_name => JSON
     Failure/Error: should serialize(:payload).
     NoMethodError:
       undefined method `cast_type' for #<ActiveRecord::ConnectionAdapters::MySQL::Column:0x007f8dcc21d778>

This cast_type attribute has been removed from ActiveRecord::ConnectionAdapters::MySQL::Column per this commit

The cast_type attribute is called from within the rails_shim

I would be happy to help fix this, but could use some direction if anyone has suggestions on how best to address this.

Note: An issue was created by the cast_type change on the oracle adapter that may have useful information as they come to a resolution.

@adammcfadden
Copy link
Author

As another note, the serialized_attributes method has been removed from Rails 5 which will cause issues within the same method on the rails_shim

@adammcfadden
Copy link
Author

I have a possible solution that is using similar logic as a fix in rails_admin

/shoulda-matchers/lib/shoulda/matchers/rails_shim.rb:22

      def self.serialized_attributes_for(model)
        if defined?(::ActiveRecord::Type::Serialized)
          # Rails 5+
          model.columns.select do |column|
            model.type_for_attribute(column.name).is_a?(::ActiveRecord::Type::Serialized)
          end.inject({}) do |hash, column|
            hash[column.name.to_s] = model.type_for_attribute(column.name).coder.name.split("::").last.constantize
            hash
          end
        else
          model.serialized_attributes
        end
      end

Fix on forked repo: adammcfadden@9af2999

I have tested this against the rails 5 app that I originally discovered the issue with, and the above solution seems to be working. I will work on adding more holistic testing in with a pull request, but that will take some time as it looks like shoulda-matchers is not yet testing against rails 5.

If anyone has suggestions for refactoring (I would love to clean up hash[column.name.to_s] = model.type_for_attribute(column.name).coder.name.split("::").last.constantize ), or info about testing against rails 5 please let me know.

@engineersmnky
Copy link

@adammcfadden model.type_for_attribute(column.name).coder.name.split("::").last.constantize seems like it could be model.type_for_attribute(column.name).coder.name.demodulize.constantize more or less the same thing under the covers but reads a little nicer.

@fubarius
Copy link

fubarius commented May 3, 2016

Any update on this?

@adammcfadden
Copy link
Author

@fubarius I haven't put in any more work on this issue. I'm assuming that Rails 5 issues are waiting until rails 5 at least has a release candidate. @engineersmnky - nice, much cleaner!

@fubarius
Copy link

fubarius commented May 6, 2016

@adammcfadden with your fix in place I hit another issue that's possibly in -context. I'll try to get the test suite passing against Rails 5 and start opening pull requests, though who knows when they'll get accepted thanks to the rather vague "changing of the guard" notice on the shoulda* projects.

fubarius added a commit to fubarius/shoulda-matchers that referenced this issue May 9, 2016
@bsodmike
Copy link
Contributor

@adammcfadden I had to make a small change with respect to shoulda master, and it works a treat. Thanks!

# spec/rails_helper.rb
module Shoulda
  module Matchers
    RailsShim.class_eval do
      def self.serialized_attributes_for(model)
        if defined?(::ActiveRecord::Type::Serialized)
          # Rails 5+
          model.columns.select do |column|
            model.type_for_attribute(column.name).is_a?(::ActiveRecord::Type::Serialized)
          end.inject({}) do |hash, column|
            hash[column.name.to_s] = model.type_for_attribute(column.name).coder
            hash
          end
        else
          model.serialized_attributes
        end
      end
    end
  end
end

@aried3r
Copy link

aried3r commented Jul 8, 2016

Any news on this?

@anthonator
Copy link

I'm seeing this as well

@leifg
Copy link

leifg commented Jul 18, 2016

Is there any particular problem with applying the mentioned monkey_patch of @bsodmike to RailsShim?

@Shomari
Copy link

Shomari commented Aug 9, 2016

any update on this being patched?

@mcmire mcmire mentioned this issue Sep 4, 2016
9 tasks
@n-rodriguez
Copy link

Hi! I confirm that @bsodmike's patch works :)

bsodmike added a commit to bsodmike/shoulda-matchers that referenced this issue Oct 18, 2016
…_type' in Rails 5

This patch is based on the discussed held in issue thoughtbot#913, and a solution
that I've proposed and others have confirmed as good to go 👏.

Ref: thoughtbot#913 (comment)
@bsodmike
Copy link
Contributor

Thanks @leifg @n-rodriguez - I've just posted a PR for this patch.

bsodmike added a commit to bsodmike/shoulda-matchers that referenced this issue Oct 18, 2016
…_type' in Rails 5

This patch is based on the discussed held in issue thoughtbot#913, and a solution
that I've proposed and others have confirmed as good to go 👏.

Ref: thoughtbot#913 (comment)
pikender pushed a commit to vinsol-spree-contrib/spree-account-recurring that referenced this issue Jan 30, 2017
Fixed Strong Parameters issue - not saving stripe keys
Fixed ActionView::Template::Error
 (Asset was not declared to be precompiled in production.
 Add Rails.application.config.assets.precompile += %w(spree/frontend/stripe.js )
 to config/initializers/assets.rb and restart your server):
Added alternate for serailize :response due to bug in shoulda matchers
 thoughtbot/shoulda-matchers#913
@laertispappas
Copy link

When will this be fixed and released?

mcmire pushed a commit that referenced this issue Jul 25, 2017
…Rails 5

This patch is based on the discussed held in issue #913, and a solution
that I've proposed and others have confirmed as good to go 👏.

Ref: #913 (comment)
@mcmire
Copy link
Collaborator

mcmire commented Jul 25, 2017

I merged @bsodmike's fix into the rails-5 branch in 28a43a3, so I'm going to close this to clean things up. We'll have a new pre-release out soon so people can use that instead of having to use a branch in their Gemfiles.

@mcmire mcmire closed this as completed Jul 25, 2017
mcmire pushed a commit that referenced this issue Sep 17, 2017
…Rails 5

This patch is based on the discussed held in issue #913, and a solution
that I've proposed and others have confirmed as good to go 👏.

Ref: #913 (comment)
@mcmire mcmire added this to the v4.0 milestone Sep 19, 2017
@jeremywadsack
Copy link

@mcmire Any progress on a pre-release? We're still using the branch.

@mcmire
Copy link
Collaborator

mcmire commented Jan 20, 2018

@jeremywadsack Sigh. Somewhat. I've been (very slowly) working on improving some of the messages so I can debug some of the current issues better. But at this point it's completely backing things up. I'll take a look at what's left over and see if I can just finish up the remaining Rails 5 issues and put something out (even it's just an alpha) soon.

@javierojeda94
Copy link

javierojeda94 commented Jan 26, 2018

@mcmire For the people who are using the master branch, how can we fix this? Is there a workaround while we wait for the fix?

@mcmire
Copy link
Collaborator

mcmire commented Jan 26, 2018

@javierojeda94 It should be fixed already, in master. Are you still experiencing issues?

@javierojeda94
Copy link

javierojeda94 commented Jan 26, 2018

@mcmire unfortunately, yes. I'm using rspec to test a serialization field on my model but I'm facint some errors on those tests:

My model make.rb

class Make < ApplicationRecord
  serialize :vehicle_types, Array
end

When I run the example it { is_expected.to serialize(:vehicle_types).as(Array) }

I got this error:

NoMethodError:
       undefined method `cast_type' for #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x0000564663634340>

In my Gemfile.lock I have shoulda-matchers (3.1.2)

@mcmire
Copy link
Collaborator

mcmire commented Jan 27, 2018

@javierojeda94 Hmm, you shouldn't be seeing that. You sure you're pointed to master? You may want to run bundle update to ensure that you're pointed to the latest commit. (The version number is the same for now -- I haven't updated it yet.)

@dillionverma
Copy link

Any update on this? I recently experienced this issue.

@blackst0ne
Copy link

@mcmire

Any news?

@mcmire
Copy link
Collaborator

mcmire commented Apr 4, 2018

@dillionverma @blackst0ne As far as I know this is fixed. Are you receiving the same exact error, or some variation? And you're pointed to master (and if so, you've run bundle update shoulda-matchers to update it)?

@steobrien
Copy link

I can confirm this works perfectly on master.

A quick note to others, ensure you're following the master branch of this repository:

# Gemfile
gem "shoulda-matchers", git: "https://github.com/thoughtbot/shoulda-matchers"

@mcmire thanks again for the patch! It would be great to get this into an official release soon.

@aried3r
Copy link

aried3r commented Apr 18, 2018

Even a .beta or .rc1 version would be very much appreciated. :)

@yagudaev
Copy link

Yep it would be nice to get a new version of shoulda-matchers out, last update was over a year ago.

https://rubygems.org/gems/shoulda-matchers/versions/3.1.2

@jeremywadsack
Copy link

For the record, it looks like 4.0.0.rc1 was released in Oct with Rails 5 support.

tigefa4u pushed a commit to tigefa4u/gitlabhq that referenced this issue May 19, 2019
This removes the warning when using Ruby 2.6:

BigDecimal.new is deprecated; use BigDecimal() method instead.

This also adds Rails 5 support, which eliminates the need for the monkey
patch to handle
thoughtbot/shoulda-matchers#913.
tigefa4u pushed a commit to tigefa4u/gitlabhq that referenced this issue May 21, 2019
This removes the warning when using Ruby 2.6:

BigDecimal.new is deprecated; use BigDecimal() method instead.

This also adds Rails 5 support, which eliminates the need for the monkey
patch to handle
thoughtbot/shoulda-matchers#913.
tigefa4u pushed a commit to tigefa4u/gitlabhq that referenced this issue May 22, 2019
This removes the warning when using Ruby 2.6:

BigDecimal.new is deprecated; use BigDecimal() method instead.

This also adds Rails 5 support, which eliminates the need for the monkey
patch to handle
thoughtbot/shoulda-matchers#913.
tigefa4u pushed a commit to tigefa4u/gitlabhq that referenced this issue Jun 3, 2019
This removes the warning when using Ruby 2.6:

BigDecimal.new is deprecated; use BigDecimal() method instead.

This also adds Rails 5 support, which eliminates the need for the monkey
patch to handle
thoughtbot/shoulda-matchers#913.

CE port: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests