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

Mark required elements inside group in mutually exclusive set as optional #207

Merged
merged 1 commit into from
Feb 9, 2015
Merged

Mark required elements inside group in mutually exclusive set as optional #207

merged 1 commit into from
Feb 9, 2015

Conversation

mintuhouse
Copy link
Contributor

No description provided.

is_nested_param = /^#{ Regexp.quote param }\[.+\]$/
params.each do |p, _|
if p.match(is_nested_param)
params[p][:required] = false unless value[:required].presence
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is altering the original params, I don't think we want that side effect.

@dblock
Copy link
Member

dblock commented Jan 31, 2015

Please update CHANGELOG, too. Thanks!

@dblock
Copy link
Member

dblock commented Feb 9, 2015

I'm merging this.

Do you see any reason not to use deep_dup, would make things a bit shorter?

        dup_params = params.deep_dup # duplicate the params as we are going to modify them
        dup_params.reject do |param, value|
          is_nested_param = /^#{ Regexp.quote param }\[.+\]$/
          0 < params.count do |p, _|
            match = p.match(is_nested_param)
            dup_params[p][:required] = false if match && !value[:required]
            match
          end
        end

dblock added a commit that referenced this pull request Feb 9, 2015
Mark required elements inside group in mutually exclusive set as optional
@dblock dblock merged commit 8f2febe into ruby-grape:master Feb 9, 2015
@mintuhouse
Copy link
Contributor Author

deep_dup is a rails method. I am not sure if everyone who uses grape-swagger use it along with rails. So avoided using it.

@mintuhouse mintuhouse deleted the feature_mutually_exclusive branch February 9, 2015 12:07
@dblock
Copy link
Member

dblock commented Feb 9, 2015

Makes sense.

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.

2 participants