From 8902a81f23b9db6d4efcaa712d9e755bd1b17b05 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Tue, 20 Feb 2018 11:24:50 +0100 Subject: [PATCH 1/8] Remove a useless assignment --- spec/models/gestionnaire_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb index 4c13fcb54..7fde9cc4f 100644 --- a/spec/models/gestionnaire_spec.rb +++ b/spec/models/gestionnaire_spec.rb @@ -188,7 +188,6 @@ describe Gestionnaire, type: :model do end describe "procedure_presentation_for_procedure_id" do - let!(:procedure_assign_2) { create :assign_to, gestionnaire: gestionnaire, procedure: procedure_2 } let!(:pp) { ProcedurePresentation.create(assign_to: procedure_assign) } it { expect(gestionnaire.procedure_presentation_for_procedure_id(procedure.id)).to eq(pp) } From deeda0e5403d1b7fe1f721412737c52b60fc14ae Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Mon, 19 Feb 2018 18:20:58 +0100 Subject: [PATCH 2/8] Add a unicity constraint on AssignTo --- ...nique_index_on_assign_tos_gestionnaire_id_procedure_id.rb | 5 +++++ db/schema.rb | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180219170553_add_unique_index_on_assign_tos_gestionnaire_id_procedure_id.rb diff --git a/db/migrate/20180219170553_add_unique_index_on_assign_tos_gestionnaire_id_procedure_id.rb b/db/migrate/20180219170553_add_unique_index_on_assign_tos_gestionnaire_id_procedure_id.rb new file mode 100644 index 000000000..77efa47ae --- /dev/null +++ b/db/migrate/20180219170553_add_unique_index_on_assign_tos_gestionnaire_id_procedure_id.rb @@ -0,0 +1,5 @@ +class AddUniqueIndexOnAssignTosGestionnaireIdProcedureId < ActiveRecord::Migration[5.2] + def change + add_index :assign_tos, [:gestionnaire_id, :procedure_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index d5260c7ee..41dfe638a 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: 2018_02_09_133452) do +ActiveRecord::Schema.define(version: 2018_02_19_170553) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -84,6 +84,7 @@ ActiveRecord::Schema.define(version: 2018_02_09_133452) do create_table "assign_tos", id: :serial, force: :cascade do |t| t.integer "gestionnaire_id" t.integer "procedure_id" + t.index ["gestionnaire_id", "procedure_id"], name: "index_assign_tos_on_gestionnaire_id_and_procedure_id", unique: true t.index ["gestionnaire_id"], name: "index_assign_tos_on_gestionnaire_id" t.index ["procedure_id"], name: "index_assign_tos_on_procedure_id" end From 1b1460c19102147b84af9f94b3649869843927c8 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Mon, 19 Feb 2018 11:20:57 +0100 Subject: [PATCH 3/8] Move a service to the gestionnaire model --- .../admin/accompagnateurs_controller.rb | 9 +++-- app/models/gestionnaire.rb | 8 +++++ app/services/accompagnateur_service.rb | 19 ----------- .../accompagnateurs/_list_assign.html.haml | 2 +- .../_list_not_assign.html.haml | 2 +- spec/services/accompagnateur_service_spec.rb | 33 ------------------- 6 files changed, 16 insertions(+), 57 deletions(-) delete mode 100644 app/services/accompagnateur_service.rb delete mode 100644 spec/services/accompagnateur_service_spec.rb diff --git a/app/controllers/admin/accompagnateurs_controller.rb b/app/controllers/admin/accompagnateurs_controller.rb index 1f7ca7c7f..e63e58f57 100644 --- a/app/controllers/admin/accompagnateurs_controller.rb +++ b/app/controllers/admin/accompagnateurs_controller.rb @@ -46,9 +46,12 @@ class Admin::AccompagnateursController < AdminController procedure = Procedure.find(params[:procedure_id]) to = params[:to] - accompagnateur_service = AccompagnateurService.new gestionnaire, procedure, to - - accompagnateur_service.change_assignement! + case to + when 'assign' + gestionnaire.assign_to_procedure(procedure) + when 'not_assign' + gestionnaire.remove_from_procedure(procedure) + end flash.notice = "Assignement effectué" redirect_to admin_procedure_accompagnateurs_path, procedure_id: params[:procedure_id] diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 9a9a01b0b..f2d314cea 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -47,6 +47,14 @@ class Gestionnaire < ActiveRecord::Base procedures.find_by(id: procedure_id).present? end + def assign_to_procedure(procedure) + AssignTo.create(gestionnaire: self, procedure: procedure) + end + + def remove_from_procedure(procedure) + AssignTo.where(gestionnaire: self, procedure: procedure).delete_all + end + def last_week_overview start_date = DateTime.now.beginning_of_week diff --git a/app/services/accompagnateur_service.rb b/app/services/accompagnateur_service.rb deleted file mode 100644 index 2546506a9..000000000 --- a/app/services/accompagnateur_service.rb +++ /dev/null @@ -1,19 +0,0 @@ -class AccompagnateurService - ASSIGN = 'assign' - NOT_ASSIGN = 'not_assign' - - def initialize accompagnateur, procedure, to - @accompagnateur = accompagnateur - @procedure = procedure - @to = to - end - - def change_assignement! - case @to - when ASSIGN - AssignTo.create(gestionnaire: @accompagnateur, procedure: @procedure) - when NOT_ASSIGN - AssignTo.where(gestionnaire: @accompagnateur, procedure: @procedure).delete_all - end - end -end diff --git a/app/views/admin/accompagnateurs/_list_assign.html.haml b/app/views/admin/accompagnateurs/_list_assign.html.haml index cd7d1621a..5160a8292 100644 --- a/app/views/admin/accompagnateurs/_list_assign.html.haml +++ b/app/views/admin/accompagnateurs/_list_assign.html.haml @@ -9,7 +9,7 @@ - @accompagnateurs_assign.each do |accompagnateur| %tr %td.col-md-1.col-lg-1.col-sm-1.col-xs-1.col-sm-1.col-xs-1.center - %a.btn.btn-primary{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: AccompagnateurService::NOT_ASSIGN)}", 'data-method' => 'put' } + %a.btn.btn-primary{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: 'not_assign')}", 'data-method' => 'put' } .fa.fa-arrow-left %td{ style: 'padding-top: 11px; font-size: 15px; text-align: right;' }= accompagnateur.email diff --git a/app/views/admin/accompagnateurs/_list_not_assign.html.haml b/app/views/admin/accompagnateurs/_list_not_assign.html.haml index 51e8c5326..9cecdbee1 100644 --- a/app/views/admin/accompagnateurs/_list_not_assign.html.haml +++ b/app/views/admin/accompagnateurs/_list_not_assign.html.haml @@ -16,7 +16,7 @@ %tr %td.col-xs-11{ style: 'padding-top: 11px; font-size: 15px;' }= accompagnateur.email %td.center - %a.btn.btn-success.gestionnaire-affectation{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: AccompagnateurService::ASSIGN)}", 'data-method' => 'put' } + %a.btn.btn-success.gestionnaire-affectation{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: 'assign')}", 'data-method' => 'put' } .fa.fa-arrow-right diff --git a/spec/services/accompagnateur_service_spec.rb b/spec/services/accompagnateur_service_spec.rb deleted file mode 100644 index a5b71d612..000000000 --- a/spec/services/accompagnateur_service_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -require 'spec_helper' - -describe AccompagnateurService do - let(:procedure) { create :procedure, :published } - let(:accompagnateur) { create :gestionnaire } - - let(:accompagnateur_service) { AccompagnateurService.new accompagnateur, procedure, to } - - describe '#change_assignement!' do - subject { accompagnateur_service.change_assignement! } - - context 'when accompagnateur is not assign at the procedure' do - let(:to) { AccompagnateurService::ASSIGN } - - before do - subject - end - - it { expect(accompagnateur.procedures).to include procedure } - end - - context 'when accompagnateur is assign at the procedure' do - let(:to) { AccompagnateurService::NOT_ASSIGN } - - before do - create :assign_to, gestionnaire: accompagnateur, procedure: procedure - subject - end - - it { expect(accompagnateur.procedures).not_to include procedure } - end - end -end From 86d867a7ead00f2defa249d018c0e265ceaca6f9 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Mon, 19 Feb 2018 11:22:20 +0100 Subject: [PATCH 4/8] Add more precise info messages --- app/controllers/admin/accompagnateurs_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/accompagnateurs_controller.rb b/app/controllers/admin/accompagnateurs_controller.rb index e63e58f57..86adfbe91 100644 --- a/app/controllers/admin/accompagnateurs_controller.rb +++ b/app/controllers/admin/accompagnateurs_controller.rb @@ -49,11 +49,12 @@ class Admin::AccompagnateursController < AdminController case to when 'assign' gestionnaire.assign_to_procedure(procedure) + flash.notice = "L'accompagnateur a bien été affecté" when 'not_assign' gestionnaire.remove_from_procedure(procedure) + flash.notice = "L'accompagnateur a bien été désaffecté" end - flash.notice = "Assignement effectué" redirect_to admin_procedure_accompagnateurs_path, procedure_id: params[:procedure_id] end end From 08f364ccdc36c1937eab36b1eb1dbf6cf75c6c54 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Mon, 19 Feb 2018 11:35:45 +0100 Subject: [PATCH 5/8] Add constants --- app/controllers/admin/accompagnateurs_controller.rb | 7 +++++-- app/views/admin/accompagnateurs/_list_assign.html.haml | 2 +- app/views/admin/accompagnateurs/_list_not_assign.html.haml | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/accompagnateurs_controller.rb b/app/controllers/admin/accompagnateurs_controller.rb index 86adfbe91..7222fc0ca 100644 --- a/app/controllers/admin/accompagnateurs_controller.rb +++ b/app/controllers/admin/accompagnateurs_controller.rb @@ -4,6 +4,9 @@ class Admin::AccompagnateursController < AdminController before_action :retrieve_procedure + ASSIGN = 'assign' + NOT_ASSIGN = 'not_assign' + def show assign_scope = @procedure.gestionnaires @@ -47,10 +50,10 @@ class Admin::AccompagnateursController < AdminController to = params[:to] case to - when 'assign' + when ASSIGN gestionnaire.assign_to_procedure(procedure) flash.notice = "L'accompagnateur a bien été affecté" - when 'not_assign' + when NOT_ASSIGN gestionnaire.remove_from_procedure(procedure) flash.notice = "L'accompagnateur a bien été désaffecté" end diff --git a/app/views/admin/accompagnateurs/_list_assign.html.haml b/app/views/admin/accompagnateurs/_list_assign.html.haml index 5160a8292..4e9171ccf 100644 --- a/app/views/admin/accompagnateurs/_list_assign.html.haml +++ b/app/views/admin/accompagnateurs/_list_assign.html.haml @@ -9,7 +9,7 @@ - @accompagnateurs_assign.each do |accompagnateur| %tr %td.col-md-1.col-lg-1.col-sm-1.col-xs-1.col-sm-1.col-xs-1.center - %a.btn.btn-primary{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: 'not_assign')}", 'data-method' => 'put' } + %a.btn.btn-primary{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: Admin::AccompagnateursController::NOT_ASSIGN)}", 'data-method' => 'put' } .fa.fa-arrow-left %td{ style: 'padding-top: 11px; font-size: 15px; text-align: right;' }= accompagnateur.email diff --git a/app/views/admin/accompagnateurs/_list_not_assign.html.haml b/app/views/admin/accompagnateurs/_list_not_assign.html.haml index 9cecdbee1..bba53bfd1 100644 --- a/app/views/admin/accompagnateurs/_list_not_assign.html.haml +++ b/app/views/admin/accompagnateurs/_list_not_assign.html.haml @@ -16,7 +16,7 @@ %tr %td.col-xs-11{ style: 'padding-top: 11px; font-size: 15px;' }= accompagnateur.email %td.center - %a.btn.btn-success.gestionnaire-affectation{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: 'assign')}", 'data-method' => 'put' } + %a.btn.btn-success.gestionnaire-affectation{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: Admin::AccompagnateursController::ASSIGN)}", 'data-method' => 'put' } .fa.fa-arrow-right From 948a29aea27f3e82bd5774f40d1413e678586b8a Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Mon, 19 Feb 2018 17:46:36 +0100 Subject: [PATCH 6/8] Use link_to helper --- app/views/admin/accompagnateurs/_list_assign.html.haml | 2 +- app/views/admin/accompagnateurs/_list_not_assign.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/accompagnateurs/_list_assign.html.haml b/app/views/admin/accompagnateurs/_list_assign.html.haml index 4e9171ccf..8afe8e25f 100644 --- a/app/views/admin/accompagnateurs/_list_assign.html.haml +++ b/app/views/admin/accompagnateurs/_list_assign.html.haml @@ -9,7 +9,7 @@ - @accompagnateurs_assign.each do |accompagnateur| %tr %td.col-md-1.col-lg-1.col-sm-1.col-xs-1.col-sm-1.col-xs-1.center - %a.btn.btn-primary{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: Admin::AccompagnateursController::NOT_ASSIGN)}", 'data-method' => 'put' } + = link_to "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: Admin::AccompagnateursController::NOT_ASSIGN)}", class: "btn btn-primary", 'data-method' => 'put' do .fa.fa-arrow-left %td{ style: 'padding-top: 11px; font-size: 15px; text-align: right;' }= accompagnateur.email diff --git a/app/views/admin/accompagnateurs/_list_not_assign.html.haml b/app/views/admin/accompagnateurs/_list_not_assign.html.haml index bba53bfd1..e9e156a07 100644 --- a/app/views/admin/accompagnateurs/_list_not_assign.html.haml +++ b/app/views/admin/accompagnateurs/_list_not_assign.html.haml @@ -16,7 +16,7 @@ %tr %td.col-xs-11{ style: 'padding-top: 11px; font-size: 15px;' }= accompagnateur.email %td.center - %a.btn.btn-success.gestionnaire-affectation{ href: "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: Admin::AccompagnateursController::ASSIGN)}", 'data-method' => 'put' } + = link_to "#{admin_procedure_accompagnateurs_path(procedure_id: @procedure.id, accompagnateur_id: accompagnateur.id, to: Admin::AccompagnateursController::ASSIGN)}", class: "btn btn-success gestionnaire-affectation", 'data-method' => 'put' do .fa.fa-arrow-right From ac6ba40598e7d5e22a0b1b0228c1216591d32d13 Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Mon, 19 Feb 2018 19:25:34 +0100 Subject: [PATCH 7/8] Unscope the procedures on Gestionnaire and add #visible_procedures --- app/controllers/new_gestionnaire/procedures_controller.rb | 4 ++-- app/models/gestionnaire.rb | 6 +++++- app/views/layouts/_new_header.haml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/new_gestionnaire/procedures_controller.rb b/app/controllers/new_gestionnaire/procedures_controller.rb index 0131bcb61..7b4d958a6 100644 --- a/app/controllers/new_gestionnaire/procedures_controller.rb +++ b/app/controllers/new_gestionnaire/procedures_controller.rb @@ -6,7 +6,7 @@ module NewGestionnaire ITEMS_PER_PAGE = 25 def index - @procedures = current_gestionnaire.procedures.order(archived_at: :desc, published_at: :desc) + @procedures = current_gestionnaire.visible_procedures.order(archived_at: :desc, published_at: :desc) dossiers = current_gestionnaire.dossiers @dossiers_count_per_procedure = dossiers.all_state.group(:procedure_id).reorder(nil).count @@ -202,7 +202,7 @@ module NewGestionnaire end def redirect_to_avis_if_needed - if current_gestionnaire.procedures.count == 0 && current_gestionnaire.avis.count > 0 + if current_gestionnaire.visible_procedures.count == 0 && current_gestionnaire.avis.count > 0 redirect_to gestionnaire_avis_index_path end end diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index f2d314cea..149c697bc 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -5,7 +5,7 @@ class Gestionnaire < ActiveRecord::Base has_and_belongs_to_many :administrateurs has_many :assign_to, dependent: :destroy - has_many :procedures, -> { publiees_ou_archivees }, through: :assign_to + has_many :procedures, through: :assign_to has_many :dossiers, -> { state_not_brouillon }, through: :procedures has_many :follows has_many :followed_dossiers, through: :follows, source: :dossier @@ -14,6 +14,10 @@ class Gestionnaire < ActiveRecord::Base include CredentialsSyncableConcern + def visible_procedures + procedures.publiees_ou_archivees + end + def procedure_filter procedure_id = self[:procedure_filter] if procedures.find_by(id: procedure_id).present? diff --git a/app/views/layouts/_new_header.haml b/app/views/layouts/_new_header.haml index 3f73c7cba..f4e088c14 100644 --- a/app/views/layouts/_new_header.haml +++ b/app/views/layouts/_new_header.haml @@ -10,7 +10,7 @@ - if nav_bar_profile == :gestionnaire && gestionnaire_signed_in? - current_url = request.path_info %ul.header-tabs - - if current_gestionnaire.procedures.count > 0 + - if current_gestionnaire.visible_procedures.count > 0 %li = link_to "Procédures", gestionnaire_procedures_path, class: (controller_name != 'avis') ? "tab-link active" : 'tab-link' - if current_gestionnaire.avis.count > 0 From 3d10dbf0e4a9a581cd40710e0860e58f9db0c87e Mon Sep 17 00:00:00 2001 From: gregoirenovel Date: Tue, 20 Feb 2018 11:24:32 +0100 Subject: [PATCH 8/8] Refactor assignement-related methods --- .../admin/accompagnateurs_controller.rb | 14 ++++-- app/models/gestionnaire.rb | 9 +++- spec/models/gestionnaire_spec.rb | 48 +++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/accompagnateurs_controller.rb b/app/controllers/admin/accompagnateurs_controller.rb index 7222fc0ca..f695dca07 100644 --- a/app/controllers/admin/accompagnateurs_controller.rb +++ b/app/controllers/admin/accompagnateurs_controller.rb @@ -51,11 +51,17 @@ class Admin::AccompagnateursController < AdminController case to when ASSIGN - gestionnaire.assign_to_procedure(procedure) - flash.notice = "L'accompagnateur a bien été affecté" + if gestionnaire.assign_to_procedure(procedure) + flash.notice = "L'accompagnateur a bien été affecté" + else + flash.alert = "L'accompagnateur a déjà été affecté" + end when NOT_ASSIGN - gestionnaire.remove_from_procedure(procedure) - flash.notice = "L'accompagnateur a bien été désaffecté" + if gestionnaire.remove_from_procedure(procedure) + flash.notice = "L'accompagnateur a bien été désaffecté" + else + flash.alert = "L'accompagnateur a déjà été désaffecté" + end end redirect_to admin_procedure_accompagnateurs_path, procedure_id: params[:procedure_id] diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 149c697bc..4b4a98aa2 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -52,11 +52,16 @@ class Gestionnaire < ActiveRecord::Base end def assign_to_procedure(procedure) - AssignTo.create(gestionnaire: self, procedure: procedure) + begin + procedures << procedure + true + rescue ActiveRecord::RecordNotUnique + false + end end def remove_from_procedure(procedure) - AssignTo.where(gestionnaire: self, procedure: procedure).delete_all + !!(procedure.in?(procedures) && procedures.destroy(procedure)) end def last_week_overview diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb index 7fde9cc4f..0a92a7bad 100644 --- a/spec/models/gestionnaire_spec.rb +++ b/spec/models/gestionnaire_spec.rb @@ -64,6 +64,54 @@ describe Gestionnaire, type: :model do end end + describe "#assign_to_procedure" do + subject { gestionnaire.assign_to_procedure(procedure_to_assign) } + + context "with a procedure not already assigned" do + let(:procedure_to_assign) { procedure_3 } + + it { is_expected.to be_truthy } + it { expect{ subject }.to change(gestionnaire.procedures, :count) } + end + + context "with an already assigned procedure" do + let(:procedure_to_assign) { procedure } + + it { is_expected.to be_falsey } + it { expect{ subject }.not_to change(gestionnaire.procedures, :count) } + end + end + + describe "#remove_from_procedure" do + subject { gestionnaire.remove_from_procedure(procedure_to_remove) } + + context "with an assigned procedure" do + let(:procedure_to_remove) { procedure } + let!(:procedure_presentation) { procedure_assign.procedure_presentation } + + it { is_expected.to be_truthy } + + describe "consequences" do + before do + procedure_assign.build_procedure_presentation + procedure_assign.save + subject + end + + it "removes the assign_to and procedure_presentation" do + expect(AssignTo.where(id: procedure_assign).count).to eq(0) + expect(ProcedurePresentation.where(assign_to_id: procedure_assign.id).count).to eq(0) + end + end + end + + context "with an already unassigned procedure" do + let(:procedure_to_remove) { procedure_3 } + + it { is_expected.to be_falsey } + end + end + context 'unified login' do it 'syncs credentials to associated user' do gestionnaire = create(:gestionnaire)