From f11f2cd3dddd8538f1cf1dcda284f6e0fe61bf15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 15 Feb 2021 23:36:58 +0100
Subject: [PATCH 1/6] Extract component to render notifications item

---
 app/assets/stylesheets/application.scss       |  1 +
 app/assets/stylesheets/layout.scss            | 26 ----------------
 app/assets/stylesheets/notification_item.scss | 25 +++++++++++++++
 .../notification_item_component.html.erb      | 30 ++++++++++++++++++
 .../layout/notification_item_component.rb     |  7 +++++
 app/views/layouts/_notification_item.html.erb | 31 +------------------
 6 files changed, 64 insertions(+), 56 deletions(-)
 create mode 100644 app/assets/stylesheets/notification_item.scss
 create mode 100644 app/components/layout/notification_item_component.html.erb
 create mode 100644 app/components/layout/notification_item_component.rb

diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
index ffb1517de..44dc26563 100644
--- a/app/assets/stylesheets/application.scss
+++ b/app/assets/stylesheets/application.scss
@@ -15,6 +15,7 @@
 @import "legislation";
 @import "legislation_process";
 @import "legislation_process_form";
+@import "notification_item";
 @import "community";
 @import "stats";
 @import "custom";
diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss
index d993372ab..cc2d92de1 100644
--- a/app/assets/stylesheets/layout.scss
+++ b/app/assets/stylesheets/layout.scss
@@ -1345,32 +1345,6 @@ form {
   }
 }
 
-.notifications {
-  position: relative;
-
-  &:hover {
-    text-decoration: none;
-  }
-
-  [class^="icon-"] {
-    font-size: rem-calc(19);
-    vertical-align: middle;
-  }
-
-  .icon-circle {
-    color: #ecf00b;
-    font-size: rem-calc(10);
-    position: absolute;
-    left: 12px;
-    top: 6px;
-
-    @include breakpoint(medium) {
-      left: auto;
-      right: 8px;
-    }
-  }
-}
-
 .notifications-list {
   position: relative;
 
diff --git a/app/assets/stylesheets/notification_item.scss b/app/assets/stylesheets/notification_item.scss
new file mode 100644
index 000000000..fd272250a
--- /dev/null
+++ b/app/assets/stylesheets/notification_item.scss
@@ -0,0 +1,25 @@
+.notifications {
+  position: relative;
+
+  &:hover {
+    text-decoration: none;
+  }
+
+  [class^="icon-"] {
+    font-size: rem-calc(19);
+    vertical-align: middle;
+  }
+
+  .icon-circle {
+    color: #ecf00b;
+    font-size: rem-calc(10);
+    position: absolute;
+    left: 12px;
+    top: 6px;
+
+    @include breakpoint(medium) {
+      left: auto;
+      right: 8px;
+    }
+  }
+}
diff --git a/app/components/layout/notification_item_component.html.erb b/app/components/layout/notification_item_component.html.erb
new file mode 100644
index 000000000..954b60420
--- /dev/null
+++ b/app/components/layout/notification_item_component.html.erb
@@ -0,0 +1,30 @@
+<% if user %>
+  <li id="notifications">
+    <%= link_to notifications_path, rel: "nofollow",
+                class: "notifications" do %>
+      <span class="show-for-sr">
+        <%= t("layouts.header.notification_item.notifications") %>
+      </span>
+
+      <% if user.notifications.unread.count > 0 %>
+        <span class="icon-circle" aria-hidden="true"></span>
+        <span class="icon-notification" aria-hidden="true"
+              title="<%= t("layouts.header.notification_item.new_notifications",
+                         count: user.notifications_count) %>">
+        </span>
+        <span class="show-for-small-only">
+          <%= t("layouts.header.notification_item.new_notifications",
+              count: user.notifications_count) %>
+        </span>
+      <% else %>
+        <span class="icon-no-notification" aria-hidden="true"
+              title="<%= t("layouts.header.notification_item.no_notifications") %>">
+        </span>
+        <span class="show-for-small-only">
+          <%= t("layouts.header.notification_item.no_notifications") %>
+        </span>
+      <% end %>
+
+    <% end %>
+  </li>
+<% end %>
diff --git a/app/components/layout/notification_item_component.rb b/app/components/layout/notification_item_component.rb
new file mode 100644
index 000000000..62952be1c
--- /dev/null
+++ b/app/components/layout/notification_item_component.rb
@@ -0,0 +1,7 @@
+class Layout::NotificationItemComponent < ApplicationComponent
+  attr_reader :user
+
+  def initialize(user)
+    @user = user
+  end
+end
diff --git a/app/views/layouts/_notification_item.html.erb b/app/views/layouts/_notification_item.html.erb
index c766f862a..2e51085b6 100644
--- a/app/views/layouts/_notification_item.html.erb
+++ b/app/views/layouts/_notification_item.html.erb
@@ -1,30 +1 @@
-<% if user_signed_in? %>
-  <li id="notifications">
-    <%= link_to notifications_path, rel: "nofollow",
-                class: "notifications" do %>
-      <span class="show-for-sr">
-        <%= t("layouts.header.notification_item.notifications") %>
-      </span>
-
-      <% if current_user.notifications.unread.count > 0 %>
-        <span class="icon-circle" aria-hidden="true"></span>
-        <span class="icon-notification" aria-hidden="true"
-              title="<%= t("layouts.header.notification_item.new_notifications",
-                         count: current_user.notifications_count) %>">
-        </span>
-        <span class="show-for-small-only">
-          <%= t("layouts.header.notification_item.new_notifications",
-              count: current_user.notifications_count) %>
-        </span>
-      <% else %>
-        <span class="icon-no-notification" aria-hidden="true"
-              title="<%= t("layouts.header.notification_item.no_notifications") %>">
-        </span>
-        <span class="show-for-small-only">
-          <%= t("layouts.header.notification_item.no_notifications") %>
-        </span>
-      <% end %>
-
-    <% end %>
-  </li>
-<% end %>
+<%= render Layout::NotificationItemComponent.new(current_user) %>

From 5b6551f6d7139d2f399fa1dee8ff34775de0bb3b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 15 Feb 2021 23:45:57 +0100
Subject: [PATCH 2/6] Extract method to get the notifications link text

---
 .../layout/notification_item_component.html.erb | 17 +++--------------
 .../layout/notification_item_component.rb       | 10 ++++++++++
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/app/components/layout/notification_item_component.html.erb b/app/components/layout/notification_item_component.html.erb
index 954b60420..2fbae2250 100644
--- a/app/components/layout/notification_item_component.html.erb
+++ b/app/components/layout/notification_item_component.html.erb
@@ -8,23 +8,12 @@
 
       <% if user.notifications.unread.count > 0 %>
         <span class="icon-circle" aria-hidden="true"></span>
-        <span class="icon-notification" aria-hidden="true"
-              title="<%= t("layouts.header.notification_item.new_notifications",
-                         count: user.notifications_count) %>">
-        </span>
-        <span class="show-for-small-only">
-          <%= t("layouts.header.notification_item.new_notifications",
-              count: user.notifications_count) %>
-        </span>
+        <span class="icon-notification" aria-hidden="true" title="<%= text %>"></span>
       <% else %>
-        <span class="icon-no-notification" aria-hidden="true"
-              title="<%= t("layouts.header.notification_item.no_notifications") %>">
-        </span>
-        <span class="show-for-small-only">
-          <%= t("layouts.header.notification_item.no_notifications") %>
-        </span>
+        <span class="icon-no-notification" aria-hidden="true" title="<%= text %>"></span>
       <% end %>
 
+      <span class="show-for-small-only"><%= text %></span>
     <% end %>
   </li>
 <% end %>
diff --git a/app/components/layout/notification_item_component.rb b/app/components/layout/notification_item_component.rb
index 62952be1c..37be31de1 100644
--- a/app/components/layout/notification_item_component.rb
+++ b/app/components/layout/notification_item_component.rb
@@ -4,4 +4,14 @@ class Layout::NotificationItemComponent < ApplicationComponent
   def initialize(user)
     @user = user
   end
+
+  private
+
+    def text
+      if user.notifications.unread.count > 0
+        t("layouts.header.notification_item.new_notifications", count: user.notifications_count)
+      else
+        t("layouts.header.notification_item.no_notifications")
+      end
+    end
 end

From 4c289f2489797d334ec143eba67eccc13270bc1e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 16 Feb 2021 00:30:01 +0100
Subject: [PATCH 3/6] Simplify notification item HTML

Other than simplifying the view, we can write tests using `click_link`,
which makes the tests more robust. Clicking the `.icon-notification`
element was causing some tests to fail when defining a window height of
750 pixels in the `admin_budgets` branch.
---
 app/assets/stylesheets/admin.scss             |  4 +-
 app/assets/stylesheets/mixins.scss            |  8 +--
 app/assets/stylesheets/notification_item.scss | 51 ++++++++++++-------
 .../notification_item_component.html.erb      | 12 +----
 .../layout/notification_item_component.rb     | 14 ++++-
 spec/shared/system/notifiable_in_app.rb       | 13 +++--
 spec/system/notifications_spec.rb             |  4 +-
 spec/system/proposal_notifications_spec.rb    | 17 +++----
 8 files changed, 72 insertions(+), 51 deletions(-)

diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss
index 51a554038..c67de27e5 100644
--- a/app/assets/stylesheets/admin.scss
+++ b/app/assets/stylesheets/admin.scss
@@ -99,7 +99,7 @@ $sidebar-active: #f4fcd0;
       }
     }
 
-    [class^="icon-"]:not(.icon-circle) {
+    [class^="icon-"] {
       font-size: $base-font-size;
     }
   }
@@ -156,7 +156,7 @@ $sidebar-active: #f4fcd0;
     }
   }
 
-  .notifications .icon-circle {
+  .notifications.unread-notifications::after {
     color: $admin-color;
   }
 
diff --git a/app/assets/stylesheets/mixins.scss b/app/assets/stylesheets/mixins.scss
index bad75babd..f6a5bef0e 100644
--- a/app/assets/stylesheets/mixins.scss
+++ b/app/assets/stylesheets/mixins.scss
@@ -163,11 +163,11 @@
   margin-right: rem-calc(10);
 }
 
-@mixin has-fa-icon($icon, $style) {
-  @extend .fa-#{$icon};
+@mixin has-fa-icon($icon, $style, $position: "before") {
 
-  &::before {
+  &::#{$position} {
     @extend %font-icon;
+    @extend .fa-#{$icon}:before;
 
     @if $style == "solid" {
       font-weight: bold;
@@ -182,7 +182,7 @@
 
   @supports (mask-image: url()) {
 
-    &::before {
+    &::#{$position} {
       @extend %svg-icon;
       mask-image: image-url("fontawesome/#{$style}/#{$icon}.svg");
     }
diff --git a/app/assets/stylesheets/notification_item.scss b/app/assets/stylesheets/notification_item.scss
index fd272250a..5d182a7db 100644
--- a/app/assets/stylesheets/notification_item.scss
+++ b/app/assets/stylesheets/notification_item.scss
@@ -1,25 +1,40 @@
 .notifications {
   position: relative;
 
+  &.unread-notifications {
+    @include has-fa-icon(bell, solid);
+    @include has-fa-icon(circle, solid, after);
+
+    &::before {
+      @include breakpoint(medium) {
+        margin-right: 0;
+      }
+    }
+
+    &::after {
+      color: #ecf00b;
+      font-size: rem-calc(10);
+      margin-right: 0;
+      position: absolute;
+      left: 12px;
+      top: 6px;
+
+      @include breakpoint(medium) {
+        left: auto;
+        right: 8px;
+      }
+    }
+  }
+
+  &.no-notifications {
+    @include has-fa-icon(bell, regular);
+  }
+
+  &::before {
+    font-size: rem-calc(19);
+  }
+
   &:hover {
     text-decoration: none;
   }
-
-  [class^="icon-"] {
-    font-size: rem-calc(19);
-    vertical-align: middle;
-  }
-
-  .icon-circle {
-    color: #ecf00b;
-    font-size: rem-calc(10);
-    position: absolute;
-    left: 12px;
-    top: 6px;
-
-    @include breakpoint(medium) {
-      left: auto;
-      right: 8px;
-    }
-  }
 }
diff --git a/app/components/layout/notification_item_component.html.erb b/app/components/layout/notification_item_component.html.erb
index 2fbae2250..363c2fd2d 100644
--- a/app/components/layout/notification_item_component.html.erb
+++ b/app/components/layout/notification_item_component.html.erb
@@ -1,18 +1,10 @@
 <% if user %>
   <li id="notifications">
-    <%= link_to notifications_path, rel: "nofollow",
-                class: "notifications" do %>
+    <%= link_to notifications_path, rel: "nofollow", title: text,
+                class: "notifications #{notifications_class}" do %>
       <span class="show-for-sr">
         <%= t("layouts.header.notification_item.notifications") %>
       </span>
-
-      <% if user.notifications.unread.count > 0 %>
-        <span class="icon-circle" aria-hidden="true"></span>
-        <span class="icon-notification" aria-hidden="true" title="<%= text %>"></span>
-      <% else %>
-        <span class="icon-no-notification" aria-hidden="true" title="<%= text %>"></span>
-      <% end %>
-
       <span class="show-for-small-only"><%= text %></span>
     <% end %>
   </li>
diff --git a/app/components/layout/notification_item_component.rb b/app/components/layout/notification_item_component.rb
index 37be31de1..4c23a2a4a 100644
--- a/app/components/layout/notification_item_component.rb
+++ b/app/components/layout/notification_item_component.rb
@@ -8,10 +8,22 @@ class Layout::NotificationItemComponent < ApplicationComponent
   private
 
     def text
-      if user.notifications.unread.count > 0
+      if unread_notifications?
         t("layouts.header.notification_item.new_notifications", count: user.notifications_count)
       else
         t("layouts.header.notification_item.no_notifications")
       end
     end
+
+    def notifications_class
+      if unread_notifications?
+        "unread-notifications"
+      else
+        "no-notifications"
+      end
+    end
+
+    def unread_notifications?
+      user.notifications.unread.count > 0
+    end
 end
diff --git a/spec/shared/system/notifiable_in_app.rb b/spec/shared/system/notifiable_in_app.rb
index afddae9d3..08e30299d 100644
--- a/spec/shared/system/notifiable_in_app.rb
+++ b/spec/shared/system/notifiable_in_app.rb
@@ -2,13 +2,13 @@ shared_examples "notifiable in-app" do |factory_name|
   let(:author) { create(:user, :verified) }
   let!(:notifiable) { create(factory_name, author: author) }
 
-  scenario "Notification icon is shown" do
+  scenario "Notification message is shown" do
     create(:notification, notifiable: notifiable, user: author)
 
     login_as author
     visit root_path
 
-    expect(page).to have_css ".icon-notification"
+    expect(page).to have_link "You have a new notification"
   end
 
   scenario "A user commented on my notifiable", :js do
@@ -16,7 +16,8 @@ shared_examples "notifiable in-app" do |factory_name|
 
     login_as author
     visit root_path
-    find(".icon-notification").click
+
+    click_link "You have a new notification"
 
     expect(page).to have_css ".notification", count: 1
     expect(page).to have_content "Someone commented on"
@@ -108,7 +109,8 @@ shared_examples "notifiable in-app" do |factory_name|
     end
 
     within("#notifications") do
-      find(".icon-no-notification").click
+      click_link "You don't have new notifications"
+
       expect(page).to have_css ".notification", count: 0
     end
   end
@@ -130,7 +132,8 @@ shared_examples "notifiable in-app" do |factory_name|
     end
 
     within("#notifications") do
-      find(".icon-no-notification").click
+      click_link "You don't have new notifications"
+
       expect(page).to have_css ".notification", count: 0
     end
   end
diff --git a/spec/system/notifications_spec.rb b/spec/system/notifications_spec.rb
index 186727ad2..9dba969ad 100644
--- a/spec/system/notifications_spec.rb
+++ b/spec/system/notifications_spec.rb
@@ -103,14 +103,14 @@ describe "Notifications" do
     visit root_path
 
     within("#notifications") do
-      expect(page).to have_css(".icon-circle")
+      expect(page).to have_css(".unread-notifications")
     end
 
     click_notifications_icon
     first(".notification a").click
 
     within("#notifications") do
-      expect(page).not_to have_css(".icon-circle")
+      expect(page).not_to have_css(".unread-notifications")
     end
   end
 
diff --git a/spec/system/proposal_notifications_spec.rb b/spec/system/proposal_notifications_spec.rb
index 7a9377962..15f384763 100644
--- a/spec/system/proposal_notifications_spec.rb
+++ b/spec/system/proposal_notifications_spec.rb
@@ -198,7 +198,7 @@ describe "Proposal Notifications" do
       login_as user1
       visit root_path
 
-      find(".icon-notification").click
+      find(".unread-notifications").click
 
       expect(page).to have_css ".notification", count: 1
 
@@ -210,7 +210,7 @@ describe "Proposal Notifications" do
       login_as user2
       visit root_path
 
-      find(".icon-notification").click
+      find(".unread-notifications").click
 
       expect(page).to have_css ".notification", count: 1
 
@@ -222,7 +222,7 @@ describe "Proposal Notifications" do
       login_as user3
       visit root_path
 
-      find(".icon-no-notification").click
+      click_link "You don't have new notifications"
 
       expect(page).to have_css ".notification", count: 0
     end
@@ -251,7 +251,7 @@ describe "Proposal Notifications" do
       login_as user1.reload
       visit root_path
 
-      find(".icon-notification").click
+      click_link "You have a new notification"
 
       expect(page).to have_css ".notification", count: 1
 
@@ -263,7 +263,7 @@ describe "Proposal Notifications" do
       login_as user2.reload
       visit root_path
 
-      find(".icon-notification").click
+      click_link "You have a new notification"
 
       expect(page).to have_css ".notification", count: 1
 
@@ -275,7 +275,7 @@ describe "Proposal Notifications" do
       login_as user3.reload
       visit root_path
 
-      find(".icon-no-notification").click
+      click_link "You don't have new notifications"
 
       expect(page).to have_css ".notification", count: 0
     end
@@ -303,7 +303,7 @@ describe "Proposal Notifications" do
       login_as user
       visit root_path
 
-      find(".icon-notification").click
+      find(".unread-notifications").click
 
       expect(page).to have_css ".notification", count: 1
       expect(page).to have_content "This resource is not available anymore"
@@ -346,8 +346,7 @@ describe "Proposal Notifications" do
         login_as user.reload
         visit root_path
 
-        within("#notifications") { expect(page).to have_content :all, "You have 3 new notifications" }
-        find(".icon-notification").click
+        click_link "You have 3 new notifications"
 
         expect(page).to have_css ".notification", count: 3
         expect(page).to have_content "There is one new notification on #{proposal.title}", count: 3

From 0839c5ea75ae0c4bb3f83f39aea2016371753f49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 16 Feb 2021 01:13:15 +0100
Subject: [PATCH 4/6] Adjust new notifications icon position

It was hard to know where the numbers were coming from; they depended on
the padding of the link and the size of the notification icon size. So
we're using variables to make it more explicit.

However, the code is now too complex, so we'll probably have to simplify
it in the future.
---
 app/assets/stylesheets/notification_item.scss | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/app/assets/stylesheets/notification_item.scss b/app/assets/stylesheets/notification_item.scss
index 5d182a7db..a6d2ee1e5 100644
--- a/app/assets/stylesheets/notification_item.scss
+++ b/app/assets/stylesheets/notification_item.scss
@@ -1,4 +1,5 @@
 .notifications {
+  $notification-icon-size: rem-calc(19);
   position: relative;
 
   &.unread-notifications {
@@ -12,16 +13,19 @@
     }
 
     &::after {
+      $menu-link-top-padding: rem-calc(11);
+      $circle-icon-size: rem-calc(10);
+
       color: #ecf00b;
-      font-size: rem-calc(10);
+      font-size: $circle-icon-size;
       margin-right: 0;
       position: absolute;
-      left: 12px;
-      top: 6px;
+      left: $notification-icon-size - rem-calc(5);
+      top: $menu-link-top-padding - $circle-icon-size / 2;
 
       @include breakpoint(medium) {
-        left: auto;
-        right: 8px;
+        $menu-link-left-padding: rem-calc(16);
+        left: $notification-icon-size + $menu-link-left-padding;
       }
     }
   }
@@ -31,7 +35,7 @@
   }
 
   &::before {
-    font-size: rem-calc(19);
+    font-size: $notification-icon-size;
   }
 
   &:hover {

From fb88e0b77c5f7b076a72bcc5e4d72907d39719ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 16 Feb 2021 01:24:33 +0100
Subject: [PATCH 5/6] Fix number of new notifications

We were displaying the total number of notifications with a message "You
have N unread notifications", but were using the total number of
notifications instead of the unread ones.
---
 app/components/layout/notification_item_component.rb | 8 ++++++--
 spec/shared/system/notifiable_in_app.rb              | 2 ++
 spec/system/proposal_notifications_spec.rb           | 6 +++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/components/layout/notification_item_component.rb b/app/components/layout/notification_item_component.rb
index 4c23a2a4a..c63205f12 100644
--- a/app/components/layout/notification_item_component.rb
+++ b/app/components/layout/notification_item_component.rb
@@ -9,7 +9,7 @@ class Layout::NotificationItemComponent < ApplicationComponent
 
     def text
       if unread_notifications?
-        t("layouts.header.notification_item.new_notifications", count: user.notifications_count)
+        t("layouts.header.notification_item.new_notifications", count: unread_notifications.count)
       else
         t("layouts.header.notification_item.no_notifications")
       end
@@ -24,6 +24,10 @@ class Layout::NotificationItemComponent < ApplicationComponent
     end
 
     def unread_notifications?
-      user.notifications.unread.count > 0
+      unread_notifications.count > 0
+    end
+
+    def unread_notifications
+      user.notifications.unread
     end
 end
diff --git a/spec/shared/system/notifiable_in_app.rb b/spec/shared/system/notifiable_in_app.rb
index 08e30299d..048eb81f0 100644
--- a/spec/shared/system/notifiable_in_app.rb
+++ b/spec/shared/system/notifiable_in_app.rb
@@ -2,6 +2,8 @@ shared_examples "notifiable in-app" do |factory_name|
   let(:author) { create(:user, :verified) }
   let!(:notifiable) { create(factory_name, author: author) }
 
+  before { create(:notification, :read, notifiable: notifiable, user: author) }
+
   scenario "Notification message is shown" do
     create(:notification, notifiable: notifiable, user: author)
 
diff --git a/spec/system/proposal_notifications_spec.rb b/spec/system/proposal_notifications_spec.rb
index 15f384763..998dd1223 100644
--- a/spec/system/proposal_notifications_spec.rb
+++ b/spec/system/proposal_notifications_spec.rb
@@ -198,7 +198,7 @@ describe "Proposal Notifications" do
       login_as user1
       visit root_path
 
-      find(".unread-notifications").click
+      click_link "You have a new notification"
 
       expect(page).to have_css ".notification", count: 1
 
@@ -210,7 +210,7 @@ describe "Proposal Notifications" do
       login_as user2
       visit root_path
 
-      find(".unread-notifications").click
+      click_link "You have a new notification"
 
       expect(page).to have_css ".notification", count: 1
 
@@ -303,7 +303,7 @@ describe "Proposal Notifications" do
       login_as user
       visit root_path
 
-      find(".unread-notifications").click
+      click_link "You have a new notification"
 
       expect(page).to have_css ".notification", count: 1
       expect(page).to have_content "This resource is not available anymore"

From e266e0e0e29fb9bd44fbd2716ec92542361aa780 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 16 Feb 2021 01:26:57 +0100
Subject: [PATCH 6/6] Simplify code to display text of new notifications

We couldn't do this refactoring earlier because we weren't using the
unread notifications count. This was fixed in the previous commit.
---
 app/components/layout/notification_item_component.rb | 12 ++----------
 config/locales/en/general.yml                        |  2 +-
 config/locales/es/general.yml                        |  2 +-
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/app/components/layout/notification_item_component.rb b/app/components/layout/notification_item_component.rb
index c63205f12..c620180b0 100644
--- a/app/components/layout/notification_item_component.rb
+++ b/app/components/layout/notification_item_component.rb
@@ -8,25 +8,17 @@ class Layout::NotificationItemComponent < ApplicationComponent
   private
 
     def text
-      if unread_notifications?
-        t("layouts.header.notification_item.new_notifications", count: unread_notifications.count)
-      else
-        t("layouts.header.notification_item.no_notifications")
-      end
+      t("layouts.header.notification_item.new_notifications", count: unread_notifications.count)
     end
 
     def notifications_class
-      if unread_notifications?
+      if unread_notifications.count > 0
         "unread-notifications"
       else
         "no-notifications"
       end
     end
 
-    def unread_notifications?
-      unread_notifications.count > 0
-    end
-
     def unread_notifications
       user.notifications.unread
     end
diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml
index 6f6672013..450f51b8a 100644
--- a/config/locales/en/general.yml
+++ b/config/locales/en/general.yml
@@ -249,8 +249,8 @@ en:
         new_notifications:
           one: You have a new notification
           other: You have %{count} new notifications
+          zero: "You don't have new notifications"
         notifications: Notifications
-        no_notifications: "You don't have new notifications"
       sdg: "SDG"
   notifications:
     index:
diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml
index d1fff5341..67bc57a8a 100644
--- a/config/locales/es/general.yml
+++ b/config/locales/es/general.yml
@@ -249,8 +249,8 @@ es:
         new_notifications:
           one: Tienes una nueva notificación
           other: Tienes %{count} notificaciones nuevas
+          zero: "No tienes notificaciones nuevas"
         notifications: Notificaciones
-        no_notifications: "No tienes notificaciones nuevas"
       sdg: "ODS"
   notifications:
     index: