Skip to content

Do not verify_filters in the RequestParser#1111

Merged
lgebhardt merged 1 commit intoJSONAPI-Resources:release-0-9from
stevenharman:fix_verify_filter_backport
Dec 10, 2017
Merged

Do not verify_filters in the RequestParser#1111
lgebhardt merged 1 commit intoJSONAPI-Resources:release-0-9from
stevenharman:fix_verify_filter_backport

Conversation

@stevenharman
Copy link
Copy Markdown

A prior change to allow filters to be passed to related resources was incompletely backported from master to the release-0-9 branch (see: 3a691b2). This completes the backport, which moves the verify_filters from the RequestParser down to the
Processor. When verify_filters happens in both places, we end up running verify_filter twice for each filter, which can result in the filter values being wrapped in nested Arrays.

fixes #1110

A prior change to allow filters to be passed to related resources was
incompletely backported from `master` to the `release-0-9` branch (see:
3a691b2). This completes the backport,
which moves the `verify_filters` from the `RequestParser` down to the
`Processor`. When `verify_filters` happens in both places, we end up
running `verify_filter` twice for each filter, which can result in the
filter values being wrapped in nested Arrays.

fixes JSONAPI-Resources#1110
@stevenharman
Copy link
Copy Markdown
Author

These failures seem to be a problem in minitest rather than with any changes I made. Thoughts?

@stevenharman
Copy link
Copy Markdown
Author

Any thoughts on this PR? It would be nice to get the 0.9 series working - at least from the official branch, so we don't have to maintain our own forks. As mentioned in #1110, this change is finishing an incomplete cherry-pick.

@stevenharman
Copy link
Copy Markdown
Author

@lgebhardt Sorry to @ you on this one, but any chance of merging this into the release-0-9 branch to finish the cherry pick? The original author agreed the changes here should have been picked over originally. As for the test failures... thanks minitest?

@lgebhardt lgebhardt merged commit 7ab7f4e into JSONAPI-Resources:release-0-9 Dec 10, 2017
@lgebhardt
Copy link
Copy Markdown
Contributor

@stevenharman Thanks, and sorry for the delay. Thanks for pointing out the errors in the tests, though I must admit I missed that when i first reviewed this PR.

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