From 02c83766e5ad8302391a7c68c47c97ceea100261 Mon Sep 17 00:00:00 2001
From: Mathieu Magnin <mathieu.magnin@gmail.com>
Date: Fri, 20 Oct 2017 10:32:28 +0200
Subject: [PATCH] [Fix #839] Fix mail template modification by side effect

---
 app/models/concerns/mail_template_concern.rb  |  2 +-
 .../concern/mail_template_concern_spec.rb     | 51 ++++++++++++++++---
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/app/models/concerns/mail_template_concern.rb b/app/models/concerns/mail_template_concern.rb
index cd44676f5..8431ef00b 100644
--- a/app/models/concerns/mail_template_concern.rb
+++ b/app/models/concerns/mail_template_concern.rb
@@ -36,7 +36,7 @@ module MailTemplateConcern
 
   def replace_tags(string, dossier)
     TAGS.inject(string) do |acc, tag|
-      acc.gsub!("--#{tag[:name]}--", replace_tag(tag, dossier)) || acc
+      acc.gsub("--#{tag[:name]}--", replace_tag(tag, dossier)) || acc
     end
   end
 
diff --git a/spec/models/concern/mail_template_concern_spec.rb b/spec/models/concern/mail_template_concern_spec.rb
index 843c4c896..e249b73be 100644
--- a/spec/models/concern/mail_template_concern_spec.rb
+++ b/spec/models/concern/mail_template_concern_spec.rb
@@ -1,17 +1,56 @@
 require 'spec_helper'
 
 describe MailTemplateConcern do
-  describe '.replace_tags' do
-    let(:dossier) { create :dossier }
-    let(:initiated_mail) { Mails::InitiatedMail.default }
+  let(:dossier) { create :dossier }
+  let(:dossier2) { create :dossier }
+  let(:initiated_mail) { Mails::InitiatedMail.default }
 
-    it 'works' do
-      initiated_mail.object = '[TPS] --numero_dossier-- --libelle_procedure-- --lien_dossier--'
+  shared_examples "can replace tokens in template" do
+    describe 'with no token to replace' do
+      let(:template) { '[TPS] rien à remplacer' }
+      it do
+        is_expected.to eq("[TPS] rien à remplacer")
+      end
+    end
+
+    describe 'with one token to replace' do
+      let(:template) { '[TPS] Dossier : --numero_dossier--' }
+      it do
+        is_expected.to eq("[TPS] Dossier : #{dossier.id}")
+      end
+    end
+
+    describe 'with multiples tokens to replace' do
+      let(:template) { '[TPS] --numero_dossier-- --libelle_procedure-- --lien_dossier--' }
+      it do
         expected =
           "[TPS] #{dossier.id} #{dossier.procedure.libelle} " +
           "<a target=\"_blank\" href=\"http://localhost:3000/users/dossiers/#{dossier.id}/recapitulatif\">http://localhost:3000/users/dossiers/#{dossier.id}/recapitulatif</a>"
 
-        expect(initiated_mail.object_for_dossier(dossier)).to eq(expected)
+        is_expected.to eq(expected)
+      end
+    end
+  end
+
+  describe '.object_for_dossier' do
+    before { initiated_mail.object = template }
+    subject { initiated_mail.object_for_dossier(dossier) }
+
+    it_behaves_like "can replace tokens in template"
+  end
+
+  describe '.body_for_dossier' do
+    before { initiated_mail.body = template }
+    subject { initiated_mail.body_for_dossier(dossier) }
+
+    it_behaves_like "can replace tokens in template"
+  end
+
+  describe '.replace_tags' do
+    it "avoids side effects" do
+      subject = "n --numero_dossier--"
+      expect(initiated_mail.replace_tags(subject, dossier)).to eq("n #{dossier.id}")
+      expect(initiated_mail.replace_tags(subject, dossier2)).to eq("n #{dossier2.id}")
     end
   end
 end