Add MongoMapper.filter_attributes#709
Conversation
…attributes as default
| if ActiveSupport.version >= Gem::Version.new("6.0") | ||
| @filter ||= begin | ||
| require "active_support/parameter_filter" |
There was a problem hiding this comment.
parameter_filter exists in ActionPack in Rails 5.x.
Then, we have to check the version and add branches to support it.
Rails 5.x was already EOL.
So we suggest dropping the support for Rails 5.x to keep the implementation simple.
Rails 5.2.Z is included in the list of supported series until June 1st 2022.
https://guides.rubyonrails.org/maintenance_policy.html#severe-security-issues
There was a problem hiding this comment.
Great feature!
I think we should leave in the 5.x support for now since it was so recently removed and since the conditional code is already there. I suspect there are also a lot of people running rails 5.2. (looks like ~ 17% are on 5.2 - https://rails-hosting.com/2022/#what-versions-of-rails-are-you-using-in-your-applications).
With that said, I think in the next few months we should drop rails 5.2 support, but only do it with a major version upgrade.
Also, I suspect that once the rails team stops providing security updates to rails 5.2 it will encourage a majority of users to upgrade.
So let's leave in the conditional code for now but plan to remove it in the next few months with mongomapper v16
There was a problem hiding this comment.
Okay, thanks for your comment based on the evidence 👍
Let's keep the conditional code!
remove it in the next few months with mongomapper v16
👍 👍 👍
There was a problem hiding this comment.
Sounds good 👍
I will add a deprecation warning for v16 with another PR.
a2ikm
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
This feature improves the security and the compatibility with ActiveRecord.
I think it's ok to merge this PR now.
On the other hand, as @mopp commented, the incompatibility between Rails 5.x and 6.0+ makes the code complex. I think we should drop Rails 5.x support.
(Of course, we need some deprecation warning before that)
@smtlaissezfaire How do you think?
|
Please merge this PR if you are ready ✨ |
Background
The ActiveRecord has
filter_attributesoption in order to conceal private information such as user email address.See also rails/rails#33756
So we would like to do the same thing on the mongomapper.
What we did
We added
filter_attributesoption and made#inspectfilter the value based on the given keywords.