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

Makes #declared unavailable to before filters #1142

Merged
merged 1 commit into from Sep 3, 2015

Conversation

jrforrest
Copy link
Contributor

In response to issue #1074

This adds a mechanism by which Grape::DSL::InsideRoute methods may be
overriden after certain filters are run.

The first problem case of a filter being utilized before it should that
necessitated this enhancement is the #declared method, which was
returning un-coerced params in before filters.

There may be other problem cases, which may be rectified using this same
mechanism.

In response to issue ruby-grape#1074

This adds a mechanism by which Grape::DSL::InsideRoute methods may be
overriden after certain filters are run.

The first problem case of a filter being utilized before it should that
necessitated this enhancement is the `#declared` method, which was
returning un-coerced params in `before` filters.

There may be other problem cases, which may be rectified using this same
mechanism.
end
end
end
end

# A filtering method that will return a hash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little weird to have the docs on the method stub rather than the full version in the PostBeforeFilter module. Anyone more savvy with YARD know if this is the best way to be doing this?

@dblock
Copy link
Member

dblock commented Sep 1, 2015

Please do fix the build, thx.

@jrforrest
Copy link
Contributor Author

Sorry about that. Didn't realize Rubocop was doing its thing.

Should be good now.

@dblock
Copy link
Member

dblock commented Sep 2, 2015

This looks good, mind squashing the commits please?

@dblock
Copy link
Member

dblock commented Sep 2, 2015

Needs a CHANGELOG entry and I think something in UPGRADING that says if you used this method before it will now raise ...

@jrforrest jrforrest force-pushed the bugfix/declared_scope branch 3 times, most recently from a2acb8e to ef2948f Compare September 2, 2015 20:37
@jrforrest
Copy link
Contributor Author

Squashed, and updated CHANGELOG and UPGRADING docs.

dblock added a commit that referenced this pull request Sep 3, 2015
Makes #declared unavailable to before filters
@dblock dblock merged commit 6167847 into ruby-grape:master Sep 3, 2015
@dblock
Copy link
Member

dblock commented Sep 3, 2015

Merged, thanks.

@jrforrest
Copy link
Contributor Author

Sure thing. Thank you for the thorough review.

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.

None yet

2 participants