westonganger / protected_attributes_continued

The community continued version of protected_attributes for Rails 5+
MIT License
45 stars 33 forks source link

ActiveRecord 5.2 compatibility #9

Closed willbryant closed 5 years ago

willbryant commented 6 years ago

On 5.2 calling create! raises the following error:

 ArgumentError:
   wrong number of arguments (given 2, expected 0..1)

From lib/active_record/mass_assignment_security/validations.rb:15.

The new method no longer accepts an options argument.

So it would need some kind of arity check or explicit version check. But I've never actually seen that second argument used so perhaps it would be better just to remove it?

willbryant commented 6 years ago

Looks like create_scope also needs to be replaced with scope.scope_for_create as that was removed in 01c85097d4977b0c141b7c89df15c0750f37c62d..

willbryant commented 6 years ago

Similar problem with the options argument to all the build calls in associations.rb.

westonganger commented 6 years ago

I was starting to remove the options behavior from new but then I remembered the roles portion of this library. The effects of changing this are far and wide. So instead I would rather monkey patch new to use assign_attributes under the hood, however I was having trouble coming up with a working patch. Would happily welcome a PR or advice on the matter.

willbryant commented 6 years ago

Oh, I'm not familiar with that. Is that what the as: option did?

westonganger commented 6 years ago

Yes exactly. To be honest I don't know it existing until earlier today as well.

willbryant commented 6 years ago

Hmm. So what's confusing me is that presumably that option was not supported by rails itself in say 5.1, because it was part of the protected_attributes gem? But the options hash was passed around anyway?

jihao commented 6 years ago

I'm trying to lift up my old rails 3 app directly to 5 by copy & paste some folders and add protected_attributes_continued to Gemfile.

Meet the same issue when try to assign a has_many field. Thanks in advance if this can be patched.

wrong number of arguments (given 2, expected 0..1)

def build_association(*options, &block)
          klass.new(*options, &block)
        end
      end
    else

Call Stack:

activerecord (5.2.0) lib/active_record/inheritance.rb:50:in `new'
protected_attributes_continued (1.3.0) lib/active_record/mass_assignment_security/reflection.rb:8:in `build_association'
protected_attributes_continued (1.3.0) lib/active_record/mass_assignment_security/associations.rb:7:in `build_record'
protected_attributes_continued (1.3.0) lib/active_record/mass_assignment_security/associations.rb:25:in `build'
activerecord (5.2.0) lib/active_record/associations/has_many_through_association.rb:60:in `build_through_record'
activerecord (5.2.0) lib/active_record/associations/has_many_through_association.rb:31:in `block in concat_records'
activerecord (5.2.0) lib/active_record/associations/has_many_through_association.rb:30:in `each'
activerecord (5.2.0) lib/active_record/associations/has_many_through_association.rb:30:in `concat_records'
activerecord (5.2.0) lib/active_record/associations/collection_association.rb:119:in `concat'
activerecord (5.2.0) lib/active_record/associations/has_many_through_association.rb:21:in `concat'
activerecord (5.2.0) lib/active_record/associations/collection_association.rb:413:in `replace_records'
activerecord (5.2.0) lib/active_record/associations/collection_association.rb:248:in `replace'
activerecord (5.2.0) lib/active_record/associations/collection_association.rb:69:in `ids_writer'
activerecord (5.2.0) lib/active_record/associations/builder/collection_association.rb:65:in `user_ids='
SalMac86 commented 6 years ago

I am also in the midst of a Rail 3 to 5 update and trying to use the protected_attributes_continued gem to minimize the re-writes.

I can't seem to get any rake task to complete - I end up with something like this:

ArgumentError: wrong number of arguments (given 2, expected 0..1)
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/activerecord-5.2.0/lib/active_record/relation.rb:56:in 'new'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/protected_attributes_continued-1.3.0/lib/active_record/mass_assignment_security/relation.rb:53:in 'find_or_initialize_by'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/activerecord-5.2.0/lib/active_record/querying.rb:8:in 'find_or_initialize_by'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/activerecord-5.2.0/lib/active_record/internal_metadata.rb:20:in '[]='
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/activerecord-5.2.0/lib/active_record/railties/databases.rake:9:in 'block (2 levels) in <top (required)>'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/railties-5.2.0/lib/rails/commands/rake/rake_command.rb:23:in 'block in perform'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/railties-5.2.0/lib/rails/commands/rake/rake_command.rb:20:in 'perform'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/railties-5.2.0/lib/rails/command.rb:48:in 'invoke'
/home/vagrant/.rvm/gems/ruby-2.5.1/gems/railties-5.2.0/lib/rails/commands.rb:18:in '<top (required)>'
bin/rails:4:in 'require'
bin/rails:4:in '<main>'
Tasks: TOP => db:environment:set
cvincent commented 6 years ago

Can confirm this issue also exists in Rails 5.1.

diogenes commented 5 years ago

@westonganger @willbryant Does it make sense the old options hash (that were being passed on those new calls) be part of the Relation object as a new internal attribute?

Let's say, for example we could have something like this on active_record/mass_assignment_security/relation.rb:

private

def build_relation(attributes, mass_assignment_options = {}, &block)
  @mass_assignment_options = mass_assignment_options

  new(attributes, &block)
end

Do you see a better approach for that? What's the best place for those roles options reside?

willbryant commented 5 years ago

I'm no longer working for the company that used this gem, but I am pretty sure that they didn't ever use that options thing. I certainly didn't know about it until we ran into the issue here.

westonganger commented 5 years ago

Since it seems most don't use the roles feature, I am willing to accept a PR that removes the roles portion of this library for Rails 5.2+ compatibility. After thats in, I'll do a major version bump for the breaking change.

thtonon commented 5 years ago

Removing the options argument from every #new and #create methods solved the issue for me since my application is not using this option argument at all.

westonganger commented 5 years ago

This bug has now been fixed. v1.4.0 has now been released which is fully now compatible with Rails 5.2