Pull `hashie-forbidden_attributes` into our codebase
What does this MR do and why?
We suspect we don't use the monkey patch from https://github.com/Maxim-Filimonov/hashie-forbidden_attributes at all.
Overall, the gem was last updated 5 years and does not have regular CI runs. Its tests are currently outdated using old version of Rails.
To remove it safely, we pull this code in our codebase and add a logging event.
If no logging will be invoked after running this code for a milestone, this code could be removed.
Discussed in: #378398 (comment 1143133426)
How to set up and validate locally
On this branch, in the rails console:
[6] pry(main)> Hashie::Mash.new.respond_to?(:permitted?)
=> false
[7] pry(main)> Hashie::Mash.new.permitted?
ArgumentError: ArgumentError
from /Users/al/dev/gitlab-development-kit/gitlab/config/initializers/hashie_mash_permitted_patch.rb:41:in `method_missing'
If you tail -f log/application_json.log
:
{"severity":"INFO","time":"2022-10-25T16:54:52.764Z","correlation_id":null,"message":"Hashie::Mash#respond_to?(:permitted?)","caller":["(pry):1:in `respond_to?'","(pry):1:in `__pry__'"]}
{"severity":"INFO","time":"2022-10-25T16:55:14.180Z","correlation_id":null,"message":"Hashie::Mash#permitted?","caller":["(pry):2:in `__pry__'"]}
On master
:
[1] pry(main)> Hashie::Mash.new.respond_to?(:permitted?)
=> true
[2] pry(main)> Hashie::Mash.new.permitted?
=> false
How to validate in more functional fashion
The examples above are rather "unit-test-style" validations.
To validate the changes in context of controller, I did:
-
Pull this Branch
-
I picked a convenient controller covered with tests and wrapped its params into
Mash
:
diff --git a/app/controllers/admin/application_settings/appearances_controller.rb b/app/controllers/admin/application_settings/appearances_controller.rb
index cf765c96a8f7..2c6bd917ab75 100644
--- a/app/controllers/admin/application_settings/appearances_controller.rb
+++ b/app/controllers/admin/application_settings/appearances_controller.rb
@@ -24,7 +24,7 @@ def create
end
def update
- if @appearance.update(appearance_params)
+ if @appearance.update(Hashie::Mash.new(appearance_params))
redirect_to admin_application_settings_appearances_path, notice: _('Appearance was successfully updated.')
else
render action: 'show'
- And run
bundle exec rspec spec/controllers/admin/application_settings/appearances_controller_spec.rb:51
. - It should be green
- Then comment out the contents of
config/initializers/hashie_mash_permitted_patch.rb
- You will get:
1) Admin::ApplicationSettings::AppearancesController PUT #update updates appearance with footer and header message
Failure/Error: connection.send(...)
ActiveModel::ForbiddenAttributesError:
ActiveModel::ForbiddenAttributesError
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:120:in `block in write_using_load_balancer'
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #378398 (closed)