4653: Permit :date_range filter param in DistributionsController#index#4776
4653: Permit :date_range filter param in DistributionsController#index#4776dorner merged 5 commits intorubyforgood:mainfrom coalest:4653-fix-unpermitted-param-error-in-distributions-controller
Conversation
|
Looks good from a functional pov -- Over to @dorner for technical comments |
There was a problem hiding this comment.
Can this be handled by a request test? Controller tests are kind of missing some of the extra validations and coverage that request tests give us.
There was a problem hiding this comment.
I can change it to a request spec. I had figured that this permit shoulda-matcher on the controller was enough, but I guess with a request spec I could also ensure no "unpermitted params" messages are being logged.
There was a problem hiding this comment.
Change the controller spec to a request spec. Let me know if this is the kind of implementation you had in mind.
dorner
left a comment
dorner
left a comment
There was a problem hiding this comment.
Sorry, I don't think I was looking closely enough. My apologies. I don't think we need a spec just to test that a particular log is or isn't written. If this isn't affecting actual application behavior, I'd rather not have a spec at all.
…d-param-error-in-distributions-controller
Removed the spec. Yea, I was a little confused when writing it :) |
|
Looks good, thanks! |
rubyforgood#4776) * 4653: Permit :date_range filter param in DistributionsController#index * Move controller spec to request spec * Remove unneeded specs
|
@coalest: Your PR |
3 participants
Resolves #4653
Description
This "unpermitted parameter" warning was logged because we were permitting other filter params but not the
date_rangeparam. This parameter is needed for date filtering, so I added it to the permitted params list.Type of change
Minor change in logging.
How Has This Been Tested?
Added a spec to test permitted filter params.
And manually noticed that the "unpermitted_parameter" warning was no longer being logged.