Merge pull request #7069 from betagouv/main

2022-03-22-01

app/controllers/instructeurs/dossiers_controller.rb

@@ -203,8 +203,7 @@ module Instructeurs
     end
 
     def update_annotations
-      dossier = current_instructeur.dossiers.includes(champs_private: :type_de_champ).find(params[:dossier_id])
-      dossier.assign_attributes(champs_private_params)
+      dossier_with_champs.assign_attributes(champs_private_params)
       if dossier.champs_private.any?(&:changed?)
         dossier.last_champ_private_updated_at = Time.zone.now
       end
@@ -278,19 +277,19 @@ module Instructeurs
     end
 
     def mark_demande_as_read
-      current_instructeur.mark_tab_as_seen(@dossier, :demande)
+      current_instructeur.mark_tab_as_seen(dossier, :demande)
     end
 
     def mark_messagerie_as_read
-      current_instructeur.mark_tab_as_seen(@dossier, :messagerie)
+      current_instructeur.mark_tab_as_seen(dossier, :messagerie)
     end
 
     def mark_avis_as_read
-      current_instructeur.mark_tab_as_seen(@dossier, :avis)
+      current_instructeur.mark_tab_as_seen(dossier, :avis)
     end
 
     def mark_annotations_privees_as_read
-      current_instructeur.mark_tab_as_seen(@dossier, :annotations_privees)
+      current_instructeur.mark_tab_as_seen(dossier, :annotations_privees)
     end
 
     def aasm_error_message(exception, target_state:)

app/javascript/components/MapReader/components/GeoJSONLayer.tsx

@@ -144,20 +144,24 @@ function LineStringLayer({
   const sourceId = String(feature.properties?.id);
   const layerId = `${sourceId}-layer`;
 
-  useEffect(() => {
-    map
-      .addSource(sourceId, {
-        type: 'geojson',
-        data: feature
-      })
-      .addLayer({
-        id: layerId,
-        source: sourceId,
-        type: 'line',
-        paint: lineStringSelectionLine
-      });
-  }, [map, layerId, sourceId, feature]);
+  const render = () => {
+    if (!map.getSource(sourceId)) {
+      map
+        .addSource(sourceId, {
+          type: 'geojson',
+          data: feature
+        })
+        .addLayer({
+          id: layerId,
+          source: sourceId,
+          type: 'line',
+          paint: lineStringSelectionLine
+        });
+    }
+  };
 
+  useEffect(render, [map, layerId, sourceId, feature]);
+  useMapEvent('styledata', render);
   useMapEvent('mouseenter', onMouseEnter, layerId);
   useMapEvent('mouseleave', onMouseLeave, layerId);
 
@@ -177,20 +181,24 @@ function PointLayer({
   const sourceId = String(feature.properties?.id);
   const layerId = `${sourceId}-layer`;
 
-  useEffect(() => {
-    map
-      .addSource(sourceId, {
-        type: 'geojson',
-        data: feature
-      })
-      .addLayer({
-        id: layerId,
-        source: sourceId,
-        type: 'circle',
-        paint: pointSelectionCircle
-      });
-  }, [map, layerId, sourceId, feature]);
+  const render = () => {
+    if (!map.getSource(sourceId)) {
+      map
+        .addSource(sourceId, {
+          type: 'geojson',
+          data: feature
+        })
+        .addLayer({
+          id: layerId,
+          source: sourceId,
+          type: 'circle',
+          paint: pointSelectionCircle
+        });
+    }
+  };
 
+  useEffect(render, [map, layerId, sourceId, feature]);
+  useMapEvent('styledata', render);
   useMapEvent('mouseenter', onMouseEnter, layerId);
   useMapEvent('mouseleave', onMouseLeave, layerId);
 
@@ -211,26 +219,30 @@ function PolygonLayer({
   const layerId = `${sourceId}-layer`;
   const lineLayerId = `${sourceId}-line-layer`;
 
-  useEffect(() => {
-    map
-      .addSource(sourceId, {
-        type: 'geojson',
-        data: feature
-      })
-      .addLayer({
-        id: lineLayerId,
-        source: sourceId,
-        type: 'line',
-        paint: polygonSelectionLine
-      })
-      .addLayer({
-        id: layerId,
-        source: sourceId,
-        type: 'fill',
-        paint: polygonSelectionFill
-      });
-  }, [map, layerId, lineLayerId, sourceId, feature]);
+  const render = () => {
+    if (!map.getSource(sourceId)) {
+      map
+        .addSource(sourceId, {
+          type: 'geojson',
+          data: feature
+        })
+        .addLayer({
+          id: lineLayerId,
+          source: sourceId,
+          type: 'line',
+          paint: polygonSelectionLine
+        })
+        .addLayer({
+          id: layerId,
+          source: sourceId,
+          type: 'fill',
+          paint: polygonSelectionFill
+        });
+    }
+  };
 
+  useEffect(render, [map, layerId, lineLayerId, sourceId, feature]);
+  useMapEvent('styledata', render);
   useMapEvent('mouseenter', onMouseEnter, layerId);
   useMapEvent('mouseleave', onMouseLeave, layerId);
 

app/models/administrateur.rb

@@ -16,7 +16,7 @@ class Administrateur < ApplicationRecord
   has_and_belongs_to_many :procedures
   has_many :services
 
-  has_one :user, dependent: :nullify
+  belongs_to :user
 
   default_scope { eager_load(:user) }
 

app/models/expert.rb

@@ -8,7 +8,7 @@
 #  user_id    :bigint
 #
 class Expert < ApplicationRecord
-  has_one :user
+  belongs_to :user
   has_many :experts_procedures
   has_many :procedures, through: :experts_procedures
   has_many :avis, through: :experts_procedures

app/models/instructeur.rb

@@ -31,7 +31,7 @@ class Instructeur < ApplicationRecord
   has_many :archives
   has_many :bulk_messages, dependent: :destroy
 
-  has_one :user, dependent: :nullify
+  belongs_to :user
 
   scope :with_instant_email_message_notifications, -> {
     includes(:assign_to).where(assign_tos: { instant_email_message_notifications_enabled: true })

app/models/procedure_presentation.rb

@@ -121,10 +121,10 @@ class ProcedurePresentation < ApplicationRecord
       end
     when 'followers_instructeurs'
       assert_supported_column(table, column)
-      # LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet
+      # LEFT OUTER JOIN allows to keep dossiers without assigned instructeurs yet
       dossiers
         .includes(:followers_instructeurs)
-        .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id')
+        .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id')
         .order("instructeurs_users.email #{order}")
         .pluck(:id)
         .uniq
@@ -167,7 +167,7 @@ class ProcedurePresentation < ApplicationRecord
         assert_supported_column(table, column)
         dossiers
           .includes(:followers_instructeurs)
-          .joins('INNER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id')
+          .joins('INNER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id')
           .filter_ilike('instructeurs_users', :email, values)
       when 'user', 'individual'
         dossiers

app/models/user.rb

@@ -25,9 +25,6 @@
 #  unlock_token                 :string
 #  created_at                   :datetime
 #  updated_at                   :datetime
-#  administrateur_id            :bigint
-#  expert_id                    :bigint
-#  instructeur_id               :bigint
 #  requested_merge_into_id      :bigint
 #
 class User < ApplicationRecord
@@ -44,6 +41,8 @@ class User < ApplicationRecord
   devise :database_authenticatable, :registerable, :async,
     :recoverable, :rememberable, :trackable, :validatable, :confirmable, :lockable
 
+  self.ignored_columns = [:administrateur_id, :instructeur_id, :expert_id]
+
   has_many :dossiers, dependent: :destroy
   has_many :invites, dependent: :destroy
   has_many :dossiers_invites, through: :invites, source: :dossier
@@ -52,9 +51,9 @@ class User < ApplicationRecord
   has_many :requested_merge_from, class_name: 'User', dependent: :nullify, inverse_of: :requested_merge_into, foreign_key: :requested_merge_into_id
 
   has_one :france_connect_information, dependent: :destroy
-  belongs_to :instructeur, optional: true, dependent: :destroy
-  belongs_to :administrateur, optional: true, dependent: :destroy
-  belongs_to :expert, optional: true, dependent: :destroy
+  has_one :instructeur, dependent: :destroy
+  has_one :administrateur, dependent: :destroy
+  has_one :expert, dependent: :destroy
   belongs_to :requested_merge_into, class_name: 'User', optional: true
 
   accepts_nested_attributes_for :france_connect_information
@@ -65,21 +64,6 @@ class User < ApplicationRecord
 
   validate :does_not_merge_on_self, if: :requested_merge_into_id_changed?
 
-  # Temporary code for double writing the admin, instructeur and expert id to the foreign key
-  after_save do
-    if saved_change_to_attribute?(:administrateur_id) && administrateur_id.present?
-      Administrateur.find(administrateur_id).update!(user_id: id)
-    end
-
-    if saved_change_to_attribute?(:instructeur_id) && instructeur_id.present?
-      Instructeur.find(instructeur_id).update!(user_id: id)
-    end
-
-    if saved_change_to_attribute?(:expert_id) && expert_id.present?
-      Expert.find(expert_id).update!(user_id: id)
-    end
-  end
-
   def validate_password_complexity?
     administrateur?
   end
@@ -135,7 +119,7 @@ class User < ApplicationRecord
       .find_or_create_by(email: email)
 
     if user.valid?
-      if user.instructeur_id.nil?
+      if user.instructeur.nil?
         user.create_instructeur!
         user.update(france_connect_information: nil)
       end
@@ -149,7 +133,7 @@ class User < ApplicationRecord
   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?
+    if user.valid? && user.administrateur.nil?
       user.create_administrateur!
       user.update(france_connect_information: nil)
     end
@@ -163,7 +147,7 @@ class User < ApplicationRecord
       .find_or_create_by(email: email)
 
     if user.valid?
-      if user.expert_id.nil?
+      if user.expert.nil?
         user.create_expert!
       end
     end
@@ -180,15 +164,15 @@ class User < ApplicationRecord
   end
 
   def administrateur?
-    administrateur_id.present?
+    administrateur.present?
   end
 
   def instructeur?
-    instructeur_id.present?
+    instructeur.present?
   end
 
   def expert?
-    expert_id.present?
+    expert.present?
   end
 
   def can_france_connect?

spec/controllers/instructeurs/dossiers_controller_spec.rb

@@ -408,8 +408,9 @@ describe Instructeurs::DossiersController, type: :controller do
   end
 
   describe '#messagerie' do
+    before { expect(controller.current_instructeur).to receive(:mark_tab_as_seen).with(dossier, :messagerie) }
     subject { get :messagerie, params: { procedure_id: procedure.id, dossier_id: dossier.id } }
-    it { expect(response).to have_http_status(:ok) }
+    it { expect(subject).to have_http_status(:ok) }
   end
 
   describe "#create_commentaire" do
@@ -431,6 +432,7 @@ describe Instructeurs::DossiersController, type: :controller do
     }
 
     before do
+      expect(controller.current_instructeur).to receive(:mark_tab_as_seen).with(dossier, :messagerie)
       allow(ClamavService).to receive(:safe_file?).and_return(scan_result)
       Timecop.freeze(now)
     end
@@ -480,6 +482,10 @@ describe Instructeurs::DossiersController, type: :controller do
     let(:saved_avis) { dossier.avis.first }
     let!(:old_avis_count) { Avis.count }
 
+    before do
+      expect(controller.current_instructeur).to receive(:mark_tab_as_seen).with(dossier, :avis)
+    end
+
     subject do
       post :create_avis, params: {
         procedure_id: procedure.id,
@@ -617,7 +623,10 @@ describe Instructeurs::DossiersController, type: :controller do
         }
       end
 
-      before { subject }
+      before do
+        expect(controller.current_instructeur).to receive(:mark_tab_as_seen).with(dossier, :demande)
+        subject
+      end
 
       it { expect(assigns(:include_infos_administration)).to eq(true) }
       it { expect(response).to render_template 'dossiers/show' }
@@ -654,6 +663,7 @@ describe Instructeurs::DossiersController, type: :controller do
     end
 
     before do
+      expect(controller.current_instructeur).to receive(:mark_tab_as_seen).with(dossier, :annotations_privees)
       another_instructeur.follow(dossier)
       Timecop.freeze(now)
       patch :update_annotations, params: params

spec/models/procedure_presentation_spec.rb

@@ -347,14 +347,14 @@ describe ProcedurePresentation do
       let(:table) { 'followers_instructeurs' }
       let(:order) { 'asc' } # Desc works the same, no extra test required
 
-      let!(:dossier_a) { create(:dossier, :en_construction, procedure: procedure) }
       let!(:dossier_z) { create(:dossier, :en_construction, procedure: procedure) }
+      let!(:dossier_a) { create(:dossier, :en_construction, procedure: procedure) }
       let!(:dossier_without_instructeur) { create(:dossier, :en_construction, procedure: procedure) }
 
       before do
+        create(:follow, dossier: dossier_z, instructeur: create(:instructeur, email: 'zythum@exemple.fr'))
         create(:follow, dossier: dossier_a, instructeur: create(:instructeur, email: 'abaca@exemple.fr'))
         create(:follow, dossier: dossier_a, instructeur: create(:instructeur, email: 'abaca2@exemple.fr'))
-        create(:follow, dossier: dossier_z, instructeur: create(:instructeur, email: 'zythum@exemple.fr'))
       end
 
       context 'for email column' do

spec/models/user_spec.rb

@@ -372,7 +372,7 @@ describe User, type: :model do
     end
 
     context 'for administrateurs' do
-      let(:user) { build(:user, email: 'admin@exemple.fr', password: password, administrateur: create(:administrateur, user: nil)) }
+      let(:user) { build(:user, email: 'admin@exemple.fr', password: password, administrateur: build(:administrateur, user: nil)) }
 
       context 'when the password is too short' do
         let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) }
@@ -453,8 +453,8 @@ describe User, type: :model do
         targeted_user.reload
 
         expect(targeted_user.instructeur).to match(instructeur)
-        expect(targeted_user.expert).to match(expert)
         expect(targeted_user.administrateur).to match(administrateur)
+        expect(targeted_user.expert).to match(expert)
       end
 
       context 'and the targeted account owns an instructeur and expert as well' do