From 8f7907b6f7bce91d0a6814516de1bbfb0f7cd94d Mon Sep 17 00:00:00 2001
From: Jonas Heinrich <j.heinrich@finn.de>
Date: Mon, 16 Mar 2015 18:17:39 +0100
Subject: [PATCH 1/6] Catches error while sending notifications

---
 app/controllers/meeting_contents_controller.rb       | 12 +++++++++++-
 config/locales/de.yml                                |  1 +
 config/locales/en.yml                                |  1 +
 spec/controllers/meeting_contents_controller_spec.rb |  2 +-
 4 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/app/controllers/meeting_contents_controller.rb b/app/controllers/meeting_contents_controller.rb
index ec79cfc9af..3242cf0822 100644
--- a/app/controllers/meeting_contents_controller.rb
+++ b/app/controllers/meeting_contents_controller.rb
@@ -85,8 +85,18 @@ class MeetingContentsController < ApplicationController
     unless @content.new_record?
       recipients = @content.meeting.participants.collect(&:mail).reject { |r| r == @content.meeting.author.mail }
       recipients << @content.meeting.author.mail unless @content.meeting.author.preference[:no_self_notified]
+      recipients_with_errors = []
       recipients.each do |recipient|
-        MeetingMailer.content_for_review(@content, @content_type, recipient).deliver
+        begin
+          MeetingMailer.content_for_review(@content, @content_type, recipient).deliver
+        rescue
+          recipients_with_errors << recipient
+        end
+      end
+      if recipients_with_errors == []
+        flash[:notice] = l(:notice_successful_notification)
+      else
+        flash[:notice] = l(:notice_notification_with_errors) + recipients_with_errors.join(", ")
       end
     end
     redirect_back_or_default controller: '/meetings', action: 'show', id: @meeting
diff --git a/config/locales/de.yml b/config/locales/de.yml
index c8656f2489..5246480554 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -64,6 +64,7 @@ de:
   label_version: "Version"
 
   notice_successful_notification: "Benachrichtigung erfolgreich gesendet"
+  notice_notification_with_errors: "Benachrichtigung gesendet. Folgende Empfänger konnten nicht benachrichtigt werden: "
   notice_timezone_missing: Keine Zeitzone eingestellt und daher %{zone} angenommen. Um Ihre Zeitzone einzustellen, klicken Sie bitte hier.
 
   permission_create_meetings: "Besprechungen erstellen"
diff --git a/config/locales/en.yml b/config/locales/en.yml
index da68ad38bc..93bc30353a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -64,6 +64,7 @@ en:
   label_version: "Version"
 
   notice_successful_notification: "Notification sent successfully"
+  notice_notification_with_errors: "Notification sent. The following recipients could not be notified: "
   notice_timezone_missing: No time zone is set and %{zone} is assumed. To choose your time zone, please click here.
 
   permission_create_meetings: "Create meetings"
diff --git a/spec/controllers/meeting_contents_controller_spec.rb b/spec/controllers/meeting_contents_controller_spec.rb
index 6a2ad1fb4b..fedd5854c1 100644
--- a/spec/controllers/meeting_contents_controller_spec.rb
+++ b/spec/controllers/meeting_contents_controller_spec.rb
@@ -86,7 +86,7 @@ describe MeetingContentsController do
         it 'produces a flash message containing the mail addresses raising the error' do
           put 'notify', meeting_id: meeting.id
           meeting.participants.each do |participant|
-            expect(flash[:error]).to include(participant.name)
+            expect(flash[:notice]).to include(participant.mail)
           end
         end
       end

From 9db7d41311b490f339a0e9867231d607c2325c2c Mon Sep 17 00:00:00 2001
From: Jonas Heinrich <j.heinrich@finn.de>
Date: Thu, 19 Mar 2015 17:59:07 +0100
Subject: [PATCH 2/6] Makes the notification to an error

---
 app/controllers/meeting_contents_controller.rb       | 2 +-
 config/locales/de.yml                                | 1 -
 config/locales/en.yml                                | 1 -
 spec/controllers/meeting_contents_controller_spec.rb | 2 +-
 4 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/app/controllers/meeting_contents_controller.rb b/app/controllers/meeting_contents_controller.rb
index 3242cf0822..8127f3917e 100644
--- a/app/controllers/meeting_contents_controller.rb
+++ b/app/controllers/meeting_contents_controller.rb
@@ -96,7 +96,7 @@ class MeetingContentsController < ApplicationController
       if recipients_with_errors == []
         flash[:notice] = l(:notice_successful_notification)
       else
-        flash[:notice] = l(:notice_notification_with_errors) + recipients_with_errors.join(", ")
+        flash[:error] = l(:error_notification_with_errors) + recipients_with_errors.join(", ")
       end
     end
     redirect_back_or_default controller: '/meetings', action: 'show', id: @meeting
diff --git a/config/locales/de.yml b/config/locales/de.yml
index 5246480554..c8656f2489 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -64,7 +64,6 @@ de:
   label_version: "Version"
 
   notice_successful_notification: "Benachrichtigung erfolgreich gesendet"
-  notice_notification_with_errors: "Benachrichtigung gesendet. Folgende Empfänger konnten nicht benachrichtigt werden: "
   notice_timezone_missing: Keine Zeitzone eingestellt und daher %{zone} angenommen. Um Ihre Zeitzone einzustellen, klicken Sie bitte hier.
 
   permission_create_meetings: "Besprechungen erstellen"
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 93bc30353a..da68ad38bc 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -64,7 +64,6 @@ en:
   label_version: "Version"
 
   notice_successful_notification: "Notification sent successfully"
-  notice_notification_with_errors: "Notification sent. The following recipients could not be notified: "
   notice_timezone_missing: No time zone is set and %{zone} is assumed. To choose your time zone, please click here.
 
   permission_create_meetings: "Create meetings"
diff --git a/spec/controllers/meeting_contents_controller_spec.rb b/spec/controllers/meeting_contents_controller_spec.rb
index fedd5854c1..0cb3f3b964 100644
--- a/spec/controllers/meeting_contents_controller_spec.rb
+++ b/spec/controllers/meeting_contents_controller_spec.rb
@@ -86,7 +86,7 @@ describe MeetingContentsController do
         it 'produces a flash message containing the mail addresses raising the error' do
           put 'notify', meeting_id: meeting.id
           meeting.participants.each do |participant|
-            expect(flash[:notice]).to include(participant.mail)
+            expect(flash[:error]).to include(participant.mail)
           end
         end
       end

From 62607c177900df60ab64a5a5f84b50052bffdd4b Mon Sep 17 00:00:00 2001
From: kgalli <mail@kgalli.de>
Date: Fri, 20 Mar 2015 16:26:37 +0000
Subject: [PATCH 3/6] Refactoring: reduce of unnecessary block chaining

---
 app/controllers/meeting_contents_controller.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/controllers/meeting_contents_controller.rb b/app/controllers/meeting_contents_controller.rb
index 8127f3917e..4544bd8eb9 100644
--- a/app/controllers/meeting_contents_controller.rb
+++ b/app/controllers/meeting_contents_controller.rb
@@ -83,9 +83,10 @@ class MeetingContentsController < ApplicationController
 
   def notify
     unless @content.new_record?
-      recipients = @content.meeting.participants.collect(&:mail).reject { |r| r == @content.meeting.author.mail }
-      recipients << @content.meeting.author.mail unless @content.meeting.author.preference[:no_self_notified]
+      recipients = @content.meeting.participants.collect{ |p| p.mail }
+      recipients.reject!{ |r| r == @content.meeting.author.mail } if @content.meeting.author.preference[:no_self_notified]
       recipients_with_errors = []
+
       recipients.each do |recipient|
         begin
           MeetingMailer.content_for_review(@content, @content_type, recipient).deliver

From d1b82564af7fbd98309b4bfc734b7e06a4a48e1e Mon Sep 17 00:00:00 2001
From: kgalli <mail@kgalli.de>
Date: Fri, 20 Mar 2015 17:17:44 +0000
Subject: [PATCH 4/6] Refactoring: increase readability

---
 app/controllers/meeting_contents_controller.rb      | 13 +++++++------
 .../controllers/meeting_contents_controller_spec.rb |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/app/controllers/meeting_contents_controller.rb b/app/controllers/meeting_contents_controller.rb
index 4544bd8eb9..5eb04979f0 100644
--- a/app/controllers/meeting_contents_controller.rb
+++ b/app/controllers/meeting_contents_controller.rb
@@ -83,13 +83,14 @@ class MeetingContentsController < ApplicationController
 
   def notify
     unless @content.new_record?
-      recipients = @content.meeting.participants.collect{ |p| p.mail }
-      recipients.reject!{ |r| r == @content.meeting.author.mail } if @content.meeting.author.preference[:no_self_notified]
-      recipients_with_errors = []
+      author_mail = @content.meeting.author.mail
+      do_not_notify_author = @content.meeting.author.preference[:no_self_notified]
 
-      recipients.each do |recipient|
+      recipients_with_errors = []
+      @content.meeting.participants.each do |recipient|
         begin
-          MeetingMailer.content_for_review(@content, @content_type, recipient).deliver
+          next if recipient.mail == author_mail and do_not_notify_author
+          MeetingMailer.content_for_review(@content, @content_type, recipient.mail).deliver
         rescue
           recipients_with_errors << recipient
         end
@@ -97,7 +98,7 @@ class MeetingContentsController < ApplicationController
       if recipients_with_errors == []
         flash[:notice] = l(:notice_successful_notification)
       else
-        flash[:error] = l(:error_notification_with_errors) + recipients_with_errors.join(", ")
+        flash[:error] = l(:error_notification_with_errors) + recipients_with_errors.map(&:name).join("; ")
       end
     end
     redirect_back_or_default controller: '/meetings', action: 'show', id: @meeting
diff --git a/spec/controllers/meeting_contents_controller_spec.rb b/spec/controllers/meeting_contents_controller_spec.rb
index 0cb3f3b964..6a2ad1fb4b 100644
--- a/spec/controllers/meeting_contents_controller_spec.rb
+++ b/spec/controllers/meeting_contents_controller_spec.rb
@@ -86,7 +86,7 @@ describe MeetingContentsController do
         it 'produces a flash message containing the mail addresses raising the error' do
           put 'notify', meeting_id: meeting.id
           meeting.participants.each do |participant|
-            expect(flash[:error]).to include(participant.mail)
+            expect(flash[:error]).to include(participant.name)
           end
         end
       end

From 32655ef1ddaf7e1cb0d4750d1d0a52b098241c98 Mon Sep 17 00:00:00 2001
From: kgalli <mail@kgalli.de>
Date: Fri, 20 Mar 2015 18:05:02 +0000
Subject: [PATCH 5/6] Refactoring: replacement of `and` with `&&`

---
 app/controllers/meeting_contents_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/meeting_contents_controller.rb b/app/controllers/meeting_contents_controller.rb
index 5eb04979f0..5cfcb7667f 100644
--- a/app/controllers/meeting_contents_controller.rb
+++ b/app/controllers/meeting_contents_controller.rb
@@ -89,7 +89,7 @@ class MeetingContentsController < ApplicationController
       recipients_with_errors = []
       @content.meeting.participants.each do |recipient|
         begin
-          next if recipient.mail == author_mail and do_not_notify_author
+          next if recipient.mail == author_mail && do_not_notify_author
           MeetingMailer.content_for_review(@content, @content_type, recipient.mail).deliver
         rescue
           recipients_with_errors << recipient

From 9b92187bae152d3f27b15305fed8ba1eead3d56e Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Wed, 1 Jul 2015 10:53:00 +0200
Subject: [PATCH 6/6] use interpolation in I18n

Relying on the position of some variable by appending in the code is bad practice
---
 app/controllers/meeting_contents_controller.rb | 3 ++-
 config/locales/de.yml                          | 2 +-
 config/locales/en.yml                          | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/controllers/meeting_contents_controller.rb b/app/controllers/meeting_contents_controller.rb
index 5cfcb7667f..58260e3c30 100644
--- a/app/controllers/meeting_contents_controller.rb
+++ b/app/controllers/meeting_contents_controller.rb
@@ -98,7 +98,8 @@ class MeetingContentsController < ApplicationController
       if recipients_with_errors == []
         flash[:notice] = l(:notice_successful_notification)
       else
-        flash[:error] = l(:error_notification_with_errors) + recipients_with_errors.map(&:name).join("; ")
+        flash[:error] = l(:error_notification_with_errors,
+                          recipients: recipients_with_errors.map(&:name).join('; '))
       end
     end
     redirect_back_or_default controller: '/meetings', action: 'show', id: @meeting
diff --git a/config/locales/de.yml b/config/locales/de.yml
index c8656f2489..1448a6b0eb 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -36,7 +36,7 @@ de:
   description_attended: "teilgenommen"
   description_invite: "eingeladen"
 
-  error_notification_with_errors: "Benachrichtigungversenden fehlgeschlagen. Folgende Empfänger konnten nicht benachrichtigt werden: "
+  error_notification_with_errors: "Benachrichtigungversenden fehlgeschlagen. Folgende Empfänger konnten nicht benachrichtigt werden: %{recipients}"
 
   events:
     meeting: Besprechung bearbeitet
diff --git a/config/locales/en.yml b/config/locales/en.yml
index da68ad38bc..9dab2f2420 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -44,7 +44,7 @@ en:
     meeting_minutes: Meeting minutes edited
     meeting_minutes_created: Meeting minutes created
 
-  error_notification_with_errors: "Failed to send notification. The following recipients could not be notified: "
+  error_notification_with_errors: "Failed to send notification. The following recipients could not be notified: %{recipients}"
 
   label_meeting_hour: "Time (hour)"
   label_meeting_minute: "Time (minute)"