Merge pull request #6107 from tchak/refactor-commentaires

Remove the link between commentaire and user
This commit is contained in:
Paul Chavard 2021-04-22 15:32:36 +01:00 committed by GitHub
commit dc73500d61
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 45 additions and 25 deletions

View file

@ -80,10 +80,10 @@ module Types
def messages(id: nil) def messages(id: nil)
if id.present? if id.present?
Loaders::Record 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)) .load(ApplicationRecord.id_from_typed_id(id))
else else
Loaders::Association.for(object.class, commentaires: [:instructeur, :user]).load(object) Loaders::Association.for(object.class, commentaires: [:instructeur, :expert]).load(object)
end end
end end

View file

@ -21,7 +21,6 @@ class Champ < ApplicationRecord
belongs_to :dossier, -> { with_discarded }, inverse_of: false, touch: true, optional: false belongs_to :dossier, -> { with_discarded }, inverse_of: false, touch: true, optional: false
belongs_to :type_de_champ, inverse_of: :champ, optional: false belongs_to :type_de_champ, inverse_of: :champ, optional: false
belongs_to :parent, class_name: 'Champ', optional: true belongs_to :parent, class_name: 'Champ', optional: true
has_many :commentaires
has_one_attached :piece_justificative_file has_one_attached :piece_justificative_file
# We declare champ specific relationships (Champs::CarteChamp, Champs::SiretChamp and Champs::RepetitionChamp) # We declare champ specific relationships (Champs::CarteChamp, Champs::SiretChamp and Champs::RepetitionChamp)

View file

@ -8,14 +8,15 @@
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# dossier_id :integer # dossier_id :integer
# expert_id :bigint
# instructeur_id :bigint # instructeur_id :bigint
# user_id :bigint
# #
class Commentaire < ApplicationRecord class Commentaire < ApplicationRecord
self.ignored_columns = [:user_id]
belongs_to :dossier, inverse_of: :commentaires, touch: true, optional: false belongs_to :dossier, inverse_of: :commentaires, touch: true, optional: false
belongs_to :user, optional: true
belongs_to :instructeur, optional: true belongs_to :instructeur, optional: true
belongs_to :expert, optional: true
validate :messagerie_available?, on: :create validate :messagerie_available?, on: :create
@ -33,10 +34,10 @@ class Commentaire < ApplicationRecord
after_create :notify after_create :notify
def email def email
if user if sent_by_instructeur?
user.email
elsif instructeur
instructeur.email instructeur.email
elsif sent_by_expert?
expert.email
else else
read_attribute(:email) read_attribute(:email)
end end
@ -47,7 +48,7 @@ class Commentaire < ApplicationRecord
end end
def redacted_email def redacted_email
if instructeur.present? if sent_by_instructeur?
if Flipper.enabled?(:hide_instructeur_email, dossier.procedure) if Flipper.enabled?(:hide_instructeur_email, dossier.procedure)
"Instructeur n° #{instructeur.id}" "Instructeur n° #{instructeur.id}"
else else
@ -59,8 +60,15 @@ class Commentaire < ApplicationRecord
end end
def sent_by_system? def sent_by_system?
[CONTACT_EMAIL, OLD_CONTACT_EMAIL].include?(email) && [CONTACT_EMAIL, OLD_CONTACT_EMAIL].include?(email)
user.nil? && instructeur.nil? end
def sent_by_instructeur?
instructeur_id.present?
end
def sent_by_expert?
expert_id.present?
end end
def sent_by?(someone) def sent_by?(someone)
@ -76,15 +84,12 @@ class Commentaire < ApplicationRecord
private private
def notify 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 # - 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. # 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, # - If a user or an invited user posted a commentaire, do nothing,
# the notification system will properly # the notification system will properly
# - Otherwise, a instructeur posted a commentaire, we need to notify the user # - 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 notify_user
end end
end end

View file

@ -2,10 +2,10 @@ class CommentaireService
class << self class << self
def build(sender, dossier, params) def build(sender, dossier, params)
case sender case sender
when User
params[:user] = sender
when Instructeur when Instructeur
params[:instructeur] = sender params[:instructeur] = sender
when Expert
params[:expert] = sender
end end
build_with_email(sender.email, dossier, params) build_with_email(sender.email, dossier, params)

View file

@ -0,0 +1,5 @@
class AddExpertIdToCommentaires < ActiveRecord::Migration[6.1]
def change
add_belongs_to :commentaires, :expert, type: :bigint, foreign_key: true
end
end

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -175,7 +175,9 @@ ActiveRecord::Schema.define(version: 2021_04_16_160721) do
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.bigint "user_id" t.bigint "user_id"
t.bigint "instructeur_id" t.bigint "instructeur_id"
t.bigint "expert_id"
t.index ["dossier_id"], name: "index_commentaires_on_dossier_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 ["instructeur_id"], name: "index_commentaires_on_instructeur_id"
t.index ["user_id"], name: "index_commentaires_on_user_id" t.index ["user_id"], name: "index_commentaires_on_user_id"
end 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 "champs", "champs", column: "parent_id"
add_foreign_key "closed_mails", "procedures" add_foreign_key "closed_mails", "procedures"
add_foreign_key "commentaires", "dossiers" add_foreign_key "commentaires", "dossiers"
add_foreign_key "commentaires", "experts"
add_foreign_key "dossier_operation_logs", "bill_signatures" add_foreign_key "dossier_operation_logs", "bill_signatures"
add_foreign_key "dossier_operation_logs", "instructeurs" add_foreign_key "dossier_operation_logs", "instructeurs"
add_foreign_key "dossiers", "groupe_instructeurs" add_foreign_key "dossiers", "groupe_instructeurs"

View file

@ -63,7 +63,7 @@ describe Commentaire do
end end
context 'with a commentaire created by a user' do 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' } let(:user) { build :user, email: 'some_user@exemple.fr' }
it { is_expected.to eq 'some_user@exemple.fr' } it { is_expected.to eq 'some_user@exemple.fr' }
@ -73,26 +73,34 @@ describe Commentaire do
describe "#notify" do describe "#notify" do
let(:procedure) { create(:procedure) } let(:procedure) { create(:procedure) }
let(:instructeur) { create(:instructeur) } let(:instructeur) { create(:instructeur) }
let(:expert) { create(:expert) }
let(:assign_to) { create(:assign_to, instructeur: instructeur, procedure: procedure) } let(:assign_to) { create(:assign_to, instructeur: instructeur, procedure: procedure) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:dossier) { create(:dossier, :en_construction, procedure: procedure, user: 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 context "with a commentaire created by a instructeur" do
let(:commentaire) { CommentaireService.build(instructeur, dossier, body: "Mon commentaire") }
it "calls notify_user" do it "calls notify_user" do
expect(commentaire).to receive(:notify_user) 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 commentaire.save
end end
end end
context "with a commentaire automatically created (notification)" do context "with a commentaire automatically created (notification)" do
it "does not call notify_user or notify_instructeurs" do let(:commentaire) { CommentaireService.build_with_email(CONTACT_EMAIL, dossier, body: "Mon commentaire") }
expect(commentaire).not_to receive(:notify_user)
expect(commentaire).not_to receive(:notify_instructeurs)
commentaire.email = CONTACT_EMAIL it "does not call notify_user" do
expect(commentaire).not_to receive(:notify_user)
commentaire.save commentaire.save
end end
end end