From df474b60cf6991abc8704b967f974bb5d9cd2d75 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 17 Nov 2021 12:53:43 +0300 Subject: [PATCH 01/47] WIP --- app/models/dossier.rb | 45 +++++++++++++++---- app/models/traitement.rb | 8 ---- .../users/dossiers/show/_header.html.haml | 24 +++++----- 3 files changed, 51 insertions(+), 26 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 391a2fa0d..1e835c786 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -60,8 +60,11 @@ class Dossier < ApplicationRecord REMAINING_DAYS_BEFORE_CLOSING = 2 INTERVAL_BEFORE_CLOSING = "#{REMAINING_DAYS_BEFORE_CLOSING} days" - INTERVAL_BEFORE_EXPIRATION = '2 weeks' - INTERVAL_EXPIRATION = '1 month 5 days' + REMAINING_WEEKS_BEFORE_EXPIRATION = 2 + INTERVAL_BEFORE_EXPIRATION = "#{REMAINING_WEEKS_BEFORE_EXPIRATION} weeks" + MONTHS_AFTER_EXPIRATION = 1 + DAYS_AFTER_EXPIRATION = 5 + INTERVAL_EXPIRATION = "#{MONTHS_AFTER_EXPIRATION} month #{DAYS_AFTER_EXPIRATION} days" has_one :etablissement, dependent: :destroy has_one :individual, validate: false, dependent: :destroy @@ -304,7 +307,7 @@ class Dossier < ApplicationRecord scope :termine_close_to_expiration, -> do state_termine .joins(:procedure) - .where(id: Traitement.termine_close_to_expiration.select(:dossier_id).distinct) + .where("dossiers.processed_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :brouillon_expired, -> do @@ -522,16 +525,42 @@ class Dossier < ApplicationRecord !brouillon? && !user_deleted? && !archived end - def en_construction_close_to_expiration? - self.class.en_construction_close_to_expiration.exists?(id: self) + def close_to_expiration_at + if brouillon? + created_at + elsif en_construction? + en_construction + elsif en_instruction? + en_instruction + else + processed_at + end + conservation_extension + duree_conservation_dossiers_dans_ds.months - REMAINING_WEEKS_BEFORE_EXPIRATION.weeks end - def brouillon_close_to_expiration? - self.class.brouillon_close_to_expiration.exists?(id: self) + def duration_after_notice + MONTHS_AFTER_EXPIRATION.month + DAYS_AFTER_EXPIRATION.days + end + + def expiration_date + if brouillon? && brouillon_close_to_expiration_notice_sent_at.present? + brouillon_close_to_expiration_notice_sent_at + duration_after_notice + elsif en_construction? && en_construction_close_to_expiration_notice_sent_at.present? + en_construction_close_to_expiration_notice_sent_at + duration_after_notice + elsif termine? && termine_close_to_expiration_notice_sent_at.present? + termine_close_to_expiration_notice_sent_at + duration_after_notice + end end def close_to_expiration? - en_construction_close_to_expiration? || brouillon_close_to_expiration? + !en_instruction? && close_to_expiration_at < Time.zone.now + end + + def close_to_expiration_notice_sent? + expiration_date.present? + end + + def expiration_can_be_extended? + brouillon? || en_construction? end def show_groupe_instructeur_details? diff --git a/app/models/traitement.rb b/app/models/traitement.rb index 3d8f5f09c..3e7f264f5 100644 --- a/app/models/traitement.rb +++ b/app/models/traitement.rb @@ -17,14 +17,6 @@ class Traitement < ApplicationRecord scope :en_instruction, -> { where(state: Dossier.states.fetch(:en_instruction)) } scope :termine, -> { where(state: Dossier::TERMINE) } - scope :termine_close_to_expiration, -> do - joins(dossier: :procedure) - .termine - .where(process_expired: true) - .where('dossiers.state' => Dossier::TERMINE) - .where("traitements.processed_at + (procedures.duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: Dossier::INTERVAL_BEFORE_EXPIRATION }) - end - scope :for_traitement_time_stats, -> (procedure) do includes(:dossier) .termine diff --git a/app/views/users/dossiers/show/_header.html.haml b/app/views/users/dossiers/show/_header.html.haml index dc13805c8..46516aed8 100644 --- a/app/views/users/dossiers/show/_header.html.haml +++ b/app/views/users/dossiers/show/_header.html.haml @@ -25,18 +25,22 @@ - if dossier.close_to_expiration? .card.warning .card-title Votre dossier va expirer + %p - if dossier.brouillon? - %p - Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. + Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. + Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver + un mois de plus en cliquant sur le bouton ci-dessous. + - elsif dossier.termine? + Le traitement de votre dossier est terminé, mais il va bientôt expirer. Cela signifie qu’il va bientôt être supprimé. + Si vous souhaitez conserver une trace, vous pouvez le télécharger au format PDF. - else - %p - Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été traité par l’administration. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. - %br - = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary' + Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été traité par l’administration. + Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver + un mois de plus en cliquant sur le bouton ci-dessous. + + - if dossier.expiration_can_be_extended? + %br + = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary' %ul.tabs = dynamic_tab_item(t('views.users.dossiers.show.header.summary'), dossier_path(dossier)) From 646459a1da6eceaabae352f2ed7885234ba21985 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 18 Nov 2021 18:15:57 +0100 Subject: [PATCH 02/47] fix(spec): at least let us start from a green suite --- app/models/dossier.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 1e835c786..47b6950ea 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -529,12 +529,12 @@ class Dossier < ApplicationRecord if brouillon? created_at elsif en_construction? - en_construction + en_construction_at elsif en_instruction? - en_instruction + en_instruction_at else processed_at - end + conservation_extension + duree_conservation_dossiers_dans_ds.months - REMAINING_WEEKS_BEFORE_EXPIRATION.weeks + end + conservation_extension + procedure.duree_conservation_dossiers_dans_ds.months - REMAINING_WEEKS_BEFORE_EXPIRATION.weeks end def duration_after_notice From 1d721f14a08f21b421d753e1c342ad899b7534fc Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 12:36:03 +0100 Subject: [PATCH 03/47] fix(spec): get back to stable state --- app/models/dossier.rb | 13 ++++++++++++- app/models/traitement.rb | 8 ++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 47b6950ea..10e8c6ce1 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -304,10 +304,21 @@ class Dossier < ApplicationRecord .joins(:procedure) .where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end + + # TODO/MFO + # maybe reconsider `termine_close_to_expiration` implementation, but for now this implementation is missing + # 1. the check on traitements.processed_at? -> is is replicated on dossier? + # 2. the check on process_expired + # scope :termine_close_to_expiration, -> do + # state_termine + # .joins(:procedure) + # .where("dossiers.processed_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + # end + # TODO/MFO scope :termine_close_to_expiration, -> do state_termine .joins(:procedure) - .where("dossiers.processed_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + .where(id: Traitement.termine_close_to_expiration.select(:dossier_id).distinct) end scope :brouillon_expired, -> do diff --git a/app/models/traitement.rb b/app/models/traitement.rb index 3e7f264f5..8e6ce610f 100644 --- a/app/models/traitement.rb +++ b/app/models/traitement.rb @@ -25,6 +25,14 @@ class Traitement < ApplicationRecord .order(:processed_at) end + scope :termine_close_to_expiration, -> do + joins(dossier: :procedure) + .termine + .where(process_expired: true) + .where('dossiers.state' => Dossier::TERMINE) + .where("traitements.processed_at + (procedures.duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: Dossier::INTERVAL_BEFORE_EXPIRATION }) + end + def self.count_dossiers_termines_by_month(groupe_instructeurs) last_traitements_per_dossier = Traitement .select('max(traitements.processed_at) as processed_at') From d8257284efada3f1639a74b18b419208b7445a20 Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 14:07:47 +0100 Subject: [PATCH 04/47] tech(refactor): extract states close to expiration within their scope. --- app/models/dossier.rb | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 10e8c6ce1..1f0ded697 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -289,20 +289,35 @@ class Dossier < ApplicationRecord .where.not(user_id: nil) end + scope :interval_brouillon_close_to_expiration, -> do + where("dossiers.created_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + end scope :brouillon_close_to_expiration, -> do - state_brouillon - .joins(:procedure) - .where("dossiers.created_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + state_brouillon.joins(:procedure).interval_brouillon_close_to_expiration + end + + scope :interval_en_construction_close_to_expiration, -> do + where("dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :en_construction_close_to_expiration, -> do - state_en_construction - .joins(:procedure) - .where("dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + state_en_construction.joins(:procedure).interval_en_construction_close_to_expiration + end + + scope :interval_en_instruction_close_to_expiration, -> do + where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :en_instruction_close_to_expiration, -> do - state_en_instruction + state_en_instruction.joins(:procedure).interval_en_instruction_close_to_expiration + end + + + scope :interval_termine_close_to_expiration, -> do + where(id: Traitement.termine_close_to_expiration.select(:dossier_id).distinct) + end + scope :termine_close_to_expiration, -> do + state_termine .joins(:procedure) - .where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + .interval_termine_close_to_expiration end # TODO/MFO From 608a85148f0ea960ba7b2a8d21c452b09c8a0c6c Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 14:28:54 +0100 Subject: [PATCH 05/47] feat(Dossier.close_to_expiration): add method to find any kind of dossier close to expiration [reuse previous scopes], add missing spec to termine_close_to_expiration, complement spec for each kind of expiration scope with general close_to_expiration spec --- app/models/dossier.rb | 12 +++++--- spec/factories/traitement.rb | 8 +++++ spec/models/dossier_spec.rb | 57 ++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 spec/factories/traitement.rb diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 1f0ded697..d60fd757b 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -330,10 +330,14 @@ class Dossier < ApplicationRecord # .where("dossiers.processed_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) # end # TODO/MFO - scope :termine_close_to_expiration, -> do - state_termine - .joins(:procedure) - .where(id: Traitement.termine_close_to_expiration.select(:dossier_id).distinct) + + scope :close_to_expiration, -> do + joins(:procedure).scoping do + state_brouillon.and(interval_brouillon_close_to_expiration) + .or(state_en_construction.and(interval_en_construction_close_to_expiration)) + .or(state_en_instruction.and(interval_en_instruction_close_to_expiration)) + .or(state_termine.and(interval_termine_close_to_expiration)) + end end scope :brouillon_expired, -> do diff --git a/spec/factories/traitement.rb b/spec/factories/traitement.rb new file mode 100644 index 000000000..8c4ab5123 --- /dev/null +++ b/spec/factories/traitement.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :traitement do + trait :accepte do + process_expired {true} + state {:accepte} + end + end +end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index d740cacbf..d6f7be081 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -61,6 +61,16 @@ describe Dossier do it { is_expected.not_to include(expiring_dossier) } end + + context 'when .close_to_expiration' do + subject { Dossier.close_to_expiration } + it do + is_expected.not_to include(young_dossier) + is_expected.to include(expiring_dossier) + is_expected.to include(just_expired_dossier) + is_expected.to include(long_expired_dossier) + end + end end describe 'en_construction_close_to_expiration' do @@ -87,6 +97,16 @@ describe Dossier do it { is_expected.not_to include(expiring_dossier) } end + + context 'when .close_to_expiration' do + subject { Dossier.close_to_expiration } + it do + is_expected.not_to include(young_dossier) + is_expected.to include(expiring_dossier) + is_expected.to include(just_expired_dossier) + is_expected.to include(long_expired_dossier) + end + end end describe 'en_instruction_close_to_expiration' do @@ -104,6 +124,43 @@ describe Dossier do is_expected.to include(just_expired_dossier) is_expected.to include(long_expired_dossier) end + + context 'when .close_to_expiration' do + subject { Dossier.close_to_expiration } + it do + is_expected.not_to include(young_dossier) + is_expected.to include(expiring_dossier) + is_expected.to include(just_expired_dossier) + is_expected.to include(long_expired_dossier) + end + end + end + + describe 'termine_close_to_expiration' do + let(:procedure) { create(:procedure, :published, duree_conservation_dossiers_dans_ds: 6) } + let!(:young_dossier) { create(:dossier, :accepte, procedure: procedure, traitements: [build(:traitement, :accepte)]) } + let!(:expiring_dossier) { create(:dossier, :accepte, procedure: procedure, traitements: [build(:traitement, :accepte, processed_at: 175.days.ago)]) } + let!(:just_expired_dossier) { create(:dossier, :accepte, procedure: procedure, traitements: [build(:traitement, :accepte, processed_at: (6.months + 1.hour + 10.seconds).ago)]) } + let!(:long_expired_dossier) { create(:dossier, :accepte, procedure: procedure, traitements: [build(:traitement, :accepte, processed_at: 1.year.ago)]) } + + subject { Dossier.termine_close_to_expiration } + + it do + is_expected.not_to include(young_dossier) + is_expected.to include(expiring_dossier) + is_expected.to include(just_expired_dossier) + is_expected.to include(long_expired_dossier) + end + + context 'when .close_to_expiration' do + subject { Dossier.close_to_expiration } + it do + is_expected.not_to include(young_dossier) + is_expected.to include(expiring_dossier) + is_expected.to include(just_expired_dossier) + is_expected.to include(long_expired_dossier) + end + end end describe 'with_notifications' do From a9978fb70b43d67552f2b3e8df2f28e37f05c1ee Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 14:34:20 +0100 Subject: [PATCH 06/47] clean(code): move mfo comment to its rightful place --- app/models/dossier.rb | 52 +++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 34 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index d60fd757b..0117f1459 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -290,53 +290,37 @@ class Dossier < ApplicationRecord end scope :interval_brouillon_close_to_expiration, -> do - where("dossiers.created_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + state_brouillon.where("dossiers.created_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end - scope :brouillon_close_to_expiration, -> do - state_brouillon.joins(:procedure).interval_brouillon_close_to_expiration + scope :interval_en_construction_close_to_expiration, -> do + state_en_construction.where("dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + end + scope :interval_en_instruction_close_to_expiration, -> do + state_en_instruction.where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + end + scope :interval_termine_close_to_expiration, -> do + state_termine.where(id: Traitement.termine_close_to_expiration.select(:dossier_id).distinct) end - scope :interval_en_construction_close_to_expiration, -> do - where("dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + scope :brouillon_close_to_expiration, -> do + joins(:procedure).interval_brouillon_close_to_expiration end scope :en_construction_close_to_expiration, -> do - state_en_construction.joins(:procedure).interval_en_construction_close_to_expiration - end - - scope :interval_en_instruction_close_to_expiration, -> do - where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + joins(:procedure).interval_en_construction_close_to_expiration end scope :en_instruction_close_to_expiration, -> do - state_en_instruction.joins(:procedure).interval_en_instruction_close_to_expiration - end - - - scope :interval_termine_close_to_expiration, -> do - where(id: Traitement.termine_close_to_expiration.select(:dossier_id).distinct) + joins(:procedure).interval_en_instruction_close_to_expiration end scope :termine_close_to_expiration, -> do - state_termine - .joins(:procedure) - .interval_termine_close_to_expiration + joins(:procedure).interval_termine_close_to_expiration end - # TODO/MFO - # maybe reconsider `termine_close_to_expiration` implementation, but for now this implementation is missing - # 1. the check on traitements.processed_at? -> is is replicated on dossier? - # 2. the check on process_expired - # scope :termine_close_to_expiration, -> do - # state_termine - # .joins(:procedure) - # .where("dossiers.processed_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) - # end - # TODO/MFO - scope :close_to_expiration, -> do joins(:procedure).scoping do - state_brouillon.and(interval_brouillon_close_to_expiration) - .or(state_en_construction.and(interval_en_construction_close_to_expiration)) - .or(state_en_instruction.and(interval_en_instruction_close_to_expiration)) - .or(state_termine.and(interval_termine_close_to_expiration)) + interval_brouillon_close_to_expiration + .or(interval_en_construction_close_to_expiration) + .or(interval_en_instruction_close_to_expiration) + .or(interval_termine_close_to_expiration) end end From eecc0c38b6260910e7cb54fbbc92eae173dd99dc Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 14:36:21 +0100 Subject: [PATCH 07/47] lint(rubocop): come on, maybe i will find the inspiration to setup it up in my ide --- spec/factories/traitement.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/factories/traitement.rb b/spec/factories/traitement.rb index 8c4ab5123..f6bb83407 100644 --- a/spec/factories/traitement.rb +++ b/spec/factories/traitement.rb @@ -1,8 +1,8 @@ FactoryBot.define do factory :traitement do trait :accepte do - process_expired {true} - state {:accepte} + process_expired { true } + state { :accepte } end end end From 3d1533dee931027c8d2d647dbe56cb99c3552bfb Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 15:14:39 +0100 Subject: [PATCH 08/47] feat(users/dossiers?statut=dossiers-expirant): add dossiers-expirant tab --- app/controllers/users/dossiers_controller.rb | 8 +++++--- app/views/users/dossiers/index.html.haml | 10 +++++++++- config/locales/en.yml | 4 ++++ config/locales/fr.yml | 4 ++++ spec/controllers/users/dossiers_controller_spec.rb | 10 ++++++++++ 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 0f0c33f1d..a3ca43124 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -24,7 +24,8 @@ module Users .with_dossiers .where(email: current_user.email) .page(page) - @statut = statut(@user_dossiers, @dossiers_invites, @dossiers_supprimes, @dossier_transfers, params[:statut]) + @dossiers_close_to_expiration = current_user.dossiers.close_to_expiration.page(page) + @statut = statut(@user_dossiers, @dossiers_invites, @dossiers_supprimes, @dossier_transfers, @dossiers_close_to_expiration, params[:statut]) end def show @@ -291,12 +292,13 @@ module Users # if the status tab is filled, then this tab # else first filled tab # else mes-dossiers - def statut(mes_dossiers, dossiers_invites, dossiers_supprimes, dossier_transfers, params_statut) + def statut(mes_dossiers, dossiers_invites, dossiers_supprimes, dossier_transfers, dossiers_close_to_expiration, params_statut) tabs = { 'mes-dossiers' => mes_dossiers.present?, 'dossiers-invites' => dossiers_invites.present?, 'dossiers-supprimes' => dossiers_supprimes.present?, - 'dossiers-transferes' => dossier_transfers.present? + 'dossiers-transferes' => dossier_transfers.present?, + 'dossiers-expirant' => dossiers_close_to_expiration.present? } if tabs[params_statut] params_statut diff --git a/app/views/users/dossiers/index.html.haml b/app/views/users/dossiers/index.html.haml index 49684211b..bebc753a8 100644 --- a/app/views/users/dossiers/index.html.haml +++ b/app/views/users/dossiers/index.html.haml @@ -39,6 +39,12 @@ active: @statut == 'dossiers-transferes', badge: number_with_html_delimiter(@dossier_transfers.count)) + - if @dossiers_close_to_expiration.count > 0 + = tab_item(t('pluralize.dossiers_close_to_expiration', count: @dossiers_close_to_expiration.count), + dossiers_path(statut: 'dossiers-expirant'), + active: @statut == 'dossiers-expirant', + badge: number_with_html_delimiter(@dossiers_close_to_expiration.count)) + .container - if @statut == "mes-dossiers" = render partial: "dossiers_list", locals: { dossiers: @user_dossiers } @@ -48,6 +54,8 @@ - if @statut == "dossiers-supprimes" = render partial: "deleted_dossiers_list", locals: { deleted_dossiers: @dossiers_supprimes } - - if @statut == "dossiers-transferes" = render partial: "transfered_dossiers_list", locals: { dossier_transfers: @dossier_transfers } + + - if @statut == "dossiers-expirant" + = render partial: "dossiers_list", locals: { dossiers: @dossiers_close_to_expiration } diff --git a/config/locales/en.yml b/config/locales/en.yml index 5c6bccab0..23e2369be 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -339,6 +339,10 @@ en: zero: transfer request one: transfer request other: transfer requests + dossiers_close_to_expiration: + zero: expiring file + one: expiring file + other: expiring files dossier_trouve: zero: 0 file found one: 1 file found diff --git a/config/locales/fr.yml b/config/locales/fr.yml index d2361db5a..4e5ec3b6a 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -347,6 +347,10 @@ fr: zero: demande de transfert one: demande de transfert other: demandes de transfert + dossiers_close_to_expiration: + zero: dossier expirant + one: dossier expirant + other: dossiers expirant dossier_trouve: zero: 0 dossier trouvé one: 1 dossier trouvé diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 7b41bf6be..75a0dcfb3 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -1125,4 +1125,14 @@ describe Users::DossiersController, type: :controller do it { is_expected.to be_falsy } end end + + describe '#index' do + before do + sign_in(user) + end + it 'works' do + get :index + expect(response).to have_http_status(:ok) + end + end end From 859a147c498aa902e6e8087ba16d0d4057d6f12a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 15:15:10 +0100 Subject: [PATCH 09/47] api: return error cause on parse error Currently, when a query can't be parsed, the error is: - logged to Sentry (which is useless to us), - returned as a generic 'Internal Server Error' (which is useless to the user who made the query). With this commit, the error is instead ignored from our logs (because it is a user error), but the parse error details are returned to the user, with the following format: > {'errors': [{'message': 'Parse error on ")" (RPAREN) at [3, 23]'}]} --- app/controllers/api/v2/graphql_controller.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/controllers/api/v2/graphql_controller.rb b/app/controllers/api/v2/graphql_controller.rb index 1bebb48e7..4e9802b3a 100644 --- a/app/controllers/api/v2/graphql_controller.rb +++ b/app/controllers/api/v2/graphql_controller.rb @@ -8,6 +8,8 @@ class API::V2::GraphqlController < API::V2::BaseController operation_name: params[:operationName]) render json: result + rescue GraphQL::ParseError => exception + handle_parse_error(exception) rescue => exception if Rails.env.production? handle_error_in_production(exception) @@ -88,6 +90,15 @@ class API::V2::GraphqlController < API::V2::BaseController end end + def handle_parse_error(exception) + render json: { + errors: [ + { message: exception.message } + ], + data: nil + }, status: 400 + end + def handle_error_in_development(exception) logger.error exception.message logger.error exception.backtrace.join("\n") From 8368d62ed315ac2d030ce591984b6368aebeff73 Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 19 Nov 2021 15:53:15 +0100 Subject: [PATCH 10/47] fix(spec): broken due to new view var --- spec/views/users/dossiers/index.html.haml_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/views/users/dossiers/index.html.haml_spec.rb b/spec/views/users/dossiers/index.html.haml_spec.rb index a62fba1bd..2764bb99c 100644 --- a/spec/views/users/dossiers/index.html.haml_spec.rb +++ b/spec/views/users/dossiers/index.html.haml_spec.rb @@ -13,6 +13,7 @@ describe 'users/dossiers/index.html.haml', type: :view do assign(:dossiers_invites, Kaminari.paginate_array(dossiers_invites).page(1)) assign(:dossiers_supprimes, Kaminari.paginate_array(user_dossiers).page(1)) assign(:dossier_transfers, Kaminari.paginate_array([]).page(1)) + assign(:dossiers_close_to_expiration, Kaminari.paginate_array([]).page(1)) assign(:statut, statut) render end From 2a87b9bf8925bba7e9227f411ceb411c0947a03d Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 22 Nov 2021 14:46:10 +0100 Subject: [PATCH 11/47] feat(users/dossiers/*): rework header for consistent design, add some utils spacers --- .../{dossier_show.scss => dossier_views.scss} | 4 +- app/assets/stylesheets/utils.scss | 62 ++++++++++++++++++- app/views/users/dossiers/brouillon.html.haml | 2 +- app/views/users/dossiers/demande.html.haml | 2 +- app/views/users/dossiers/messagerie.html.haml | 2 +- app/views/users/dossiers/modifier.html.haml | 2 +- app/views/users/dossiers/show.html.haml | 2 +- 7 files changed, 66 insertions(+), 10 deletions(-) rename app/assets/stylesheets/{dossier_show.scss => dossier_views.scss} (95%) diff --git a/app/assets/stylesheets/dossier_show.scss b/app/assets/stylesheets/dossier_views.scss similarity index 95% rename from app/assets/stylesheets/dossier_show.scss rename to app/assets/stylesheets/dossier_views.scss index 94781e8e6..65ab61c73 100644 --- a/app/assets/stylesheets/dossier_show.scss +++ b/app/assets/stylesheets/dossier_views.scss @@ -1,9 +1,7 @@ @import "colors"; @import "constants"; -#dossier-show { - margin-bottom: 30px; - +.dossier-container { .sub-header { .label { float: right; diff --git a/app/assets/stylesheets/utils.scss b/app/assets/stylesheets/utils.scss index a67cb63bf..72c8b8e4e 100644 --- a/app/assets/stylesheets/utils.scss +++ b/app/assets/stylesheets/utils.scss @@ -50,6 +50,8 @@ color: $black; } + + .mt-1 { margin-top: $default-spacer; } @@ -58,6 +60,10 @@ margin-top: 2 * $default-spacer; } +.mt-3 { + margin-top: 3 * $default-spacer; +} + .mt-4 { margin-top: 4 * $default-spacer; } @@ -66,10 +72,62 @@ margin-top: 8 * $default-spacer; } +.mb-1 { + margin-bottom: $default-spacer; +} + .mb-2 { margin-bottom: 2 * $default-spacer; } -.mb-1 { - margin-bottom: $default-spacer; +.mb-3 { + margin-bottom: 3 * $default-spacer; +} + +.mb-4 { + margin-bottom: 4 * $default-spacer; +} + +.mb-8 { + margin-bottom: 8 * $default-spacer; +} + +.pt-1 { + padding-top: $default-spacer; +} + +.pt-2 { + padding-top: 2 * $default-spacer; +} + +.pt-3 { + padding-top: 3 * $default-spacer; +} + +.pt-4 { + padding-top: 4 * $default-spacer; +} + +.pt-8 { + padding-top: 8 * $default-spacer; +} + +.pb-1 { + padding-bottom: $default-spacer; +} + +.pb-2 { + padding-bottom: 2 * $default-spacer; +} + +.pb-3 { + padding-bottom: 3 * $default-spacer; +} + +.pb-4 { + padding-bottom: 4 * $default-spacer; +} + +.pb-8 { + padding-bottom: 8 * $default-spacer; } diff --git a/app/views/users/dossiers/brouillon.html.haml b/app/views/users/dossiers/brouillon.html.haml index f8828703d..c838a6615 100644 --- a/app/views/users/dossiers/brouillon.html.haml +++ b/app/views/users/dossiers/brouillon.html.haml @@ -4,7 +4,7 @@ - content_for :footer do = render partial: "users/procedure_footer", locals: { procedure: @dossier.procedure, dossier: @dossier } -#dossier-draft +.dossier-container .dossier-header.sub-header .container = render partial: "shared/dossiers/header", locals: { dossier: @dossier, apercu: false } diff --git a/app/views/users/dossiers/demande.html.haml b/app/views/users/dossiers/demande.html.haml index 84cbfb04d..8a1991d43 100644 --- a/app/views/users/dossiers/demande.html.haml +++ b/app/views/users/dossiers/demande.html.haml @@ -3,7 +3,7 @@ - content_for :footer do = render partial: "users/procedure_footer", locals: { procedure: @dossier.procedure, dossier: @dossier } -#dossier-show +.dossier-container.mb-4 = render partial: 'users/dossiers/show/header', locals: { dossier: @dossier } = render partial: 'shared/dossiers/demande', locals: { dossier: @dossier, demande_seen_at: nil, profile: 'usager' } diff --git a/app/views/users/dossiers/messagerie.html.haml b/app/views/users/dossiers/messagerie.html.haml index 17bf6a9c6..fc0cd2b5b 100644 --- a/app/views/users/dossiers/messagerie.html.haml +++ b/app/views/users/dossiers/messagerie.html.haml @@ -3,7 +3,7 @@ - content_for :footer do = render partial: "users/procedure_footer", locals: { procedure: @dossier.procedure, dossier: @dossier } -#dossier-show +.dossier-container.mb-4 = render partial: 'users/dossiers/show/header', locals: { dossier: @dossier } .container diff --git a/app/views/users/dossiers/modifier.html.haml b/app/views/users/dossiers/modifier.html.haml index 24fbbf3a6..41c4a3512 100644 --- a/app/views/users/dossiers/modifier.html.haml +++ b/app/views/users/dossiers/modifier.html.haml @@ -3,7 +3,7 @@ - content_for :footer do = render partial: "users/procedure_footer", locals: { procedure: @dossier.procedure, dossier: @dossier } -#dossier-show +.dossier-container.mb-4 = render partial: 'users/dossiers/show/header', locals: { dossier: @dossier } .container diff --git a/app/views/users/dossiers/show.html.haml b/app/views/users/dossiers/show.html.haml index f23312447..2ec8ab29d 100644 --- a/app/views/users/dossiers/show.html.haml +++ b/app/views/users/dossiers/show.html.haml @@ -3,7 +3,7 @@ - content_for :footer do = render partial: "users/procedure_footer", locals: { procedure: @dossier.procedure, dossier: @dossier } -#dossier-show +.dossier-container.mb-4 = render partial: 'users/dossiers/show/header', locals: { dossier: @dossier } .container From b6adf5fc7274deb2f106cb44d355c06e996d1163 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 22 Nov 2021 14:51:52 +0100 Subject: [PATCH 12/47] feat(dossiers/show): enhance header with expirations message/banner. also ensure consistent design between dossier states fix(spec): broken due to last refactoring spec(dossier.extend_conservation): add system spec lint(ruby): still not yet ready for auto lint in IDE... --- app/helpers/dossier_helper.rb | 5 ++ app/models/dossier.rb | 34 ++++++----- .../dossiers/_expiration_banner.html.haml | 17 ++++++ app/views/shared/dossiers/_header.html.haml | 18 ++++-- .../dossiers/_short_expires_message.html.haml | 9 +++ .../users/dossiers/show/_header.html.haml | 22 +------- config/locales/en.yml | 1 + config/locales/fr.yml | 1 + config/locales/shared.en.yml | 8 +++ config/locales/shared.fr.yml | 8 +++ spec/support/capybara.rb | 2 +- spec/system/users/brouillon_spec.rb | 21 +++++++ .../shared/dossiers/_header.html.haml_spec.rb | 56 +++++++++++++++++++ 13 files changed, 164 insertions(+), 38 deletions(-) create mode 100644 app/views/shared/dossiers/_expiration_banner.html.haml create mode 100644 app/views/shared/dossiers/_short_expires_message.html.haml create mode 100644 spec/views/shared/dossiers/_header.html.haml_spec.rb diff --git a/app/helpers/dossier_helper.rb b/app/helpers/dossier_helper.rb index 1891b8485..8a2582100 100644 --- a/app/helpers/dossier_helper.rb +++ b/app/helpers/dossier_helper.rb @@ -98,6 +98,11 @@ module DossierHelper end end + def safe_expiration_date(dossier) + date = dossier.expiration_date.presence || dossier.approximative_expiration_date + l(date, format: '%d/%m/%Y') + end + def annuaire_link(siren) base_url = "https://annuaire-entreprises.data.gouv.fr" return base_url if siren.blank? diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 0117f1459..42d51a228 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -539,20 +539,32 @@ class Dossier < ApplicationRecord !brouillon? && !user_deleted? && !archived end - def close_to_expiration_at + def expirable? + [brouillon?, en_construction?, termine?].any? + end + + def approximative_expiration_date_reference if brouillon? created_at elsif en_construction? en_construction_at - elsif en_instruction? - en_instruction_at - else + elsif termine? processed_at - end + conservation_extension + procedure.duree_conservation_dossiers_dans_ds.months - REMAINING_WEEKS_BEFORE_EXPIRATION.weeks + else + fail "approximative_expiration_date_reference should not be called in state #{self.state}" + end end - def duration_after_notice - MONTHS_AFTER_EXPIRATION.month + DAYS_AFTER_EXPIRATION.days + def approximative_expiration_date + [ + approximative_expiration_date_reference, + conservation_extension, + procedure.duree_conservation_dossiers_dans_ds.months + ].sum - REMAINING_WEEKS_BEFORE_EXPIRATION.weeks + end + + def close_to_expiration? + approximative_expiration_date < Time.zone.now end def expiration_date @@ -565,12 +577,8 @@ class Dossier < ApplicationRecord end end - def close_to_expiration? - !en_instruction? && close_to_expiration_at < Time.zone.now - end - - def close_to_expiration_notice_sent? - expiration_date.present? + def duration_after_notice + MONTHS_AFTER_EXPIRATION.month + DAYS_AFTER_EXPIRATION.days end def expiration_can_be_extended? diff --git a/app/views/shared/dossiers/_expiration_banner.html.haml b/app/views/shared/dossiers/_expiration_banner.html.haml new file mode 100644 index 000000000..b01fbea34 --- /dev/null +++ b/app/views/shared/dossiers/_expiration_banner.html.haml @@ -0,0 +1,17 @@ +- if dossier.expirable? && dossier.close_to_expiration? + .card.warning.mt-2.mb-3 + .card-title Votre dossier va expirer + %p + - if dossier.brouillon? + Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. + Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver + un mois de plus en cliquant sur le bouton ci-dessous. + - elsif dossier.en_construction? + Votre dossier est en attente de prise en charge par l'administration. Le delais de prise en charge maximale est de 6 mois. Vous pouvez toutefois entendre cette durée d'un mois en cliquant sur le bouton suivant. + - elsif dossier.termine? + Le traitement de votre dossier est terminé, mais il va bientôt expirer. Cela signifie qu’il va bientôt être supprimé. + Si vous souhaitez conserver une trace, vous pouvez le télécharger au format PDF. + + - if dossier.expiration_can_be_extended? + %br + = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary mt-2' diff --git a/app/views/shared/dossiers/_header.html.haml b/app/views/shared/dossiers/_header.html.haml index aad250c4b..056dceff1 100644 --- a/app/views/shared/dossiers/_header.html.haml +++ b/app/views/shared/dossiers/_header.html.haml @@ -1,6 +1,14 @@ -%h1 - = procedure_libelle(dossier.procedure) += status_badge(dossier.state) +.title-container + %span.icon.folder + %h1= procedure_libelle(dossier.procedure) + %h2 + = t('views.users.dossiers.show.header.dossier_number', dossier_id: dossier.id) + = t('views.users.dossiers.show.header.created_date', date_du_dossier: I18n.l(dossier.created_at)) + + = render(partial: 'shared/dossiers/short_expires_message', locals: {dossier: dossier}) + + .header-actions + - if current_user.owns?(dossier) + = render partial: 'invites/dropdown', locals: { dossier: dossier } -.dossier-form-actions - - if current_user.owns?(dossier) - = render partial: 'invites/dropdown', locals: { dossier: dossier } diff --git a/app/views/shared/dossiers/_short_expires_message.html.haml b/app/views/shared/dossiers/_short_expires_message.html.haml new file mode 100644 index 000000000..466c3f880 --- /dev/null +++ b/app/views/shared/dossiers/_short_expires_message.html.haml @@ -0,0 +1,9 @@ +- if dossier.expirable? + %p.expires_at + %small= t("shared.dossiers.header.expires_at.#{dossier.state}", date: safe_expiration_date(dossier)) +- else + %p.expires_at_en_instruction + %small= t("shared.dossiers.header.expires_at.en_instruction") + + += render(partial: 'shared/dossiers/expiration_banner', locals: {dossier: dossier}) diff --git a/app/views/users/dossiers/show/_header.html.haml b/app/views/users/dossiers/show/_header.html.haml index 46516aed8..2985d024f 100644 --- a/app/views/users/dossiers/show/_header.html.haml +++ b/app/views/users/dossiers/show/_header.html.haml @@ -10,6 +10,9 @@ - if dossier.en_construction_at.present? = t('views.users.dossiers.show.header.submit_date', date_du_dossier: I18n.l(dossier.en_construction_at)) + = render(partial: 'shared/dossiers/short_expires_message', locals: {dossier: dossier}) + + - if current_user.owns?(dossier) .header-actions = render partial: 'invites/dropdown', locals: { dossier: dossier } @@ -22,25 +25,6 @@ %li = link_to t('views.users.dossiers.show.header.print_dossier'), dossier_path(dossier, format: :pdf), target: "_blank", rel: "noopener", class: "menu-item menu-link" - - if dossier.close_to_expiration? - .card.warning - .card-title Votre dossier va expirer - %p - - if dossier.brouillon? - Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. - - elsif dossier.termine? - Le traitement de votre dossier est terminé, mais il va bientôt expirer. Cela signifie qu’il va bientôt être supprimé. - Si vous souhaitez conserver une trace, vous pouvez le télécharger au format PDF. - - else - Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été traité par l’administration. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. - - - if dossier.expiration_can_be_extended? - %br - = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary' %ul.tabs = dynamic_tab_item(t('views.users.dossiers.show.header.summary'), dossier_path(dossier)) diff --git a/config/locales/en.yml b/config/locales/en.yml index 23e2369be..b0abb4c73 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -160,6 +160,7 @@ en: request: "Request" mailbox: "Mailbox" dossier_number: "File n. %{dossier_id}" + created_date: "- Draft on %{date_du_dossier}" submit_date: "- Submit on %{date_du_dossier}" print: "print" print_dossier: "All the file" diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 4e5ec3b6a..ea9696470 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -156,6 +156,7 @@ fr: request: "Demande" mailbox: "Messagerie" dossier_number: "Dossier nº %{dossier_id}" + created_date: "- En brouillon depuis le %{date_du_dossier}" submit_date: "- Déposé le %{date_du_dossier}" print: "imprimer" print_dossier: "Tout le dossier" diff --git a/config/locales/shared.en.yml b/config/locales/shared.en.yml index 7ef3d6294..fa45fd22a 100644 --- a/config/locales/shared.en.yml +++ b/config/locales/shared.en.yml @@ -7,6 +7,14 @@ en: numero_allocataire_notice: It is usually composed of 7 digits. code_postal_label: postal code code_postal_notice: It is usually composed of 5 digits. + header: + expires_at: + brouillon: "Expires at %{date}" + en_construction: "Expires at %{date}" + en_instruction: "This file is being instructed, the administration will answer as soon as possible" + accepte: "Expires at %{date}" + refuse: "Expires at %{date}" + sans_suite: "Expires at %{date}" champs: cnaf: show: diff --git a/config/locales/shared.fr.yml b/config/locales/shared.fr.yml index a0bf159e1..ffcca7a87 100644 --- a/config/locales/shared.fr.yml +++ b/config/locales/shared.fr.yml @@ -7,6 +7,14 @@ fr: numero_allocataire_notice: Il est généralement composé de 7 chiffres. code_postal_label: Le code postal code_postal_notice: Il est généralement composé de 5 chiffres. + header: + expires_at: + brouillon: "Expirera le %{date}" + en_construction: "Expirera le %{date}" + en_instruction: "Ce dossier est en instruction, il n'expirera pas" + accepte: "Expirera le %{date}" + refuse: "Expirera le %{date}" + sans_suite: "Expirera le %{date}" champs: cnaf: show: diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 10ed09c19..912664fe0 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -9,7 +9,7 @@ end Capybara.register_driver :headless_chrome do |app| options = Selenium::WebDriver::Chrome::Options.new - options.add_argument('--headless') + options.add_argument('--headless') unless ENV['NO_HEADLESS'] options.add_argument('--window-size=1440,900') capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index 29beb58b0..bc4c3f1b3 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -165,6 +165,27 @@ describe 'The user' do expect(page).to have_current_path(merci_dossier_path(user_dossier)) end + scenario 'extends dossier experation date more than one time, ', js: true do + user_old_dossier = create(:dossier, + procedure: simple_procedure, + created_at: simple_procedure.duree_conservation_dossiers_dans_ds.month.ago, + user: user) + login_as(user, scope: :user) + visit brouillon_dossier_path(user_old_dossier) + + expect(page).to have_css('.card-title', text: 'Votre dossier va expirer', visible: true) + click_on "Repousser sa suppression" + expect(page).not_to have_button("Repousser sa suppression") + + Timecop.freeze(1.month.from_now) do + visit brouillon_dossier_path(user_old_dossier) + + expect(page).to have_css('.card-title', text: 'Votre dossier va expirer', visible: true) + click_on "Repousser sa suppression" + expect(page).not_to have_button("Repousser sa suppression") + end + end + let(:procedure_with_pj) do tdcs = [build(:type_de_champ_piece_justificative, mandatory: true, libelle: 'Pièce justificative')] create(:procedure, :published, :for_individual, types_de_champ: tdcs) diff --git a/spec/views/shared/dossiers/_header.html.haml_spec.rb b/spec/views/shared/dossiers/_header.html.haml_spec.rb new file mode 100644 index 000000000..25567efea --- /dev/null +++ b/spec/views/shared/dossiers/_header.html.haml_spec.rb @@ -0,0 +1,56 @@ +describe 'shared/dossiers/short_expires_message.html.haml', type: :view do + include DossierHelper + let(:dossier) do + build(:dossier, state, attributes.merge(id: 1, state: state)) + end + let(:i18n_key_state) { state } + subject do + render('shared/dossiers/short_expires_message.html.haml', + dossier: dossier, + current_user: build(:user)) + end + + context 'with dossier.brouillon?' do + let(:attributes) { { created_at: 6.months.ago } } + let(:state) { :brouillon } + + it 'render estimated expiration date' do + expect(subject).to have_selector('.expires_at', + text: I18n.t("shared.dossiers.header.expires_at.#{i18n_key_state}", + date: safe_expiration_date(dossier))) + end + end + + context 'with dossier.en_construction?' do + let(:attributes) { { en_construction_at: 6.months.ago } } + let(:state) { :en_construction } + + it 'render estimated expiration date' do + expect(subject).to have_selector('.expires_at', + text: I18n.t("shared.dossiers.header.expires_at.#{i18n_key_state}", + date: safe_expiration_date(dossier))) + end + end + + context 'with dossier.en_instruction?' do + let(:state) { :en_instruction } + let(:attributes) { {} } + + it 'render estimated expiration date' do + expect(subject).to have_selector('p.expires_at_en_instruction', + text: I18n.t("shared.dossiers.header.expires_at.#{i18n_key_state}")) + end + end + + context 'with dossier.en_processed_at?' do + let(:state) { :accepte } + let(:attributes) { {} } + + it 'render estimated expiration date' do + allow(dossier).to receive(:processed_at).and_return(6.months.ago) + expect(subject).to have_selector('.expires_at', + text: I18n.t("shared.dossiers.header.expires_at.#{i18n_key_state}", + date: safe_expiration_date(dossier))) + end + end +end From 09ca70eef04f15313ef55f1265851980f8c61692 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 12:29:24 +0100 Subject: [PATCH 13/47] specs: improve search controller specs --- spec/controllers/recherche_controller_spec.rb | 80 ++++++++++++++----- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/spec/controllers/recherche_controller_spec.rb b/spec/controllers/recherche_controller_spec.rb index 985e4dd3e..9aa2f5664 100644 --- a/spec/controllers/recherche_controller_spec.rb +++ b/spec/controllers/recherche_controller_spec.rb @@ -1,15 +1,35 @@ describe RechercheController, type: :controller do - let(:dossier) { create(:dossier, :en_construction, :with_populated_annotations) } - let(:dossier2) { create(:dossier, :en_construction, procedure: dossier.procedure) } + let(:procedure) { + create(:procedure, + :published, + :for_individual, + :with_type_de_champ, + :with_type_de_champ_private, + types_de_champ_count: 2, + types_de_champ_private_count: 2) + } + let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } let(:instructeur) { create(:instructeur) } - let(:dossier_with_expert) { avis.dossier } - let(:avis) { create(:avis, dossier: create(:dossier, :en_construction, :with_populated_annotations)) } + let(:dossier_with_expert) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } + let(:avis) { create(:avis, dossier: dossier_with_expert) } let(:user) { instructeur.user } before do instructeur.assign_to_procedure(dossier.procedure) + + dossier.champs[0].value = "Name of district A" + dossier.champs[1].value = "Name of city A" + dossier.champs_private[0].value = "Dossier A is complete" + dossier.champs_private[1].value = "Dossier A is valid" + dossier.save! + + dossier_with_expert.champs[0].value = "Name of district B" + dossier_with_expert.champs[1].value = "name of city B" + dossier_with_expert.champs_private[0].value = "Dossier B is incomplete" + dossier_with_expert.champs_private[1].value = "Dossier B is invalid" + dossier_with_expert.save! end describe 'GET #index' do @@ -46,8 +66,8 @@ describe RechercheController, type: :controller do end context 'when instructeur do not own the dossier' do - let(:dossier3) { create(:dossier, :en_construction) } - let(:query) { dossier3.id } + let(:dossier2) { create(:dossier, :en_construction) } + let(:query) { dossier2.id } it { is_expected.to have_http_status(200) } @@ -69,29 +89,49 @@ describe RechercheController, type: :controller do end end - describe 'by private annotations' do - context 'when instructeur search by private annotations' do - let(:query) { dossier.private_search_terms } + describe 'by champs' do + let(:query) { 'district A' } - before { subject } + before { subject } - it { is_expected.to have_http_status(200) } + it { is_expected.to have_http_status(200) } - it 'returns the expected dossier' do - expect(assigns(:projected_dossiers).count).to eq(1) - expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier.id) - end + it 'returns the expected dossier' do + expect(assigns(:projected_dossiers).count).to eq(1) + expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier.id) end - context 'when expert search by private annotations' do + context 'as an expert' do let(:user) { avis.experts_procedure.expert.user } - let(:query) { dossier_with_expert.private_search_terms } - - before { subject } + let(:query) { 'district' } it { is_expected.to have_http_status(200) } - it 'returns 0 dossiers' do + it 'returns only the dossier available to the expert' do + expect(assigns(:projected_dossiers).count).to eq(1) + expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier_with_expert.id) + end + end + end + + describe 'by private annotations' do + let(:query) { 'invalid' } + + before { subject } + + it { is_expected.to have_http_status(200) } + + it 'returns the expected dossier' do + expect(assigns(:projected_dossiers).count).to eq(1) + expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier_with_expert.id) + end + + context 'as an expert' do + let(:user) { avis.experts_procedure.expert.user } + + it { is_expected.to have_http_status(200) } + + it 'does not allow experts to search in private annotations' do expect(assigns(:projected_dossiers).count).to eq(0) end end From 8d89ae366b8dd6dec9ef78915739141caabbc02b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 12 Nov 2021 08:27:02 +0100 Subject: [PATCH 14/47] specs: minor cleanups --- spec/jobs/api_entreprise/job_spec.rb | 4 ++-- spec/models/champs/iban_champ_spec.rb | 1 - spec/models/instructeur_spec.rb | 6 +++--- spec/models/procedure_presentation_and_revisions_spec.rb | 8 ++++---- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/spec/jobs/api_entreprise/job_spec.rb b/spec/jobs/api_entreprise/job_spec.rb index db03ae134..4c1cdc690 100644 --- a/spec/jobs/api_entreprise/job_spec.rb +++ b/spec/jobs/api_entreprise/job_spec.rb @@ -6,7 +6,7 @@ RSpec.describe APIEntreprise::Job, type: :job do describe '#perform' do let(:dossier) { create(:dossier, :with_entreprise) } - context 'when a un retryable error is raised' do + context 'when an un-retriable error is raised' do let(:errors) { [:standard_error] } it 'does not retry' do @@ -14,7 +14,7 @@ RSpec.describe APIEntreprise::Job, type: :job do end end - context 'when a retryable error is raised' do + context 'when a retriable error is raised' do let(:errors) { [:service_unavaible, :bad_gateway, :timed_out] } it 'retries 5 times' do diff --git a/spec/models/champs/iban_champ_spec.rb b/spec/models/champs/iban_champ_spec.rb index b23f78b89..ed7619e34 100644 --- a/spec/models/champs/iban_champ_spec.rb +++ b/spec/models/champs/iban_champ_spec.rb @@ -1,4 +1,3 @@ - describe Champs::IbanChamp do describe '#valid?' do it do diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 4426629db..3e28c4795 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -262,14 +262,14 @@ describe Instructeur, type: :model do end describe '#notifications_for_groupe_instructeurs' do - # one procedure, one group, 2 instructeurs + # a procedure, one group, 2 instructeurs let(:procedure) { create(:simple_procedure, :routee, :with_type_de_champ_private, :for_individual) } let(:gi_p1) { procedure.groupe_instructeurs.last } - let!(:dossier) { create(:dossier, :with_individual, :followed, groupe_instructeur: gi_p1, state: Dossier.states.fetch(:en_construction)) } + let!(:dossier) { create(:dossier, :with_individual, :followed, procedure: procedure, groupe_instructeur: gi_p1, state: Dossier.states.fetch(:en_construction)) } let(:instructeur) { dossier.follows.first.instructeur } let!(:instructeur_2) { create(:instructeur, groupe_instructeurs: [gi_p1]) } - # one other procedure, dossier followed by a third instructeur + # another procedure, dossier followed by a third instructeur let!(:dossier_on_procedure_2) { create(:dossier, :followed, state: Dossier.states.fetch(:en_construction)) } let!(:instructeur_on_procedure_2) { dossier_on_procedure_2.follows.first.instructeur } let(:gi_p2) { dossier.groupe_instructeur } diff --git a/spec/models/procedure_presentation_and_revisions_spec.rb b/spec/models/procedure_presentation_and_revisions_spec.rb index 4f9432d31..b16607bd5 100644 --- a/spec/models/procedure_presentation_and_revisions_spec.rb +++ b/spec/models/procedure_presentation_and_revisions_spec.rb @@ -16,7 +16,7 @@ describe ProcedurePresentation do context 'for a published procedure' do let(:procedure) { create(:procedure, :published) } - let!(:tdc) { { type_champ: :number, libelle: 'libelle 1' } } + let(:tdc) { { type_champ: :number, libelle: 'libelle 1' } } before do procedure.draft_revision.add_type_de_champ(tdc) @@ -26,7 +26,7 @@ describe ProcedurePresentation do it { is_expected.to match(['libelle 1']) } context 'when there is another published revision with an added tdc' do - let!(:added_tdc) { { type_champ: :number, libelle: 'libelle 2' } } + let(:added_tdc) { { type_champ: :number, libelle: 'libelle 2' } } before do procedure.draft_revision.add_type_de_champ(added_tdc) @@ -37,7 +37,7 @@ describe ProcedurePresentation do end context 'add one tdc above the first one' do - let!(:tdc2) { { type_champ: :number, libelle: 'libelle 2' } } + let(:tdc2) { { type_champ: :number, libelle: 'libelle 2' } } before do created_tdc2 = procedure.draft_revision.add_type_de_champ(tdc2) @@ -47,7 +47,7 @@ describe ProcedurePresentation do it { is_expected.to match(['libelle 2', 'libelle 1']) } - context 'and finaly, when this tdc is removed' do + context 'and finally, when this tdc is removed' do let!(:previous_tdc2) { procedure.published_revision.types_de_champ.find_by(libelle: 'libelle 2') } before do From 774ef00f8e3cfe5d2814e0ad29e010902344af1e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 13:24:39 +0100 Subject: [PATCH 15/47] specs: improve dossier_spec#build_default_individual --- spec/models/dossier_spec.rb | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index d740cacbf..6ad850dfb 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -193,11 +193,21 @@ describe Dossier do expect(dossier.champs.count).to eq(1) expect(dossier.champs_private.count).to eq(1) end + end + + describe '#build_default_individual' do + let(:dossier) { build(:dossier, procedure: procedure, user: user) } + + subject do + dossier.individual = nil + dossier.build_default_individual + end context 'when the dossier belongs to a procedure for individuals' do - let(:procedure) { create(:procedure, :with_type_de_champ, for_individual: true) } + let(:procedure) { create(:procedure, for_individual: true) } it 'creates a default individual' do + subject expect(dossier.individual).to be_present expect(dossier.individual.nom).to be_nil expect(dossier.individual.prenom).to be_nil @@ -209,6 +219,7 @@ describe Dossier do let(:user) { build(:user, france_connect_information: france_connect_information) } it 'fills the individual with the informations from France Connect' do + subject expect(dossier.individual.nom).to eq('DUBOIS') expect(dossier.individual.prenom).to eq('Angela Claire Louise') expect(dossier.individual.gender).to eq(Individual::GENDER_FEMALE) @@ -217,9 +228,10 @@ describe Dossier do end context 'when the dossier belongs to a procedure for moral personas' do - let(:procedure) { create(:procedure, :with_type_de_champ, for_individual: false) } + let(:procedure) { create(:procedure, for_individual: false) } it 'doesn’t create a individual' do + subject expect(dossier.individual).to be_nil end end From ef2d9a3e869548f082250e1117db517da165969f Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 16 Nov 2021 14:48:57 +0100 Subject: [PATCH 16/47] specs: fix :champ_siret factory not using the procedure Unlike all other champs factories, :champ_siret would attempt to build a champ with a `nil` procedure (instead of using the dossier's one). --- spec/factories/champ.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/champ.rb b/spec/factories/champ.rb index b438befbb..99b0fd25e 100644 --- a/spec/factories/champ.rb +++ b/spec/factories/champ.rb @@ -190,7 +190,7 @@ FactoryBot.define do end factory :champ_siret, class: 'Champs::SiretChamp' do - association :type_de_champ, factory: [:type_de_champ_siret] + type_de_champ { association :type_de_champ_siret, procedure: dossier.procedure } association :etablissement, factory: [:etablissement] value { '44011762001530' } end From d8fbcfe2e2b6fa1bcfcf2cacf9fc3eb0306cb52b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 16 Nov 2021 14:49:32 +0100 Subject: [PATCH 17/47] specs: explicitely pass the dossier to champs factories Otherwise the factory will not know in avance which procedure this champs belongs to. --- spec/serializers/dossier_serializer_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/serializers/dossier_serializer_spec.rb b/spec/serializers/dossier_serializer_spec.rb index 33a883d72..0257e634e 100644 --- a/spec/serializers/dossier_serializer_spec.rb +++ b/spec/serializers/dossier_serializer_spec.rb @@ -21,11 +21,11 @@ describe DossierSerializer do let(:dossier) { create(:dossier, :en_construction, procedure: create(:procedure, :published, :with_type_de_champ)) } before do - dossier.champs << build(:champ_carte) - dossier.champs << build(:champ_siret) - dossier.champs << build(:champ_integer_number) - dossier.champs << build(:champ_decimal_number) - dossier.champs << build(:champ_linked_drop_down_list) + dossier.champs << build(:champ_carte, dossier: dossier) + dossier.champs << build(:champ_siret, dossier: dossier) + dossier.champs << build(:champ_integer_number, dossier: dossier) + dossier.champs << build(:champ_decimal_number, dossier: dossier) + dossier.champs << build(:champ_linked_drop_down_list, dossier: dossier) end it { From 970c3e4b2bd12162691ce003c1b926aa049c3e83 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 12:48:52 +0000 Subject: [PATCH 18/47] specs: avoid executing business logic in Procedure factory Calling business logic in a factory is a code-smell, because it usually requires the object to be saved into database, and may have unintended consequences when the business logic is changed. Also, this allows to just build a published procedure, without saving it to the database. --- spec/factories/dossier.rb | 31 ++++---- spec/factories/individual.rb | 7 ++ spec/factories/procedure.rb | 76 +++++++++++-------- spec/factories/procedure_revision.rb | 17 +++++ .../procedure_revision_type_de_champ.rb | 12 +++ spec/helpers/dossier_helper_spec.rb | 2 +- spec/mailers/application_mailer_spec.rb | 2 +- spec/mailers/dossier_mailer_spec.rb | 11 +-- spec/models/procedure_presentation_spec.rb | 10 +-- .../dossier_projection_service_spec.rb | 2 +- spec/system/accessibilite/wcag_usager_spec.rb | 2 +- .../dossiers/_demande.html.haml_spec.rb | 2 +- 12 files changed, 112 insertions(+), 62 deletions(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 59f4abc37..50182c714 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -2,18 +2,20 @@ FactoryBot.define do factory :dossier do autorisation_donnees { true } state { Dossier.states.fetch(:brouillon) } - association :user + + user { association :user } transient do - procedure { nil } + for_individual? { false } + # For now a dossier must use a `create`d procedure, even if the dossier is only built (and not created). + # This is because saving the dossier fails when the procedure has not been saved beforehand + # (due to some internal ActiveRecord error). + # TODO: find a way to find the issue and just `build` the procedure. + procedure { create(:procedure, :published, :with_type_de_champ, :with_type_de_champ_private, for_individual: for_individual?) } end after(:build) do |dossier, evaluator| - if evaluator.procedure.present? - procedure = evaluator.procedure - else - procedure = create(:procedure, :published, :with_type_de_champ, :with_type_de_champ_private) - end + procedure = evaluator.procedure dossier.revision = procedure.active_revision @@ -22,7 +24,9 @@ FactoryBot.define do dossier.groupe_instructeur = procedure.routee? ? nil : procedure.defaut_groupe_instructeur end - dossier.build_default_individual + if procedure.for_individual? && dossier.individual.blank? + build(:individual, :empty, dossier: dossier) + end end trait :with_entreprise do @@ -42,12 +46,11 @@ FactoryBot.define do end trait :with_individual do - after(:build) do |dossier, evaluator| - # If the procedure was implicitely created by the factory, - # mark it automatically as for_individual. - if evaluator.procedure.nil? - dossier.procedure.update(for_individual: true) - end + transient do + for_individual? { true } + end + + after(:build) do |dossier, _evaluator| if !dossier.procedure.for_individual? raise 'Inconsistent factory: attempting to create a dossier :with_individual on a procedure that is not `for_individual?`' end diff --git a/spec/factories/individual.rb b/spec/factories/individual.rb index e93b3f6c0..c1aa66c5d 100644 --- a/spec/factories/individual.rb +++ b/spec/factories/individual.rb @@ -5,5 +5,12 @@ FactoryBot.define do prenom { 'Xavier' } birthdate { Date.new(1991, 11, 01) } association :dossier + + trait :empty do + gender { nil } + nom { nil } + prenom { nil } + birthdate { nil } + end end end diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index eb874dfa6..41e2b83fa 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -1,5 +1,6 @@ FactoryBot.define do sequence(:published_path) { |n| "fake_path#{n}" } + factory :procedure do sequence(:libelle) { |n| "Procedure #{n}" } description { "Demande de subvention à l'intention des associations" } @@ -26,24 +27,17 @@ FactoryBot.define do elsif procedure.administrateurs.empty? procedure.administrateurs = [build(:administrateur)] end - procedure.draft_revision = build(:procedure_revision, procedure: procedure) - evaluator.types_de_champ.each do |type_de_champ| - type_de_champ.revision = procedure.draft_revision - type_de_champ.private = false - type_de_champ.revision.revision_types_de_champ << build(:procedure_revision_type_de_champ, - revision: procedure.draft_revision, - position: type_de_champ.order_place, - type_de_champ: type_de_champ) - end + initial_revision = build(:procedure_revision, procedure: procedure) + add_types_de_champs(evaluator.types_de_champ, to: initial_revision, scope: :public) + add_types_de_champs(evaluator.types_de_champ_private, to: initial_revision, scope: :private) - evaluator.types_de_champ_private.each do |type_de_champ| - type_de_champ.revision = procedure.draft_revision - type_de_champ.private = true - type_de_champ.revision.revision_types_de_champ_private << build(:procedure_revision_type_de_champ, - revision: procedure.draft_revision, - position: type_de_champ.order_place, - type_de_champ: type_de_champ) + if procedure.brouillon? + procedure.draft_revision = initial_revision + else + procedure.published_revision = initial_revision + procedure.published_revision.published_at = Time.zone.now + procedure.draft_revision = build(:procedure_revision, from_original: initial_revision) end end @@ -71,11 +65,12 @@ FactoryBot.define do end factory :simple_procedure do + published + + for_individual { true } + after(:build) do |procedure, _evaluator| - procedure.for_individual = true build(:type_de_champ, libelle: 'Texte obligatoire', mandatory: true, procedure: procedure) - procedure.path = generate(:published_path) - procedure.publish! end end @@ -218,26 +213,27 @@ FactoryBot.define do end trait :published do - after(:build) do |procedure, _evaluator| - procedure.path = generate(:published_path) - procedure.publish! - end + aasm_state { :publiee } + path { generate(:published_path) } + published_at { Time.zone.now } + unpublished_at { nil } + closed_at { nil } end trait :closed do - after(:build) do |procedure, _evaluator| - procedure.path = generate(:published_path) - procedure.publish! - procedure.close! - end + published + + aasm_state { :close } + published_at { Time.zone.now - 1.second } + closed_at { Time.zone.now } end trait :unpublished do - after(:build) do |procedure, _evaluator| - procedure.path = generate(:published_path) - procedure.publish! - procedure.unpublish! - end + published + + aasm_state { :depubliee } + published_at { Time.zone.now - 1.second } + unpublished_at { Time.zone.now } end trait :discarded do @@ -308,3 +304,17 @@ FactoryBot.define do end end end + +def add_types_de_champs(types_de_champ, to: nil, scope: :public) + revision = to + association_name = scope == :private ? :revision_types_de_champ_private : :revision_types_de_champ + + types_de_champ.each do |type_de_champ| + type_de_champ.revision = revision + type_de_champ.private = (scope == :private) + type_de_champ.revision.public_send(association_name) << build(:procedure_revision_type_de_champ, + revision: revision, + position: type_de_champ.order_place, + type_de_champ: type_de_champ) + end +end diff --git a/spec/factories/procedure_revision.rb b/spec/factories/procedure_revision.rb index 050749066..1d7ee0fc3 100644 --- a/spec/factories/procedure_revision.rb +++ b/spec/factories/procedure_revision.rb @@ -1,4 +1,21 @@ FactoryBot.define do factory :procedure_revision do + transient do + from_original { nil } + end + + after(:build) do |revision, evaluator| + if evaluator.from_original + original = evaluator.from_original + + revision.procedure = original.procedure + original.revision_types_de_champ.each do |r_tdc| + revision.revision_types_de_champ << build(:procedure_revision_type_de_champ, from_original: r_tdc) + end + original.revision_types_de_champ_private.each do |r_tdc| + revision.revision_types_de_champ_private << build(:procedure_revision_type_de_champ, from_original: r_tdc) + end + end + end end end diff --git a/spec/factories/procedure_revision_type_de_champ.rb b/spec/factories/procedure_revision_type_de_champ.rb index 62359ab98..fc2b5b214 100644 --- a/spec/factories/procedure_revision_type_de_champ.rb +++ b/spec/factories/procedure_revision_type_de_champ.rb @@ -1,4 +1,16 @@ FactoryBot.define do factory :procedure_revision_type_de_champ do + transient do + from_original { nil } + end + + after(:build) do |revision_type_de_champ, evaluator| + if evaluator.from_original + original = evaluator.from_original + + revision_type_de_champ.type_de_champ = original.type_de_champ + revision_type_de_champ.position = original.position + end + end end end diff --git a/spec/helpers/dossier_helper_spec.rb b/spec/helpers/dossier_helper_spec.rb index d1e47d61d..9835cba2d 100644 --- a/spec/helpers/dossier_helper_spec.rb +++ b/spec/helpers/dossier_helper_spec.rb @@ -49,7 +49,7 @@ RSpec.describe DossierHelper, type: :helper do let(:procedure) { create(:simple_procedure, :for_individual) } context "when the individual is not provided" do - let(:individual) { nil } + let(:individual) { build(:individual, :empty) } it { is_expected.to be_blank } end diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb index c667d6cd4..f75be8e2c 100644 --- a/spec/mailers/application_mailer_spec.rb +++ b/spec/mailers/application_mailer_spec.rb @@ -1,6 +1,6 @@ RSpec.describe ApplicationMailer, type: :mailer do describe 'dealing with invalid emails' do - let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) } + let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) } subject { DossierMailer.notify_new_draft(dossier) } describe 'invalid emails are not sent' do diff --git a/spec/mailers/dossier_mailer_spec.rb b/spec/mailers/dossier_mailer_spec.rb index e521c94de..87e4bcdca 100644 --- a/spec/mailers/dossier_mailer_spec.rb +++ b/spec/mailers/dossier_mailer_spec.rb @@ -12,7 +12,7 @@ RSpec.describe DossierMailer, type: :mailer do end describe '.notify_new_draft' do - let(:dossier) { create(:dossier, procedure: build(:simple_procedure, :with_auto_archive)) } + let(:dossier) { create(:dossier, procedure: create(:simple_procedure, :with_auto_archive)) } subject { described_class.notify_new_draft(dossier) } @@ -27,7 +27,7 @@ RSpec.describe DossierMailer, type: :mailer do end describe '.notify_new_answer with dossier brouillon' do - let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) } + let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) } let(:commentaire) { create(:commentaire, dossier: dossier) } subject { described_class.with(commentaire: commentaire).notify_new_answer } @@ -39,8 +39,9 @@ RSpec.describe DossierMailer, type: :mailer do end describe '.notify_new_answer with dossier en construction' do - let(:dossier) { create(:dossier, state: "en_construction", procedure: build(:simple_procedure)) } + let(:dossier) { create(:dossier, :en_construction, procedure: create(:simple_procedure)) } let(:commentaire) { create(:commentaire, dossier: dossier) } + subject { described_class.with(commentaire: commentaire).notify_new_answer } it { expect(subject.subject).to include("Nouveau message") } @@ -51,7 +52,7 @@ RSpec.describe DossierMailer, type: :mailer do end describe '.notify_new_answer with commentaire discarded' do - let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) } + let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) } let(:commentaire) { create(:commentaire, dossier: dossier, discarded_at: 2.minutes.ago) } subject { described_class.with(commentaire: commentaire).notify_new_answer } @@ -83,7 +84,7 @@ RSpec.describe DossierMailer, type: :mailer do end describe '.notify_revert_to_instruction' do - let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) } + let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) } subject { described_class.notify_revert_to_instruction(dossier) } diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index c3d45668a..9dce6c912 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -320,8 +320,8 @@ describe ProcedurePresentation do let(:procedure) { create(:procedure, :for_individual) } - let!(:first_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Alain', nom: 'Antonelli')) } - let!(:last_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'Mme', prenom: 'Zora', nom: 'Zemmour')) } + let!(:first_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'M', prenom: 'Alain', nom: 'Antonelli')) } + let!(:last_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'Mme', prenom: 'Zora', nom: 'Zemmour')) } context 'for gender column' do let(:column) { 'gender' } @@ -617,8 +617,8 @@ describe ProcedurePresentation do context 'for individual table' do let(:procedure) { create(:procedure, :for_individual) } - let!(:kept_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'Mme', prenom: 'Josephine', nom: 'Baker')) } - let!(:discarded_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Jean', nom: 'Tremblay')) } + let!(:kept_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'Mme', prenom: 'Josephine', nom: 'Baker')) } + let!(:discarded_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'M', prenom: 'Jean', nom: 'Tremblay')) } context 'for gender column' do let(:filter) { [{ 'table' => 'individual', 'column' => 'gender', 'value' => 'Mme' }] } @@ -646,7 +646,7 @@ describe ProcedurePresentation do ] end - let!(:other_kept_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Romuald', nom: 'Pistis')) } + let!(:other_kept_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'M', prenom: 'Romuald', nom: 'Pistis')) } it 'returns every dossier that matches any of the search criteria for a given column' do is_expected.to contain_exactly(kept_dossier.id, other_kept_dossier.id) diff --git a/spec/services/dossier_projection_service_spec.rb b/spec/services/dossier_projection_service_spec.rb index 096ac0e07..d08d8d396 100644 --- a/spec/services/dossier_projection_service_spec.rb +++ b/spec/services/dossier_projection_service_spec.rb @@ -92,7 +92,7 @@ describe DossierProjectionService do context 'for individual table' do let(:table) { 'individual' } let(:procedure) { create(:procedure, :for_individual, :with_type_de_champ, :with_type_de_champ_private) } - let(:dossier) { create(:dossier, procedure: procedure, individual: create(:individual, nom: 'Martin', prenom: 'Jacques', gender: 'M.')) } + let(:dossier) { create(:dossier, procedure: procedure, individual: build(:individual, nom: 'Martin', prenom: 'Jacques', gender: 'M.')) } context 'for prenom column' do let(:column) { 'prenom' } diff --git a/spec/system/accessibilite/wcag_usager_spec.rb b/spec/system/accessibilite/wcag_usager_spec.rb index 5ac9658aa..a563d5e0e 100644 --- a/spec/system/accessibilite/wcag_usager_spec.rb +++ b/spec/system/accessibilite/wcag_usager_spec.rb @@ -1,5 +1,5 @@ describe 'wcag rules for usager', js: true do - let(:procedure) { create(:procedure, :with_type_de_champ, :with_all_champs, :with_service, :for_individual, :published) } + let(:procedure) { create(:procedure, :published, :with_all_champs, :with_service, :for_individual) } let(:password) { 'a very complicated password' } let(:litteraire_user) { create(:user, password: password) } diff --git a/spec/views/shared/dossiers/_demande.html.haml_spec.rb b/spec/views/shared/dossiers/_demande.html.haml_spec.rb index cb48f4de2..9d166cc43 100644 --- a/spec/views/shared/dossiers/_demande.html.haml_spec.rb +++ b/spec/views/shared/dossiers/_demande.html.haml_spec.rb @@ -32,7 +32,7 @@ describe 'shared/dossiers/demande.html.haml', type: :view do end context 'when dossier was created by an individual' do - let(:individual) { create(:individual) } + let(:individual) { build(:individual) } it 'renders the individual identity infos' do expect(subject).to include(individual.gender) From 032e3e2cb17f958793c7e5398e1ce189e72d563b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 14:41:47 +0000 Subject: [PATCH 19/47] specs: create the procedure administrateur declaratively It seems better to create associations in an declarative fashion, rather than using imperative code. This also makes the attribute compatible with build_stubbed. --- spec/factories/procedure.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 41e2b83fa..e7f7aee85 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -13,6 +13,8 @@ FactoryBot.define do lien_site_web { "https://mon-site.gouv" } path { SecureRandom.uuid } + administrateurs { administrateur.present? ? [administrateur] : [association(:administrateur)] } + transient do administrateur { } instructeurs { [] } @@ -22,12 +24,6 @@ FactoryBot.define do end after(:build) do |procedure, evaluator| - if evaluator.administrateur - procedure.administrateurs = [evaluator.administrateur] - elsif procedure.administrateurs.empty? - procedure.administrateurs = [build(:administrateur)] - end - initial_revision = build(:procedure_revision, procedure: procedure) add_types_de_champs(evaluator.types_de_champ, to: initial_revision, scope: :public) add_types_de_champs(evaluator.types_de_champ_private, to: initial_revision, scope: :private) From 0e809ac1a2e9775e1a7418c9b94412d27babfce6 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 15:14:40 +0000 Subject: [PATCH 20/47] specs: build the default groupe instructeur before the create callback This allows for better ergonomics, by having the default groupe instructeur available even before the procedure is saved. --- spec/factories/groupe_instructeur.rb | 4 ++++ spec/factories/procedure.rb | 1 + 2 files changed, 5 insertions(+) diff --git a/spec/factories/groupe_instructeur.rb b/spec/factories/groupe_instructeur.rb index f86ab4c11..db563a2f1 100644 --- a/spec/factories/groupe_instructeur.rb +++ b/spec/factories/groupe_instructeur.rb @@ -4,5 +4,9 @@ FactoryBot.define do factory :groupe_instructeur do label { generate(:groupe_label) } association :procedure + + trait :default do + label { GroupeInstructeur::DEFAUT_LABEL } + end end end diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index e7f7aee85..36925e66b 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -13,6 +13,7 @@ FactoryBot.define do lien_site_web { "https://mon-site.gouv" } path { SecureRandom.uuid } + groupe_instructeurs { [association(:groupe_instructeur, :default, procedure: instance, strategy: :build)] } administrateurs { administrateur.present? ? [administrateur] : [association(:administrateur)] } transient do From f2233300b6fcec5cb29dacc71f9d7d683372998a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 15:23:48 +0000 Subject: [PATCH 21/47] specs: assign the default groupe instructeur declaratively --- spec/factories/dossier.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 50182c714..0d603c789 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -4,6 +4,7 @@ FactoryBot.define do state { Dossier.states.fetch(:brouillon) } user { association :user } + groupe_instructeur { procedure.routee? ? nil : procedure.defaut_groupe_instructeur } transient do for_individual? { false } @@ -19,11 +20,6 @@ FactoryBot.define do dossier.revision = procedure.active_revision - # Assign the procedure to the dossier through the groupe_instructeur - if dossier.groupe_instructeur.nil? - dossier.groupe_instructeur = procedure.routee? ? nil : procedure.defaut_groupe_instructeur - end - if procedure.for_individual? && dossier.individual.blank? build(:individual, :empty, dossier: dossier) end From 2203a7762f7c7e4256163228535f83b06c35c683 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 16:42:54 +0100 Subject: [PATCH 22/47] specs: create dossier revision and individual declaratively --- spec/factories/dossier.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 0d603c789..f2452791a 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -5,6 +5,8 @@ FactoryBot.define do user { association :user } groupe_instructeur { procedure.routee? ? nil : procedure.defaut_groupe_instructeur } + revision { procedure.active_revision } + individual { association(:individual, :empty, dossier: instance, strategy: :build) if procedure.for_individual? } transient do for_individual? { false } @@ -15,16 +17,6 @@ FactoryBot.define do procedure { create(:procedure, :published, :with_type_de_champ, :with_type_de_champ_private, for_individual: for_individual?) } end - after(:build) do |dossier, evaluator| - procedure = evaluator.procedure - - dossier.revision = procedure.active_revision - - if procedure.for_individual? && dossier.individual.blank? - build(:individual, :empty, dossier: dossier) - end - end - trait :with_entreprise do after(:build) do |dossier, _evaluator| if dossier.procedure.for_individual? From 997c1979807fd315a8b340108f3bbcfb54cb14d6 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Fri, 19 Nov 2021 16:25:32 +0000 Subject: [PATCH 23/47] specs: simplify the syntax of some factories --- .../new_administrateur/services_controller_spec.rb | 2 +- spec/factories/dossier.rb | 4 +--- spec/factories/procedure.rb | 8 ++------ spec/factories/service.rb | 2 +- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/spec/controllers/new_administrateur/services_controller_spec.rb b/spec/controllers/new_administrateur/services_controller_spec.rb index 9634148f3..21c0ca13d 100644 --- a/spec/controllers/new_administrateur/services_controller_spec.rb +++ b/spec/controllers/new_administrateur/services_controller_spec.rb @@ -131,7 +131,7 @@ describe NewAdministrateur::ServicesController, type: :controller do end it { expect(service.reload).not_to be_nil } - it { expect(flash.alert).to eq("la démarche #{procedure.libelle} utilise encore le service service. Veuillez l'affecter à un autre service avant de pouvoir le supprimer") } + it { expect(flash.alert).to eq("la démarche #{procedure.libelle} utilise encore le service #{service.nom}. Veuillez l'affecter à un autre service avant de pouvoir le supprimer") } it { expect(flash.notice).to be_nil } it { expect(response).to redirect_to(admin_services_path(procedure_id: 12)) } end diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index f2452791a..6bee4d8d0 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -90,9 +90,7 @@ FactoryBot.define do end trait :with_commentaires do - after(:create) do |dossier, _evaluator| - dossier.commentaires += create_list(:commentaire, 2) - end + commentaires { [build(:commentaire), build(:commentaire)] } end trait :followed do diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 36925e66b..1b6a622d2 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -80,9 +80,7 @@ FactoryBot.define do end trait :with_service do - after(:build) do |procedure, _evaluator| - procedure.service = create(:service) - end + service { association :service, administrateur: administrateurs.first } end trait :with_instructeur do @@ -98,9 +96,7 @@ FactoryBot.define do end trait :for_individual do - after(:build) do |procedure, _evaluator| - procedure.for_individual = true - end + for_individual { true } end trait :with_auto_archive do diff --git a/spec/factories/service.rb b/spec/factories/service.rb index 64dcaa9f9..a3215328e 100644 --- a/spec/factories/service.rb +++ b/spec/factories/service.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :service do - nom { 'service' } + sequence(:nom) { |n| "Service #{n}" } organisme { 'organisme' } type_organisme { Service.type_organismes.fetch(:association) } email { 'email@toto.com' } From 91109dc944c48a8f80c522f531f42622cd17703e Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 15 Oct 2021 16:15:14 +0200 Subject: [PATCH 24/47] bump openid connect (mandatory to avoid lets encrypt certs pb) --- Gemfile.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 913c4f27a..ffa5f6467 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -421,7 +421,7 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2021.0212) mini_magick (4.11.0) - mini_mime (1.1.1) + mini_mime (1.1.2) mini_portile2 (2.6.1) minitest (5.14.4) momentjs-rails (2.20.1) @@ -437,7 +437,7 @@ GEM mini_portile2 (~> 2.6.1) racc (~> 1.4) open4 (1.3.4) - openid_connect (1.2.0) + openid_connect (1.3.0) activemodel attr_required (>= 1.0.0) json-jwt (>= 1.5.0) @@ -492,7 +492,7 @@ GEM rack (>= 1.0, < 3) rack-mini-profiler (2.3.1) rack (>= 1.2.0) - rack-oauth2 (1.16.0) + rack-oauth2 (1.19.0) activesupport attr_required httpclient @@ -700,7 +700,7 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - swd (1.2.0) + swd (1.3.0) activesupport (>= 3) attr_required (>= 0.0.5) httpclient (>= 2.4) @@ -747,7 +747,7 @@ GEM nokogiri (~> 1.6) rubyzip (>= 1.3.0) selenium-webdriver (>= 3.0, < 4.0) - webfinger (1.1.0) + webfinger (1.2.0) activesupport httpclient (>= 2.4) webmock (3.11.2) From ed7d776727d15c586c092fbb6657d287ea9eb654 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 17 Nov 2021 17:49:58 +0100 Subject: [PATCH 25/47] Bump jwt dep to decrypt ES256 FCA response --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index ffa5f6467..7c61d7b1c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -376,7 +376,7 @@ GEM regexp_parser (~> 2.0) uri_template (~> 0.7) jsonapi-renderer (0.2.2) - jwt (2.2.2) + jwt (2.3.0) kaminari (1.2.1) activesupport (>= 4.1.0) kaminari-actionview (= 1.2.1) From d2432e34ebf61ee6bd8861ee0352a75c4a594ef2 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 10:00:04 +0100 Subject: [PATCH 26/47] AgentConnect UI --- app/assets/images/logo-agent-connect.png | Bin 0 -> 3587 bytes .../stylesheets/france-connect-agent-login.scss | 11 +++++++++++ .../agent_connect/agent_controller.rb | 4 ++++ app/services/agent_connect_service.rb | 5 +++++ app/views/agent_connect/agent/index.html.haml | 6 ++++++ app/views/users/sessions/new.html.haml | 8 +++++++- config/env.example.optional | 3 +++ config/locales/en.yml | 2 ++ config/locales/fr.yml | 2 ++ config/locales/views/agent_connect/agent/en.yml | 11 +++++++++++ config/locales/views/agent_connect/agent/fr.yml | 11 +++++++++++ config/routes.rb | 4 ++++ 12 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 app/assets/images/logo-agent-connect.png create mode 100644 app/assets/stylesheets/france-connect-agent-login.scss create mode 100644 app/controllers/agent_connect/agent_controller.rb create mode 100644 app/services/agent_connect_service.rb create mode 100644 app/views/agent_connect/agent/index.html.haml create mode 100644 config/locales/views/agent_connect/agent/en.yml create mode 100644 config/locales/views/agent_connect/agent/fr.yml diff --git a/app/assets/images/logo-agent-connect.png b/app/assets/images/logo-agent-connect.png new file mode 100644 index 0000000000000000000000000000000000000000..6ad68703c82105c2ece68128f5aca64c62d7ffcc GIT binary patch literal 3587 zcmV+e4*c0K4`Y)9GFO~Z)l>07}`!1FHE|vQ-m;W=F|NsC0E|mHMPNMA`Bmim5GM4)1^8Wt* z|N8v?w%PsU@&2*b{e8Uq_WJ%Wl=`mK{qgwzX0ZGH{{Ft+{YjzvO{4l>uKP5X`n%ly zLZ15J@c#7q{%o}So6P(>oBBVU`nKBr&E@^z@BV1A`*OGY`1}6+{r*#@`l8VM`uqJo zoccDH`qAh8jm7+bzWc4!{dT$g%;f!BtNY93{qXnxM4$SL!~8gz`k>DIOQZUT!u;s- z{!OI%W3T&qyZfcl{Oi*m8{+P=A&*uF_p!%WD{Di># z$m9Lp?f$vj{RL2?fxi4trTW$B{=whkJ0dAj@VCNA0Q{_ytv4OXY~_WH)%^t0Ig)am{I`TB>e)4|l~?>a&5F*&@@=23;e z7hI}Gq55!}%bU2`V35X+uhlMWvl3aTCTFh?X;_CE2d%fT7`P}OGgtq2~Yn61U-d}O2{p96roYfp&sraL&|NZ^HO=7!U zcH}5CIa!;3wC47Xl7n%g=X8Aj@$znav))f#p}pKLSe~E9?C|vZugBs_f4tY@@m*n; zE`G~Nf4s5L@3m)vNQTGvmYU7s^_#@&da>nFn%%|Vuopm~3o)tBdT+%{e8^+!nWOF(2WW z##lc982Td`v#JS_E-$Y)oH7)N}h}Pb!~y{r7_2CMb@@4B1%Zi zTQa7obV@Nt@+6M${_-J#a>Xkm=I=HMocDk41KC|zHL0@20C+Z7iTW|OH2#)A{=AIi zwxZFTK$HMi+i0(%(kaBetW#pR=;xa>*S&K&$8lfRikRHf8jWVtQ_Ho7eij$PU2rti zR9MpVhEhtm5 z@@lD=enRI%A9D{Ah`0I1y$FfvN(?5?`}D@9*!t&g(8%(%U`zp%d-5r*EVthwmKpk6!k8o|g~m8}aJbE-_#@Ow4ZuDK)=7 zeDz)8;&RFEEeDndIUiHOq=XvGUM)Z9qA6v)pScpVkU=yPY;NxKl#>qmL=IiNQSP*{ zNS2ejXew>!;EBc%L$UB%B;V3n>mbTRV@iA~`=0vRpgw-Ip^nJ7q%Ta&Ef33(*Ss}D zu|c`%(su)t&umh|Bn%zN*ZF|XICViFbjkru#AgHxV@awWeC zi-nHK)|^6h%wegZTg(qE$yFsw;!SfeNv+FLiIU8Xh5=IO>-KaCdn#x6nBV`&HIpd;_pPt8Mdjdjbsi=8QF!E3H}A0lhw-by~_t9P2beea4w?=3*D01m=sXlz5%v-qk*{ zDHJAAvb}qJMrl=*BM{ZL*G-x&jODp1Tfut!q%PQ!#~@`|`6 z3poW%gZ*Xz^Q91&Z*bhR*G3NJqaiSFDBI}hB-aomHEgSl7Dl%9Pr zzQkUB?SGBwu+;Pa56rHWaVnU5x?VcEg5$WKJ|7aZ96)uLjp%im0rx!nr;qu*_HQbf zGxi+CK4#7zhs10FP%|aVw9gl#TgDi9W6*fA6tb#tF)u%;y;83}2bo{yIPUEop)d^? z1*T+|HWUfJeLz#D!$B90#Dy9Xn8;~kG-c4@Z zR+FiOsPx>;wuLO5kz+Dh5^D@CJk2pR!*4S6QnDUR!q(ZjNU&|i>}4H3D|2>Ci%K@z zin7Nii82RkDmw*7Yjw1%Cb1x>Doo4^@ZsUxkNx|oI_9H7T<8gP%vtdAX<$2F8Ulo) zrgh}(5X!a@B>^6mJePeBCTWxpC=O-#`9gNhDxw&%Xq- z>v(dpK*>F*ikV`AkH!FIm`YSInCgD2hBvFc4CXP0>8>p?K%X-wrUU+%m@KLW6n(W3 zNQ~`?!zfQNKvTt5Gy6J#JuwufIE(b_78+bMGdn6 zEUeLtc|^RK1wm?1PrM#2R%@{z?F=(k=-HOQfJ%i;;o#aZAgbx77>wo;ZNLUnCVSv5 zw_1;&hK=b$BEJ5l$L5vV!yny!=q`24OC`)l)G*fq8wLjaKsiwds+|kqDW%nN)nSG? zhsdbM#z5QB0IPC2+9-C4G>SpGU|ts~lXAEmDe3I-lT1Oj{~Xe4@1CJb=@Btk-1&kE zrqv@C>mjlWgQ$gQ1js95vQfgMIJRMw+EyTTaZy%*1$bG1@JVblVIXC40DeK&01aU9 zlVJiM{P*5}sT=cxE=GCjeNYQWmjKZ|pU=0RHKfG3+$h!z0e_BHi=HjFwb4-MIrn+nhB5SVtLFNGXSlPMFZ z4xaC>hYQ`*DCzNT< zCn_`pn+*`BpaA9^58xhVab)?+wi9K0fc0*$Sh_kU4Fa}2H`eD;B4G1_^;cO{IOm&z zJ2K`A14lx^g`{h@yqu<%Ppnx6NFnzDEKBKu*~30aEYXOIg)>^lTa#jK(r4*qul0Gt z@>$=^ik28dVy3jQG2h%xlv2jj7g?36!a1!D+;}=aNV!uOlibVSl%5w_PT+vmn{`g2 z;v&o!4ir_wBVi2RjdF2002ov JPDHLkV1iTadZYjV literal 0 HcmV?d00001 diff --git a/app/assets/stylesheets/france-connect-agent-login.scss b/app/assets/stylesheets/france-connect-agent-login.scss new file mode 100644 index 000000000..1d1139385 --- /dev/null +++ b/app/assets/stylesheets/france-connect-agent-login.scss @@ -0,0 +1,11 @@ +@import "colors"; +@import "constants"; + +.france-connect-agent-login-button { + background-image: image-url("logo-agent-connect.png"); + display: block; + height: 60px; + width: 230px; + margin: 20px auto; + font-size: 0; +} diff --git a/app/controllers/agent_connect/agent_controller.rb b/app/controllers/agent_connect/agent_controller.rb new file mode 100644 index 000000000..17bf6cf92 --- /dev/null +++ b/app/controllers/agent_connect/agent_controller.rb @@ -0,0 +1,4 @@ +class AgentConnect::AgentController < ApplicationController + def index + end +end diff --git a/app/services/agent_connect_service.rb b/app/services/agent_connect_service.rb new file mode 100644 index 000000000..1887d39f4 --- /dev/null +++ b/app/services/agent_connect_service.rb @@ -0,0 +1,5 @@ +class AgentConnectService + def self.enabled? + ENV.fetch("AGENT_CONNECT_ENABLED", "enabled") == "enabled" + end +end diff --git a/app/views/agent_connect/agent/index.html.haml b/app/views/agent_connect/agent/index.html.haml new file mode 100644 index 000000000..04340689f --- /dev/null +++ b/app/views/agent_connect/agent/index.html.haml @@ -0,0 +1,6 @@ +.container + %h1.mt-2.mb-2= t('.connect') + + %p= t('.intro_html', app_name: APPLICATION_NAME) + + = link_to t('.cta'), agent_connect_login_path, class: "france-connect-agent-login-button" diff --git a/app/views/users/sessions/new.html.haml b/app/views/users/sessions/new.html.haml index 77e9c79ca..5e72e1f9b 100644 --- a/app/views/users/sessions/new.html.haml +++ b/app/views/users/sessions/new.html.haml @@ -23,7 +23,13 @@ = f.submit t('views.users.sessions.new.connection'), class: "button large primary expand" - %hr + .france-connect-login-separator + = t('views.shared.france_connect_login.separator') + - if AgentConnectService.enabled? + .center + %p.mb-2= t('views.users.sessions.new.instructor_or_admin') + = link_to t('views.users.sessions.new.connect_with_agent_connect'), agent_connect_path + %hr %p.center %span= t('views.users.sessions.new.are_you_new', app_name: APPLICATION_NAME.gsub("-","‑")).html_safe %br diff --git a/config/env.example.optional b/config/env.example.optional index 0c2b3a4c4..ec0247dac 100644 --- a/config/env.example.optional +++ b/config/env.example.optional @@ -11,6 +11,9 @@ DS_ENV="staging" # Utilisation de France Connect # FRANCE_CONNECT_ENABLED="disabled" # "enabled" par défaut +# Utilisation de Agent Connect +# AGENT_CONNECT_ENABLED="disabled" # "enabled" par défaut + # Personnalisation d'instance - URLs des CGU et des mentions légales # CGU_URL="" # MENTIONS_LEGALES_URL="" diff --git a/config/locales/en.yml b/config/locales/en.yml index 5c6bccab0..024f61992 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -203,6 +203,8 @@ en: connection: Sign in are_you_new: First time on %{app_name}? find_procedure: Find your procedure + instructor_or_admin: Instructor or Administrator ? + connect_with_agent_connect: Connect with AgentConnect passwords: reset_link_sent: got_it: Got it! diff --git a/config/locales/fr.yml b/config/locales/fr.yml index d2361db5a..7db748382 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -199,6 +199,8 @@ fr: connection: Se connecter are_you_new: Vous êtes nouveau sur %{app_name} ? find_procedure: Trouvez votre démarche + instructor_or_admin: Vous êtes instructeur ou administrateur ? + connect_with_agent_connect: Se connecter avec AgentConnect passwords: reset_link_sent: email_sent_html: "Nous vous avons envoyé un email à l’adresse %{email}." diff --git a/config/locales/views/agent_connect/agent/en.yml b/config/locales/views/agent_connect/agent/en.yml new file mode 100644 index 000000000..7000b794f --- /dev/null +++ b/config/locales/views/agent_connect/agent/en.yml @@ -0,0 +1,11 @@ +en: + agent_connect: + agent: + index: + connect: Connect with AgentConnect + intro_html: | + AgentConnect allows instructors et administrators to use their usual login credentials to connect to %{app_name}. +
+
+ Only agents of the Ministry of Ecological Transition can currently benefit from it. + cta: Connect with AgentConnect diff --git a/config/locales/views/agent_connect/agent/fr.yml b/config/locales/views/agent_connect/agent/fr.yml new file mode 100644 index 000000000..ede7c2ece --- /dev/null +++ b/config/locales/views/agent_connect/agent/fr.yml @@ -0,0 +1,11 @@ +fr: + agent_connect: + agent: + index: + connect: Connectez-vous avec AgentConnect + intro_html: | + AgentConnect permet aux instructeurs et administrateurs d’utiliser leurs identifiants habituels pour se connecter à %{app_name}. +
+
+ Seul les agents du ministère de la Transition écologique peuvent actuellement en bénéficier. + cta: S’identifier avec Agent Connect diff --git a/config/routes.rb b/config/routes.rb index b9c643cdb..738af7695 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -129,6 +129,10 @@ Rails.application.routes.draw do post 'particulier/merge_with_new_account' => 'particulier#merge_with_new_account' end + namespace :agent_connect do + get '' => 'agent#index' + end + namespace :champs do get ':position/siret', to: 'siret#show', as: :siret get ':position/dossier_link', to: 'dossier_link#show', as: :dossier_link From 45ce27472186c0660dcb981c156efc859a9b20a5 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 12:01:27 +0100 Subject: [PATCH 27/47] add agent connect secrets --- config/env.example | 6 ++++++ config/initializers/agent_connect.rb | 1 + config/secrets.yml | 8 ++++++++ 3 files changed, 15 insertions(+) create mode 100644 config/initializers/agent_connect.rb diff --git a/config/env.example b/config/env.example index 48266c857..d959bd33d 100644 --- a/config/env.example +++ b/config/env.example @@ -45,6 +45,12 @@ FC_PARTICULIER_ID="" FC_PARTICULIER_SECRET="" FC_PARTICULIER_BASE_URL="" +# Service externe: authentification Agent Connect +AGENT_CONNECT_ID="" +AGENT_CONNECT_SECRET="" +AGENT_CONNECT_BASE_URL="" +AGENT_CONNECT_JWKS="" + # Service externe: Support Utilisateur HelpScout | Spécifique démarches-simplifiées.fr HELPSCOUT_MAILBOX_ID="" HELPSCOUT_CLIENT_ID="" diff --git a/config/initializers/agent_connect.rb b/config/initializers/agent_connect.rb new file mode 100644 index 000000000..1e9f1d32c --- /dev/null +++ b/config/initializers/agent_connect.rb @@ -0,0 +1 @@ +AGENT_CONNECT = Rails.application.secrets.agent_connect diff --git a/config/secrets.yml b/config/secrets.yml index 1199d404a..d6c545141 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -25,6 +25,14 @@ defaults: &defaults token_endpoint: <%= ENV['FC_PARTICULIER_BASE_URL'] %>/api/v1/token userinfo_endpoint: <%= ENV['FC_PARTICULIER_BASE_URL'] %>/api/v1/userinfo logout_endpoint: <%= ENV['FC_PARTICULIER_BASE_URL'] %>/api/v1/logout + agent_connect: + identifier: <%= ENV['AGENT_CONNECT_ID'] %> + secret: <%= ENV['AGENT_CONNECT_SECRET'] %> + redirect_uri: http://test.localhost:3000/agent_connect/callback + authorization_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/authorize + token_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/token + userinfo_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/userinfo + logout_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/session/end mailjet: api_key: <%= ENV['MAILJET_API_KEY'] %> secret_key: <%= ENV['MAILJET_SECRET_KEY'] %> From 898df449d42c7c82fe471aa0723595124cdf1373 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 10:21:47 +0100 Subject: [PATCH 28/47] redirect to AgentConnect --- app/controllers/agent_connect/agent_controller.rb | 4 ++++ app/models/agent_connect_client.rb | 5 +++++ app/services/agent_connect_service.rb | 11 +++++++++++ config/routes.rb | 1 + 4 files changed, 21 insertions(+) create mode 100644 app/models/agent_connect_client.rb diff --git a/app/controllers/agent_connect/agent_controller.rb b/app/controllers/agent_connect/agent_controller.rb index 17bf6cf92..718a866de 100644 --- a/app/controllers/agent_connect/agent_controller.rb +++ b/app/controllers/agent_connect/agent_controller.rb @@ -1,4 +1,8 @@ class AgentConnect::AgentController < ApplicationController def index end + + def login + redirect_to AgentConnectService.authorization_uri + end end diff --git a/app/models/agent_connect_client.rb b/app/models/agent_connect_client.rb new file mode 100644 index 000000000..45cab0bb0 --- /dev/null +++ b/app/models/agent_connect_client.rb @@ -0,0 +1,5 @@ +class AgentConnectClient < OpenIDConnect::Client + def initialize + super(AGENT_CONNECT) + end +end diff --git a/app/services/agent_connect_service.rb b/app/services/agent_connect_service.rb index 1887d39f4..95d422f37 100644 --- a/app/services/agent_connect_service.rb +++ b/app/services/agent_connect_service.rb @@ -2,4 +2,15 @@ class AgentConnectService def self.enabled? ENV.fetch("AGENT_CONNECT_ENABLED", "enabled") == "enabled" end + + def self.authorization_uri + client = AgentConnectClient.new + + client.authorization_uri( + scope: [:openid, :email], + state: SecureRandom.hex(16), + nonce: SecureRandom.hex(16), + acr_values: 'eidas1' + ) + end end diff --git a/config/routes.rb b/config/routes.rb index 738af7695..fa89733a0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -131,6 +131,7 @@ Rails.application.routes.draw do namespace :agent_connect do get '' => 'agent#index' + get 'login' => 'agent#login' end namespace :champs do From 3316dfc866928e1cf3414e7e7f0402fcee0917b5 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 10:23:30 +0100 Subject: [PATCH 29/47] reopen openid_connect gem to support AC encrypted jwt response --- config/initializers/open_id_connect.rb | 56 ++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/config/initializers/open_id_connect.rb b/config/initializers/open_id_connect.rb index 333374682..5266653b8 100644 --- a/config/initializers/open_id_connect.rb +++ b/config/initializers/open_id_connect.rb @@ -3,3 +3,59 @@ OpenIDConnect.logger = Rails.logger Rack::OAuth2.logger = Rails.logger # Webfinger.logger = Rails.logger SWD.logger = Rails.logger + +# the openid_connect gem does not support +# jwt format in the userinfo call. +# A PR is open to improve the situation +# https://github.com/nov/openid_connect/pull/54 +module OpenIDConnect + class AccessToken < Rack::OAuth2::AccessToken::Bearer + private + + def jwk_loader + JSON.parse(URI.parse(ENV['AGENT_CONNECT_JWKS']).read).deep_symbolize_keys + end + + def decode_jwt(requested_host, jwt) + agent_connect_host = URI.parse(ENV['AGENT_CONNECT_BASE_URL']).host + + if requested_host == agent_connect_host + # rubocop:disable Lint/UselessAssignment + JWT.decode(jwt, key = nil, verify = true, { algorithms: ['ES256'], jwks: jwk_loader })[0] + # rubocop:enable Lint/UselessAssignment + else + raise "unknwon host : #{requested_host}" + end + end + + def resource_request + res = yield + case res.status + when 200 + hash = case parse_type_and_subtype(res.content_type) + when 'application/jwt' + requested_host = URI.parse(client.userinfo_endpoint).host + decode_jwt(requested_host, res.body) + when 'application/json' + JSON.parse(res.body) + end + hash&.with_indifferent_access + when 400 + raise BadRequest.new('API Access Faild', res) + when 401 + raise Unauthorized.new('Access Token Invalid or Expired', res) + when 403 + raise Forbidden.new('Insufficient Scope', res) + else + raise HttpError.new(res.status, 'Unknown HttpError', res) + end + end + + # https://datatracker.ietf.org/doc/html/rfc2045#section-5.1 + # - type and subtype are the first member + # they are case insensitive + def parse_type_and_subtype(content_type) + content_type.split(';')[0].strip.downcase + end + end +end From 1926a630f9d0e6600a8903ae3708a292d66a6598 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 15:24:54 +0100 Subject: [PATCH 30/47] add agent_connect_id to instructeur --- app/models/instructeur.rb | 1 + ...46_add_agent_connect_sub_column_to_instructeurs_table.rb | 6 ++++++ db/schema.rb | 5 ++++- 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20211119112046_add_agent_connect_sub_column_to_instructeurs_table.rb diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index de70468f2..4dd9fb139 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -8,6 +8,7 @@ # login_token_created_at :datetime # created_at :datetime # updated_at :datetime +# agent_connect_id :string # class Instructeur < ApplicationRecord has_many :administrateurs_instructeurs diff --git a/db/migrate/20211119112046_add_agent_connect_sub_column_to_instructeurs_table.rb b/db/migrate/20211119112046_add_agent_connect_sub_column_to_instructeurs_table.rb new file mode 100644 index 000000000..0a710ebc8 --- /dev/null +++ b/db/migrate/20211119112046_add_agent_connect_sub_column_to_instructeurs_table.rb @@ -0,0 +1,6 @@ +class AddAgentConnectSubColumnToInstructeursTable < ActiveRecord::Migration[6.1] + def change + add_column :instructeurs, :agent_connect_id, :string + add_index :instructeurs, :agent_connect_id, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 0ad37d610..f2d3b5e78 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,8 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_11_15_112933) do +ActiveRecord::Schema.define(version: 2021_11_19_112046) do + # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "unaccent" @@ -534,6 +535,8 @@ ActiveRecord::Schema.define(version: 2021_11_15_112933) do t.text "encrypted_login_token" t.datetime "login_token_created_at" t.boolean "bypass_email_login_token", default: false, null: false + t.string "agent_connect_id" + t.index ["agent_connect_id"], name: "index_instructeurs_on_agent_connect_id", unique: true end create_table "invites", id: :serial, force: :cascade do |t| From 5234a1854cc84ae5a7bbbdc5dea09d7c26fd9604 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 15:24:54 +0100 Subject: [PATCH 31/47] manage AgentConnect callback --- .../agent_connect/agent_controller.rb | 32 +++++++++++++++++++ app/models/agent_connect_client.rb | 6 +++- app/services/agent_connect_service.rb | 8 +++++ config/routes.rb | 1 + 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/app/controllers/agent_connect/agent_controller.rb b/app/controllers/agent_connect/agent_controller.rb index 718a866de..b24369c1b 100644 --- a/app/controllers/agent_connect/agent_controller.rb +++ b/app/controllers/agent_connect/agent_controller.rb @@ -1,3 +1,4 @@ +# doc: https://github.com/france-connect/Documentation-AgentConnect class AgentConnect::AgentController < ApplicationController def index end @@ -5,4 +6,35 @@ class AgentConnect::AgentController < ApplicationController def login redirect_to AgentConnectService.authorization_uri end + + def callback + user_info = AgentConnectService.user_info(params[:code]) + + instructeur = Instructeur.find_by(agent_connect_id: user_info['sub']) + + if instructeur.nil? + instructeur = Instructeur.find_by(users: { email: santized_email(user_info) }) + instructeur&.update(agent_connect_id: user_info['sub']) + end + + if instructeur.nil? + user = User.create_or_promote_to_instructeur(santized_email(user_info), Devise.friendly_token[0, 20]) + instructeur = user.instructeur + instructeur.update(agent_connect_id: user_info['sub']) + end + + sign_in(:user, instructeur.user) + + redirect_to instructeur_procedures_path + + rescue Rack::OAuth2::Client::Error => e + Rails.logger.error e.message + redirect_france_connect_error_connection + end + + private + + def santized_email(user_info) + user_info['email'].strip.downcase + end end diff --git a/app/models/agent_connect_client.rb b/app/models/agent_connect_client.rb index 45cab0bb0..f18eb4a63 100644 --- a/app/models/agent_connect_client.rb +++ b/app/models/agent_connect_client.rb @@ -1,5 +1,9 @@ class AgentConnectClient < OpenIDConnect::Client - def initialize + def initialize(code = nil) super(AGENT_CONNECT) + + if code.present? + self.authorization_code = code + end end end diff --git a/app/services/agent_connect_service.rb b/app/services/agent_connect_service.rb index 95d422f37..beefe3b99 100644 --- a/app/services/agent_connect_service.rb +++ b/app/services/agent_connect_service.rb @@ -13,4 +13,12 @@ class AgentConnectService acr_values: 'eidas1' ) end + + def self.user_info(code) + client = AgentConnectClient.new(code) + + client.access_token!(client_auth_method: :secret) + .userinfo! + .raw_attributes + end end diff --git a/config/routes.rb b/config/routes.rb index fa89733a0..3b4812185 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -132,6 +132,7 @@ Rails.application.routes.draw do namespace :agent_connect do get '' => 'agent#index' get 'login' => 'agent#login' + get 'callback' => 'agent#callback' end namespace :champs do From c5097451ef580b04451b930481ba9ece43155a77 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 15:24:54 +0100 Subject: [PATCH 32/47] add redirect --- app/controllers/agent_connect/agent_controller.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/controllers/agent_connect/agent_controller.rb b/app/controllers/agent_connect/agent_controller.rb index b24369c1b..c550284e4 100644 --- a/app/controllers/agent_connect/agent_controller.rb +++ b/app/controllers/agent_connect/agent_controller.rb @@ -1,5 +1,7 @@ # doc: https://github.com/france-connect/Documentation-AgentConnect class AgentConnect::AgentController < ApplicationController + before_action :redirect_to_login_if_fc_aborted, only: [:callback] + def index end @@ -37,4 +39,15 @@ class AgentConnect::AgentController < ApplicationController def santized_email(user_info) user_info['email'].strip.downcase end + + def redirect_to_login_if_fc_aborted + if params[:code].blank? + redirect_to new_user_session_path + end + end + + def redirect_france_connect_error_connection + flash.alert = t('errors.messages.france_connect.connexion') + redirect_to(new_user_session_path) + end end From 4efd15377cdd97687f1d33eb6d51a3c670a4e9a5 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 19 Nov 2021 15:24:54 +0100 Subject: [PATCH 33/47] spec --- .../agent_connect/agent_controller_spec.rb | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 spec/controllers/agent_connect/agent_controller_spec.rb diff --git a/spec/controllers/agent_connect/agent_controller_spec.rb b/spec/controllers/agent_connect/agent_controller_spec.rb new file mode 100644 index 000000000..2ed41acdb --- /dev/null +++ b/spec/controllers/agent_connect/agent_controller_spec.rb @@ -0,0 +1,90 @@ +describe AgentConnect::AgentController, type: :controller do + describe '#callback' do + let(:email) { 'i@email.com' } + subject { get :callback, params: { code: code } } + + context 'when the callback code is correct' do + let(:code) { 'correct' } + let(:user_info) { { 'sub' => 'sub', 'email' => ' I@email.com' } } + + context 'and user_info returns some info' do + before do + expect(AgentConnectService).to receive(:user_info).and_return(user_info) + end + + context 'and the instructeur does not have an account yet' do + before do + expect(controller).to receive(:sign_in) + end + + it 'creates the user, signs in and redirects to procedure_path' do + expect { subject }.to change { User.count }.by(1).and change { Instructeur.count }.by(1) + + last_user = User.last + + expect(last_user.email).to eq(email) + expect(last_user.confirmed_at).to be_present + expect(last_user.instructeur.agent_connect_id).to eq('sub') + expect(response).to redirect_to(instructeur_procedures_path) + end + end + + context 'and the instructeur already has an account' do + let!(:instructeur) { create(:instructeur, email: email) } + + before do + expect(controller).to receive(:sign_in) + end + + it 'reuses the account, signs in and redirects to procedure_path' do + expect { subject }.to change { User.count }.by(0).and change { Instructeur.count }.by(0) + + instructeur.reload + + expect(instructeur.agent_connect_id).to eq('sub') + expect(response).to redirect_to(instructeur_procedures_path) + end + end + + context 'and the instructeur already has an account as a user' do + let!(:user) { create(:user, email: email) } + + before do + expect(controller).to receive(:sign_in) + end + + it 'reuses the account, signs in and redirects to procedure_path' do + expect { subject }.to change { User.count }.by(0).and change { Instructeur.count }.by(1) + + instructeur = user.reload.instructeur + + expect(instructeur.agent_connect_id).to eq('sub') + expect(response).to redirect_to(instructeur_procedures_path) + end + end + end + + context 'but user_info raises and error' do + before do + expect(AgentConnectService).to receive(:user_info).and_raise(Rack::OAuth2::Client::Error.new(500, error: 'Unknown')) + end + + it 'aborts the processus' do + expect { subject }.to change { User.count }.by(0).and change { Instructeur.count }.by(0) + + expect(response).to redirect_to(new_user_session_path) + end + end + end + + context 'when the callback code is blank' do + let(:code) { '' } + + it 'aborts the processus' do + expect { subject }.to change { User.count }.by(0).and change { Instructeur.count }.by(0) + + expect(response).to redirect_to(new_user_session_path) + end + end + end +end From 2227dcc1e774d748a5281567345295b43f8f4b07 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Nov 2021 11:39:50 +0100 Subject: [PATCH 34/47] app: remove leftovers of procedure path autocomplete We used to pre-validate the procedure, to display in advance if the path could be used. Now that the path autocomplete is long gone, we can remove this kludgy code. --- app/models/procedure.rb | 13 ------------- .../procedures/_publication_form.html.haml | 11 +---------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index a4e839346..2d2289148 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -235,7 +235,6 @@ class Procedure < ApplicationRecord validates :description, presence: true, allow_blank: false, allow_nil: false validates :administrateurs, presence: true validates :lien_site_web, presence: true, if: :publiee? - validate :validate_for_publication, on: :publication validate :check_juridique validates :path, presence: true, format: { with: /\A[a-z0-9_\-]{3,200}\z/ }, uniqueness: { scope: [:path, :closed_at, :hidden_at, :unpublished_at], case_sensitive: false } validates :duree_conservation_dossiers_dans_ds, allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: MAX_DUREE_CONSERVATION } @@ -325,18 +324,6 @@ class Procedure < ApplicationRecord end end - def validate_for_publication - old_attributes = self.slice(:aasm_state, :closed_at) - self.aasm_state = :publiee - self.closed_at = nil - - is_valid = validate - - self.attributes = old_attributes - - is_valid - end - def suggested_path(administrateur) if path_customized? return path diff --git a/app/views/new_administrateur/procedures/_publication_form.html.haml b/app/views/new_administrateur/procedures/_publication_form.html.haml index 0989ab17b..76900b50e 100644 --- a/app/views/new_administrateur/procedures/_publication_form.html.haml +++ b/app/views/new_administrateur/procedures/_publication_form.html.haml @@ -30,14 +30,5 @@ autocomplete: 'off', placeholder: 'https://exemple.gouv.fr/ma_demarche') - - procedure.validate(:publication) - - errors = procedure.errors - -# Ignore the :taken error if the path can be claimed - - if errors.details[:path]&.pluck(:error)&.include?(:taken) && procedure.path_available?(administrateur, procedure.path) - - errors.delete(:path) - - - options = { class: "button primary", id: 'publish' } - - if errors.details[:path].present? - - options[:disabled] = :disabled .flex.justify-end - = submit_tag procedure_publish_label(procedure, :submit), options + = submit_tag procedure_publish_label(procedure, :submit), { class: "button primary", id: 'publish' } From 0fd9e15cc11015744366a7ad9a98b5c1e00a422c Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 23 Nov 2021 15:19:20 +0100 Subject: [PATCH 35/47] i18n(expiration_banner): extract test in i18n files --- .../shared/dossiers/_expiration_banner.html.haml | 13 +++++-------- config/locales/shared.en.yml | 7 +++++++ config/locales/shared.fr.yml | 8 ++++++++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/app/views/shared/dossiers/_expiration_banner.html.haml b/app/views/shared/dossiers/_expiration_banner.html.haml index b01fbea34..5f55b4be9 100644 --- a/app/views/shared/dossiers/_expiration_banner.html.haml +++ b/app/views/shared/dossiers/_expiration_banner.html.haml @@ -1,17 +1,14 @@ - if dossier.expirable? && dossier.close_to_expiration? .card.warning.mt-2.mb-3 - .card-title Votre dossier va expirer + .card-title= t('shared.dossiers.header.banner.title') %p - if dossier.brouillon? - Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. + = t('shared.dossiers.header.banner.states.brouillon') - elsif dossier.en_construction? - Votre dossier est en attente de prise en charge par l'administration. Le delais de prise en charge maximale est de 6 mois. Vous pouvez toutefois entendre cette durée d'un mois en cliquant sur le bouton suivant. + = t('shared.dossiers.header.banner.states.en_construction') - elsif dossier.termine? - Le traitement de votre dossier est terminé, mais il va bientôt expirer. Cela signifie qu’il va bientôt être supprimé. - Si vous souhaitez conserver une trace, vous pouvez le télécharger au format PDF. + = t('shared.dossiers.header.banner.states.termine') - if dossier.expiration_can_be_extended? %br - = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary mt-2' + = button_to t('shared.dossiers.header.banner.button_delay_expiration'), users_dossier_repousser_expiration_path(dossier), class: 'button secondary mt-2' diff --git a/config/locales/shared.en.yml b/config/locales/shared.en.yml index fa45fd22a..718513321 100644 --- a/config/locales/shared.en.yml +++ b/config/locales/shared.en.yml @@ -15,6 +15,13 @@ en: accepte: "Expires at %{date}" refuse: "Expires at %{date}" sans_suite: "Expires at %{date}" + banner: + title: Your file will expire + states: + brouillon: Your file is still in draft and will soon expire. So it will be deleted soon without being instructed. If you want to pursue your procedure you can submit it now. Otherwise you are able to delay its expiration by clicking on the underneath button. + en_construction: Your file is pending for instruction. The maximum delay is 6 months, but your can extend the duration by a month by clicking on the underneath button. + termine: Your file had been processed and will soon expire.So it will be deleted soon. If you want to keep it, your can dowload a PDF file of it. + button_delay_expiration: "Delay deletion" champs: cnaf: show: diff --git a/config/locales/shared.fr.yml b/config/locales/shared.fr.yml index ffcca7a87..cbeb937df 100644 --- a/config/locales/shared.fr.yml +++ b/config/locales/shared.fr.yml @@ -15,6 +15,14 @@ fr: accepte: "Expirera le %{date}" refuse: "Expirera le %{date}" sans_suite: "Expirera le %{date}" + banner: + title: Votre dossier va expirer + states: + brouillon: Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver un mois de plus en cliquant sur le bouton ci-dessous. + en_construction: Votre dossier est en attente de prise en charge par l'administration. Le delais de prise en charge maximale est de 6 mois. Vous pouvez toutefois entendre cette durée d'un mois en cliquant sur le bouton suivant. + termine: Le traitement de votre dossier est terminé, mais il va bientôt expirer. Cela signifie qu’il va bientôt être supprimé. Si vous souhaitez conserver une trace, vous pouvez le télécharger au format PDF. + button_delay_expiration: "Repousser sa suppression" + champs: cnaf: show: From 4b557a4f18159456f9ba44ac6e69211433dea1ca Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 24 Nov 2021 06:55:56 +0000 Subject: [PATCH 36/47] gems: update active_storage_validation to 0.9.6 This allows us to have the %{file_max_size} variable defined in the error message. See https://github.com/igorkasyanchuk/active_storage_validations/pull/134 --- Gemfile.lock | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7c61d7b1c..25da7fb86 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -67,8 +67,11 @@ GEM activemodel (>= 4.1, < 6.2) case_transform (>= 0.2) jsonapi-renderer (>= 0.1.1.beta1, < 0.3) - active_storage_validations (0.9.2) - rails (>= 5.2.0) + active_storage_validations (0.9.6) + activejob (>= 5.2.0) + activemodel (>= 5.2.0) + activestorage (>= 5.2.0) + activesupport (>= 5.2.0) activejob (6.1.4.1) activesupport (= 6.1.4.1) globalid (>= 0.3.6) From 60c2718f29c135097719f1751dc5448a5f2db4eb Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 24 Nov 2021 09:04:20 +0000 Subject: [PATCH 37/47] models: remove custom code for file size validation message With active_storage_validations 0.9.6, we can use the %{max_size} variable directly in the error message. --- app/models/attestation_template.rb | 5 ++--- app/models/avis.rb | 9 ++++----- app/models/champs/piece_justificative_champ.rb | 3 +-- app/models/champs/titre_identite_champ.rb | 15 ++------------- app/models/commentaire.rb | 3 +-- app/models/concerns/file_validation_concern.rb | 12 ------------ app/models/procedure.rb | 7 +++---- config/i18n-tasks.yml | 1 + config/locales/active_storage_validations.en.yml | 4 ++-- config/locales/active_storage_validations.fr.yml | 4 ++-- 10 files changed, 18 insertions(+), 45 deletions(-) delete mode 100644 app/models/concerns/file_validation_concern.rb diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb index 4414167aa..16f463d7f 100644 --- a/app/models/attestation_template.rb +++ b/app/models/attestation_template.rb @@ -14,7 +14,6 @@ class AttestationTemplate < ApplicationRecord include ActionView::Helpers::NumberHelper include TagsSubstitutionConcern - include FileValidationConcern belongs_to :procedure, optional: false @@ -24,8 +23,8 @@ class AttestationTemplate < ApplicationRecord validates :footer, length: { maximum: 190 } FILE_MAX_SIZE = 1.megabytes - validates :logo, content_type: ['image/png', 'image/jpg', 'image/jpeg'], size: file_size_validation(FILE_MAX_SIZE) - validates :signature, content_type: ['image/png', 'image/jpg', 'image/jpeg'], size: file_size_validation(FILE_MAX_SIZE) + validates :logo, content_type: ['image/png', 'image/jpg', 'image/jpeg'], size: { less_than: FILE_MAX_SIZE } + validates :signature, content_type: ['image/png', 'image/jpg', 'image/jpeg'], size: { less_than: FILE_MAX_SIZE } DOSSIER_STATE = Dossier.states.fetch(:accepte) diff --git a/app/models/avis.rb b/app/models/avis.rb index 2f56b1b89..3065aadf8 100644 --- a/app/models/avis.rb +++ b/app/models/avis.rb @@ -17,7 +17,6 @@ # class Avis < ApplicationRecord include EmailSanitizableConcern - include FileValidationConcern belongs_to :dossier, inverse_of: :avis, touch: true, optional: false belongs_to :experts_procedure, optional: false @@ -31,16 +30,16 @@ class Avis < ApplicationRecord FILE_MAX_SIZE = 20.megabytes validates :piece_justificative_file, content_type: AUTHORIZED_CONTENT_TYPES, - size: file_size_validation(FILE_MAX_SIZE) + size: { less_than: FILE_MAX_SIZE } validates :introduction_file, content_type: AUTHORIZED_CONTENT_TYPES, - size: file_size_validation(FILE_MAX_SIZE) + size: { less_than: FILE_MAX_SIZE } validates :email, format: { with: Devise.email_regexp, message: "n'est pas valide" }, allow_nil: true validates :claimant, presence: true - validates :piece_justificative_file, size: file_size_validation(FILE_MAX_SIZE) - validates :introduction_file, size: file_size_validation(FILE_MAX_SIZE) + validates :piece_justificative_file, size: { less_than: FILE_MAX_SIZE } + validates :introduction_file, size: { less_than: FILE_MAX_SIZE } before_validation -> { sanitize_email(:email) } default_scope { joins(:dossier) } diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index 5076f963a..ecd534fd7 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -20,11 +20,10 @@ # type_de_champ_id :integer # class Champs::PieceJustificativeChamp < Champ - include FileValidationConcern FILE_MAX_SIZE = 200.megabytes validates :piece_justificative_file, - size: file_size_validation(FILE_MAX_SIZE), + size: { less_than: FILE_MAX_SIZE }, if: -> { !type_de_champ.skip_pj_validation } validates :piece_justificative_file, diff --git a/app/models/champs/titre_identite_champ.rb b/app/models/champs/titre_identite_champ.rb index 1d74c3245..7fa81233d 100644 --- a/app/models/champs/titre_identite_champ.rb +++ b/app/models/champs/titre_identite_champ.rb @@ -20,20 +20,9 @@ # type_de_champ_id :integer # class Champs::TitreIdentiteChamp < Champ - include FileValidationConcern FILE_MAX_SIZE = 20.megabytes - - ACCEPTED_FORMATS = [ - "image/png", - "image/jpeg" - ] - - # TODO: once we're running on Rails 6, re-enable this validation. - # See https://github.com/betagouv/demarches-simplifiees.fr/issues/4926 - # - validates :piece_justificative_file, - content_type: ACCEPTED_FORMATS, - size: file_size_validation(FILE_MAX_SIZE) + ACCEPTED_FORMATS = ['image/png', 'image/jpeg'] + validates :piece_justificative_file, content_type: ACCEPTED_FORMATS, size: { less_than: FILE_MAX_SIZE } def main_value_name :piece_justificative_file diff --git a/app/models/commentaire.rb b/app/models/commentaire.rb index 6e430649e..b98cc70c3 100644 --- a/app/models/commentaire.rb +++ b/app/models/commentaire.rb @@ -13,7 +13,6 @@ # instructeur_id :bigint # class Commentaire < ApplicationRecord - include FileValidationConcern include Discard::Model self.ignored_columns = [:user_id] @@ -31,7 +30,7 @@ class Commentaire < ApplicationRecord FILE_MAX_SIZE = 20.megabytes validates :piece_jointe, content_type: AUTHORIZED_CONTENT_TYPES, - size: file_size_validation(FILE_MAX_SIZE) + size: { less_than: FILE_MAX_SIZE } default_scope { order(created_at: :asc) } scope :updated_since?, -> (date) { where('commentaires.updated_at > ?', date) } diff --git a/app/models/concerns/file_validation_concern.rb b/app/models/concerns/file_validation_concern.rb deleted file mode 100644 index a3237d43f..000000000 --- a/app/models/concerns/file_validation_concern.rb +++ /dev/null @@ -1,12 +0,0 @@ -module FileValidationConcern - extend ActiveSupport::Concern - class_methods do - # This method works around missing `%{min_size}` and `%{max_size}` variables in active_record_validation - # default error message. - # - # Hopefully this will be fixed upstream in https://github.com/igorkasyanchuk/active_storage_validations/pull/134 - def file_size_validation(file_max_size = 200.megabytes) - { less_than: file_max_size, message: I18n.t('errors.messages.file_size_out_of_range', file_size_limit: ActiveSupport::NumberHelper.number_to_human_size(file_max_size)) } - end - end -end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 2d2289148..cb2bfad02 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -54,7 +54,6 @@ class Procedure < ApplicationRecord self.ignored_columns = [:duree_conservation_dossiers_hors_ds] include ProcedureStatsConcern include EncryptableConcern - include FileValidationConcern include Discard::Model self.discard_column = :hidden_at @@ -253,7 +252,7 @@ class Procedure < ApplicationRecord "image/jpg", "image/png", "text/plain" - ], size: file_size_validation(FILE_MAX_SIZE), if: -> { new_record? || created_at > Date.new(2020, 2, 28) } + ], size: { less_than: FILE_MAX_SIZE }, if: -> { new_record? || created_at > Date.new(2020, 2, 28) } validates :deliberation, content_type: [ "application/msword", @@ -264,11 +263,11 @@ class Procedure < ApplicationRecord "image/jpg", "image/png", "text/plain" - ], size: file_size_validation(FILE_MAX_SIZE), if: -> { new_record? || created_at > Date.new(2020, 4, 29) } + ], size: { less_than: FILE_MAX_SIZE }, if: -> { new_record? || created_at > Date.new(2020, 4, 29) } LOGO_MAX_SIZE = 5.megabytes validates :logo, content_type: ['image/png', 'image/jpg', 'image/jpeg'], - size: file_size_validation(LOGO_MAX_SIZE), + size: { less_than: LOGO_MAX_SIZE }, if: -> { new_record? || created_at > Date.new(2020, 11, 13) } validates :api_entreprise_token, jwt_token: true, allow_blank: true diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 0d03716b7..a4e992c04 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -101,6 +101,7 @@ ignore_unused: - 'activerecord.errors.*' - 'errors.messages.blank' - 'errors.messages.content_type_invalid' +- 'errors.messages.file_size_out_of_range' - 'pluralize.*' - 'views.pagination.*' - 'time.formats.default' diff --git a/config/locales/active_storage_validations.en.yml b/config/locales/active_storage_validations.en.yml index a73f5e3a6..5297cf5a9 100644 --- a/config/locales/active_storage_validations.en.yml +++ b/config/locales/active_storage_validations.en.yml @@ -1,5 +1,5 @@ en: errors: messages: - content_type_invalid: "is not of an accepted type" - file_size_out_of_range: "is too big. The file must be at most %{file_size_limit}." + content_type_invalid: is not of an accepted type + file_size_out_of_range: is too big. The file must be at most %{max_size}. diff --git a/config/locales/active_storage_validations.fr.yml b/config/locales/active_storage_validations.fr.yml index 1fcec73ce..596e836b8 100644 --- a/config/locales/active_storage_validations.fr.yml +++ b/config/locales/active_storage_validations.fr.yml @@ -1,5 +1,5 @@ fr: errors: messages: - content_type_invalid: "n’est pas d’un type accepté" - file_size_out_of_range: "est trop lourd(e). Le fichier doit faire au plus %{file_size_limit}." + content_type_invalid: n’est pas d’un type accepté + file_size_out_of_range: est trop lourd(e). Le fichier doit faire au plus %{max_size}. From 758e7d68e632bb27eb16ffcd12a35c1bc016ceb7 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 11 Nov 2021 10:31:01 +0100 Subject: [PATCH 38/47] fix(graphql): fix and improuve query parsing for logs --- app/controllers/api/v2/graphql_controller.rb | 37 +---------- .../concerns/graphql_operation_log_concern.rb | 62 +++++++++++++++++++ .../graphql_operation_log_concern_spec.rb | 59 ++++++++++++++++++ 3 files changed, 124 insertions(+), 34 deletions(-) create mode 100644 app/controllers/concerns/graphql_operation_log_concern.rb create mode 100644 spec/controllers/concerns/graphql_operation_log_concern_spec.rb diff --git a/app/controllers/api/v2/graphql_controller.rb b/app/controllers/api/v2/graphql_controller.rb index 4e9802b3a..11a5ce654 100644 --- a/app/controllers/api/v2/graphql_controller.rb +++ b/app/controllers/api/v2/graphql_controller.rb @@ -1,4 +1,6 @@ class API::V2::GraphqlController < API::V2::BaseController + include GraphqlOperationLogConcern + def execute variables = ensure_hash(params[:variables]) @@ -24,43 +26,10 @@ class API::V2::GraphqlController < API::V2::BaseController super payload.merge!({ - graphql_operation: operation_log(params[:query], params[:operationName], params[:variables]) + graphql_operation: operation_log(params[:query], params[:operationName], params[:variables]&.to_unsafe_h) }) end - def operation_log(query, operation_name, variables) - return "NoQuery" if query.nil? - - operation = GraphQL.parse(query).children.find do |node| - if node.is_a?(GraphQL::Language::Nodes::OperationDefinition) - node.name == operation_name - end - end - - return "InvalidQuery" if operation.nil? - return "IntrospectionQuery" if operation.name == "IntrospectionQuery" - - message = operation.operation_type - if operation.name - message += ": #{operation.name} { " - end - message += operation.selections.map(&:name).join(', ') - message += " }" - if variables.present? - message += " " - message += variables.to_unsafe_h.flat_map do |(name, value)| - if name == "input" - value.map do |(name, value)| - "#{name}: \"#{value.to_s.truncate(10)}\"" - end - else - "#{name}: \"#{value.to_s.truncate(10)}\"" - end - end.join(', ') - end - message - end - def process_action(*args) super rescue ActionDispatch::Http::Parameters::ParseError => exception diff --git a/app/controllers/concerns/graphql_operation_log_concern.rb b/app/controllers/concerns/graphql_operation_log_concern.rb new file mode 100644 index 000000000..86650bfa0 --- /dev/null +++ b/app/controllers/concerns/graphql_operation_log_concern.rb @@ -0,0 +1,62 @@ +module GraphqlOperationLogConcern + extend ActiveSupport::Concern + + # This method parses GraphQL query and creates a short description of the query. It is useful for logging. + def operation_log(query, operation_name, variables) + return "NoQuery" if query.nil? + + operation = parse_graphql_query(query, operation_name) + + return "InvalidQuery" if operation.nil? + return "IntrospectionQuery" if operation.name == "IntrospectionQuery" + + message = "#{operation.operation_type}: " + message += if operation.name.present? + "#{operation.name} { " + else + "{ " + end + message += operation.selections.map(&:name).join(', ') + message += " } " + message += if variables.present? + variables.flat_map do |(name, value)| + format_graphql_variable(name, value) + end + else + operation.selections.flat_map(&:arguments).flat_map do |argument| + format_graphql_variable(argument.name, argument.value) + end + end.join(', ') + + message.strip + end + + private + + def parse_graphql_query(query, operation_name) + operations = GraphQL.parse(query).children.filter do |node| + node.is_a?(GraphQL::Language::Nodes::OperationDefinition) + end + if operations.size == 1 + operations.first + else + operations.find { |node| node.name == operation_name } + end + rescue + nil + end + + def format_graphql_variable(name, value) + if value.is_a?(Hash) + value.map do |(name, value)| + format_graphql_variable(name, value) + end + elsif value.is_a?(GraphQL::Language::Nodes::InputObject) + value.arguments.map do |argument| + format_graphql_variable(argument.name, argument.value) + end + else + "#{name}: \"#{value.to_s.truncate(10)}\"" + end + end +end diff --git a/spec/controllers/concerns/graphql_operation_log_concern_spec.rb b/spec/controllers/concerns/graphql_operation_log_concern_spec.rb new file mode 100644 index 000000000..074a6eaf6 --- /dev/null +++ b/spec/controllers/concerns/graphql_operation_log_concern_spec.rb @@ -0,0 +1,59 @@ +RSpec.describe GraphqlOperationLogConcern, type: :controller do + class TestController < ActionController::Base + include GraphqlOperationLogConcern + end + + controller TestController do + end + + describe '#operation_log' do + let(:query) { nil } + let(:variables) { nil } + let(:operation_name) { nil } + + subject { controller.operation_log(query, operation_name, variables) } + + context 'with no query' do + it { expect(subject).to eq('NoQuery') } + end + + context 'with invalid query' do + let(:query) { 'query { demarche {} }' } + + it { expect(subject).to eq('InvalidQuery') } + end + + context 'with two queries' do + let(:query) { 'query demarche { demarche } query dossier { dossier }' } + let(:operation_name) { 'dossier' } + + it { expect(subject).to eq('query: dossier { dossier }') } + end + + context 'with arguments' do + let(:query) { 'query demarche { demarche(number: 123) { id } }' } + + it { expect(subject).to eq('query: demarche { demarche } number: "123"') } + end + + context 'with variables' do + let(:query) { 'query { demarche(number: 123) { id } }' } + let(:variables) { { number: 124 } } + + it { expect(subject).to eq('query: { demarche } number: "124"') } + end + + context 'with mutation and arguments' do + let(:query) { 'mutation { passerDossierEnInstruction(input: { number: 123 }) { id } }' } + + it { expect(subject).to eq('mutation: { passerDossierEnInstruction } number: "123"') } + end + + context 'with mutation and variables' do + let(:query) { 'mutation { passerDossierEnInstruction(input: { number: 123 }) { id } }' } + let(:variables) { { input: { number: 124 } } } + + it { expect(subject).to eq('mutation: { passerDossierEnInstruction } number: "124"') } + end + end +end From f60055637aea210c62f8bf827140e612c53d9133 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 24 Nov 2021 12:47:01 +0100 Subject: [PATCH 39/47] add missing redirect_uri env --- config/env.example | 1 + config/secrets.yml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/config/env.example b/config/env.example index d959bd33d..d00111dd7 100644 --- a/config/env.example +++ b/config/env.example @@ -50,6 +50,7 @@ AGENT_CONNECT_ID="" AGENT_CONNECT_SECRET="" AGENT_CONNECT_BASE_URL="" AGENT_CONNECT_JWKS="" +AGENT_CONNECT_REDIRECT="" # Service externe: Support Utilisateur HelpScout | Spécifique démarches-simplifiées.fr HELPSCOUT_MAILBOX_ID="" diff --git a/config/secrets.yml b/config/secrets.yml index d6c545141..86e7a5022 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -28,7 +28,7 @@ defaults: &defaults agent_connect: identifier: <%= ENV['AGENT_CONNECT_ID'] %> secret: <%= ENV['AGENT_CONNECT_SECRET'] %> - redirect_uri: http://test.localhost:3000/agent_connect/callback + redirect_uri: <%= ENV['AGENT_CONNECT_REDIRECT'] %> authorization_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/authorize token_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/token userinfo_endpoint: <%= ENV['AGENT_CONNECT_BASE_URL'] %>/api/v2/userinfo From bd4a5c419b1486715f51332e2c37860bdef72f31 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 24 Nov 2021 12:50:47 +0100 Subject: [PATCH 40/47] add title to agentconnect page --- app/views/agent_connect/agent/index.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/agent_connect/agent/index.html.haml b/app/views/agent_connect/agent/index.html.haml index 04340689f..c3533aeb5 100644 --- a/app/views/agent_connect/agent/index.html.haml +++ b/app/views/agent_connect/agent/index.html.haml @@ -1,3 +1,5 @@ +- content_for(:title, t('.cta')) + .container %h1.mt-2.mb-2= t('.connect') From 6e7d2e057a008015f511d580bbef43db1e674966 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 24 Nov 2021 12:50:52 +0100 Subject: [PATCH 41/47] typo --- config/locales/views/agent_connect/agent/fr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/views/agent_connect/agent/fr.yml b/config/locales/views/agent_connect/agent/fr.yml index ede7c2ece..dbf7c019d 100644 --- a/config/locales/views/agent_connect/agent/fr.yml +++ b/config/locales/views/agent_connect/agent/fr.yml @@ -8,4 +8,4 @@ fr:

Seul les agents du ministère de la Transition écologique peuvent actuellement en bénéficier. - cta: S’identifier avec Agent Connect + cta: S’identifier avec AgentConnect From e5f544066303b360a018d2159a7db5f39e89343b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 23 Nov 2021 13:27:28 +0100 Subject: [PATCH 42/47] models: explicitely save procedure's new revision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-cloned objects have all their relationships stale. Thus, for a newly deep-cloned revision, `revision.types_de_champs` returns `[]`, even when it actually has associated types de champ. This causes consecutive champs creations and re-ordering to fail in subtle ways, like: ``` procedure.draft_revision.add_type_de_champ(…) procedure.publish_revision! procedure.draft_revision.add_type_de_champ(…) procedure.draft_revision.move_type_de_champ(…) # this will fail ``` As `publish_revision!` created a new stale revision, moving the type de champ fails because not all existing champs are found until the object is refreshed. We don't hit this path in production, because usually only a single operation is made in a request. To fix this, save the new revision before associating it as the draft procedure. (Another option would be to `reload` the revision after creation, but this seems better contained and matches the name of the method.) --- app/models/procedure.rb | 4 +++- spec/models/procedure_spec.rb | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index cb2bfad02..0179b6219 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -678,7 +678,9 @@ class Procedure < ApplicationRecord end def create_new_revision - draft_revision.deep_clone(include: [:revision_types_de_champ, :revision_types_de_champ_private]) + draft_revision + .deep_clone(include: [:revision_types_de_champ, :revision_types_de_champ_private]) + .tap(&:save!) end def average_dossier_weight diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 8db6d4716..dc659c6e2 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -695,6 +695,50 @@ describe Procedure do end end + describe "#publish_revision!" do + let(:procedure) { create(:procedure, :published) } + let(:tdc_attributes) { { type_champ: :number, libelle: 'libelle 1' } } + let(:publication_date) { Time.zone.local(2021, 1, 1, 12, 00, 00) } + + before do + procedure.draft_revision.add_type_de_champ(tdc_attributes) + end + + subject do + Timecop.freeze(publication_date) do + procedure.publish_revision! + end + end + + it 'publishes the new revision' do + subject + expect(procedure.published_revision).to be_present + expect(procedure.published_revision.published_at).to eq(publication_date) + expect(procedure.published_revision.types_de_champ.first.libelle).to eq('libelle 1') + end + + it 'creates a new draft revision' do + expect { subject }.to change(ProcedureRevision, :count).by(1) + expect(procedure.draft_revision).to be_present + expect(procedure.draft_revision.revision_types_de_champ).to be_present + expect(procedure.draft_revision.types_de_champ).to be_present + expect(procedure.draft_revision.types_de_champ.first.libelle).to eq('libelle 1') + end + + context 'when the procedure has dossiers' do + let(:dossier_draft) { create(:dossier, :brouillon, procedure: procedure) } + let(:dossier_submitted) { create(:dossier, :en_construction, procedure: procedure) } + + before { [dossier_draft, dossier_submitted] } + + it 'enqueues rebase jobs for draft dossiers' do + subject + expect(DossierRebaseJob).to have_been_enqueued.with(dossier_draft) + expect(DossierRebaseJob).not_to have_been_enqueued.with(dossier_submitted) + end + end + end + describe "#unpublish!" do let(:procedure) { create(:procedure, :published) } let(:now) { Time.zone.now.beginning_of_minute } From 1f1dd9fce5706de845446d33797e4900ade2ca56 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 24 Nov 2021 13:54:18 +0300 Subject: [PATCH 43/47] fix(traitements): add missing traitements to dossiers when reverting a decision --- app/models/dossier.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 42d51a228..803b40a46 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -809,6 +809,8 @@ class Dossier < ApplicationRecord end def after_repasser_en_construction(instructeur) + create_missing_traitemets + self.en_construction_close_to_expiration_notice_sent_at = nil self.conservation_extension = 0.days self.en_construction_at = self.traitements @@ -819,6 +821,8 @@ class Dossier < ApplicationRecord end def after_repasser_en_instruction(instructeur, disable_notification: false) + create_missing_traitemets + self.archived = false self.termine_close_to_expiration_notice_sent_at = nil self.conservation_extension = 0.days @@ -1088,6 +1092,15 @@ class Dossier < ApplicationRecord private + def create_missing_traitemets + if en_construction_at.present? && traitements.en_construction.empty? + self.traitements.passer_en_construction(processed_at: en_construction_at) + end + if en_instruction_at.present? && traitements.en_instruction.empty? + self.traitements.passer_en_instruction(processed_at: en_instruction_at) + end + end + def deleted_dossier @deleted_dossier ||= DeletedDossier.find_by(dossier_id: id) end From d847775c686c18057b4ec35f753ee436788d4a82 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 24 Nov 2021 14:27:32 +0300 Subject: [PATCH 44/47] feat(traitements): add depose_at to dossiers --- app/models/dossier.rb | 4 +++- db/migrate/20211124111429_add_depose_at_to_dossiers.rb | 5 +++++ db/schema.rb | 3 ++- spec/models/dossier_spec.rb | 7 +++++-- 4 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20211124111429_add_depose_at_to_dossiers.rb diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 803b40a46..12dd9b761 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -10,6 +10,7 @@ # conservation_extension :interval default(0 seconds) # declarative_triggered_at :datetime # deleted_user_email_never_send :string +# depose_at :datetime # en_construction_at :datetime # en_construction_close_to_expiration_notice_sent_at :datetime # en_instruction_at :datetime @@ -776,7 +777,7 @@ class Dossier < ApplicationRecord def after_passer_en_construction self.conservation_extension = 0.days - self.en_construction_at = self.traitements + self.depose_at = self.en_construction_at = self.traitements .passer_en_construction .processed_at save! @@ -1095,6 +1096,7 @@ class Dossier < ApplicationRecord def create_missing_traitemets if en_construction_at.present? && traitements.en_construction.empty? self.traitements.passer_en_construction(processed_at: en_construction_at) + self.depose_at ||= en_construction_at end if en_instruction_at.present? && traitements.en_instruction.empty? self.traitements.passer_en_instruction(processed_at: en_instruction_at) diff --git a/db/migrate/20211124111429_add_depose_at_to_dossiers.rb b/db/migrate/20211124111429_add_depose_at_to_dossiers.rb new file mode 100644 index 000000000..a1ad2ed42 --- /dev/null +++ b/db/migrate/20211124111429_add_depose_at_to_dossiers.rb @@ -0,0 +1,5 @@ +class AddDeposeAtToDossiers < ActiveRecord::Migration[6.1] + def change + add_column :dossiers, :depose_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index f2d3b5e78..c4625c91d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_11_19_112046) do +ActiveRecord::Schema.define(version: 2021_11_24_111429) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -323,6 +323,7 @@ ActiveRecord::Schema.define(version: 2021_11_19_112046) do t.index "to_tsvector('french'::regconfig, search_terms)", name: "index_dossiers_on_search_terms", using: :gin t.bigint "dossier_transfer_id" t.datetime "identity_updated_at" + t.datetime "depose_at" t.index ["archived"], name: "index_dossiers_on_archived" t.index ["dossier_transfer_id"], name: "index_dossiers_on_dossier_transfer_id" t.index ["groupe_instructeur_id"], name: "index_dossiers_on_groupe_instructeur_id" diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 423da756d..0649180f1 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -456,6 +456,7 @@ describe Dossier do it { expect(dossier.state).to eq(Dossier.states.fetch(:en_construction)) } it { expect(dossier.en_construction_at).to eq(beginning_of_day) } + it { expect(dossier.depose_at).to eq(beginning_of_day) } it { expect(dossier.traitement.state).to eq(Dossier.states.fetch(:en_construction)) } it { expect(dossier.traitement.processed_at).to eq(beginning_of_day) } @@ -467,6 +468,7 @@ describe Dossier do expect(dossier.traitements.size).to eq(3) expect(dossier.traitements.first.processed_at).to eq(beginning_of_day) expect(dossier.traitement.processed_at.round).to eq(dossier.en_construction_at.round) + expect(dossier.depose_at).to eq(beginning_of_day) expect(dossier.en_construction_at).to be > beginning_of_day end end @@ -490,8 +492,9 @@ describe Dossier do dossier.repasser_en_construction!(instructeur) dossier.passer_en_instruction!(instructeur) - expect(dossier.traitements.size).to eq(3) - expect(dossier.traitements.first.processed_at).to eq(beginning_of_day) + expect(dossier.traitements.size).to eq(4) + expect(dossier.traitements.en_construction.first.processed_at).to eq(dossier.depose_at) + expect(dossier.traitements.en_instruction.first.processed_at).to eq(beginning_of_day) expect(dossier.traitement.processed_at.round).to eq(dossier.en_instruction_at.round) expect(dossier.en_instruction_at).to be > beginning_of_day end From 68f6c01548d37db9d34024444006418fb085ad00 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 24 Nov 2021 14:39:16 +0300 Subject: [PATCH 45/47] task(traitements): add depose_at to existing dossiers --- ...11124112843_add_depose_at_to_dossiers.rake | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 lib/tasks/deployment/20211124112843_add_depose_at_to_dossiers.rake diff --git a/lib/tasks/deployment/20211124112843_add_depose_at_to_dossiers.rake b/lib/tasks/deployment/20211124112843_add_depose_at_to_dossiers.rake new file mode 100644 index 000000000..10959ccf7 --- /dev/null +++ b/lib/tasks/deployment/20211124112843_add_depose_at_to_dossiers.rake @@ -0,0 +1,22 @@ +namespace :after_party do + desc 'Deployment task: add_depose_at_to_dossiers' + task add_depose_at_to_dossiers: :environment do + puts "Running deploy task 'add_depose_at_to_dossiers'" + + dossiers = Dossier.includes(:traitements).where(depose_at: nil).where.not(en_construction_at: nil) + progress = ProgressReport.new(dossiers.count) + + dossiers.find_each do |dossier| + traitement = dossier.traitements.find { |traitement| traitement.state == :en_construction } + depose_at = traitement&.processed_at || dossier.en_construction_at + dossier.update_column(:depose_at, depose_at) + progress.inc + end + progress.finish + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end From 0d486981a6ab86f298001d4ffc0ebabfe4db067d Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 25 Nov 2021 10:40:22 +0100 Subject: [PATCH 46/47] fix(ComboCOmmunesSearch.typo): fix combo communes search typo --- app/javascript/components/ComboCommunesSearch.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/components/ComboCommunesSearch.jsx b/app/javascript/components/ComboCommunesSearch.jsx index ae2b005ba..c64116a79 100644 --- a/app/javascript/components/ComboCommunesSearch.jsx +++ b/app/javascript/components/ComboCommunesSearch.jsx @@ -105,7 +105,7 @@ function ComboCommunesSearch(params) {

- Choisissez la commune. Vous pouver entre le nom ou le code postal. + Choisissez la commune. Vous pouvez entrer le nom ou le code postal.

Date: Thu, 25 Nov 2021 11:11:03 +0100 Subject: [PATCH 47/47] fix(eslint): max columns --- app/javascript/components/ComboCommunesSearch.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/javascript/components/ComboCommunesSearch.jsx b/app/javascript/components/ComboCommunesSearch.jsx index c64116a79..fa9aca1ab 100644 --- a/app/javascript/components/ComboCommunesSearch.jsx +++ b/app/javascript/components/ComboCommunesSearch.jsx @@ -105,7 +105,8 @@ function ComboCommunesSearch(params) {

- Choisissez la commune. Vous pouvez entrer le nom ou le code postal. + Choisissez la commune. Vous pouvez entrer le nom ou le code + postal.