Using #fetch with #permit isn't fully tested #899
Conversation
|
Oops, it turns out the matcher does work as expected, but the test case was missing params to get it to work, since the regular empty hash returned by |
aripollak
changed the title
|
|
||
| expect(controller). | ||
| to permit(:eta, :diner_id). | ||
| for(:create, params: { order: { foo: '' } }) |
There was a problem hiding this comment.
Thanks for this -- this is definitely a more realistic way to test this. You're using foo as a key just so the order hash is non-empty, is that correct?
There was a problem hiding this comment.
Yeah.. Ideally that shouldn't need to happen for this, but I'm not sure why it's needed. I get why it's needed when using require, but not fetch.
When the permit matcher is used without `#on`, the controller does not use `params#require`, and the params object is duplicated, the matcher did not recognize the `#permit` call inside the controller. This happened because the matcher overwrote double registries with the same parameter hash whenever ActionController::Parameters was instantiated. This is related to #899.
|
Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this PR. Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this PR, then please let us know so that we can reprioritize it. Thanks! |
The fix for #495 doesn't seem to work currently. There's a test case for it, but it didn't test the full behavior, so I fixed that and now it fails as expected. Unfortunately I haven't figured out what the appropriate fix is for this, so some guidance would be appreciated.