Refactor OnboardingsController (#17329)

* Refactor OnboardingsController

* Fix broken spec

* Update policy

* Fix spec

* Update config/routes.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Touchup

* Update routes

* Screw it, let's move notification_settings too

* Revert "Screw it, let's move notification_settings too"

This reverts commit aead8c05f4dda62cbc46cdd033afd0acdef2ad73.

* Temp .travis.yml changes

* Revert "Temp .travis.yml changes"

This reverts commit c26109843ba027f9a524e66282a9b01f0341f836.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

app/controllers/onboardings_controller.rb

@@ -1,8 +0,0 @@
-class OnboardingsController < ApplicationController
-  before_action :authenticate_user!
-  before_action :set_cache_control_headers, only: %i[show]
-
-  def show
-    set_surrogate_key_header "onboarding-slideshow"
-  end
-end

app/controllers/users/onboardings_controller.rb

@@ -1,14 +1,19 @@
 module Users
   class OnboardingsController < ApplicationController
-    before_action :set_no_cache_header
     before_action :authenticate_user!
-    after_action :verify_authorized
+    before_action :set_cache_control_headers, only: [:show]
+    before_action :set_no_cache_header, only: %i[update onboarding_checkbox_update]
+    after_action :verify_authorized, except: [:show]
 
     ALLOWED_USER_PARAMS = %i[last_onboarding_page username].freeze
     ALLOWED_CHECKBOX_PARAMS = %i[checked_code_of_conduct checked_terms_and_conditions].freeze
 
-    def onboarding_update
-      authorize User
+    def show
+      set_surrogate_key_header "onboarding-slideshow"
+    end
+
+    def update
+      authorize User, :onboarding_update?
 
       user_params = {}
 

app/javascript/onboarding/components/FollowTags.jsx

@@ -25,7 +25,7 @@ export class FollowTags extends Component {
       });
 
     const csrfToken = getContentOfToken('csrf-token');
-    fetch('/onboarding_update', {
+    fetch('/onboarding', {
       method: 'PATCH',
       headers: {
         'X-CSRF-Token': csrfToken,

app/javascript/onboarding/components/FollowUsers.jsx

@@ -31,7 +31,7 @@ export class FollowUsers extends Component {
       });
 
     const csrfToken = getContentOfToken('csrf-token');
-    fetch('/onboarding_update', {
+    fetch('/onboarding', {
       method: 'PATCH',
       headers: {
         'X-CSRF-Token': csrfToken,

app/javascript/onboarding/components/ProfileForm.jsx

@@ -50,7 +50,7 @@ export class ProfileForm extends Component {
     const { formValues, last_onboarding_page } = this.state;
     const { username, ...newFormValues } = formValues;
     try {
-      const response = await request('/onboarding_update', {
+      const response = await request('/onboarding', {
         method: 'PATCH',
         body: {
           user: { last_onboarding_page, username },
@@ -66,7 +66,7 @@ export class ProfileForm extends Component {
       Honeybadger.notify(error.statusText);
       let errorMessage = 'Unable to continue, please try again.';
       if (error.status === 422) {
-        // parse validation error messages from UsersController#onboarding_update
+        // parse validation error messages from UsersController#onboarding
         const errorData = await error.json();
         errorMessage = errorData.errors;
         this.setState({ error: true, errorMessage });

app/javascript/onboarding/utilities.js

@@ -9,7 +9,7 @@ export const getContentOfToken = (token) =>
 
 export const updateOnboarding = (lastPage) => {
   const csrfToken = getContentOfToken('csrf-token');
-  fetch('/onboarding_update', {
+  fetch('/onboarding', {
     method: 'PATCH',
     headers: {
       'X-CSRF-Token': csrfToken,

app/views/articles/index.html.erb

@@ -36,7 +36,7 @@
 
     <%# BEGIN Feed menu bar %>
     <main class="articles-list crayons-layout__content" id="main-content" data-follow-button-container="true">
-      <%= render(partial: "onboardings/task_card") if user_signed_in? %>
+      <%= render(partial: "users/onboardings/task_card") if user_signed_in? %>
 
       <header class="p-2 px-3 m:p-0 m:px-0 m:mb-2 fs-l">
         <h1 class="screen-reader-only"><%= t("views.stories.heading") %></h1>

config/routes.rb

@@ -194,7 +194,11 @@ Rails.application.routes.draw do
       get :podcasts
     end
 
-    resource :onboarding, only: :show
+    scope module: "users" do
+      resource :onboarding, only: %i[show update]
+      patch "/onboarding_checkbox_update", to: "onboardings#onboarding_checkbox_update"
+    end
+
     resources :profiles, only: %i[update]
     resources :profile_field_groups, only: %i[index], defaults: { format: :json }
 
@@ -211,8 +215,6 @@ Rails.application.routes.draw do
     get "/notifications/:filter/:org_id", to: "notifications#index", as: :notifications_filter_org
     get "/notification_subscriptions/:notifiable_type/:notifiable_id", to: "notification_subscriptions#show"
     post "/notification_subscriptions/:notifiable_type/:notifiable_id", to: "notification_subscriptions#upsert"
-    patch "/onboarding_update", to: "users/onboardings#onboarding_update"
-    patch "/onboarding_checkbox_update", to: "users/onboardings#onboarding_checkbox_update"
     patch "/onboarding_notifications_checkbox_update",
           to: "users/notification_settings#onboarding_notifications_checkbox_update"
     get "email_subscriptions/unsubscribe"

spec/requests/users_onboarding_spec.rb

@@ -8,48 +8,48 @@ RSpec.describe "UsersOnboarding", type: :request do
            profile: create(:profile, location: "Llama Town"))
   end
 
-  describe "PATCH /onboarding_update" do
+  describe "PATCH /onboarding" do
     context "when signed in" do
       before { sign_in user }
 
       it "updates the user's last_onboarding_page attribute" do
         params = { user: { last_onboarding_page: "v2: personal info form", username: "test" } }
         expect do
-          patch "/onboarding_update.json", params: params
+          patch "/onboarding", params: params
         end.to change(user, :last_onboarding_page)
       end
 
       it "updates the user's username attribute" do
         params = { user: { username: "WilhuffTarkin" } }
         expect do
-          patch "/onboarding_update.json", params: params
+          patch "/onboarding", params: params
         end.to change(user, :username).to("wilhufftarkin")
       end
 
       it "returns a 422 error if the username is blank" do
         params = { user: { username: "" } }
-        patch "/onboarding_update.json", params: params
+        patch "/onboarding", params: params
         expect(response).to have_http_status(:unprocessable_entity)
       end
 
       it "updates the user's profile" do
         params = { profile: { location: "Galactic Empire" } }
         expect do
-          patch "/onboarding_update.json", params: params
+          patch "/onboarding", params: params
         end.to change(user.profile, :location).to("Galactic Empire")
       end
 
       it "does not update the user's last_onboarding_page if it is empty" do
         params = { user: { last_onboarding_page: "" } }
         expect do
-          patch "/onboarding_update.json", params: params
+          patch "/onboarding", params: params
         end.not_to change(user, :last_onboarding_page)
       end
     end
 
     context "when signed out" do
       it "returns a not found error if user is not signed in" do
-        patch "/onboarding_update.json", params: {}
+        patch "/onboarding.json", params: {}
         expect(response.parsed_body["error"]).to include("Please sign in")
       end
     end