Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close #797 #876

Merged
merged 1 commit into from
Jan 4, 2015
Merged

Close #797 #876

merged 1 commit into from
Jan 4, 2015

Conversation

rodzyn
Copy link
Contributor

@rodzyn rodzyn commented Jan 4, 2015

Hashie::Mash for declared params

@rodzyn rodzyn force-pushed the declared_params branch 2 times, most recently from 08b64d8 to cfd3879 Compare January 4, 2015 00:52
@dblock
Copy link
Member

dblock commented Jan 4, 2015

This needs a CHANGELOG entry please, and possible documentation in README.

@rodzyn
Copy link
Contributor Author

rodzyn commented Jan 4, 2015

Added.

dblock added a commit that referenced this pull request Jan 4, 2015
@dblock dblock merged commit 811eca6 into ruby-grape:master Jan 4, 2015
@dblock
Copy link
Member

dblock commented Jan 4, 2015

Merged, thanks.

@jwkoelewijn
Copy link
Contributor

Mash does not seem to play well with Rails::StrongParameters, because of the reimplementation of #respond_to?:

  • Mash#respond_to? will always return true for methods ending in '!' or '?'
  • Mash#identifier? will return true or false, based on the identifier being present in the Mash
  • Mash#identifier! will make sure the identifier key is present in the Mash

Rails::StrongParameters utilizes the #sanitize_for_mass_assignment method, which raises an
ActiveModel::ForbiddenAttributesError whenever the provided attributes respond to :permitted?
(which it does for all Mash'es, because it ends with a '?') AND when !attributes.permitted?
(which always returns true when the 'permitted' key is not present in the Mash):

module ActiveModel
  class ForbiddenAttributesError < StandardError
  end

  module ForbiddenAttributesProtection
    protected
    def sanitize_for_mass_assignment(attributes)
      if attributes.respond_to?(:permitted?) && !attributes.permitted?
        raise ActiveModel::ForbiddenAttributesError
      else
        attributes
      end
    end
  end
end

This results in always raising a ActiveModel::ForbiddenAttributeError

I do not see how to write a simple test-case for this and for now I reimplemented the #declared(params) helper in my own helpers utilizing a regular Hash.

@kuraga
Copy link
Contributor

kuraga commented May 19, 2015

@jwkoelewijn @dblock Are Hashie::* useful for us, at all?

@jwkoelewijn
Copy link
Contributor

I would propose to use the standard Hash and add a line or two to the readme to show how one could overwrite the #declared(params) method, for instance by wrapping it in a Hashie::Mash.

I would argue that the need for a Mash is not standard, rather an exception.

Apart from that, I never felt the need to access parameters as methods....

@dblock
Copy link
Member

dblock commented May 19, 2015

Please see /~https://github.com/Maxim-Filimonov/hashie-forbidden_attributes, what am I missing?

@jwkoelewijn
Copy link
Contributor

Ok, so there is a gem to fix a bug introduced by a change to introduce mashes. What's still not entirely clear though is the added value of using this Mash thing in the first place...

@dblock
Copy link
Member

dblock commented May 20, 2015

To be fair Mash was used all over the place before Rails4 strong attributes :) But I agree, Mash is "everything to everyone", and possibly there's a better fix that provides Mash semantics by mixing parts of Hashie (indifferent access? method access?) without needing another gem, I would welcome a PR like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants