Address reviewer feedback; simplify controller

app/controllers/registrations/experience_levels_controller.rb

 # frozen_string_literal: true
 
 module Registrations
-  class ExperienceLevelsController < DeviseController
+  class ExperienceLevelsController < ApplicationController
     # This will need to be changed to simply 'devise' as part of
     # https://gitlab.com/gitlab-org/growth/engineering/issues/64
     layout 'devise_experimental_separate_sign_up_flow'
 
-    before_action :authenticate_user!
     before_action :check_experiment_enabled
 
     def update
       current_user.experience_level = params[:experience_level]
 
       if current_user.save
-        set_flash_message! :notice, :signed_up
+        flash[:message] = I18n.t('devise.registrations.signed_up')
         redirect_to group_path(params[:namespace_path] || current_user)
       else
         render :show
@@ -25,13 +24,5 @@ module Registrations
     def check_experiment_enabled
       access_denied! unless experiment_enabled?(:onboarding_issues)
     end
-
-    # Override the default translation scope of "devise.#{controller_name}" to
-    # reuse existing translations from the RegistrationsController. Also, this
-    # way we're much more likely to catch problems early if that controller is
-    # ever renamed.
-    def translation_scope
-      "devise.#{RegistrationsController.controller_name}"
-    end
   end
 end

config/routes.rb

@@ -48,9 +48,7 @@ Rails.application.routes.draw do
   scope path: '/users/sign_up', module: :registrations, as: :users_sign_up do
     get :welcome
     patch :update_registration
-    devise_scope :user do
-      resource :experience_level, only: [:show, :update]
-    end
+    resource :experience_level, only: [:show, :update]
 
     Gitlab.ee do
       resources :groups, only: [:new, :create]

spec/controllers/registrations/experience_levels_controller_spec.rb

@@ -5,17 +5,10 @@ require 'spec_helper'
 describe Registrations::ExperienceLevelsController do
   let_it_be(:user) { create(:user) }
 
-  before do
-    @request.env['devise.mapping'] = Devise.mappings[:user]
-  end
-
   describe 'GET #show' do
     subject { get :show }
 
-    # I don't understand why these specs are failing. It's like we never
-    # actually hit the `authenticate_user!` hook, or that it doesn't really do
-    # what I expect it to do based on other tests.
-    xcontext 'with an unauthenticated user' do
+    context 'with an unauthenticated user' do
       it { is_expected.to have_gitlab_http_status(:redirect) }
       it { is_expected.to redirect_to(new_user_session_path) }
     end
@@ -46,10 +39,7 @@ describe Registrations::ExperienceLevelsController do
 
     let(:params) { {} }
 
-    # I don't understand why these specs are failing. It's like we never
-    # actually hit the `authenticate_user!` hook, or that it doesn't really do
-    # what I expect it to do based on other tests.
-    xcontext 'with an unauthenticated user' do
+    context 'with an unauthenticated user' do
       it { is_expected.to have_gitlab_http_status(:redirect) }
       it { is_expected.to redirect_to(new_user_session_path) }
     end
@@ -60,42 +50,62 @@ describe Registrations::ExperienceLevelsController do
         stub_experiment_for_user(onboarding_issues: true)
       end
 
-      context 'when no experience_level is sent' do
+      context 'when not part of the onboarding issues experiment' do
         before do
-          user.user_preference.update_attribute(:experience_level, :novice)
+          stub_experiment_for_user(onboarding_issues: false)
         end
 
-        it 'will unset the user’s experience level' do
-          expect { subject }.to change { user.reload.experience_level }.to(nil)
-        end
+        it { is_expected.to have_gitlab_http_status(:not_found) }
       end
 
-      context 'when an expected experience level is sent' do
-        let(:params) { { experience_level: :novice } }
+      context 'when user is successfully updated' do
+        it { is_expected.to set_flash[:message].to('Welcome! You have signed up successfully.') }
+
+        context 'when no experience_level is sent' do
+          before do
+            user.user_preference.update_attribute(:experience_level, :novice)
+          end
 
-        it 'sets the user’s experience level' do
-          expect { subject }.to change { user.reload.experience_level }
+          it 'will unset the user’s experience level' do
+            expect { subject }.to change { user.reload.experience_level }.to(nil)
+          end
         end
-      end
 
-      context 'when an unexpected experience level is sent' do
-        let(:params) { { experience_level: :nonexistent } }
+        context 'when an expected experience level is sent' do
+          let(:params) { { experience_level: :novice } }
 
-        it 'raises an exception' do
-          expect { subject }.to raise_error(ArgumentError, "'nonexistent' is not a valid experience_level")
+          it 'sets the user’s experience level' do
+            expect { subject }.to change { user.reload.experience_level }.to('novice')
+          end
         end
-      end
 
-      context 'when a namespace_path is sent' do
-        let(:params) { { namespace_path: namespace.to_param } }
+        context 'when an unexpected experience level is sent' do
+          let(:params) { { experience_level: :nonexistent } }
 
-        it { is_expected.to have_gitlab_http_status(:redirect) }
-        it { is_expected.to redirect_to(group_path(namespace)) }
+          it 'raises an exception' do
+            expect { subject }.to raise_error(ArgumentError, "'nonexistent' is not a valid experience_level")
+          end
+        end
+
+        context 'when a namespace_path is sent' do
+          let(:params) { { namespace_path: namespace.to_param } }
+
+          it { is_expected.to have_gitlab_http_status(:redirect) }
+          it { is_expected.to redirect_to(group_path(namespace)) }
+        end
+
+        context 'when no namespace_path is sent' do
+          it { is_expected.to have_gitlab_http_status(:redirect) }
+          it { is_expected.to redirect_to(user_path(user)) }
+        end
       end
 
-      context 'when no namespace_path is sent' do
-        it { is_expected.to have_gitlab_http_status(:redirect) }
-        it { is_expected.to redirect_to(user_path(user)) }
+      context 'when user update fails' do
+        before do
+          allow_any_instance_of(User).to receive(:save).and_return(false)
+        end
+
+        it { is_expected.to render_template(:show) }
       end
     end
   end