search

rubymonolith / superform

Build highly customizable forms in Rails
MIT License
263 stars 14 forks source link

Optional authenticity token #38

Open nolantait opened 1 month ago

nolantait commented 1 month ago

Rails doesn't check GET requests for authenticity_token. Currently if you make GET requests the token will be littered in the URL. Rails also has the same option.

I also added some docs in the README. This change also contains some feedback I got from my friend who was reading the docs and got confused about #row mentions in the README. I've changed them to #labelled so they are unified.

This PR also adds phlex-testing-capybara which is quite familiar to rails devs and could help make writing specs for rails components more approachable. Its still a bit awkward to magically stub helpers in the tests but they work and are easy to write.

Happy for any feedback

bradgessler commented 1 month ago

Thanks for contributing! I'd rather eliminate the configuration and instead not include the authenticity token for a GET request.

Here's the steps that would take:

  1. Remove the :authenticity_token option from the constructor.
  2. Create a protected method within Superform like def has_authenticity_token? = form_method != "get"
  3. Add a check to the form template that looks something like authenticity_token_field if has_authenticity_token?

If you include specs for all that and I can merge it.

For the docs, you can tell people they can force that field for get requests by overriding the method like this: has_authenticity_token? = true

Could you put the row/labeled changes in a different PR? I like keeping PRs related to one issue so the conversation doesn't fork.

nolantait commented 3 weeks ago

Sounds good, I've removed the docs changes in this PR. Not sure what I buggered with the Gemfile.lock for the CI.