API change in rails master breaks paperclip #983

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@adamtrilling

Any model save involving a paperclip field in rails master results in the following exception:

wrong number of arguments (2 for 1)

vendor/bundle/ruby/1.9.1/bundler/gems/rails-c5807728d52c/activesupport/lib/active_support/callbacks.rb:73:in `run_callbacks'
vendor/bundle/ruby/1.9.1/gems/paperclip-3.1.4/lib/paperclip/callbacks.rb:26:in `run_paperclip_callbacks'
...

From those two files, it's pretty obvious what happened: the run_callbacks method used to take two arguments and now it only takes one. I did some grepping and it doesn't look like paperclip was using the second argument, so I removed it, and now my models work.

I don't know a whole lot about the internals of paperclip, so feel free to reject this if I horribly broke everything.

@jyurek
Member
jyurek commented Aug 17, 2012

This actually passes current tests, but it would be awesome if you could add a test (or appraisal) showing that it passes in rails master.

@adamtrilling

Turns out, there are other issues with the tests in rails 4. I'm working on the appraisal and I'll push more code once I get that sorted out.

@adamtrilling

Shoulda doesn't work in Rails 4 either:

/Users/adam/code/paperclip/test/generator_test.rb:10:in <class:GeneratorTest>': undefined methodcontext' for GeneratorTest:Class (NoMethodError)
from /Users/adam/code/paperclip/test/generator_test.rb:5:in `<top (required)>'

I'll take a look at this when I have more time.

@travisbot

This pull request passes (merged 176bc59 into cb7ad7b).

@jyurek
Member
jyurek commented Aug 24, 2012

Ok, since there are other issues that need to be addressed and you're still working on it, I'm going to close this request so it doesn't get huge and ungainly. When you're done, please create a new one and we'll work on getting it merged. Thanks very much for your work here.

Also, if you haven't already, please make an issue and/or PR for shoulda so we can track that for Rails 4 as well. Thanks!

@jyurek jyurek closed this Aug 24, 2012
@sandelius sandelius referenced this pull request Nov 14, 2012
Closed

Support for Rails4 #1086

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