From 0e610179fb491924ff569f5e889743974dd79bec Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 12:22:12 +0100 Subject: [PATCH 1/8] refactor(champ_label): use ApplicationHelper --- .../editable_champ/champ_label_content_component.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/components/editable_champ/champ_label_content_component.rb b/app/components/editable_champ/champ_label_content_component.rb index 9981e297b..60025939e 100644 --- a/app/components/editable_champ/champ_label_content_component.rb +++ b/app/components/editable_champ/champ_label_content_component.rb @@ -1,4 +1,6 @@ class EditableChamp::ChampLabelContentComponent < ApplicationComponent + include ApplicationHelper + def initialize(champ:, seen_at: nil) @champ, @seen_at = champ, seen_at end @@ -12,8 +14,4 @@ class EditableChamp::ChampLabelContentComponent < ApplicationComponent def highlight? @champ.updated_at.present? && @seen_at&.<(@champ.updated_at) end - - def try_format_datetime(datetime) - datetime.present? ? I18n.l(datetime) : '' - end end From 7e7363c8c3c4114f1e09cc79003cfeda0e0f7052 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 12:22:41 +0100 Subject: [PATCH 2/8] refactor(application_component): add current_administrateur --- app/components/application_component.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/components/application_component.rb b/app/components/application_component.rb index 8d88db6c2..9681ab890 100644 --- a/app/components/application_component.rb +++ b/app/components/application_component.rb @@ -9,4 +9,8 @@ class ApplicationComponent < ViewComponent::Base def current_user controller.current_user end + + def current_administrateur + controller.current_administrateur + end end From 30b53ec9272954677b47346933a17c9928bd1758 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 12:29:12 +0100 Subject: [PATCH 3/8] feat(procedure_admins): allow self remove from procedure --- .../procedure_administrateurs_controller.rb | 28 +++++++------ ...ocedure_administrateurs_controller_spec.rb | 40 ++++++++++++------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/app/controllers/administrateurs/procedure_administrateurs_controller.rb b/app/controllers/administrateurs/procedure_administrateurs_controller.rb index eea1c45f6..952d73afd 100644 --- a/app/controllers/administrateurs/procedure_administrateurs_controller.rb +++ b/app/controllers/administrateurs/procedure_administrateurs_controller.rb @@ -29,20 +29,24 @@ module Administrateurs end def destroy - administrateur = @procedure.administrateurs.find(params[:id]) + admin_to_delete = @procedure.administrateurs.find(params[:id]) - # Prevent self-removal (Also enforced in the UI) - if administrateur == current_administrateur - flash.alert = "Vous ne pouvez pas vous retirer vous-même d’une démarche." - return + if (@procedure.administrateurs - [admin_to_delete]).filter(&:active?).empty? + flash.alert = "Il doit rester au moins un administrateur actif." + else + begin + # Actually remove the admin + @procedure.administrateurs.delete(admin_to_delete) + @administrateur = admin_to_delete + flash.notice = "L’administrateur \« #{admin_to_delete.email} » a été retiré de la démarche « #{@procedure.libelle} »." + + if current_administrateur == admin_to_delete + redirect_to admin_procedures_path + end + rescue ActiveRecord::ActiveRecordError => e + flash.alert = e.message + end end - - # Actually remove the admin - @procedure.administrateurs.delete(administrateur) - @administrateur = administrateur - flash.notice = "L’administrateur \« #{administrateur.email} » a été retiré de la démarche « #{@procedure.libelle} »." - rescue ActiveRecord::ActiveRecordError => e - flash.alert = e.message end end end diff --git a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb index 1f277b2fa..a21981e46 100644 --- a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb +++ b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb @@ -1,6 +1,6 @@ describe Administrateurs::ProcedureAdministrateursController, type: :controller do - let(:signed_in_admin) { create(:administrateur) } - let(:other_admin) { create(:administrateur) } + let(:signed_in_admin) { create(:administrateur, active: true) } + let(:other_admin) { create(:administrateur, active: true) } let!(:administrateurs_procedure) { create(:administrateurs_procedure, administrateur: signed_in_admin, procedure: procedure, manager: manager) } let!(:procedure) { create(:procedure, administrateurs: [other_admin]) } render_views @@ -19,29 +19,41 @@ describe Administrateurs::ProcedureAdministrateursController, type: :controller describe '#destroy' do let(:manager) { false } - subject do + + def destroy_admin(admin_to_remove) delete :destroy, params: { procedure_id: procedure.id, id: admin_to_remove.id }, format: :turbo_stream end context 'when removing another admin' do - let(:admin_to_remove) { other_admin } + before do + destroy_admin(other_admin) + end it 'removes the admin from the procedure' do - subject - expect(response).to have_http_status(:ok) - expect(subject.body).to include('alert-success') - expect(admin_to_remove.procedures.reload).not_to include(procedure) + expect(response.body).to include('alert-success') + expect(other_admin.procedures.reload).not_to include(procedure) + end + + context 'then removing oneself' do + before do + destroy_admin(signed_in_admin) + end + + it 'removes the admin from the procedure' do + expect(response.body).to include('alert-danger') + expect(signed_in_admin.procedures.reload).to include(procedure) + end end end context 'when removing oneself from a procedure' do - let(:admin_to_remove) { signed_in_admin } + before do + destroy_admin(signed_in_admin) + end - it 'denies the right for an admin to remove itself' do - subject - expect(response).to have_http_status(:ok) - expect(subject.body).to include('alert-danger') - expect(admin_to_remove.procedures.reload).to include(procedure) + it 'removes the admin from the procedure' do + expect(response).to redirect_to admin_procedures_path + expect(signed_in_admin.procedures.reload).not_to include(procedure) end end end From 924d2ae15d683d5b02dd9244f1dbff8466971728 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 12:25:14 +0100 Subject: [PATCH 4/8] refactor(procedure_adminis): extract component --- .../administrateur_component.rb | 42 +++++++++++++++++++ .../administrateur_component.html.haml | 5 +++ 2 files changed, 47 insertions(+) create mode 100644 app/components/procedure/procedure_administrateurs/administrateur_component.rb create mode 100644 app/components/procedure/procedure_administrateurs/administrateur_component/administrateur_component.html.haml diff --git a/app/components/procedure/procedure_administrateurs/administrateur_component.rb b/app/components/procedure/procedure_administrateurs/administrateur_component.rb new file mode 100644 index 000000000..47a5f49ca --- /dev/null +++ b/app/components/procedure/procedure_administrateurs/administrateur_component.rb @@ -0,0 +1,42 @@ +class Procedure::ProcedureAdministrateurs::AdministrateurComponent < ApplicationComponent + include ApplicationHelper + + def initialize(procedure:, administrateur:) + @procedure = procedure + @administrateur = administrateur + end + + def email + if @administrateur == current_administrateur + "#{@administrateur.email} (C’est vous !)" + else + @administrateur.email + end + end + + def created_at + try_format_datetime(@administrateur.created_at) + end + + def registration_state + @administrateur.registration_state + end + + def remove_button + if is_there_at_least_another_active_admin? + button_to 'Retirer', + admin_procedure_administrateur_path(@procedure, @administrateur), + method: :delete, + class: 'button', + form: { data: { turbo: true, turbo_confirm: "Retirer « #{@administrateur.email} » des administrateurs de « #{@procedure.libelle} » ?" } } + end + end + + def is_there_at_least_another_active_admin? + if @administrateur.active? + @procedure.administrateurs.count(&:active?) > 1 + else + @procedure.administrateurs.count(&:active?) >= 1 + end + end +end diff --git a/app/components/procedure/procedure_administrateurs/administrateur_component/administrateur_component.html.haml b/app/components/procedure/procedure_administrateurs/administrateur_component/administrateur_component.html.haml new file mode 100644 index 000000000..9678adb08 --- /dev/null +++ b/app/components/procedure/procedure_administrateurs/administrateur_component/administrateur_component.html.haml @@ -0,0 +1,5 @@ +%tr{ id: dom_id(@administrateur) } + %td= email + %td= created_at + %td= registration_state + %td= remove_button From 066c441b61243e226183527d5a0ef4475c5082e9 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 12:31:41 +0100 Subject: [PATCH 5/8] refactor(procedure_admins): use component --- .../procedure_administrateurs/create.turbo_stream.haml | 3 ++- .../procedure_administrateurs/destroy.turbo_stream.haml | 4 ++-- .../administrateurs/procedure_administrateurs/index.html.haml | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml b/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml index 6f363a5b3..ea1bb3da3 100644 --- a/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml +++ b/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml @@ -1,3 +1,4 @@ - if @administrateur.present? - = turbo_stream.append "administrateurs", partial: 'administrateur', locals: { procedure: @procedure, administrateur: @administrateur } + = turbo_stream.update 'administrateurs', + render(Procedure::ProcedureAdministrateurs::AdministrateurComponent.with_collection(@procedure.administrateurs.order('users.email'), procedure: @procedure)) = turbo_stream.replace "new_administrateur", partial: 'add_admin_form', locals: { procedure: @procedure, disabled_as_super_admin: administrateur_as_manager? } diff --git a/app/views/administrateurs/procedure_administrateurs/destroy.turbo_stream.haml b/app/views/administrateurs/procedure_administrateurs/destroy.turbo_stream.haml index 4b4645f2d..e4aee0e28 100644 --- a/app/views/administrateurs/procedure_administrateurs/destroy.turbo_stream.haml +++ b/app/views/administrateurs/procedure_administrateurs/destroy.turbo_stream.haml @@ -1,2 +1,2 @@ -- if @administrateur.present? - = turbo_stream.remove(@administrateur) += turbo_stream.update 'administrateurs', + render(Procedure::ProcedureAdministrateurs::AdministrateurComponent.with_collection(@procedure.administrateurs.order('users.email'), procedure: @procedure)) diff --git a/app/views/administrateurs/procedure_administrateurs/index.html.haml b/app/views/administrateurs/procedure_administrateurs/index.html.haml index bc4c87ffc..26f43982a 100644 --- a/app/views/administrateurs/procedure_administrateurs/index.html.haml +++ b/app/views/administrateurs/procedure_administrateurs/index.html.haml @@ -12,7 +12,7 @@ %th= 'Enregistré le' %th= 'État' %tbody#administrateurs - = render partial: 'administrateur', collection: @procedure.administrateurs.order('users.email'), locals: { procedure: @procedure } + = render(Procedure::ProcedureAdministrateurs::AdministrateurComponent.with_collection(@procedure.administrateurs.order('users.email'), procedure: @procedure)) %tfoot %tr %th{ colspan: 4 } From bcccd575736bbd3c499410e90bb934be601a8d42 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 12:31:48 +0100 Subject: [PATCH 6/8] refactor(procedure_admins): delete old partial --- .../_administrateur.html.haml | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 app/views/administrateurs/procedure_administrateurs/_administrateur.html.haml diff --git a/app/views/administrateurs/procedure_administrateurs/_administrateur.html.haml b/app/views/administrateurs/procedure_administrateurs/_administrateur.html.haml deleted file mode 100644 index dce0a1a83..000000000 --- a/app/views/administrateurs/procedure_administrateurs/_administrateur.html.haml +++ /dev/null @@ -1,13 +0,0 @@ -%tr{ id: dom_id(administrateur) } - %td= administrateur.email - %td= try_format_datetime(administrateur.created_at) - %td= administrateur.registration_state - %td - - if administrateur == current_administrateur - C’est vous ! - - else - = button_to 'Retirer', - admin_procedure_administrateur_path(procedure, administrateur), - method: :delete, - class: 'button', - form: { data: { turbo: true, turbo_confirm: "Retirer « #{administrateur.email} » des administrateurs de « #{procedure.libelle} » ?" } } From 4babee259167b3c562abfec6bd8f437ad2ec2202 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 15:19:02 +0100 Subject: [PATCH 7/8] fix(admin): ignore active column the attribut should be retrieve from the user model as the administrateur is no more a devise model --- app/models/administrateur.rb | 6 ++++++ .../procedure_administrateurs_controller_spec.rb | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 41c71ced2..27114acdb 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -12,6 +12,8 @@ class Administrateur < ApplicationRecord include ActiveRecord::SecureToken + self.ignored_columns = [:active] + UNUSED_ADMIN_THRESHOLD = 6.months has_and_belongs_to_many :instructeurs @@ -42,6 +44,10 @@ class Administrateur < ApplicationRecord user&.email end + def active? + user&.active? + end + def self.find_inactive_by_token(reset_password_token) self.inactive.with_reset_password_token(reset_password_token) end diff --git a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb index a21981e46..753a3c29e 100644 --- a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb +++ b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb @@ -1,6 +1,6 @@ describe Administrateurs::ProcedureAdministrateursController, type: :controller do - let(:signed_in_admin) { create(:administrateur, active: true) } - let(:other_admin) { create(:administrateur, active: true) } + let(:signed_in_admin) { create(:administrateur).tap { _1.user.update(last_sign_in_at: Time.zone.now) } } + let(:other_admin) { create(:administrateur).tap { _1.user.update(last_sign_in_at: Time.zone.now) } } let!(:administrateurs_procedure) { create(:administrateurs_procedure, administrateur: signed_in_admin, procedure: procedure, manager: manager) } let!(:procedure) { create(:procedure, administrateurs: [other_admin]) } render_views From f532eee9f67e0099e5a4b2da9c88471d514a59fd Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 21 Nov 2022 15:19:19 +0100 Subject: [PATCH 8/8] fix(spec): remove strange click --- spec/system/administrateurs/procedure_administrateurs_spec.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/system/administrateurs/procedure_administrateurs_spec.rb b/spec/system/administrateurs/procedure_administrateurs_spec.rb index 5a3988e6b..8d6d3bea6 100644 --- a/spec/system/administrateurs/procedure_administrateurs_spec.rb +++ b/spec/system/administrateurs/procedure_administrateurs_spec.rb @@ -23,7 +23,6 @@ describe 'Administrateurs can manage administrateurs', js: true do scenario 'the administrator can add another administrator' do another_administrateur = create(:administrateur) visit admin_procedure_administrateurs_path(procedure) - find('#administrateurs').click fill_in('administrateur_email', with: another_administrateur.email) @@ -41,8 +40,6 @@ describe 'Administrateurs can manage administrateurs', js: true do administrateur.administrateurs_procedures.update_all(manager: true) visit admin_procedure_administrateurs_path(procedure) - find('#administrateurs').click - expect(page).to have_css("#administrateur_email[disabled=\"disabled\"]") end end