Refactor notification helper and fix notification service

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

app/helpers/notifications_helper.rb

@@ -16,8 +16,8 @@ module NotificationsHelper
     end
   end
 
-  def notification_icon(level)
-    icon("#{notification_icon_class(level)} fw")
+  def notification_icon(level, text = nil)
+    icon("#{notification_icon_class(level)} fw", text: text)
   end
 
   def notification_title(level)
@@ -39,14 +39,10 @@ module NotificationsHelper
       notification_title: title
     }
 
-    content_tag(:li, class: active_level_for(setting, level)) do
+    content_tag(:li, class: ('active' if setting.level == level)) do
       link_to '#', class: 'update-notification', data: data do
-        icon("#{notification_icon_class(level)} fw", text: title)
+        notification_icon(level, title)
       end
     end
   end
-
-  def active_level_for(setting, level)
-    'active' if setting.level == level
-  end
 end

app/services/notification_service.rb

@@ -352,7 +352,7 @@ class NotificationService
       setting = user.notification_settings.find_by(source: project)
 
       if !setting && project.group
-        setting = user.notification_settings.find_by(source: group)
+        setting = user.notification_settings.find_by(source: project.group)
       end
 
       # reject users who globally set mention notification and has no setting per project/group

spec/helpers/notifications_helper_spec.rb

@@ -2,34 +2,15 @@ require 'spec_helper'
 
 describe NotificationsHelper do
   describe 'notification_icon' do
-    let(:notification) { double(disabled?: false, participating?: false, watch?: false) }
-
-    context "disabled notification" do
-      before { allow(notification).to receive(:disabled?).and_return(true) }
-
-      it "has a red icon" do
-        expect(notification_icon(notification)).to match('class="fa fa-volume-off ns-mute"')
-      end
-    end
-
-    context "participating notification" do
-      before { allow(notification).to receive(:participating?).and_return(true) }
-
-      it "has a blue icon" do
-        expect(notification_icon(notification)).to match('class="fa fa-volume-down ns-part"')
-      end
-    end
-
-    context "watched notification" do
-      before { allow(notification).to receive(:watch?).and_return(true) }
-
-      it "has a green icon" do
-        expect(notification_icon(notification)).to match('class="fa fa-volume-up ns-watch"')
-      end
-    end
+    it { expect(notification_icon(:disabled)).to match('class="fa fa-microphone-slash fa-fw"') }
+    it { expect(notification_icon(:participating)).to match('class="fa fa-volume-up fa-fw"') }
+    it { expect(notification_icon(:mention)).to match('class="fa fa-at fa-fw"') }
+    it { expect(notification_icon(:global)).to match('class="fa fa-globe fa-fw"') }
+    it { expect(notification_icon(:watch)).to match('class="fa fa-eye fa-fw"') }
+  end
 
-    it "has a blue icon" do
-      expect(notification_icon(notification)).to match('class="fa fa-circle-o ns-default"')
-    end
+  describe 'notification_title' do
+    it { expect(notification_title(:watch)).to match('Watch') }
+    it { expect(notification_title(:mention)).to match('On mention') }
   end
 end