diff --git a/Gemfile b/Gemfile index 214c8c5b0..e03fc22c8 100644 --- a/Gemfile +++ b/Gemfile @@ -100,6 +100,7 @@ group :test do gem 'capybara-screenshot' # Save a dump of the page when an integration test fails gem 'factory_bot' gem 'launchy' + gem 'rack_session_access' gem 'rails-controller-testing' gem 'rspec_junit_formatter' gem 'selenium-devtools' diff --git a/Gemfile.lock b/Gemfile.lock index 591a7f8dd..7a794f3bf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -529,6 +529,9 @@ GEM rack rack-test (2.0.2) rack (>= 1.3) + rack_session_access (0.2.0) + builder (>= 2.0.0) + rack (>= 1.0.0) rails (6.1.7.1) actioncable (= 6.1.7.1) actionmailbox (= 6.1.7.1) @@ -905,6 +908,7 @@ DEPENDENCIES pundit rack-attack rack-mini-profiler + rack_session_access rails rails-controller-testing rails-erd diff --git a/app/controllers/concerns/query_params_store_concern.rb b/app/controllers/concerns/query_params_store_concern.rb deleted file mode 100644 index f8584e7f4..000000000 --- a/app/controllers/concerns/query_params_store_concern.rb +++ /dev/null @@ -1,31 +0,0 @@ -module QueryParamsStoreConcern - extend ActiveSupport::Concern - - included do - helper_method :stored_query_params? - end - - def store_query_params - # Don't override already stored params, because we could do goings and comings with authentication, and - # lost previously stored params - return if stored_query_params? || filtered_query_params.empty? - - session[:stored_params] = filtered_query_params.to_json - end - - def retrieve_and_delete_stored_query_params - return {} unless stored_query_params? - - JSON.parse(session.delete(:stored_params)) - end - - def stored_query_params? - session[:stored_params].present? - end - - private - - def filtered_query_params - request.query_parameters.except(:locale, "locale") - end -end diff --git a/app/controllers/users/commencer_controller.rb b/app/controllers/users/commencer_controller.rb index f8fa38aed..63e7dacdd 100644 --- a/app/controllers/users/commencer_controller.rb +++ b/app/controllers/users/commencer_controller.rb @@ -1,20 +1,23 @@ module Users class CommencerController < ApplicationController - include QueryParamsStoreConcern - layout 'procedure_context' - before_action :retrieve_prefilled_dossier, if: -> { params[:prefill_token].present? }, only: :commencer - before_action :set_prefilled_dossier_ownership, if: -> { user_signed_in? && @prefilled_dossier&.orphan? }, only: :commencer - before_action :check_prefilled_dossier_ownership, if: -> { user_signed_in? && @prefilled_dossier }, only: :commencer - def commencer @procedure = retrieve_procedure return procedure_not_found if @procedure.blank? || @procedure.brouillon? - - store_query_params - @revision = @procedure.published_revision + + if params[:prefill_token].present? || commencer_page_is_reloaded? + retrieve_prefilled_dossier(params[:prefill_token] || session[:prefill_token]) + elsif prefill_params_present? + build_prefilled_dossier + end + + if user_signed_in? + set_prefilled_dossier_ownership if @prefilled_dossier&.orphan? + check_prefilled_dossier_ownership if @prefilled_dossier + end + render 'commencer/show' end @@ -70,6 +73,14 @@ module Users private + def commencer_page_is_reloaded? + session[:prefill_token].present? && session[:prefill_params] == params.to_unsafe_h + end + + def prefill_params_present? + params.keys.find { |param| param.split('_').first == "champ" } + end + def retrieve_procedure Procedure.publiees.or(Procedure.brouillons).find_by(path: params[:path]) end @@ -78,8 +89,23 @@ module Users Procedure.publiees.or(Procedure.brouillons).or(Procedure.closes).order(published_at: :desc).find_by(path: params[:path]) end - def retrieve_prefilled_dossier - @prefilled_dossier = Dossier.state_brouillon.prefilled.find_by!(prefill_token: params[:prefill_token]) + def build_prefilled_dossier + @prefilled_dossier = Dossier.new( + revision: @revision, + groupe_instructeur: @procedure.defaut_groupe_instructeur_for_new_dossier, + state: Dossier.states.fetch(:brouillon), + prefilled: true + ) + @prefilled_dossier.build_default_individual + if @prefilled_dossier.save + @prefilled_dossier.prefill!(PrefillParams.new(@prefilled_dossier, params.to_unsafe_h).to_a) + end + session[:prefill_token] = @prefilled_dossier.prefill_token + session[:prefill_params] = params.to_unsafe_h + end + + def retrieve_prefilled_dossier(prefill_token) + @prefilled_dossier = Dossier.state_brouillon.prefilled.find_by!(prefill_token: prefill_token) end # The prefilled dossier is not owned yet, and the user is signed in: they become the new owner diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 4e2791be8..5ab9ec3bc 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -1,7 +1,6 @@ module Users class DossiersController < UserController include DossierHelper - include QueryParamsStoreConcern layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret] @@ -144,6 +143,8 @@ module Users end def brouillon + session.delete(:prefill_token) + session.delete(:prefill_params) @dossier = dossier_with_champs @dossier.valid?(context: :prefilling) @@ -295,7 +296,6 @@ module Users ) dossier.build_default_individual dossier.save! - dossier.prefill!(PrefillParams.new(dossier, retrieve_and_delete_stored_query_params).to_a) DossierMailer.with(dossier:).notify_new_draft.deliver_later if dossier.procedure.for_individual diff --git a/app/views/commencer/show.html.haml b/app/views/commencer/show.html.haml index d15e5cf6d..ae3487dd5 100644 --- a/app/views/commencer/show.html.haml +++ b/app/views/commencer/show.html.haml @@ -21,11 +21,6 @@ %p= t('views.commencer.show.prefilled_draft_detail_html', time_ago: time_ago_in_words(@prefilled_dossier.created_at), procedure: @procedure.libelle) = link_to t('views.commencer.show.go_to_prefilled_file'), brouillon_dossier_path(@prefilled_dossier), class: 'fr-btn fr-btn--lg fr-my-2w' - - elsif stored_query_params? - %h2.huge-title= t('views.commencer.show.prefilled_draft') - %p= t('views.commencer.show.prefill_dossier_detail_html', procedure: @procedure.libelle) - = link_to t('views.commencer.show.go_to_prefilled_file'), url_for_new_dossier(@revision), class: 'fr-btn fr-btn--lg fr-my-2w' - - elsif dossiers.empty? = link_to t('views.commencer.show.start_procedure'), url_for_new_dossier(@revision), class: 'fr-btn fr-btn--lg fr-my-2w' diff --git a/config/environments/test.rb b/config/environments/test.rb index ccaba8feb..4a01914e6 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -87,4 +87,6 @@ Rails.application.configure do silence_warnings do BCrypt::Engine::DEFAULT_COST = BCrypt::Engine::MIN_COST end + + config.middleware.use RackSessionAccess::Middleware end diff --git a/config/locales/en.yml b/config/locales/en.yml index 0e0e4bd1d..a9b50e61c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -107,7 +107,6 @@ en: show_dossiers: View my current files prefilled_draft: "You have a prefilled file" prefilled_draft_detail_html: "You are ready to continue a prefilled file for the \"%{procedure}\" procedure, started <strong>%{time_ago} ago</strong>." - prefill_dossier_detail_html: "You are ready to continue a prefilled file for the \"%{procedure}\" procedure." already_draft: "You already started to fill a file" already_draft_detail_html: "You started to fill a file for the \"%{procedure}\" procedure <strong>%{time_ago} ago</strong>" already_not_draft: "You already submitted a file" diff --git a/config/locales/fr.yml b/config/locales/fr.yml index bfd07b394..37a910f79 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -98,7 +98,6 @@ fr: show_dossiers: Voir mes dossiers en cours prefilled_draft: "Vous avez un dossier prérempli" prefilled_draft_detail_html: "Vous êtes prêt·e à poursuivre un dossier prérempli sur la démarche « %{procedure} », commencé il y a <strong>%{time_ago}</strong>." - prefill_dossier_detail_html: "Vous êtes prêt·e à poursuivre un dossier prérempli sur la démarche « %{procedure} »." already_draft: "Vous avez déjà commencé à remplir un dossier" already_draft_detail_html: "Il y a <strong>%{time_ago}</strong>, vous avez commencé à remplir un dossier sur la démarche « %{procedure} »." already_not_draft: "Vous avez déjà déposé un dossier" diff --git a/spec/controllers/concerns/query_params_store_concern_spec.rb b/spec/controllers/concerns/query_params_store_concern_spec.rb deleted file mode 100644 index 42b29177f..000000000 --- a/spec/controllers/concerns/query_params_store_concern_spec.rb +++ /dev/null @@ -1,91 +0,0 @@ -RSpec.describe QueryParamsStoreConcern, type: :controller do - class TestController < ActionController::Base - include QueryParamsStoreConcern - end - - controller TestController do - end - - before { allow_any_instance_of(ActionDispatch::Request).to receive(:query_parameters).and_return(params) } - - describe '#store_query_params' do - subject(:store_query_params) { controller.store_query_params } - - context 'when params are already stored' do - let(:params) { { param1: "param1" } } - - it "does nothing" do - session[:stored_params] = "there is already something in there" - - expect { store_query_params }.not_to change { session[:stored_params] } - end - end - - context 'when params are empty' do - let(:params) { {} } - - it "does nothing" do - expect { store_query_params }.not_to change { session[:stored_params] } - end - end - - context 'when the store is empty and we have params' do - let(:params) { { param1: "param1", param2: "param2" } } - - it "stores the query params" do - expect { store_query_params }.to change { session[:stored_params] }.from(nil).to(params.to_json) - end - end - - context 'when params contain a locale' do - let(:params) { { locale: "fr", param2: "param2" } } - - it "does not store the locale" do - expect { store_query_params }.to change { session[:stored_params] }.from(nil).to({ param2: "param2" }.to_json) - end - end - end - - describe '#retrieve_and_delete_stored_query_params' do - subject(:retrieve_and_delete_stored_query_params) { controller.retrieve_and_delete_stored_query_params } - - context 'when there are no stored params' do - let(:params) { {} } - - it 'returns an empty hash' do - expect(retrieve_and_delete_stored_query_params).to be_empty - end - end - - context 'when params are stored' do - let(:params) { { param1: "param1", param2: "param2" } } - - before { controller.store_query_params } - - it 'deletes the stored params' do - expect { retrieve_and_delete_stored_query_params }.to change { session[:stored_params] }.to(nil) - end - - it 'returns the stored params' do - expect(retrieve_and_delete_stored_query_params).to match(params.with_indifferent_access) - end - end - end - - describe '#stored_query_params?' do - subject(:stored_query_params?) { controller.stored_query_params? } - - before { controller.store_query_params } - context 'when query params have been stored' do - let(:params) { { param1: "param1", param2: "param2" } } - - it { is_expected.to eq(true) } - end - - context 'when query params have not been stored' do - let(:params) { {} } - - it { is_expected.to eq(false) } - end - end -end diff --git a/spec/controllers/users/commencer_controller_spec.rb b/spec/controllers/users/commencer_controller_spec.rb index f4370c526..8ca93cac3 100644 --- a/spec/controllers/users/commencer_controller_spec.rb +++ b/spec/controllers/users/commencer_controller_spec.rb @@ -15,16 +15,6 @@ describe Users::CommencerController, type: :controller do expect(assigns(:procedure)).to eq published_procedure expect(assigns(:revision)).to eq published_procedure.published_revision end - - context 'when there are query params' do - subject { get :commencer, params: { path: path, any_param: "any param" } } - - it "stores the parameters in session" do - subject - - expect(session[:stored_params]).to be_present - end - end end context 'when the path is for a draft procedure' do @@ -73,7 +63,7 @@ describe Users::CommencerController, type: :controller do end end - context 'when a dossier has been prefilled' do + context 'when a dossier has been prefilled by POST before' do let(:dossier) { create(:dossier, :brouillon, :prefilled, user: user) } let(:path) { dossier.procedure.path } @@ -151,6 +141,85 @@ describe Users::CommencerController, type: :controller do end end end + + context 'when a dossier is being prefilled by GET' do + let(:type_de_champ_text) { create(:type_de_champ_text, procedure: published_procedure) } + let(:path) { published_procedure.path } + let(:user) { create(:user) } + + context "when the dossier does not exists yet" do + subject { get :commencer, params: { path: path, "champ_#{type_de_champ_text.to_typed_id}" => "blabla" } } + + shared_examples 'a prefilled brouillon dossier creator' do + it 'creates a dossier' do + subject + expect(Dossier.count).to eq(1) + expect(session[:prefill_token]).to eq(Dossier.last.prefill_token) + expect(session[:prefill_params]).to eq({ "action" => "commencer", "champ_#{type_de_champ_text.to_typed_id}" => "blabla", "controller" => "users/commencer", "path" => path.to_s }) + expect(Dossier.last.champs.where(type_de_champ: type_de_champ_text).first.value).to eq("blabla") + end + end + + context 'when the user is unauthenticated' do + it_behaves_like 'a prefilled brouillon dossier creator' + end + + context 'when the user is authenticated' do + before { sign_in user } + + it_behaves_like 'a prefilled brouillon dossier creator' + + it { expect { subject }.to change { Dossier.last&.user }.from(nil).to(user) } + + it 'sends the notify_new_draft email' do + expect { perform_enqueued_jobs { subject } }.to change { ActionMailer::Base.deliveries.count }.by(1) + + dossier = Dossier.last + mail = ActionMailer::Base.deliveries.last + expect(mail.subject).to eq("Retrouvez votre brouillon pour la démarche « #{dossier.procedure.libelle} »") + expect(mail.html_part.body).to include(dossier_path(dossier)) + end + end + end + context "when prefilled params are passed" do + subject { get :commencer, params: { path: path, prefill_token: "token", "champ_#{type_de_champ_text.to_typed_id}" => "blabla" } } + + context "when the associated dossier exists" do + let!(:dossier) { create(:dossier, :prefilled, prefill_token: "token") } + let!(:champ_text) { create(:champ_text, dossier: dossier, type_de_champ: type_de_champ_text) } + + it "does not create a new dossier" do + subject + expect(Dossier.count).to eq(1) + expect(assigns(:prefilled_dossier)).to eq(dossier) + end + end + context "when the associated dossier does not exists" do + it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) } + end + end + context "when session params exists" do + subject { get :commencer, params: { path: path, "champ_#{type_de_champ_text.to_typed_id}" => "blabla" } } + + before do + session[:prefill_token] = "token" + session[:prefill_params] = { "action" => "commencer", "champ_#{type_de_champ_text.to_typed_id}" => "blabla", "controller" => "users/commencer", "path" => path.to_s } + end + + context "when the associated dossier exists" do + let!(:dossier) { create(:dossier, :prefilled, prefill_token: "token") } + + it "does not create a new dossier" do + subject + expect(Dossier.count).to eq(1) + expect(assigns(:prefilled_dossier)).to eq(dossier) + end + end + context "when the associated dossier does not exists" do + it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) } + end + end + end end describe '#commencer_test' do diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 5e7c57d7e..d428d6e9f 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -1068,60 +1068,6 @@ describe Users::DossiersController, type: :controller do it { is_expected.to redirect_to dossiers_path } end - - context 'when prefill values have been stored in session before' do - let!(:type_de_champ_1) { create(:type_de_champ_text, procedure: procedure) } - let(:value_1) { "any value" } - - let!(:type_de_champ_2) { create(:type_de_champ_textarea, procedure: procedure) } - let(:value_2) { "another value" } - - let(:params) { - { - procedure_id: procedure_id, - "champ_#{type_de_champ_1.to_typed_id_for_query}" => value_1, - "champ_#{type_de_champ_2.to_typed_id_for_query}" => value_2 - } - } - - before { session[:stored_params] = params.to_json } - - it { expect { subject }.to change { session[:stored_params] }.to(nil) } - - it { expect { subject }.to change { Dossier.count }.by(1) } - - it "prefills the dossier's champs with the given values" do - subject - - dossier = Dossier.last - expect(find_champ_by_stable_id(dossier, type_de_champ_1.stable_id).value).to eq(value_1) - expect(find_champ_by_stable_id(dossier, type_de_champ_2.stable_id).value).to eq(value_2) - end - - it { is_expected.to redirect_to siret_dossier_path(id: Dossier.last) } - - context 'when prefill values contain a hash' do - let(:value_2) { { evil: "payload" } } - - it "prefills the dossier's champ with the hash stored as a string" do - subject - - dossier = Dossier.last - expect(find_champ_by_stable_id(dossier, type_de_champ_2.stable_id).value).to eq("{\"evil\"=>\"payload\"}") - end - end - - context 'when prefill values contain an array' do - let(:value_2) { ["a", "b", "c"] } - - it "prefills the dossier's champ with the array stored as a string" do - subject - - dossier = Dossier.last - expect(find_champ_by_stable_id(dossier, type_de_champ_2.stable_id).value).to eq("[\"a\", \"b\", \"c\"]") - end - end - end end context 'when user is not logged' do it { is_expected.to have_http_status(302) } diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 340d3d75d..6e1b9fb87 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -14,6 +14,7 @@ require 'axe-rspec' require 'devise' require 'shoulda-matchers' require 'view_component/test_helpers' +require "rack_session_access/capybara" # Requires supporting ruby files with custom matchers and macros, etc, in # spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index 56c96ca60..93bedcaef 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -102,6 +102,39 @@ describe 'Prefilling a dossier (with a GET request):', js: true do end end + context 'when authenticated with existing dossier and session params (ie: reload the page)' do + let(:user) { create(:user, password: password) } + let(:dossier) { create(:dossier, :prefilled, procedure: procedure, prefill_token: "token", user: nil) } + + before do + create(:champ_text, dossier: dossier, type_de_champ: type_de_champ_text, value: text_value) + + page.set_rack_session(prefill_token: "token") + page.set_rack_session(prefill_params: { "action" => "commencer", "champ_#{type_de_champ_text.to_typed_id}" => text_value, "controller" => "users/commencer", "path" => procedure.path }) + + visit "/users/sign_in" + sign_in_with user.email, password + + visit commencer_path( + path: procedure.path, + "champ_#{type_de_champ_text.to_typed_id}" => text_value + ) + + click_on "Poursuivre mon dossier prérempli" + end + + it "should not create a new dossier" do + expect(Dossier.count).to eq(1) + expect(dossier.reload.user).to eq(user) + + expect(page).to have_current_path(brouillon_dossier_path(dossier)) + expect(page).to have_field(type_de_champ_text.libelle, with: text_value) + + expect(page.get_rack_session[:prefill_token]).to be_nil + expect(page.get_rack_session[:prefill_params]).to be_nil + end + end + context 'when unauthenticated' do before { visit entry_path } diff --git a/spec/views/commencer/show.html.haml_spec.rb b/spec/views/commencer/show.html.haml_spec.rb index 45d2923ef..9f07f03ba 100644 --- a/spec/views/commencer/show.html.haml_spec.rb +++ b/spec/views/commencer/show.html.haml_spec.rb @@ -89,11 +89,5 @@ RSpec.describe 'commencer/show.html.haml', type: :view do expect(rendered).to have_link('Poursuivre mon dossier prérempli', href: brouillon_dossier_path(prefilled_dossier)) end end - - context 'and they have stored query params in order to prefill a dossier' do - let(:stored_query_params) { true } - - it_behaves_like 'it renders a link to create a new dossier', 'Poursuivre mon dossier prérempli' - end end end