Merge pull request #8622 from demarches-simplifiees/fix/stored_query_issue
Dossier prefill get without stored queries
This commit is contained in:
commit
687c05e6d1
15 changed files with 160 additions and 213 deletions
1
Gemfile
1
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'
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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'
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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"
|
||||
|
|
|
@ -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"
|
||||
|
|
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -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) }
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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 }
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue