Turn on parameter whitelisting and get specs to pass #205
Conversation
|
Merged this and pushed. Pushed some tweaks of my own that fixed issues that cropped up with Gemfile.lock update |
|
Just had a look and this comes up. This pull seams to break {"utf8"=>"✓",
"authenticity_token"=>"3tTj+Kj3qZi4jhISeEG7aaT7QOlb/1CgX1Q9n3LrnyQ=",
"topic"=>{"subject"=>"wasa",
"posts_attributes"=>{"0"=>{"text"=>"mamamama"}}},
"commit"=>"Create Topic",
"forum_id"=>"4"}Will have a dig but if anyone has time before tomorrow go ahead and dig. Thanks |
|
we need a test case that shows this. Sorry. Anyone else seeing it? @jgadbois? |
|
I didn't see it - maybe your User model just doesn't have the new forem_auto_subscribe field. You will potentially need to add it with your own migration because the migration to add it is dynamically generated during install, but not if you just copy over new migrations for upgrade. |
|
ah good eye. its true in my users table there is no such field. I'll do a migration now. |
|
@jgadbois where is the migration generator for this i want to have a look. |
|
lib/generators/forem/install/templates/forem_auto_subscribe_migration.rb |
|
@jgadbois thanks, That fixed it. |
|
Yeah unless your Forem user class isn't 'User'. If it is something else then you will need to change :users. |
| @@ -56,6 +56,11 @@ def test_dummy_config | |||
| inject_into_file "#{dummy_path}/config/application.rb", | |||
| "\nrequire 'kaminari'\n", | |||
| :before => "module Dummy" | |||
|
|
|||
| inject_into_file "#{dummy_path}/config/application.rb", | |||
| "\n config.active_record.whitelist_attributes = true\n", | |||
There was a problem hiding this comment.
Strongly strongly strongly disagree.
There was a problem hiding this comment.
Suggestions? You disagree with approach or turning whitelist_attributes on?
There was a problem hiding this comment.
Disagree with turning this option on. If you turn this option on there's no way to know for certain that we have all attributes mass-assignable as required.
I'm about to head out to lunch but will look at this again when I come back.
There was a problem hiding this comment.
K, I was just turning it on so that things actually break when running the specs. Otherwise everything can be mass assigned no problem (unless attr_accessible is called).
There was a problem hiding this comment.
I've now removed this with @35bd6fa. All the tests still pass. Nice work!
Fixes #204