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

Rails: update first form project description to account for default form submission behavior with Turbo #28932

Closed
3 tasks done
jg-k opened this issue Oct 9, 2024 · 7 comments · Fixed by #29165
Closed
3 tasks done

Comments

@jg-k
Copy link
Contributor

jg-k commented Oct 9, 2024

Checks

Describe your suggestion

In the first rails form project, it is advised to start by writing the first form HTML manually, without using rails helpers.
Step 1 mentions
"You don’t want to forget about safety, so make sure you provide the form with an authenticity token. If you don’t remember how to do so, go back to the Form Basics lesson and refresh your memory."

This link points to a section of the rails form basic lesson where it suggests that "You’ll either get an error or your user session will get zeroed out (depending on your Rails version)".

But in practice, when writing the form HTML manually without hidden element with the form authentication token, the form still submits successfully without CSRF error and a record is created.

This appears to be because forms are now submitted with Turbo by default, and in this case, the token generated with the csrf tag in "head" in application.html.erb is the one that is required and validated when the form is submitted.

Alternatively, when setting data-turbo="false" on the form element to disable submission via turbo, and still omitting the hidden element with the form authenticity token, the csrf error is triggered as expected.

It could be beneficial to clarify this behavior in the project task description and/ or in the rails form basics lesson.

Path

Ruby / Rails

Lesson Url

https://www.theodinproject.com/lessons/ruby-on-rails-forms

(Optional) Discord Name

jegK

(Optional) Additional Comments

No response

@jg-k jg-k changed the title Rails: update first form project description to account for default form submission with Turbo Rails: update first form project description to account for default form submission beahvior with Turbo Oct 9, 2024
@jg-k jg-k changed the title Rails: update first form project description to account for default form submission beahvior with Turbo Rails: update first form project description to account for default form submission behavior with Turbo Oct 9, 2024
@wise-king-sullyman
Copy link
Member

@TheOdinProject/rails-path thoughts?

@CouchofTomato
Copy link
Member

Yeah this was written before turbo so it will have changed. I think we need to do a Turbo and heading towards Rails 8 overhaul of the TOP content.

Copy link

This issue is stale because it has had no activity for the last 30 days.

@github-actions github-actions bot added the Status: Stale This issue/PR has been inactive for over 30 days and will be closed if inactivity continues label Nov 21, 2024
@JoshDevHub
Copy link
Contributor

@jg-k Are you still interested in working on this issue?

@JoshDevHub JoshDevHub removed the Status: Stale This issue/PR has been inactive for over 30 days and will be closed if inactivity continues label Nov 21, 2024
@jg-k
Copy link
Contributor Author

jg-k commented Nov 26, 2024

yeah sure. I d be happy to propose a local fix to this lesson if you guys are intrested.

Regarding this comment from @CouchofTomato

Yeah this was written before turbo so it will have changed. I think we need to do a Turbo and heading towards Rails 8 overhaul of the TOP content.

I haven t progress much further, so I don't have the overview, but I suspect there are many more places that may need rewriting in light of turbo + rails 8 upgrades. so not sure how meaningful it is to update this section only, and I would not be qualified for this large scale overhaul...

Just let me know if you re interested in a local fix and i ll propose something, keeping the tasks the same, but adding a warning on how to disable turbo for the sake of the exercise and get expected behavior with/without adding csrf tag in the form.

@CouchofTomato
Copy link
Member

Thanks @jg-k

I'm happy with a local fix. I've started drafting a course update but it won't be done for a little while yet.

@jg-k
Copy link
Contributor Author

jg-k commented Dec 9, 2024

@CouchofTomato @JoshDevHub
I proposed something in the linked PR. Let me know your thoughts!

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 a pull request may close this issue.

4 participants