From c20ad5ca17d94a10cca530774827728015eea27d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 22 Apr 2021 14:44:58 +0100 Subject: [PATCH] Remove the link between commentaire and user --- app/graphql/types/dossier_type.rb | 4 +-- app/models/champ.rb | 1 - app/models/commentaire.rb | 29 +++++++++++-------- app/services/commentaire_service.rb | 4 +-- ...422101149_add_expert_id_to_commentaires.rb | 5 ++++ db/schema.rb | 5 +++- spec/models/commentaire_spec.rb | 22 +++++++++----- 7 files changed, 45 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20210422101149_add_expert_id_to_commentaires.rb diff --git a/app/graphql/types/dossier_type.rb b/app/graphql/types/dossier_type.rb index 9d611166d..7f340ffc3 100644 --- a/app/graphql/types/dossier_type.rb +++ b/app/graphql/types/dossier_type.rb @@ -80,10 +80,10 @@ module Types def messages(id: nil) if id.present? Loaders::Record - .for(Commentaire, where: { dossier: object }, includes: [:instructeur, :user], array: true) + .for(Commentaire, where: { dossier: object }, includes: [:instructeur, :expert], array: true) .load(ApplicationRecord.id_from_typed_id(id)) else - Loaders::Association.for(object.class, commentaires: [:instructeur, :user]).load(object) + Loaders::Association.for(object.class, commentaires: [:instructeur, :expert]).load(object) end end diff --git a/app/models/champ.rb b/app/models/champ.rb index fb35660ca..4b3fda82e 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -21,7 +21,6 @@ class Champ < ApplicationRecord belongs_to :dossier, -> { with_discarded }, inverse_of: false, touch: true, optional: false belongs_to :type_de_champ, inverse_of: :champ, optional: false belongs_to :parent, class_name: 'Champ', optional: true - has_many :commentaires has_one_attached :piece_justificative_file # We declare champ specific relationships (Champs::CarteChamp, Champs::SiretChamp and Champs::RepetitionChamp) diff --git a/app/models/commentaire.rb b/app/models/commentaire.rb index d55a5ce69..895bc2f3f 100644 --- a/app/models/commentaire.rb +++ b/app/models/commentaire.rb @@ -8,14 +8,15 @@ # created_at :datetime not null # updated_at :datetime not null # dossier_id :integer +# expert_id :bigint # instructeur_id :bigint -# user_id :bigint # class Commentaire < ApplicationRecord + self.ignored_columns = [:user_id] belongs_to :dossier, inverse_of: :commentaires, touch: true, optional: false - belongs_to :user, optional: true belongs_to :instructeur, optional: true + belongs_to :expert, optional: true validate :messagerie_available?, on: :create @@ -33,10 +34,10 @@ class Commentaire < ApplicationRecord after_create :notify def email - if user - user.email - elsif instructeur + if sent_by_instructeur? instructeur.email + elsif sent_by_expert? + expert.email else read_attribute(:email) end @@ -47,7 +48,7 @@ class Commentaire < ApplicationRecord end def redacted_email - if instructeur.present? + if sent_by_instructeur? if Flipper.enabled?(:hide_instructeur_email, dossier.procedure) "Instructeur n° #{instructeur.id}" else @@ -59,8 +60,15 @@ class Commentaire < ApplicationRecord end def sent_by_system? - [CONTACT_EMAIL, OLD_CONTACT_EMAIL].include?(email) && - user.nil? && instructeur.nil? + [CONTACT_EMAIL, OLD_CONTACT_EMAIL].include?(email) + end + + def sent_by_instructeur? + instructeur_id.present? + end + + def sent_by_expert? + expert_id.present? end def sent_by?(someone) @@ -76,15 +84,12 @@ class Commentaire < ApplicationRecord private def notify - dossier_user_email = dossier.user.email - invited_users_emails = dossier.invites.pluck(:email).to_a - # - If the email is the contact email, the commentaire is a copy # of an automated notification email we sent to a user, so do nothing. # - If a user or an invited user posted a commentaire, do nothing, # the notification system will properly # - Otherwise, a instructeur posted a commentaire, we need to notify the user - if !email.in?([CONTACT_EMAIL, dossier_user_email, *invited_users_emails]) + if sent_by_instructeur? || sent_by_expert? notify_user end end diff --git a/app/services/commentaire_service.rb b/app/services/commentaire_service.rb index f7611b290..7892c65f2 100644 --- a/app/services/commentaire_service.rb +++ b/app/services/commentaire_service.rb @@ -2,10 +2,10 @@ class CommentaireService class << self def build(sender, dossier, params) case sender - when User - params[:user] = sender when Instructeur params[:instructeur] = sender + when Expert + params[:expert] = sender end build_with_email(sender.email, dossier, params) diff --git a/db/migrate/20210422101149_add_expert_id_to_commentaires.rb b/db/migrate/20210422101149_add_expert_id_to_commentaires.rb new file mode 100644 index 000000000..78afbe946 --- /dev/null +++ b/db/migrate/20210422101149_add_expert_id_to_commentaires.rb @@ -0,0 +1,5 @@ +class AddExpertIdToCommentaires < ActiveRecord::Migration[6.1] + def change + add_belongs_to :commentaires, :expert, type: :bigint, foreign_key: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c282337f9..fec39e667 100644 --- a/db/schema.rb +++ b/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: 2021_04_16_160721) do +ActiveRecord::Schema.define(version: 2021_04_22_101149) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -175,7 +175,9 @@ ActiveRecord::Schema.define(version: 2021_04_16_160721) do t.datetime "updated_at", null: false t.bigint "user_id" t.bigint "instructeur_id" + t.bigint "expert_id" t.index ["dossier_id"], name: "index_commentaires_on_dossier_id" + t.index ["expert_id"], name: "index_commentaires_on_expert_id" t.index ["instructeur_id"], name: "index_commentaires_on_instructeur_id" t.index ["user_id"], name: "index_commentaires_on_user_id" end @@ -738,6 +740,7 @@ ActiveRecord::Schema.define(version: 2021_04_16_160721) do add_foreign_key "champs", "champs", column: "parent_id" add_foreign_key "closed_mails", "procedures" add_foreign_key "commentaires", "dossiers" + add_foreign_key "commentaires", "experts" add_foreign_key "dossier_operation_logs", "bill_signatures" add_foreign_key "dossier_operation_logs", "instructeurs" add_foreign_key "dossiers", "groupe_instructeurs" diff --git a/spec/models/commentaire_spec.rb b/spec/models/commentaire_spec.rb index c38a0f3d8..4e33eb5c9 100644 --- a/spec/models/commentaire_spec.rb +++ b/spec/models/commentaire_spec.rb @@ -63,7 +63,7 @@ describe Commentaire do end context 'with a commentaire created by a user' do - let(:commentaire) { build :commentaire, user: user } + let(:commentaire) { build :commentaire, email: user.email } let(:user) { build :user, email: 'some_user@exemple.fr' } it { is_expected.to eq 'some_user@exemple.fr' } @@ -73,26 +73,34 @@ describe Commentaire do describe "#notify" do let(:procedure) { create(:procedure) } let(:instructeur) { create(:instructeur) } + let(:expert) { create(:expert) } let(:assign_to) { create(:assign_to, instructeur: instructeur, procedure: procedure) } let(:user) { create(:user) } let(:dossier) { create(:dossier, :en_construction, procedure: procedure, user: user) } - let(:commentaire) { Commentaire.new(dossier: dossier, body: "Mon commentaire") } context "with a commentaire created by a instructeur" do + let(:commentaire) { CommentaireService.build(instructeur, dossier, body: "Mon commentaire") } + it "calls notify_user" do expect(commentaire).to receive(:notify_user) + commentaire.save + end + end - commentaire.email = instructeur.email + context "with a commentaire created by an expert" do + let(:commentaire) { CommentaireService.build(expert, dossier, body: "Mon commentaire") } + + it "calls notify_user" do + expect(commentaire).to receive(:notify_user) commentaire.save end end context "with a commentaire automatically created (notification)" do - it "does not call notify_user or notify_instructeurs" do - expect(commentaire).not_to receive(:notify_user) - expect(commentaire).not_to receive(:notify_instructeurs) + let(:commentaire) { CommentaireService.build_with_email(CONTACT_EMAIL, dossier, body: "Mon commentaire") } - commentaire.email = CONTACT_EMAIL + it "does not call notify_user" do + expect(commentaire).not_to receive(:notify_user) commentaire.save end end