Parameter filtering when logging parameters #443

Merged
merged 3 commits into from Nov 28, 2014

Conversation

Projects
None yet
2 participants
@ma2gedev
Contributor

ma2gedev commented Oct 25, 2014

This is discussed at #423.
I've added parameter filtering.
I'll squash commits if it's allowed to merge.
Thanks in advance!

@chrismccord

This comment has been minimized.

Show comment
Hide comment
@chrismccord

chrismccord Oct 31, 2014

Member

@ma2gedev I think this is close. Can you include docs on configuring filtered_params ?

Member

chrismccord commented Oct 31, 2014

@ma2gedev I think this is close. Can you include docs on configuring filtered_params ?

@ma2gedev

This comment has been minimized.

Show comment
Hide comment
@ma2gedev

ma2gedev Oct 31, 2014

Contributor

Thanks @chrismccord !
I've added docs about configure in this commit ma2gedev@8333b96.

Contributor

ma2gedev commented Oct 31, 2014

Thanks @chrismccord !
I've added docs about configure in this commit ma2gedev@8333b96.

@ma2gedev ma2gedev changed the title from [WIP] Parameter filtering when logging parameters to Parameter filtering when logging parameters Nov 1, 2014

@ma2gedev

This comment has been minimized.

Show comment
Hide comment
@ma2gedev

ma2gedev Nov 1, 2014

Contributor

@chrismccord Thanks for your patience.
I've squashed all commits about this pull request.
And configuring docs is here. https://github.com/ma2gedev/phoenix/blob/parameter_filter/lib/phoenix/plugs/controller_logger.ex#L9-L16

Contributor

ma2gedev commented Nov 1, 2014

@chrismccord Thanks for your patience.
I've squashed all commits about this pull request.
And configuring docs is here. https://github.com/ma2gedev/phoenix/blob/parameter_filter/lib/phoenix/plugs/controller_logger.ex#L9-L16

lib/phoenix/plugs/controller_logger.ex
+ defp filter_parameters(params) do
+ filter_parameters = Application.get_env(:phoenix, :filter_parameters)
+ Enum.into params, %{}, fn {k, v} ->
+ if String.contains?(String.downcase(k), filter_parameters) do

This comment has been minimized.

@chrismccord

chrismccord Nov 24, 2014

Member

If we are down casing the key, we should probably downcase the configured param keys as well. Case insensitive matching both ways makes the most sense to me

@chrismccord

chrismccord Nov 24, 2014

Member

If we are down casing the key, we should probably downcase the configured param keys as well. Case insensitive matching both ways makes the most sense to me

@ma2gedev

This comment has been minimized.

Show comment
Hide comment
@ma2gedev

ma2gedev Nov 27, 2014

Contributor

@chrismccord Thanks for reviewed. I support such case in commit 732793d

Contributor

ma2gedev commented Nov 27, 2014

@chrismccord Thanks for reviewed. I support such case in commit 732793d

@ma2gedev

This comment has been minimized.

Show comment
Hide comment
@ma2gedev

ma2gedev Nov 27, 2014

Contributor

OMG! It's conflicts. 😱
I rebased the code. The commit is e7fde49 that support camelcase parameters in config.filter_parameters.
@chrismccord

Contributor

ma2gedev commented Nov 27, 2014

OMG! It's conflicts. 😱
I rebased the code. The commit is e7fde49 that support camelcase parameters in config.filter_parameters.
@chrismccord

@chrismccord chrismccord merged commit e7fde49 into phoenixframework:master Nov 28, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@chrismccord

This comment has been minimized.

Show comment
Hide comment
@chrismccord

chrismccord Nov 28, 2014

Member

Thanks! <3 <3 <3

Member

chrismccord commented Nov 28, 2014

Thanks! <3 <3 <3

@ma2gedev

This comment has been minimized.

Show comment
Hide comment
@ma2gedev

ma2gedev Nov 28, 2014

Contributor

@chrismccord
Thanks!!
And sorry conflicts again.
Many many thanks for your effort to resolve conflicts at ba3a74d

❤️ 💚 💛

Contributor

ma2gedev commented Nov 28, 2014

@chrismccord
Thanks!!
And sorry conflicts again.
Many many thanks for your effort to resolve conflicts at ba3a74d

❤️ 💚 💛

@ma2gedev ma2gedev deleted the ma2gedev:parameter_filter branch Nov 28, 2014

Gazler pushed a commit that referenced this pull request Sep 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment