implementing strong_parameters support #1750

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@garysweaver
Contributor

garysweaver commented Oct 29, 2012

Discussed quite a bit here:
gregbell#1731

The idea is that we have a strong_parameters branch of ActiveAdmin that would not get merged into ActiveAdmin master unless chosen as a basis for work on Rails 4 compatibility. However, others could depend on this branch in their Gemfile (info in the README).

I had another branch based on the original patch from the google list, but this request was based on suggestions I got from a ticket in the inherited resources issue tracker as well as suggestions from those on the thread above.

Unfortunately, I don't have enough time to spend on writing tests for it, but I tried to alter the tests to indicate that it is only compatible with Rails 3.2.

I know this is not the normal method of contribution, and I apologize. The original attempt was more friendly to earlier versions of Rails, but it seems like it makes sense to start to break ties with Rails pre-3.2 considering these changes. In addition, there may still be references to mass assignment security (attr_accessible) and mass assignment security whitelisting should be turned off, etc. in the Rails used for testing in this branch.

To reiterate: this pull request should not be merged into master unless more work is done to make it compatible with past Rails versions or could be merged into a rails 4 compatibility branch, but the branch itself may benefit others to be part of the main AA project as its own branch.

Thanks in advance for feedback.

@latortuga

This comment has been minimized.

Show comment Hide comment
@latortuga

latortuga Oct 29, 2012

Typo, my friend. Bundler refuses to bundle as-is because of the mismatched paren.

Typo, my friend. Bundler refuses to bundle as-is because of the mismatched paren.

This comment has been minimized.

Show comment Hide comment
@garysweaver

garysweaver Oct 29, 2012

Owner

Thanks, Drew! Something else wrong too. Somehow GitHub is showing master as having the same changes as this branch in my fork. I'm going to have to have some fun with git now. :(

Owner

garysweaver replied Oct 29, 2012

Thanks, Drew! Something else wrong too. Somehow GitHub is showing master as having the same changes as this branch in my fork. I'm going to have to have some fun with git now. :(

This comment has been minimized.

Show comment Hide comment
@garysweaver

garysweaver Oct 29, 2012

Owner

K fixed the mistaken commit to master and removed that parenth. Sorry bout that. thx! just started rake and its tests, so will see how that goes...

Owner

garysweaver replied Oct 29, 2012

K fixed the mistaken commit to master and removed that parenth. Sorry bout that. thx! just started rake and its tests, so will see how that goes...

This comment has been minimized.

Show comment Hide comment
@garysweaver

garysweaver Oct 29, 2012

Owner

Ok, did another commit to return nil from resource_params when params[name of model] is nil. Now I don't see a relation to failing tests and the SP related code, well at the moment at least.

Owner

garysweaver replied Oct 29, 2012

Ok, did another commit to return nil from resource_params when params[name of model] is nil. Now I don't see a relation to failing tests and the SP related code, well at the moment at least.

@seanlinsley

This comment has been minimized.

Show comment Hide comment
@seanlinsley

seanlinsley May 7, 2013

Member

Strong parameters should just work. Closing this PR.

Member

seanlinsley commented May 7, 2013

Strong parameters should just work. Closing this PR.

@seanlinsley seanlinsley closed this May 7, 2013

@garysweaver garysweaver deleted the garysweaver:strong_parameters_support branch May 7, 2013

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