Merge pull request #4742 from betagouv/4532-supprime-email-column-de-la-table-admin

#4532 supprime la colonne email de la table admin

app/controllers/admin/procedures_controller.rb

@@ -79,7 +79,7 @@ class Admin::ProceduresController < AdminController
   end
 
   def transfer
-    admin = Administrateur.find_by(email: params[:email_admin].downcase)
+    admin = Administrateur.by_email(params[:email_admin].downcase)
 
     if admin.nil?
       render '/admin/procedures/transfer', formats: 'js', status: 404

app/controllers/manager/demandes_controller.rb

@@ -48,9 +48,9 @@ module Manager
     end
 
     def pending_demandes
-      already_approved_emails = Administrateur
-        .where(email: demandes.map { |d| d[:email] })
-        .pluck(:email)
+      already_approved_emails = Administrateur.eager(:user)
+        .where(users: { email: demandes.map { |d| d[:email] } })
+        .map(&:email)
 
       demandes.reject { |demande| already_approved_emails.include?(demande[:email]) }
     end

app/controllers/manager/procedures_controller.rb

@@ -29,7 +29,7 @@ module Manager
     end
 
     def add_administrateur
-      administrateur = Administrateur.find_by(email: params[:email])
+      administrateur = Administrateur.by_email(params[:email])
       if administrateur
         procedure.administrateurs << administrateur
         flash[:notice] = "L'administrateur \"#{params[:email]}\" est ajouté à la démarche."

app/controllers/new_administrateur/procedure_administrateurs_controller.rb

@@ -9,7 +9,7 @@ module NewAdministrateur
       email = params.require(:administrateur)[:email]&.strip&.downcase
 
       # Find the admin
-      administrateur = Administrateur.find_by(email: email)
+      administrateur = Administrateur.by_email(email)
       if administrateur.nil?
         flash.alert = "L’administrateur « #{email} » n’existe pas. Invitez-le à demander un compte administrateur à l’addresse <a href=#{new_demande_url}>#{new_demande_url}</a>."
         return

app/controllers/users/passwords_controller.rb

@@ -11,7 +11,7 @@ class Users::PasswordsController < Devise::PasswordsController
   def create
     # Check the credentials associated to the mail to generate a correct reset link
     email = params[:user][:email]
-    if Administrateur.find_by(email: email)
+    if Administrateur.by_email(email)
       @devise_mapping = Devise.mappings[:administrateur]
       params[:administrateur] = params[:user]
       # uncomment to check password complexity for Instructeur
@@ -56,7 +56,7 @@ class Users::PasswordsController < Devise::PasswordsController
 
   def try_to_authenticate_administrateur
     if user_signed_in?
-      administrateur = Administrateur.find_by(email: current_user.email)
+      administrateur = Administrateur.by_email(current_user.email)
 
       if administrateur
         sign_in(administrateur.user)

app/models/administrateur.rb

@@ -1,6 +1,5 @@
 class Administrateur < ApplicationRecord
-  self.ignored_columns = ['features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at']
-  include EmailSanitizableConcern
+  self.ignored_columns = ['email', 'features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at']
   include ActiveRecord::SecureToken
 
   has_and_belongs_to_many :instructeurs
@@ -10,11 +9,19 @@ class Administrateur < ApplicationRecord
 
   has_one :user, dependent: :nullify
 
-  before_validation -> { sanitize_email(:email) }
+  default_scope { eager_load(:user) }
 
   scope :inactive, -> { joins(:user).where(users: { last_sign_in_at: nil }) }
   scope :with_publiees_ou_closes, -> { joins(:procedures).where(procedures: { aasm_state: [:publiee, :close, :depubliee] }) }
 
+  def self.by_email(email)
+    Administrateur.find_by(users: { email: email })
+  end
+
+  def email
+    user.email
+  end
+
   # validate :password_complexity, if: Proc.new { |a| Devise.password_length.include?(a.password.try(:size)) }
 
   def password_complexity

app/models/dossier.rb

@@ -381,7 +381,7 @@ class Dossier < ApplicationRecord
     update(hidden_at: deleted_dossier.deleted_at)
 
     if en_construction?
-      administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.pluck(:email)
+      administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.map(&:email)
       administration_emails.each do |email|
         DossierMailer.notify_deletion_to_administration(deleted_dossier, email).deliver_later
       end

app/models/user.rb

@@ -82,7 +82,7 @@ class User < ApplicationRecord
     user = User.create_or_promote_to_instructeur(email, password)
 
     if user.valid? && user.administrateur_id.nil?
-      user.create_administrateur!(email: email)
+      user.create_administrateur!
     end
 
     user

app/services/administrateur_usage_statistics_service.rb

@@ -119,7 +119,7 @@ class AdministrateurUsageStatisticsService
   end
 
   def nb_instructeurs_by_administrateur_id
-    @nb_instructeurs_by_administrateur_id ||= with_default(0, Administrateur.joins(:instructeurs).group(:administrateur_id).count)
+    @nb_instructeurs_by_administrateur_id ||= with_default(0, Administrateur.joins(:instructeurs).group('administrateurs.id').count)
   end
 
   def nb_dossiers_by_administrateur_id_and_procedure_id_and_synthetic_state

app/views/admin/procedures/new_from_existing.html.haml

@@ -56,4 +56,4 @@
               %td.flex
                 = link_to('Consulter', apercu_procedure_path(id: procedure.id), target: "_blank", rel: "noopener", class: 'button small')
                 = link_to('Cloner', admin_procedure_clone_path(procedure.id, from_new_from_existing: true), 'data-method' => :put, class: 'button small primary')
-                = link_to('Contacter', "mailto:#{procedure.administrateurs.pluck(:email) * ","}", class: 'button small')
+                = link_to('Contacter', "mailto:#{procedure.administrateurs.map(&:email) * ","}", class: 'button small')

app/views/new_administrateur/procedure_administrateurs/_add_admin_form.html.haml

@@ -1,4 +1,4 @@
-= form_for procedure.administrateurs.new,
+= form_for procedure.administrateurs.new(user: User.new),
   url: { controller: 'procedure_administrateurs' },
   html: { class: 'form', id:  "procedure-#{procedure.id}-new_administrateur" } ,
   remote: true do |f|

app/views/new_administrateur/procedure_administrateurs/index.html.haml

@@ -11,7 +11,7 @@
       %th= 'Enregistré le'
       %th= 'État'
     %tbody{ id: "procedure-#{@procedure.id}-administrateurs" }
-      = render partial: 'administrateur', collection: @procedure.administrateurs.order(:email)
+      = render partial: 'administrateur', collection: @procedure.administrateurs.order('users.email')
     %tfoot
       %tr
         %th{ colspan: 4 }

db/migrate/20200130165328_remove_unique_constraint_on_administrateur_emails.rb

@@ -0,0 +1,13 @@
+class RemoveUniqueConstraintOnAdministrateurEmails < ActiveRecord::Migration[5.2]
+  def up
+    # Drop the index entirely
+    remove_index :administrateurs, :email
+    # Add the index again, without the unicity constraint
+    add_index :administrateurs, :email
+  end
+
+  def down
+    remove_index :administrateurs, :email
+    add_index :administrateurs, :email, unique: true
+  end
+end

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: 2020_01_14_113700) do
+ActiveRecord::Schema.define(version: 2020_01_30_165328) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -53,7 +53,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_113700) do
     t.datetime "updated_at"
     t.boolean "active", default: false
     t.string "encrypted_token"
-    t.index ["email"], name: "index_administrateurs_on_email", unique: true
+    t.index ["email"], name: "index_administrateurs_on_email"
   end
 
   create_table "administrateurs_instructeurs", id: false, force: :cascade do |t|

spec/factories/administrateur.rb

@@ -1,9 +1,8 @@
 FactoryBot.define do
   sequence(:administrateur_email) { |n| "admin#{n}@admin.com" }
   factory :administrateur do
-    email { generate(:administrateur_email) }
-
     transient do
+      email { generate(:administrateur_email) }
       password { 'mon chien aime les bananes' }
     end
 

spec/features/admin/admin_creation_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'As an administrateur', js: true do
   let(:administration) { create(:administration) }
   let(:admin_email) { 'new_admin@gouv.fr' }
-  let(:new_admin) { Administrateur.find_by(email: admin_email) }
+  let(:new_admin) { Administrateur.by_email(admin_email) }
 
   before do
     perform_enqueued_jobs do

spec/features/admin/procedure_update_spec.rb

@@ -59,4 +59,18 @@ feature 'Administrateurs can edit procedures', js: true do
       expect(page).to have_field('procedure_libelle', with: 'Ma petite démarche')
     end
   end
+
+  scenario 'the administrator can add another administrator' do
+    another_administrateur = create(:administrateur)
+    visit admin_procedure_path(procedure)
+    click_on 'Administrateurs'
+
+    fill_in('administrateur_email', with: another_administrateur.email)
+
+    click_on 'Ajouter comme administrateur'
+
+    within('.alert-success') do
+      expect(page).to have_content(another_administrateur.email)
+    end
+  end
 end

spec/mailers/previews/administration_mailer_preview.rb

@@ -41,6 +41,6 @@ class AdministrationMailerPreview < ActionMailer::Preview
   end
 
   def administrateur
-    Administrateur.new(id: 111, email: "chef.de.service@administration.gouv.fr")
+    Administrateur.new(id: 111, user: User.new(email: "chef.de.service@administration.gouv.fr"))
   end
 end