Merge pull request #4211 from betagouv/simplify_role_creation

Simplifie la création de role instructeur ou administrateur

app/controllers/admin/instructeurs_controller.rb

@@ -37,21 +37,15 @@ class Admin::InstructeursController < AdminController
   private
 
   def invite_instructeur(email)
-    user = User.find_by(email: email)
+    user = User.create_or_promote_to_instructeur(
-
+      email,
-    if user.nil?
+      SecureRandom.hex,
-      user = User.create(
+      administrateurs: [current_administrateur]
-        email: email,
+    )
-        password: SecureRandom.hex,
-        confirmed_at: Time.zone.now
-      )
-    end
-
-    if user.errors.empty?
-      @instructeur = Instructeur.create(email: email, administrateurs: [current_administrateur])
-      user.update!(instructeur: @instructeur)
 
+    if user.valid?
       user.invite!
+
       flash.notice = 'Instructeur ajouté'
     else
       flash.alert = user.errors.full_messages

app/controllers/instructeurs/avis_controller.rb

@@ -83,23 +83,13 @@ module Instructeurs
       email = params[:email]
       password = params['instructeur']['password']
 
-      user = User.find_by(email: email)
+      # Not perfect because the password will not be changed if the user already exists
-
+      user = User.create_or_promote_to_instructeur(email, password)
-      if user.nil?
-        user = User.create(
-          email: email,
-          password: password,
-          confirmed_at: Time.zone.now
-        )
-      end
-
-      if user.errors.empty?
-        instructeur = Instructeur.create(email: email)
-        user.update!(instructeur: instructeur)
 
+      if user.valid?
         sign_in(user)
 
-        Avis.link_avis_to_instructeur(instructeur)
+        Avis.link_avis_to_instructeur(user.instructeur)
         redirect_to url_for(instructeur_avis_index_path)
       else
         flash[:alert] = user.errors.full_messages

app/models/administration.rb

@@ -8,32 +8,11 @@ class Administration < ApplicationRecord
   end
 
   def invite_admin(email)
-    password = SecureRandom.hex
+    user = User.create_or_promote_to_administrateur(email, SecureRandom.hex)
 
-    user = User.find_by(email: email)
+    if user.valid?
-
+      AdministrationMailer.new_admin_email(user.administrateur, self).deliver_later
-    if user.nil?
+      user.invite_administrateur!(id)
-      # set confirmed_at otherwise admin confirmation doesnt work
-      # we somehow mess up using reset_password logic instead of
-      # confirmation_logic
-      # FIXME
-      user = User.create(
-        email: email,
-        password: password,
-        confirmed_at: Time.zone.now
-      )
-    end
-
-    if user.errors.empty?
-      if user.instructeur.nil?
-        Instructeur.create!(email: email, user: user)
-      end
-
-      if user.administrateur.nil?
-        administrateur = Administrateur.create!(email: email, active: false, user: user)
-        AdministrationMailer.new_admin_email(administrateur, self).deliver_later
-        user.invite_administrateur!(id)
-      end
     end
 
     user

app/models/user.rb

@@ -61,6 +61,32 @@ class User < ApplicationRecord
     AdministrateurMailer.activate_before_expiration(self, reset_password_token).deliver_later
   end
 
+  def self.create_or_promote_to_instructeur(email, password, administrateurs: [])
+    user = User
+      .create_with(password: password, confirmed_at: Time.zone.now)
+      .find_or_create_by(email: email)
+
+    if user.valid?
+      if user.instructeur_id.nil?
+        user.create_instructeur!(email: email)
+      end
+
+      user.instructeur.administrateurs << administrateurs
+    end
+
+    user
+  end
+
+  def self.create_or_promote_to_administrateur(email, password)
+    user = User.create_or_promote_to_instructeur(email, password)
+
+    if user.valid? && user.administrateur_id.nil?
+      user.create_administrateur!(email: email)
+    end
+
+    user
+  end
+
   private
 
   def link_invites!

spec/factories/administrateur.rb

@@ -4,18 +4,11 @@ FactoryBot.define do
     email { generate(:administrateur_email) }
 
     transient do
-      user { nil }
       password { 'mon chien aime les bananes' }
     end
 
-    after(:create) do |administrateur, evaluator|
+    initialize_with do
-      if evaluator.user.present?
+      User.create_or_promote_to_administrateur(email, password).administrateur
-        user = evaluator.user
-      else
-        user = create(:user, email: administrateur.email, password: evaluator.password, administrateur: administrateur)
-      end
-
-      create(:instructeur, email: administrateur.email, user: user)
     end
   end
 

spec/factories/instructeur.rb

@@ -8,14 +8,8 @@ FactoryBot.define do
       password { 'somethingverycomplated!' }
     end
 
-    after(:create) do |instructeur, evaluator|
+    initialize_with do
-      if evaluator.user.present?
+      User.create_or_promote_to_instructeur(email, password).instructeur
-        user = evaluator.user
-      else
-        user = create(:user, email: instructeur.email, password: evaluator.password)
-      end
-
-      instructeur.update!(user: user)
     end
   end
 end

spec/models/user_spec.rb

@@ -100,4 +100,68 @@ describe User, type: :model do
       it { is_expected.to be_falsey }
     end
   end
+
+  describe '.create_or_promote_to_instructeur' do
+    let(:email) { 'inst1@gmail.com' }
+    let(:password) { 'un super password !' }
+    let(:admins) { [] }
+
+    subject { User.create_or_promote_to_instructeur(email, password, administrateurs: admins) }
+
+    context 'without an existing user' do
+      it do
+        user = subject
+        expect(user.valid_password?(password)).to be true
+        expect(user.confirmed_at).to be_present
+        expect(user.instructeur).to be_present
+      end
+
+      context 'with an administrateur' do
+        let(:admins) { [create(:administrateur)] }
+
+        it do
+          user = subject
+          expect(user.instructeur.administrateurs).to eq(admins)
+        end
+      end
+    end
+
+    context 'with an existing user' do
+      before { create(:user, email: email, password: 'démarches-simplifiées-pwd') }
+
+      it 'keeps the previous password' do
+        user = subject
+        expect(user.valid_password?('démarches-simplifiées-pwd')).to be true
+        expect(user.instructeur).to be_present
+      end
+
+      context 'with an existing instructeur' do
+        let(:old_admins) { [create(:administrateur)] }
+        let(:admins) { [create(:administrateur)] }
+        let!(:instructeur) { Instructeur.create(email: 'i@mail.com', administrateurs: old_admins) }
+
+        before do
+          User
+            .find_by(email: email)
+            .update!(instructeur: instructeur)
+        end
+
+        it 'keeps the existing instructeurs and adds administrateur' do
+          user = subject
+          expect(user.instructeur).to eq(instructeur)
+          expect(user.instructeur.administrateurs).to eq(old_admins + admins)
+        end
+      end
+    end
+
+    context 'with an invalid email' do
+      let(:email) { 'invalid' }
+
+      it 'does not build an instructeur' do
+        user = subject
+        expect(user.valid?).to be false
+        expect(user.instructeur).to be_nil
+      end
+    end
+  end
 end