From a683fcff98ed2f21449b277619544cbeb199b56c Mon Sep 17 00:00:00 2001
From: Bertocq <bertocq@gmail.com>
Date: Wed, 28 Feb 2018 20:57:39 +0100
Subject: [PATCH] Refactor notification partials and index view

The notification body has been extracted to a new partial to allow
notifications without link to be rendered without needing an if-else
duplicating view code.

Note the `link_to_if` at _notification partial, as well as the optional
body attribute.
---
 .../notifications/_notification.html.erb      | 26 ++++++-------------
 .../notifications/_notification_body.html.erb | 17 ++++++++++++
 app/views/notifications/index.html.erb        |  4 ++-
 3 files changed, 28 insertions(+), 19 deletions(-)
 create mode 100644 app/views/notifications/_notification_body.html.erb

diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb
index 2d8b115e5..34f1b96b1 100644
--- a/app/views/notifications/_notification.html.erb
+++ b/app/views/notifications/_notification.html.erb
@@ -1,21 +1,11 @@
-<li id="<%= dom_id(notification) %>" class="notification <%= "unread" if notification.unread? %>">
-
-  <% if notification.notifiable_available? %>
-    <%= link_to notification do %>
-      <p>
-        <em>
-          <%= t("notifications.notification.action.#{notification.notifiable_action}",
-                count: notification.counter) %>
-        </em>
-        <strong id="<%= dom_id(notification) %>_title">
-          <%= notification.notifiable_title %>
-        </strong>
-      </p>
-
-      <p class="time">
-        <%= l notification.timestamp, format: :datetime %>
-      </p>
-    <% end %>
+<li id="<%= dom_id(notification) %>" class="notification <%= 'unread' if notification&.unread? %>">
+  <% if notification.try(:notifiable_available?) %>
+    <% locals = { notification: notification,
+                  timestamp: notification.timestamp,
+                  title: notification.notifiable_title,
+                  body: notification.notifiable.try(:body) } %>
+    <% link_text = render partial: '/notifications/notification_body', locals: locals %>
+    <%= link_to_if notification.link.present?, link_text, notification.link %>
   <% else %>
     <p>
       <strong>
diff --git a/app/views/notifications/_notification_body.html.erb b/app/views/notifications/_notification_body.html.erb
new file mode 100644
index 000000000..d6ed86673
--- /dev/null
+++ b/app/views/notifications/_notification_body.html.erb
@@ -0,0 +1,17 @@
+<p>
+  <% if notification && notification.notifiable_action %>
+    <em>
+      <%= t("notifications.notification.action.#{notification.notifiable_action}",
+            count: notification.counter) %>
+    </em>
+  <% end %>
+  <strong id="<%= dom_id(notification) if notification %>_title">
+    <%= title %>
+  </strong>
+  <% if body %>
+    <p><%= body %></p>
+  <% end %>
+</p>
+<p class="time">
+  <%= l(timestamp, format: :datetime) %>
+</p>
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb
index eccd3ec6a..e005aad88 100644
--- a/app/views/notifications/index.html.erb
+++ b/app/views/notifications/index.html.erb
@@ -24,7 +24,9 @@
       </div>
     <% else %>
       <ul class="no-bullet clear notifications-list">
-        <%= render @notifications %>
+        <% @notifications.each do |notification| %>
+          <%= render partial: '/notifications/notification', locals: {notification: notification} %>
+        <% end %>
       </ul>
     <% end %>
   </div>