From f63cd868119b982568fc99e69753559e28529398 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 4 Nov 2019 15:18:09 +0000 Subject: [PATCH 01/10] webhook: fix a crash when the Helpscout customer is unknown --- app/controllers/webhook_controller.rb | 37 ++++++++------- spec/controllers/webhook_controller_spec.rb | 50 +++++++++++++++++++++ 2 files changed, 68 insertions(+), 19 deletions(-) create mode 100644 spec/controllers/webhook_controller_spec.rb diff --git a/app/controllers/webhook_controller.rb b/app/controllers/webhook_controller.rb index 970b07f27..5382d065f 100644 --- a/app/controllers/webhook_controller.rb +++ b/app/controllers/webhook_controller.rb @@ -4,28 +4,27 @@ class WebhookController < ActionController::Base def helpscout email = params[:customer][:email].downcase user = User.find_by(email: email) - instructeur = user.instructeur - administrateur = user.administrateur - html = [] - if user - url = manager_user_url(user) - html << link_to_manager(user, url) - end - - if instructeur - url = manager_instructeur_url(instructeur) - html << link_to_manager(instructeur, url) - end - - if administrateur - url = manager_administrateur_url(administrateur) - html << link_to_manager(administrateur, url) - end - - if html.empty? + if user.nil? head :not_found + else + instructeur = user.instructeur + administrateur = user.administrateur + + url = manager_user_url(user) + html = [link_to_manager(user, url)] + + if instructeur + url = manager_instructeur_url(instructeur) + html << link_to_manager(instructeur, url) + end + + if administrateur + url = manager_administrateur_url(administrateur) + html << link_to_manager(administrateur, url) + end + render json: { html: html.join('
') } end end diff --git a/spec/controllers/webhook_controller_spec.rb b/spec/controllers/webhook_controller_spec.rb new file mode 100644 index 000000000..81e07e8c6 --- /dev/null +++ b/spec/controllers/webhook_controller_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe WebhookController, type: :controller do + describe '#helpscout' do + before { allow(controller).to receive(:verify_signature!).and_return(true) } + + subject(:response) { get :helpscout, params: { customer: { email: customer_email } } } + + let(:payload) { JSON.parse(subject.body) } + + context 'when there is no matching user' do + let(:customer_email) { 'not-a-user@exemple.fr' } + + it 'returns an empty response' do + expect(subject.status).to eq(404) + expect(subject.body).to be_empty + end + end + + context 'when there is a matching user' do + let(:user) { create(:user) } + let(:customer_email) { user.email } + + it 'returns a 200 response' do + expect(subject.status).to eq(200) + expect(subject.body).to be_present + end + + it 'returns a link to the User profile in the Manager' do + expect(payload).to have_key('html') + expect(payload['html']).to have_selector("a[href='#{manager_user_url(user)}']") + end + + context 'when there are an associated Instructeur and Administrateur' do + let!(:instructeur) { create(:instructeur, user: user) } + let!(:admin) { create(:administrateur, user: user) } + + it 'returns a link to the Instructeur profile in the Manager' do + expect(payload).to have_key('html') + expect(payload['html']).to have_selector("a[href='#{manager_instructeur_url(instructeur)}']") + end + + it 'returns a link to the Administrateur profile in the Manager' do + expect(payload).to have_key('html') + expect(payload['html']).to have_selector("a[href='#{manager_administrateur_url(admin)}']") + end + end + end + end +end From 243bc2887f18bb7a26d743f95f78d0b3d881fff4 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 09:31:06 +0100 Subject: [PATCH 02/10] Invite_administrateur!: remove unused return --- app/models/user.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index a94d9212d..70aebf176 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -53,8 +53,6 @@ class User < ApplicationRecord reset_password_token = set_reset_password_token AdministrationMailer.invite_admin(self, reset_password_token, administration_id).deliver_later - - reset_password_token end def remind_invitation! From 53c7997081d5c9ba3b91f479cf608513d49cf921 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 09:12:14 +0100 Subject: [PATCH 03/10] Invite_administrateur!: characterization test --- spec/models/user_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 430e0b1d4..f9009e0a1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -164,4 +164,22 @@ describe User, type: :model do end end end + + describe 'invite_administrateur!' do + let(:administration) { create(:administration) } + let(:administrateur) { create(:administrateur) } + let(:user) { administrateur.user } + + subject { user.invite_administrateur!(administration.id) } + + context 'when the user is inactif' do + before do + mailer_double = double('mailer', deliver_later: true) + allow(AdministrationMailer).to receive(:invite_admin).and_return(mailer_double) + subject + end + + it { expect(AdministrationMailer).to have_received(:invite_admin).with(user, kind_of(String), administration.id) } + end + end end From e3d7688e66c4e5aca5938db94d3da1f1867fd5c7 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 09:37:53 +0100 Subject: [PATCH 04/10] [fix #4473] Invite_administrateur!: do not reset password if the user is active --- app/models/user.rb | 7 ++++--- .../administration_mailer/invite_admin.html.haml | 13 +++++++++---- .../previews/administration_mailer_preview.rb | 4 ++++ spec/models/user_spec.rb | 11 +++++++++++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 70aebf176..126c7adec 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,11 +47,12 @@ class User < ApplicationRecord end def invite_administrateur!(administration_id) - if administrateur.active? - raise "Impossible d'inviter un utilisateur déjà actif !" + reset_password_token = nil + + if !administrateur.active? + reset_password_token = set_reset_password_token end - reset_password_token = set_reset_password_token AdministrationMailer.invite_admin(self, reset_password_token, administration_id).deliver_later end diff --git a/app/views/administration_mailer/invite_admin.html.haml b/app/views/administration_mailer/invite_admin.html.haml index f646d2946..b048e8b30 100644 --- a/app/views/administration_mailer/invite_admin.html.haml +++ b/app/views/administration_mailer/invite_admin.html.haml @@ -9,10 +9,15 @@ %p Votre compte administrateur a été créé pour l'adresse email #{@admin.email}. -%p - %b - Pour l’activer, cliquez sur le lien suivant : - = link_to(admin_activate_url(token: @reset_password_token), admin_activate_url(token: @reset_password_token)) +- if @reset_password_token.present? + %p + %b + Pour l’activer, cliquez sur le lien suivant : + = link_to(admin_activate_url(token: @reset_password_token), admin_activate_url(token: @reset_password_token)) +- else + %p + Pour vous connecter, cliquez sur le lien suivant : + = link_to(new_user_session_url, new_user_session_url) %p = render partial: "layouts/mailers/bizdev_signature", locals: { author_name: @author_name } diff --git a/spec/mailers/previews/administration_mailer_preview.rb b/spec/mailers/previews/administration_mailer_preview.rb index e0fd0b0df..7527f9632 100644 --- a/spec/mailers/previews/administration_mailer_preview.rb +++ b/spec/mailers/previews/administration_mailer_preview.rb @@ -11,6 +11,10 @@ class AdministrationMailerPreview < ActionMailer::Preview AdministrationMailer.invite_admin(administrateur, "12345678", 0) end + def invite_admin_whose_already_has_an_account + AdministrationMailer.invite_admin(administrateur, nil, 0) + end + def refuse_admin AdministrationMailer.refuse_admin('bad_admin@pipo.com') end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f9009e0a1..a113cfd84 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -181,5 +181,16 @@ describe User, type: :model do it { expect(AdministrationMailer).to have_received(:invite_admin).with(user, kind_of(String), administration.id) } end + + context 'when the user is actif' do + before do + user.update(last_sign_in_at: Time.zone.now) + mailer_double = double('mailer', deliver_later: true) + allow(AdministrationMailer).to receive(:invite_admin).and_return(mailer_double) + subject + end + + it { expect(AdministrationMailer).to have_received(:invite_admin).with(user, nil, administration.id) } + end end end From aeb44dd2aadf633c5238b9bdc95d9f5e62fadf0a Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 09:52:25 +0100 Subject: [PATCH 05/10] Cleaning spec --- spec/models/user_spec.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a113cfd84..4ff839507 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -170,14 +170,14 @@ describe User, type: :model do let(:administrateur) { create(:administrateur) } let(:user) { administrateur.user } + let(:mailer_double) { double('mailer', deliver_later: true) } + + before { allow(AdministrationMailer).to receive(:invite_admin).and_return(mailer_double) } + subject { user.invite_administrateur!(administration.id) } context 'when the user is inactif' do - before do - mailer_double = double('mailer', deliver_later: true) - allow(AdministrationMailer).to receive(:invite_admin).and_return(mailer_double) - subject - end + before { subject } it { expect(AdministrationMailer).to have_received(:invite_admin).with(user, kind_of(String), administration.id) } end @@ -185,8 +185,6 @@ describe User, type: :model do context 'when the user is actif' do before do user.update(last_sign_in_at: Time.zone.now) - mailer_double = double('mailer', deliver_later: true) - allow(AdministrationMailer).to receive(:invite_admin).and_return(mailer_double) subject end From c37bfc7349b3857811b7a3f233b644a815465b11 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 4 Nov 2019 18:32:56 +0100 Subject: [PATCH 06/10] instructeurs: fix email casing in invitation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When inviting an instructeur, the code first looked up an existing instructeur using the normalized (downcased) email address–but then tried to promote it using the non-normalized version, which failed. This fixes the issue by always using the normalized email. --- app/controllers/admin/instructeurs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/instructeurs_controller.rb b/app/controllers/admin/instructeurs_controller.rb index fd35b15b5..5a179eac5 100644 --- a/app/controllers/admin/instructeurs_controller.rb +++ b/app/controllers/admin/instructeurs_controller.rb @@ -15,7 +15,7 @@ class Admin::InstructeursController < AdminController procedure_id = params[:procedure_id] if @instructeur.nil? - invite_instructeur(params[:instructeur][:email]) + invite_instructeur(email) else assign_instructeur! end From b193dd1465cdbe00b0270d7e83c0f333227d4d15 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 10:01:07 +0100 Subject: [PATCH 07/10] User get the active notion --- app/models/user.rb | 4 ++++ spec/models/user_spec.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 126c7adec..62176d692 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -92,6 +92,10 @@ class User < ApplicationRecord "User:#{id}" end + def active? + last_sign_in_at.present? + end + private def link_invites! diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4ff839507..a24949fbc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -191,4 +191,22 @@ describe User, type: :model do it { expect(AdministrationMailer).to have_received(:invite_admin).with(user, nil, administration.id) } end end + + describe '#active?' do + let!(:user) { create(:user) } + + subject { user.active? } + + context 'when the user has never signed in' do + before { user.update(last_sign_in_at: nil) } + + it { is_expected.to be false } + end + + context 'when the user has already signed in' do + before { user.update(last_sign_in_at: Time.zone.now) } + + it { is_expected.to be true } + end + end end From 5643e671a0de8896e294419fdeb8c03309250ad1 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 10:05:59 +0100 Subject: [PATCH 08/10] Code use user.active? --- app/controllers/application_controller.rb | 2 +- app/models/administrateur.rb | 4 ++-- app/models/user.rb | 2 +- app/services/administrateur_usage_statistics_service.rb | 2 +- spec/features/admin/admin_creation_spec.rb | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 03c8bcd57..1ee6f1411 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -250,7 +250,7 @@ class ApplicationController < ActionController::Base payload: { DS_SIGN_IN_COUNT: current_user&.sign_in_count, DS_CREATED_AT: current_administrateur&.created_at, - DS_ACTIVE: current_administrateur&.active?, + DS_ACTIVE: current_user&.active?, DS_ID: current_administrateur&.id, DS_GESTIONNAIRE_ID: current_instructeur&.id, DS_ROLES: current_user_roles diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index a874fbe27..2f834f39a 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -46,7 +46,7 @@ class Administrateur < ApplicationRecord end def registration_state - if active? + if user.active? 'Actif' elsif user.reset_password_period_valid? 'En attente' @@ -56,7 +56,7 @@ class Administrateur < ApplicationRecord end def invitation_expired? - !active? && !user.reset_password_period_valid? + !user.active? && !user.reset_password_period_valid? end def self.reset_password(reset_password_token, password) diff --git a/app/models/user.rb b/app/models/user.rb index 62176d692..41a4984b3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -49,7 +49,7 @@ class User < ApplicationRecord def invite_administrateur!(administration_id) reset_password_token = nil - if !administrateur.active? + if !active? reset_password_token = set_reset_password_token end diff --git a/app/services/administrateur_usage_statistics_service.rb b/app/services/administrateur_usage_statistics_service.rb index b9b38f868..5c85f2a96 100644 --- a/app/services/administrateur_usage_statistics_service.rb +++ b/app/services/administrateur_usage_statistics_service.rb @@ -29,7 +29,7 @@ class AdministrateurUsageStatisticsService result = { ds_sign_in_count: administrateur.user.sign_in_count, ds_created_at: administrateur.created_at, - ds_active: administrateur.active?, + ds_active: administrateur.user.active?, ds_id: administrateur.id, nb_services: nb_services_by_administrateur_id[administrateur.id], nb_instructeurs: nb_instructeurs_by_administrateur_id[administrateur.id], diff --git a/spec/features/admin/admin_creation_spec.rb b/spec/features/admin/admin_creation_spec.rb index 166983b64..de719edc9 100644 --- a/spec/features/admin/admin_creation_spec.rb +++ b/spec/features/admin/admin_creation_spec.rb @@ -12,7 +12,7 @@ feature 'As an administrateur', js: true do end scenario 'I can register' do - expect(new_admin.reload.active?).to be(false) + expect(new_admin.reload.user.active?).to be(false) confirmation_email = open_email(admin_email) token_params = confirmation_email.body.match(/token=[^"]+/) @@ -24,6 +24,6 @@ feature 'As an administrateur', js: true do expect(page).to have_content 'Mot de passe enregistré' - expect(new_admin.reload.active?).to be(true) + expect(new_admin.reload.user.active?).to be(true) end end From 395aba8bbcb825b0214db8a1df23fc8da15fdb6f Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 10:06:54 +0100 Subject: [PATCH 09/10] Remove administrateur active notion --- app/models/administrateur.rb | 4 ---- spec/models/administrateur_spec.rb | 18 ------------------ 2 files changed, 22 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 2f834f39a..0c43f0eca 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -80,8 +80,4 @@ class Administrateur < ApplicationRecord def can_be_deleted? dossiers.state_instruction_commencee.none? && procedures.none? end - - def active? - user.last_sign_in_at.present? - end end diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index e06c53630..4b9fef1f0 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -50,22 +50,4 @@ describe Administrateur, type: :model do # it { expect(subject).to eq([]) } # end # end - - describe '#active?' do - let!(:administrateur) { create(:administrateur) } - - subject { administrateur.active? } - - context 'when the user has never signed in' do - before { administrateur.user.update(last_sign_in_at: nil) } - - it { is_expected.to be false } - end - - context 'when the user has already signed in' do - before { administrateur.user.update(last_sign_in_at: Time.zone.now) } - - it { is_expected.to be true } - end - end end From 9f5169eb7d59b1ff40c041cd7e4e93b40c8525ed Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Nov 2019 10:18:37 +0100 Subject: [PATCH 10/10] Remove unused Administrateur.reset_password method --- app/models/administrateur.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 0c43f0eca..d097ebd74 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -59,16 +59,6 @@ class Administrateur < ApplicationRecord !user.active? && !user.reset_password_period_valid? end - def self.reset_password(reset_password_token, password) - administrateur = self.reset_password_by_token({ - password: password, - password_confirmation: password, - reset_password_token: reset_password_token - }) - - administrateur - end - def owns?(procedure) procedure.administrateurs.include?(self) end