vinistock / sail

Sail is a lightweight Rails engine that brings an admin panel for managing configuration settings on a live Rails app
Other
507 stars 32 forks source link

Allow extra params in settings and profiles controllers #338

Closed denny closed 3 years ago

denny commented 3 years ago
denny commented 3 years ago

I guess these are new to Rails 6.1 maybe?

vinistock commented 3 years ago

Once again, thank you so much for the interest and help with Sail.

I think this PR should be merged regardless, since the strong parameters aren't including all that's needed. However, I couldn't quite reproduce the issue with Rails 6.1.1. I get the warnings in the console about unpermitted params, but requests are successful.

Did you verify that this fixes the issue?

denny commented 3 years ago

Hrm. It did for me, but maybe it's tangled up with the app I'm embedding Sail into. I'll try to isolate things a bit more if I get a chance later this week coming.

vinistock commented 3 years ago

Well, if it fixes the issue, I'm fine with merging. We should fix the unpermitted parameters anyway.

denny commented 3 years ago

Having looked at my own forms again, I think there might be a more 'correct' way of fixing this, but it involves a bit more work. Leave this open for now and I'll see if I can tackle it in the next week or two? I think Sail might need to move from form_tag to form_for or form_with, and then you can use a .require().permit() chain in the receiving methods instead of just .permit(), which will handle (or ignore) the authenticity_token param automatically, and I think _method too. I haven't seen a locale param in my forms though, is that a part of Sail?

vinistock commented 3 years ago

Sounds good. Sail will handle the locale parameter, as long as the main app passes it along.

denny commented 3 years ago

By the way, in case you're interested, here's a screenshot of Sail embedded into the ShinyCMS admin area 🙂 Screenshot from 2021-01-18 22-08-42

vinistock commented 3 years ago

That looks fancy. I love it!