should_not allow_mass_assignment_of broken in rails3 RC

[justinko]

I forked remarkable, updated the Gemfile to use rails 3 RC and everything passes. Very strange why the rubygems package breaks...

[hosh]

Yeah, there're are some things broken in there. I don't use allow_mass_assignments ... Have you tried should allow_mass_assignment_of in RC?

[justinko]

Yes, should_allow_mass... works, but should_not_allow_mass.... does not

[justinko]

If you use attr_accessible in RC, Model.protected_attributes will NOT list the attributes that are NOT passed to attr_accessible.

[xxx]

This is still an issue as of Rails 3.0.3 and Remarkable 4.0.0.alpha4

[orslumen]

Came across the same issue using Rails 3.0.9, Remarkable 4.0.0.alpha4. Seems like AR adds "id" and "type" as attributes_protected_by_default, see activerecord-3.0.9/lib/active_record/base.rb#1708.

If you are using attr_accessible in your model and should_not allow_mass_assignment_of in the spec, the AllowMassAssignmentOfMatcher finds the default protected_attributes from AR and searches in vain for the attribute you provided to the matcher.

[sjmadsen]

Is there any reason not to query Model.column_names, exclude Model.accessible_attributes and use the resulting array for the list of protected attributes? I created a fork, made this change and it works for me.

I believe this is the same logic that Rails applies. If there is a whitelist, only attributes in that list are valid for mass-assignment. If there is no whitelist, only then is the protected blacklist consulted.

[hosh]

@sjmadsen The only reason was ignorance. I had been fixing this up for Rails 3.1 and had not dived into it, other than that Rails 3.1 uses a new ActiveModel::MassAssignmentSecurity module.

Is this logic you are talking about for this new 3.1 code, or the 3.0 base?

[sjmadsen]

My apologies if my comment came across as critical. One advantage to the current technique is that it doesn't touch the database, whereas querying Model.column_names necessarily has to do that. If someone wanted to avoid touching the database entirely, my solution isn't ideal.

I made the change and am currently working against Rails 3.0. I pushed my change up, so feel free to pull it if you feel it's appropriate. (lightyear@1c27093)

[hosh]

Nah, you didn't sound critical. I've dived deep into some parts of the code and ignored others, so bug fixes come with the help of people like you. Would you mind turning that commit into a pull request? Thanks

[rurounijones]

Just to confirm a few things. should I be expecting the allow_mass_assignment macro to work on a Rails 3.1 project with remarkable_mongoid 0.6.0 or is that version not yet compatible? (And would switching to master help?)

If switching to master works should I be expecting to be able to use "should_not allow_mass_assignment_of" as I was with shoulda-matchers?

[EDIT ]Just saw that the master of remarkable has a few bits related to this. I am having trouble using master with remarkable_mongoid at the moment so will wait for the next tagged release

[hosh]

@rurunijones remarkable_mongoid did not even pop up on my radar (since I don't use it).

Expect it to work with 3.1, and if mongoid builds off of ActiveModel, then getting that to work should not be so hard.

[rurounijones]

Ok, remarkable_mongoid relies on remarkable for the mass-assignment checks and mongoid uses the ActiveModel for for that I think so it should hopefully not be a problem.

I will try using remarkable_mongoid with remarkable master instead of the dependency set in the gem and see if it works. If not then I will wait for the next release of remarkable 4 alpha which remarkable_mongoid should hopefully pickup.