From 4fd9fa6610a441a013803988dce345b15592a27c Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 14 Jan 2019 16:25:48 +0100 Subject: [PATCH] sign_in: extract the procedure context to a ProcedureContextConcern --- .../concerns/procedure_context_concern.rb | 34 ++++++++ .../devise/store_location_extension.rb | 9 +++ app/controllers/users/sessions_controller.rb | 25 +----- app/services/number_service.rb | 5 -- .../procedure_context_concern_spec.rb | 77 +++++++++++++++++++ .../devise/store_location_extension_spec.rb | 15 ++++ .../users/sessions_controller_spec.rb | 36 ++------- 7 files changed, 147 insertions(+), 54 deletions(-) create mode 100644 app/controllers/concerns/procedure_context_concern.rb delete mode 100644 app/services/number_service.rb create mode 100644 spec/controllers/concerns/procedure_context_concern_spec.rb diff --git a/app/controllers/concerns/procedure_context_concern.rb b/app/controllers/concerns/procedure_context_concern.rb new file mode 100644 index 000000000..307dba95e --- /dev/null +++ b/app/controllers/concerns/procedure_context_concern.rb @@ -0,0 +1,34 @@ +module ProcedureContextConcern + extend ActiveSupport::Concern + + include Devise::Controllers::StoreLocation + include Devise::StoreLocationExtension + + def restore_procedure_context + if stored_procedure_id.present? + @procedure = Procedure.publiees.find_by(id: stored_procedure_id) + + if @procedure.blank? + invalid_procedure_context + end + end + end + + private + + def stored_procedure_id + stored_location = get_stored_location_for(:user) + + if stored_location.present? && stored_location.include?('procedure_id=') + stored_location.split('procedure_id=').second + else + nil + end + end + + def invalid_procedure_context + clear_stored_location_for(:user) + flash.alert = t('errors.messages.procedure_not_found') + redirect_to root_path + end +end diff --git a/app/controllers/devise/store_location_extension.rb b/app/controllers/devise/store_location_extension.rb index 00c8791eb..c6496bb03 100644 --- a/app/controllers/devise/store_location_extension.rb +++ b/app/controllers/devise/store_location_extension.rb @@ -1,6 +1,15 @@ module Devise # Useful helpers additions to Devise::Controllers::StoreLocation module StoreLocationExtension + # A variant of `stored_location_key_for` which doesn't delete the stored path. + def get_stored_location_for(resource_or_scope) + location = stored_location_for(resource_or_scope) + if location + store_location_for(resource_or_scope, location) + end + location + end + # Delete the url stored in the session for the given scope. def clear_stored_location_for(resource_or_scope) session_key = send(:stored_location_key_for, resource_or_scope) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index d63881c96..ee94aed4a 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -1,18 +1,15 @@ class Users::SessionsController < Sessions::SessionsController + include ProcedureContextConcern include TrustedDeviceConcern include ActionView::Helpers::DateHelper layout 'procedure_context', only: [:new, :create] + before_action :restore_procedure_context, only: [:new, :create] + # GET /resource/sign_in def new - if user_return_to_procedure_id.present? - @procedure = Procedure.active(user_return_to_procedure_id) - end - @user = User.new - rescue ActiveRecord::RecordNotFound - error_procedure end # POST /resource/sign_in @@ -80,7 +77,7 @@ class Users::SessionsController < Sessions::SessionsController end def no_procedure - session['user_return_to'] = nil + clear_stored_location_for(:user) redirect_to new_user_session_path end @@ -112,20 +109,6 @@ class Users::SessionsController < Sessions::SessionsController end end - def error_procedure - session["user_return_to"] = nil - flash.alert = t('errors.messages.procedure_not_found') - redirect_to url_for root_path - end - - def user_return_to_procedure_id - if session["user_return_to"].nil? - return nil - end - - NumberService.to_number session["user_return_to"].split("?procedure_id=").second - end - def try_to_authenticate(klass, remember_me = false) resource = klass.find_for_database_authentication(email: params[:user][:email]) diff --git a/app/services/number_service.rb b/app/services/number_service.rb deleted file mode 100644 index 981f47d9c..000000000 --- a/app/services/number_service.rb +++ /dev/null @@ -1,5 +0,0 @@ -class NumberService - def self.to_number(string) - string.to_s - end -end diff --git a/spec/controllers/concerns/procedure_context_concern_spec.rb b/spec/controllers/concerns/procedure_context_concern_spec.rb new file mode 100644 index 000000000..42e616305 --- /dev/null +++ b/spec/controllers/concerns/procedure_context_concern_spec.rb @@ -0,0 +1,77 @@ +require 'rails_helper' + +RSpec.describe ProcedureContextConcern, type: :controller do + class TestController < ActionController::Base + include ProcedureContextConcern + + before_action :restore_procedure_context + + def index + head :ok + end + end + + controller TestController do + end + + describe '#restore_procedure_context' do + subject { get :index } + + context 'when no return location has been stored' do + it 'succeeds, without defining a procedure on the controller' do + expect(subject.status).to eq 200 + expect(assigns(:procedure)).to be nil + end + end + + context 'when no procedure_id is present in the stored return location' do + before do + controller.store_location_for(:user, dossiers_path) + end + + it 'succeeds, without assigns a procedure on the controller' do + expect(subject.status).to eq 200 + expect(assigns(:procedure)).to be nil + end + end + + context 'when a procedure location has been stored' do + context 'when the stored procedure does not exist' do + before do + controller.store_location_for(:user, new_dossier_path(procedure_id: '0')) + end + + it 'redirects with an error' do + expect(subject.status).to eq 302 + expect(subject).to redirect_to root_path + end + end + + context 'when the stored procedure is not published' do + let(:procedure) { create :procedure } + + before do + controller.store_location_for(:user, new_dossier_path(procedure_id: procedure.id)) + end + + it 'redirects with an error' do + expect(subject.status).to eq 302 + expect(subject).to redirect_to root_path + end + end + + context 'when the stored procedure exists' do + let(:procedure) { create :procedure, :published } + + before do + controller.store_location_for(:user, new_dossier_path(procedure_id: procedure.id)) + end + + it 'succeeds, and assigns the procedure on the controller' do + expect(subject.status).to eq 200 + expect(assigns(:procedure)).to eq procedure + end + end + end + end +end diff --git a/spec/controllers/devise/store_location_extension_spec.rb b/spec/controllers/devise/store_location_extension_spec.rb index 89a42c6f5..f43a041b4 100644 --- a/spec/controllers/devise/store_location_extension_spec.rb +++ b/spec/controllers/devise/store_location_extension_spec.rb @@ -9,6 +9,21 @@ RSpec.describe Devise::StoreLocationExtension, type: :controller do controller TestController do end + describe '#get_stored_location_for' do + context 'when a location has been previously stored' do + before { subject.store_location_for(:user, dossiers_path) } + + it 'returns the stored location without clearing it' do + expect(subject.get_stored_location_for(:user)).to eq dossiers_path + expect(subject.stored_location_for(:user)).to eq dossiers_path + end + end + + context 'when no location has been stored' do + it { expect(subject.get_stored_location_for(:user)).to be nil } + end + end + describe "#clear_stored_location_for" do context 'when a location has been previously stored' do before { subject.store_location_for(:user, dossiers_path) } diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index a5ef7a201..e12521849 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -173,38 +173,18 @@ describe Users::SessionsController, type: :controller do describe '#new' do subject { get :new } - context 'when procedure_id is not present in user_return_to session params' do - it { expect(subject.status).to eq 200 } - end + it { expect(subject.status).to eq 200 } - context 'when procedure_id is present in user_return_to session params' do - context 'when procedure_id does not exist' do - before do - session["user_return_to"] = '?procedure_id=0' - end + context 'when a procedure location has been stored' do + let(:procedure) { create :procedure, :published } - it { expect(subject.status).to eq 302 } - it { expect(subject).to redirect_to root_path } + before do + controller.store_location_for(:user, new_dossier_path(procedure_id: procedure.id)) end - context 'when procedure is not published' do - let(:procedure) { create :procedure } - before do - session["user_return_to"] = "?procedure_id=#{procedure.id}" - end - - it { expect(subject.status).to eq 302 } - it { expect(subject).to redirect_to root_path } - end - - context 'when procedure_id exist' do - let(:procedure) { create :procedure, :published } - - before do - session["user_return_to"] = "?procedure_id=#{procedure.id}" - end - - it { expect(subject.status).to eq 200 } + it 'makes the saved procedure available' do + expect(subject.status).to eq 200 + expect(assigns(:procedure)).to eq procedure end end end