Merge pull request #5550 from betagouv/dev

2020-09-08-01

app/graphql/schema.graphql

@@ -304,6 +304,7 @@ type Demarche {
     """
     updatedSince: ISO8601DateTime
   ): DossierConnection!
+  draftRevision: Revision!
   groupeInstructeurs: [GroupeInstructeur!]!
   id: ID!
 
@@ -311,6 +312,8 @@ type Demarche {
   Le numero de la démarche.
   """
   number: Int!
+  publishedRevision: Revision
+  revisions: [Revision!]!
   service: Service!
 
   """
@@ -411,6 +414,7 @@ type Dossier {
   Le numero du dossier.
   """
   number: Int!
+  revision: Revision!
 
   """
   L'état du dossier.
@@ -1097,6 +1101,17 @@ type RepetitionChamp implements Champ {
   stringValue: String
 }
 
+type Revision {
+  annotationDescriptors: [ChampDescriptor!]!
+  champDescriptors: [ChampDescriptor!]!
+
+  """
+  Date de la création.
+  """
+  dateCreation: ISO8601DateTime!
+  id: ID!
+}
+
 type SelectionUtilisateur implements GeoArea {
   geometry: GeoJSON!
   id: ID!

app/graphql/types/demarche_type.rb

@@ -43,6 +43,10 @@ module Types
     field :champ_descriptors, [Types::ChampDescriptorType], null: false, method: :types_de_champ
     field :annotation_descriptors, [Types::ChampDescriptorType], null: false, method: :types_de_champ_private
 
+    field :draft_revision, Types::RevisionType, null: false
+    field :published_revision, Types::RevisionType, null: true
+    field :revisions, [Types::RevisionType], null: false
+
     def state
       object.aasm.current_state
     end
@@ -55,6 +59,10 @@ module Types
       Loaders::Record.for(Service).load(object.service_id)
     end
 
+    def revisions
+      Loaders::Association.for(object.class, :revisions).load(object)
+    end
+
     def dossiers(updated_since: nil, created_since: nil, state: nil, archived: nil, order:)
       dossiers = object.dossiers.state_not_brouillon.for_api_v2
 

app/graphql/types/dossier_type.rb

@@ -24,18 +24,18 @@ module Types
       { Extensions::Attachment => { attachment: :justificatif_motivation } }
     ]
 
+    field :usager, Types::ProfileType, null: false
+    field :groupe_instructeur, Types::GroupeInstructeurType, null: false
+    field :revision, Types::RevisionType, null: false
+
     field :demandeur, Types::DemandeurType, null: false
 
-    field :usager, Types::ProfileType, null: false
     field :instructeurs, [Types::ProfileType], null: false
-
-    field :champs, [Types::ChampType], null: false
-    field :annotations, [Types::ChampType], null: false
-
     field :messages, [Types::MessageType], null: false
     field :avis, [Types::AvisType], null: false
 
-    field :groupe_instructeur, Types::GroupeInstructeurType, null: false
+    field :champs, [Types::ChampType], null: false
+    field :annotations, [Types::ChampType], null: false
 
     def state
       object.state
@@ -45,14 +45,26 @@ module Types
       Loaders::Record.for(User).load(object.user_id)
     end
 
-    def instructeurs
-      Loaders::Association.for(object.class, :followers_instructeurs).load(object)
-    end
-
     def groupe_instructeur
       Loaders::Record.for(GroupeInstructeur).load(object.groupe_instructeur_id)
     end
 
+    def revision
+      Loaders::Record.for(ProcedureRevision).load(object.revision_id)
+    end
+
+    def demandeur
+      if object.procedure.for_individual
+        Loaders::Association.for(object.class, :individual).load(object)
+      else
+        Loaders::Association.for(object.class, :etablissement).load(object)
+      end
+    end
+
+    def instructeurs
+      Loaders::Association.for(object.class, :followers_instructeurs).load(object)
+    end
+
     def messages
       Loaders::Association.for(object.class, commentaires: [:instructeur, :user]).load(object)
     end
@@ -69,14 +81,6 @@ module Types
       Loaders::Association.for(object.class, :champs_private).load(object)
     end
 
-    def demandeur
-      if object.procedure.for_individual
-        Loaders::Association.for(object.class, :individual).load(object)
-      else
-        Loaders::Association.for(object.class, :etablissement).load(object)
-      end
-    end
-
     def self.authorized?(object, context)
       authorized_demarche?(object.procedure, context)
     end

app/graphql/types/revision_type.rb

@@ -0,0 +1,17 @@
+module Types
+  class RevisionType < Types::BaseObject
+    global_id_field :id
+    field :date_creation, GraphQL::Types::ISO8601DateTime, "Date de la création.", null: false, method: :created_at
+
+    field :champ_descriptors, [Types::ChampDescriptorType], null: false
+    field :annotation_descriptors, [Types::ChampDescriptorType], null: false
+
+    def champ_descriptors
+      Loaders::Association.for(object.class, :types_de_champ).load(object)
+    end
+
+    def annotation_descriptors
+      Loaders::Association.for(object.class, :types_de_champ_private).load(object)
+    end
+  end
+end

app/models/dossier.rb

@@ -288,7 +288,27 @@ class Dossier < ApplicationRecord
   scope :for_procedure, -> (procedure) { includes(:user, :groupe_instructeur).where(groupe_instructeurs: { procedure: procedure }) }
   scope :for_api_v2, -> { includes(procedure: [:administrateurs], etablissement: [], individual: [], traitements: []) }
 
-  scope :with_notifications, -> do
+  # todo: once we are sure with_cached_notifications does not introduce regressions, remove with_unoptimized_notifications
+  # and use with_cached_notifications instead
+  scope :with_notifications, -> (instructeur) do
+    if Flipper.enabled?(:cached_notifications, instructeur)
+      return with_cached_notifications
+    else
+      return with_unoptimized_notifications
+    end
+  end
+
+  scope :with_cached_notifications, -> do
+    joins(:follows)
+      .where('last_champ_updated_at > follows.demande_seen_at' \
+      ' OR groupe_instructeur_updated_at > follows.demande_seen_at' \
+      ' OR last_champ_private_updated_at > follows.annotations_privees_seen_at' \
+      ' OR last_avis_updated_at > follows.avis_seen_at' \
+      ' OR last_commentaire_updated_at > follows.messagerie_seen_at')
+      .distinct
+  end
+
+  scope :with_unoptimized_notifications, -> do
     # This scope is meant to be composed, typically with Instructeur.followed_dossiers, which means that the :follows table is already INNER JOINed;
     # it will fail otherwise
     # rubocop:disable DS/ApplicationName

app/models/instructeur.rb

@@ -141,14 +141,14 @@ class Instructeur < ApplicationRecord
       .where(groupe_instructeur: target_groupes)
       .send(scope) # :en_cours or :termine or :not_archived (or any other Dossier scope)
       .merge(followed_dossiers)
-      .with_notifications
+      .with_notifications(self)
   end
 
   def procedures_with_notifications(scope)
     dossiers = Dossier
       .send(scope) # :en_cours or :termine (or any other Dossier scope)
       .merge(followed_dossiers)
-      .with_notifications
+      .with_notifications(self)
 
     Procedure
       .where(id: dossiers.joins(:groupe_instructeur)
@@ -213,6 +213,10 @@ class Instructeur < ApplicationRecord
   def features
   end
 
+  def flipper_id
+    "Instructeur:#{id}"
+  end
+
   private
 
   def annotations_hash(demande, annotations_privees, avis, messagerie)

app/models/procedure.rb

@@ -140,7 +140,7 @@ class Procedure < ApplicationRecord
   }
 
   scope :for_api_v2, -> {
-    includes(administrateurs: :user)
+    includes(:draft_revision, :published_revision, administrateurs: :user)
   }
 
   validates :libelle, presence: true, allow_blank: false, allow_nil: false

app/models/procedure_presentation.rb

@@ -91,7 +91,7 @@ class ProcedurePresentation < ApplicationRecord
 
     case table
     when 'notifications'
-      dossiers_id_with_notification = dossiers.merge(instructeur.followed_dossiers).with_notifications.ids
+      dossiers_id_with_notification = dossiers.merge(instructeur.followed_dossiers).with_notifications(instructeur).ids
       if order == 'desc'
         return dossiers_id_with_notification +
             (dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)

app/views/layouts/_pre_maintenance.html.haml

@@ -1,9 +1,4 @@
-- if feature_enabled?(:pre_maintenance_mode)
+- if ENV['BANNER_MESSAGE'].present?
   .site-banner.warning
     .container
-      .site-banner-icon 🕚
+      = sanitize(ENV['BANNER_MESSAGE'])
-      .site-banner-text
-        %strong
-          Une opération de maintenance est prévue sur #{APPLICATION_NAME} à 23 h 00.
-        %br
-        La plateforme sera inaccessible pendant une vingtaine de minutes.

config/initializers/flipper.rb

@@ -34,7 +34,6 @@ features = [
   :maintenance_mode,
   :mini_profiler,
   :operation_log_serialize_subject,
-  :pre_maintenance_mode,
   :xray,
   :carte_ign,
   :localization

spec/controllers/api/v2/graphql_controller_spec.rb

@@ -63,6 +63,18 @@ describe API::V2::GraphqlController do
             email
           }
         }
+        revisions {
+          id
+        }
+        draftRevision {
+          id
+        }
+        publishedRevision {
+          id
+          champDescriptors {
+            type
+          }
+        }
         service {
           nom
           typeOrganisme
@@ -123,6 +135,16 @@ describe API::V2::GraphqlController do
               label: "défaut"
             }
           ],
+          revisions: procedure.revisions.map { |revision| { id: revision.to_typed_id } },
+          draftRevision: { id: procedure.draft_revision.to_typed_id },
+          publishedRevision: {
+            id: procedure.published_revision.to_typed_id,
+            champDescriptors: procedure.published_types_de_champ.map do |tdc|
+              {
+                type: tdc.type_champ
+              }
+            end
+          },
           service: {
             nom: procedure.service.nom,
             typeOrganisme: procedure.service.type_organisme,
@@ -255,6 +277,12 @@ describe API::V2::GraphqlController do
                 number
                 label
               }
+              revision {
+                id
+                champDescriptors {
+                  type
+                }
+              }
               messages {
                 email
                 body
@@ -314,6 +342,14 @@ describe API::V2::GraphqlController do
               number: dossier.groupe_instructeur.id,
               label: dossier.groupe_instructeur.label
             },
+            revision: {
+              id: dossier.revision.to_typed_id,
+              champDescriptors: dossier.types_de_champ.map do |tdc|
+                {
+                  type: tdc.type_champ
+                }
+              end
+            },
             demandeur: {
               id: dossier.individual.to_typed_id,
               nom: dossier.individual.nom,

spec/controllers/instructeurs/dossiers_controller_spec.rb

@@ -445,12 +445,26 @@ describe Instructeurs::DossiersController, type: :controller do
       }
     end
 
+    let(:emails) { ['email@a.com'] }
+
+    context "notifications updates" do
+      context 'when an instructeur follows the dossier' do
+        let(:follower) { create(:instructeur) }
+        before { follower.follow(dossier) }
+
+        it 'the follower has a notification' do
+          expect(follower.followed_dossiers.with_notifications(follower)).to eq([])
+          subject
+          expect(follower.followed_dossiers.with_notifications(follower)).to eq([dossier.reload])
+        end
+      end
+    end
+
+    context 'email sending' do
       before do
         subject
       end
 
-    let(:emails) { ['email@a.com'] }
-
       it { expect(saved_avis.email).to eq('email@a.com') }
       it { expect(saved_avis.introduction).to eq('intro') }
       it { expect(saved_avis.confidentiel).to eq(true) }
@@ -461,6 +475,8 @@ describe Instructeurs::DossiersController, type: :controller do
       context "with an invalid email" do
         let(:emails) { ['emaila.com'] }
 
+        before { subject }
+
         it { expect(response).to render_template :avis }
         it { expect(flash.alert).to eq(["emaila.com : Email n'est pas valide"]) }
         it { expect { subject }.not_to change(Avis, :count) }
@@ -470,6 +486,8 @@ describe Instructeurs::DossiersController, type: :controller do
       context 'with multiple emails' do
         let(:emails) { ["toto.fr,titi@titimail.com"] }
 
+        before { subject }
+
         it { expect(response).to render_template :avis }
         it { expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) }
         it { expect(flash.notice).to eq("Une demande d'avis a été envoyée à titi@titimail.com") }
@@ -481,7 +499,7 @@ describe Instructeurs::DossiersController, type: :controller do
         let(:asked_confidentiel) { false }
         let(:previous_avis_confidentiel) { false }
         let(:dossier) { create(:dossier, :en_construction, :with_dossier_link, procedure: procedure) }
-
+        before { subject }
         context 'when the expert doesn’t share linked dossiers' do
           let(:invite_linked_dossiers) { false }
 
@@ -528,6 +546,7 @@ describe Instructeurs::DossiersController, type: :controller do
         end
       end
     end
+  end
 
   describe "#show" do
     context "when the dossier is exported as PDF" do
@@ -569,6 +588,7 @@ describe Instructeurs::DossiersController, type: :controller do
       ], instructeurs: instructeurs)
     end
     let(:dossier) { create(:dossier, :en_construction, :with_all_annotations, procedure: procedure) }
+    let(:another_instructeur) { create(:instructeur) }
     let(:now) { Time.zone.parse('01/01/2100') }
 
     let(:champ_multiple_drop_down_list) do
@@ -588,6 +608,7 @@ describe Instructeurs::DossiersController, type: :controller do
     end
 
     before do
+      another_instructeur.follow(dossier)
       Timecop.freeze(now)
       patch :update_annotations, params: params
 
@@ -646,6 +667,12 @@ describe Instructeurs::DossiersController, type: :controller do
         expect(dossier.reload.last_champ_private_updated_at).to eq(now)
         expect(response).to redirect_to(annotations_privees_instructeur_dossier_path(dossier.procedure, dossier))
       }
+
+      it 'updates the annotations' do
+        Timecop.travel(now + 1.hour)
+        expect(instructeur.followed_dossiers.with_notifications(instructeur)).to eq([])
+        expect(another_instructeur.followed_dossiers.with_notifications(instructeur)).to eq([dossier.reload])
+      end
     end
 
     context "without new values for champs_private" do

spec/controllers/users/dossiers_controller_spec.rb

@@ -682,6 +682,22 @@ describe Users::DossiersController, type: :controller do
       it { expect(dossier.reload.state).to eq(Dossier.states.fetch(:en_construction)) }
       it { expect(response).to redirect_to(demande_dossier_path(dossier)) }
     end
+
+    context 'when the dossier is followed by an instructeur' do
+      let(:dossier) { create(:dossier) }
+      let(:instructeur) { create(:instructeur) }
+      let!(:invite) { create(:invite, dossier: dossier, user: user) }
+
+      before do
+        instructeur.follow(dossier)
+      end
+
+      it 'the follower has a notification' do
+        expect(instructeur.reload.followed_dossiers.with_notifications(instructeur)).to eq([])
+        subject
+        expect(instructeur.reload.followed_dossiers.with_notifications(instructeur)).to eq([dossier.reload])
+      end
+    end
   end
 
   describe '#index' do
@@ -834,6 +850,8 @@ describe Users::DossiersController, type: :controller do
     before do
       Timecop.freeze(now)
       sign_in(user)
+      # Flipper.enable(:cached_notifications, instructeur_with_instant_message)
+      # Flipper.enable(:cached_notifications, instructeur_without_instant_message)
       allow(ClamavService).to receive(:safe_file?).and_return(scan_result)
       allow(DossierMailer).to receive(:notify_new_commentaire_to_instructeur).and_return(double(deliver_later: nil))
       instructeur_with_instant_message.follow(dossier)
@@ -844,6 +862,7 @@ describe Users::DossiersController, type: :controller do
 
     after { Timecop.return }
 
+    context 'commentaire creation' do
       it "creates a commentaire" do
         expect { subject }.to change(Commentaire, :count).by(1)
 
@@ -855,6 +874,21 @@ describe Users::DossiersController, type: :controller do
       end
     end
 
+    context 'notification' do
+      before 'instructeurs have no notification before the message' do
+        expect(instructeur_with_instant_message.followed_dossiers.with_notifications(instructeur_with_instant_message)).to eq([])
+        expect(instructeur_without_instant_message.followed_dossiers.with_notifications(instructeur_without_instant_message)).to eq([])
+        Timecop.travel(now + 1.day)
+        subject
+      end
+
+      it 'adds them a notification' do
+        expect(instructeur_with_instant_message.reload.followed_dossiers.with_notifications(instructeur_with_instant_message)).to eq([dossier.reload])
+        expect(instructeur_without_instant_message.reload.followed_dossiers.with_notifications(instructeur_without_instant_message)).to eq([dossier.reload])
+      end
+    end
+  end
+
   describe '#ask_deletion' do
     before { sign_in(user) }
 

spec/models/dossier_spec.rb

@@ -103,19 +103,40 @@ describe Dossier do
 
     before do
       create(:follow, dossier: dossier, instructeur: instructeur, messagerie_seen_at: 2.hours.ago)
+      Flipper.enable_actor(:cached_notifications, instructeur)
     end
 
-    subject { instructeur.followed_dossiers.with_notifications }
+    subject { instructeur.followed_dossiers.with_notifications(instructeur) }
 
     context('without changes') do
       it { is_expected.to eq [] }
     end
 
     context('with changes') do
-      before { dossier.commentaires << create(:commentaire, email: 'test@exemple.fr') }
+      context 'when there is a new commentaire' do
+        before { dossier.update!(last_commentaire_updated_at: Time.zone.now) }
 
         it { is_expected.to match([dossier]) }
       end
+
+      context 'when there is a new avis' do
+        before { dossier.update!(last_avis_updated_at: Time.zone.now) }
+
+        it { is_expected.to match([dossier]) }
+      end
+
+      context 'when a public champ is updated' do
+        before { dossier.update!(last_champ_updated_at: Time.zone.now) }
+
+        it { is_expected.to match([dossier]) }
+      end
+
+      context 'when a private champ is updated' do
+        before { dossier.update!(last_champ_private_updated_at: Time.zone.now) }
+
+        it { is_expected.to match([dossier]) }
+      end
+    end
   end
 
   describe 'methods' do

spec/models/instructeur_spec.rb

@@ -8,6 +8,7 @@ describe Instructeur, type: :model do
 
   before do
     assign(procedure_2)
+    Flipper.enable_actor(:cached_notifications, instructeur)
   end
 
   describe 'follow' do
@@ -263,12 +264,20 @@ describe Instructeur, type: :model do
 
     let!(:dossier_on_procedure_2) { create(:dossier, :followed, state: Dossier.states.fetch(:en_construction)) }
     let!(:instructeur_on_procedure_2) { dossier_on_procedure_2.follows.first.instructeur }
+    let(:now) { Time.zone.parse("14/09/1867") }
+    let(:follow) { instructeur.follows.find_by(dossier: dossier) }
+    let(:follow2) { instructeur_2.follows.find_by(dossier: dossier) }
+    let(:seen_at_instructeur) { now - 1.hour }
+    let(:seen_at_instructeur2) { now - 1.hour }
 
     before do
       procedure.groupe_instructeurs.last.instructeurs << instructeur
       instructeur_2.followed_dossiers << dossier
+      Timecop.freeze(now)
     end
 
+    after { Timecop.return }
+
     subject { instructeur.notifications_for_procedure(procedure, :en_cours) }
 
     context 'when the instructeur has just followed the dossier' do
@@ -276,7 +285,11 @@ describe Instructeur, type: :model do
     end
 
     context 'when there is a modification on public champs' do
-      before { dossier.champs.first.update_attribute('value', 'toto') }
+      before do
+        dossier.update!(last_champ_updated_at: now)
+        follow.update_attribute('demande_seen_at', seen_at_instructeur)
+        follow2.update_attribute('demande_seen_at', seen_at_instructeur2)
+      end
 
       it { is_expected.to match([dossier]) }
       it { expect(instructeur_2.notifications_for_procedure(procedure, :en_cours)).to match([dossier]) }
@@ -289,9 +302,8 @@ describe Instructeur, type: :model do
       end
 
       context 'when instructeur update it s public champs last seen' do
-        let(:follow) { instructeur.follows.find_by(dossier: dossier) }
+        let(:seen_at_instructeur) { now + 1.hour }
-
+        let(:seen_at_instructeur2) { now - 1.hour }
-        before { follow.update_attribute('demande_seen_at', Time.zone.now) }
 
         it { is_expected.to match([]) }
         it { expect(instructeur_2.notifications_for_procedure(procedure, :en_cours)).to match([dossier]) }
@@ -305,20 +317,29 @@ describe Instructeur, type: :model do
     end
 
     context 'when there is a modification on private champs' do
-      before { dossier.champs_private.first.update_attribute('value', 'toto') }
+      before do
+        dossier.update!(last_champ_private_updated_at: now)
+        follow.update_attribute('annotations_privees_seen_at', seen_at_instructeur)
+      end
 
       it { is_expected.to match([dossier]) }
     end
 
     context 'when there is a modification on avis' do
-      before { create(:avis, dossier: dossier) }
+      before do
+        dossier.update!(last_avis_updated_at: Time.zone.now)
+        follow.update_attribute('avis_seen_at', seen_at_instructeur)
+      end
 
       it { is_expected.to match([dossier]) }
     end
 
     context 'the messagerie' do
       context 'when there is a new commentaire' do
-        before { create(:commentaire, dossier: dossier, email: 'a@b.com') }
+        before do
+          dossier.update!(last_commentaire_updated_at: Time.zone.now)
+          follow.update_attribute('messagerie_seen_at', seen_at_instructeur)
+        end
 
         it { is_expected.to match([dossier]) }
       end
@@ -339,7 +360,7 @@ describe Instructeur, type: :model do
     subject { instructeur.procedures_with_notifications(:en_cours) }
 
     context 'when there is a modification on public champs' do
-      before { dossier.champs.first.update_attribute('value', 'toto') }
+      before { dossier.update!(last_champ_updated_at: Time.zone.now) }
 
       it { is_expected.to match([procedure]) }
     end

spec/models/procedure_presentation_spec.rb

@@ -262,7 +262,8 @@ describe ProcedurePresentation do
       let!(:older_dossier) { create(:dossier, :en_construction, procedure: procedure) }
 
       before do
-        notified_dossier.champs.first.touch(time: Time.zone.local(2018, 9, 20))
+        Flipper.enable_actor(:cached_notifications, instructeur)
+        notified_dossier.update!(last_champ_updated_at: Time.zone.local(2018, 9, 20))
         create(:follow, instructeur: instructeur, dossier: notified_dossier, demande_seen_at: Time.zone.local(2018, 9, 10))
         notified_dossier.touch(time: Time.zone.local(2018, 9, 20))
         recent_dossier.touch(time: Time.zone.local(2018, 9, 25))