[RF-DOCS] Sign up guide #55254
[RF-DOCS] Sign up guide #55254
Conversation
Co-authored-by: Amanda Perino <58528404+AmandaPerino@users.noreply.github.com>
Co-authored-by: Amanda Perino <58528404+AmandaPerino@users.noreply.github.com>
Co-authored-by: Amanda Perino <58528404+AmandaPerino@users.noreply.github.com>
|
@excid3 how does one get to this guide? Is it linked from the Getting Started one? |
|
@MatheusRich @excid3 It needs to be added to: |
|
There are design changes coming so it will be done in a separate PR. |
|
@MatheusRich @p8 Just like the /docs landing page we launched last year, we are building a Tutorial landing page where all of the tutorials, and all upcoming tutorials will live. (PR here.) |
|
@AmandaPerino amazing! For anyone reviewing and wants to preview the results:
|
|
|
||
| ```ruby | ||
| namespace :settings do | ||
| resource :password, only: [:show, :update] |
There was a problem hiding this comment.
Why show and not edit?
There was a problem hiding this comment.
I think it was done to create a nice "/settings/password" path.
Like you, I was confused at first, but I eventually came to like the idea because it tells readers implicitly that forms do not always have to be on new or edit routes, and that they are not constricted by the framework.
| Let's add that `update` action to the controller now. | ||
|
|
||
| ```ruby#5-17 | ||
| class Settings::PasswordsController < Settings::BaseController |
There was a problem hiding this comment.
This is using Settings::BaseController before we introduced it
|
|
||
| ### Users Controller & Views | ||
|
|
||
| We can then use this in the controller. Create `app/controllers/settings/users_controller.rb` and add the following: |
There was a problem hiding this comment.
Not a biggie, but I found it a bit confusing to mix the other settings (which seems mostly personal) with admin stuff. I'd suggest an admin namespace, but that would generate a lot of rework.
There was a problem hiding this comment.
Great tutorial, as always @excid3! So easy to follow and yet very comprehensive! I love how clean the code is and all the little details you put here and there. It shows your deep understanding of the Rails-way. It teaches so much more than the title says. Well done!
@AmandaPerino That looks great! |
| ```erb | ||
| <h1>Password</h1> | ||
| <%= form_with model: Current.user, url: settings_password_path do |form| %> |
There was a problem hiding this comment.
Feedback from reddit: https://old.reddit.com/r/rails/comments/1lmo4ud/learning_to_learn_rails_the_rails_secret_handshake/
I kind of agree that Current.user needs more explanation. I think its setup by the authentication generator in the Getting Started guide, but neither Getting Started nor this guide really explain what it is/how it works.
| Adding Sign Up | ||
| -------------- | ||
|
|
||
| We've already used the Rails authentication generator to allow users to login to their accounts. The generator created a `User` model with `email_address:string` and `password_digest:string` columns in the database. It also added `has_secure_password` to the `User` model which handles passwords and confirmations. This takes care of most of what we need to add sign up to our application. |
There was a problem hiding this comment.
What do you think about a link to /getting_started#adding-authentication here so that someone can quickly jump back to that section for context?
| <%= link_to "Back to all users", settings_users_path %> | ||
| <h1><%= @user.full_name %></h1> | ||
| <%= tag.p @user.email_address %> |
There was a problem hiding this comment.
I think this is the only usage of tag.<tag> in this guide, it may be worth replacing with <p> just to not introduce another concept?
There was a problem hiding this comment.
I like tag.p, it's a nice way to introduce that concept IMO
There was a problem hiding this comment.
It feels unnecessary complexity at this point. The plain erb html would serve this well. I wonder if this preparing for something in a future tutorial.
But the guides are all in rails/rails, right? As long as we merge both at the same time, it should be okay to keep them in their respective repos, unless I'm overlooking something. Or did you mean moving all tutorials to /website? |
| normalizes :email_address, with: ->(e) { e.strip.downcase } | ||
| validates :first_name, :last_name, presence: true |
There was a problem hiding this comment.
Maybe it would worth mention here that has_secure_password only validates the presence of the password, but not the minimum length or complexity.
Yes, I think we should move these to It's easier to keep the website and tutorials in sync if they are in the same repo. If a lot of tutorials get added to the guides (without making them part of the guides), the guides get more difficult to maintain. There will just be more text to maintain and fixes would need to get backported. I would also expect navigation for tutorials to be slightly different than for guides? |
|
|
||
| ```ruby | ||
| namespace :settings do | ||
| resource :password, only: [:show, :update] |
There was a problem hiding this comment.
How about using edit instead of show? Actually, we will be editing the password, not showing it.
|
|
||
| ```ruby#2 | ||
| namespace :settings do | ||
| resource :account, only: [ :show, :destroy ] |
There was a problem hiding this comment.
How about calling this user? Usually, in Rails an account groups many users, and even so, the controller doesn't do account.destroy, it does user.destroy.
| before_action -> { redirect_to root_path if authenticated? }, **options | ||
| end | ||
| def admin_access_only(**options) |
There was a problem hiding this comment.
This seems a bit out of place to me. This is an action more typical of an authorization concept, not so much authentication. Maybe move it somewhere else? Maybe an Authorization concern that has a require_admin as a class method.
That makes sense to me for the new tutorials, but a little less clear how that would work with the Getting Started guide. It feels like the GS guide should stay in the guides.ror.org subdomain. But I'm open to differing thoughts here- is there any valid reason stopping it from also moving to /tutorials? If it stays on the subdomain, the navigation from that tutorial to the others feels less clean. |
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com> Co-authored-by: Matheus Richard <matheusrichardt@gmail.com>
I can't imagine someone reading the "Getting Started" guide from Rails version 6.0. The use case for the "Getting Started" guide is a bit different than other guides. Most would probably read it once to build an app and never look at it again, whereas the other guides are used to look up things more often. The guides for older Rails version are useful when you are working on an application with that same version. So maybe it's a good idea to move that guide to /website as well? There are some existing guides which are more tutorial like. For example the plugins guide: https://guides.rubyonrails.org/plugins.html. I'm not sure those should be moved as well. |
Co-authored-by: Kyoungwon Lee (Stark) <kyoungwon.lee86@gmail.com>
There was a problem hiding this comment.
This is a really great guide! Despite using Rails for years, I learned a few interesting things.
What do you think about inserting tests throughout the guide, after introducing new requirements and before showing code blocks? This could help foster a TDD/test-first culture among readers.
| end | ||
| ``` | ||
|
|
||
| This test is should be invalid because the user's name is missing. Since this request is invalid, we need to assert the response is a 422 Unprocessable Entity. We can also assert that there is no difference in the `User.count` to ensure no User was created. |
There was a problem hiding this comment.
| This test is should be invalid because the user's name is missing. Since this request is invalid, we need to assert the response is a 422 Unprocessable Entity. We can also assert that there is no difference in the `User.count` to ensure no User was created. | |
| This test should be invalid because the user's name is missing. Since this request is invalid, we need to assert the response is a 422 Unprocessable Entity. We can also assert that there is no difference in the `User.count` to ensure no User was created. |
@MatheusRich I am trying to do just that, |
|
@MichaelGame-Dev the page has changed a little since i've last saw this, but you can find the docs link in the Annotations section: 
Click on Guides and add |
Motivation / Background
This is a continuation of the Getting Started guide.
Detail
After building the e-commerce store, this guide adds:
Additional information
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]cc @AmandaPerino