Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

permitted_attributes to take param_key into consideration. #16

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

hamajyotan
Copy link
Collaborator

I really like permitted_attributes and would like to apply it to many cases, but I am having a little trouble with it.
It is not possible to determine the namespace-aware scope name from a nested model name such as Admin::User.

Admin::User -> :user

This pull request solves that problem.

Admin::User -> :admin_user

Specifically, for objects that respond to model_name (assuming ActiveRecord::Base, etc.), we'll apply model_name.param_key This is similar to how form_with in Rails determines the scope.
See https://github.com/rails/rails/blob/main/actionview/lib/action_view/helpers/form_helper.rb#L750

@YasuakiOmokawa
Copy link

Good solution for the usecase👍

@kyuden
Copy link
Owner

kyuden commented Nov 28, 2021

Hi @hamajyotan . Sorry for the late reply.
Since you suggested that you would like to help Banken as a maintainer in that email you sent to me, I added you as a new maintainer for this repository. Thank you for the offering.

My concern is that the current Travis CI in this repository hasn't been as free as it used to be due to their policy changes. So we can't confirm that this PR's test passes with various ruby ​​versions. Also before releasing this change, we need to check the test with 2.7 and later versions of ruby. You can continue to use Travis, check it locally, or move to another CI like Github Action, Please make sure that the tests pass with various Ruby versions before the next release.

@hamajyotan
Copy link
Collaborator Author

@kyuden Thanks for the reply and adding me as a maintainer.

I'll run the checks locally on various versions of Ruby first.
Then I'd like to set up a CI environment for the Ruby 2.7 migration with another Github Actions (is that right?). to set up a CI environment for Ruby 2.7 or later.

@kyuden
Copy link
Owner

kyuden commented Dec 7, 2021

@hamajyotan Great! 👍

@hamajyotan
Copy link
Collaborator Author

@kyuden I apologize for the extended period of silence.

I have made adjustments to ensure that tests run successfully on Ruby 2.3 and later using Github Actions. Tests are executed in a matrix with various versions of Rails.

As a point of note, for Rails 3.x, initially, strong_parameters is not built into Rails itself but is excluded from the test configuration due to its complexity. On the other hand, considering the age of Rails 3.x, it might be reasonable to exclude it from gem support.

Furthermore, in this pull request, GitHub Actions were not functioning, so I separately confirmed the CI behavior at hamajyotan#2. The results can be found at https://github.com/hamajyotan/banken/actions/runs/7873598059/.

That's all, please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants