Merge pull request #12586 from saraycp/fix_metrics-for_notifications

Fix metrics for notifications

src/api/app/controllers/webui/users/notifications_controller.rb

@@ -21,13 +21,14 @@ def update
                       fetch_notifications.where(id: params[:notification_ids])
                     end
     # rubocop:disable Rails/SkipsModelValidations
-    updated_notifications = notifications.update_all('delivered = !delivered')
+    read_count = notifications.where(delivered: false).update_all('delivered = !delivered')
+    unread_count = notifications.where(delivered: true).update_all('delivered = !delivered')
     # rubocop:enable Rails/SkipsModelValidations
 
-    if updated_notifications.zero?
+    if read_count.zero? && unread_count.zero?
       flash.now[:error] = "Couldn't update the notifications"
     else
-      send_notifications_information_rabbitmq(updated_notifications)
+      send_notifications_information_rabbitmq(read_count, unread_count)
     end
 
     respond_to do |format|
@@ -91,8 +92,8 @@ def show_read_all_button?
     fetch_notifications.count > Notification::MAX_PER_PAGE
   end
 
-  def send_notifications_information_rabbitmq(number_of_records)
-    RabbitmqBus.send_to_bus('metrics',
-                            "notification.delivered,web=true,rss=false value=#{number_of_records}")
+  def send_notifications_information_rabbitmq(read_count, unread_count)
+    RabbitmqBus.send_to_bus('metrics', "notification,action=read value=#{read_count}") if read_count.positive?
+    RabbitmqBus.send_to_bus('metrics', "notification,action=unread value=#{unread_count}") if unread_count.positive?
   end
 end

src/api/app/models/notification.rb

@@ -54,9 +54,11 @@ def track_notification_creation
                             "notification.create,notifiable_type=#{notifiable_type},web=#{web},rss=#{rss} value=1")
   end
 
+  # This is only called when the request come from the API. The UI performs 'update_all' that does not trigger callbacks.
+  # This metrics complements the same metrics tracked in Webui::Users::NotificationsController#send_notifications_information_rabbitmq
   def track_notification_delivered
     RabbitmqBus.send_to_bus('metrics',
-                            "notification.delivered,notifiable_type=#{notifiable_type},web=#{web},rss=#{rss} value=1")
+                            "notification,action=#{delivered ? 'read' : 'unread'} value=1")
   end
 end
 

src/api/spec/models/notification_spec.rb

@@ -68,31 +68,31 @@
 
   describe 'Instrumentation' do
     let!(:test_user) { create(:confirmed_user, login: 'foo') }
-    let!(:rss_notification) { create(:rss_notification, subscriber: test_user) }
+    let!(:web_notification) { create(:web_notification, subscriber: test_user) }
 
     before do
-      allow(RabbitmqBus).to receive(:send_to_bus).with('metrics', 'notification.delivered,notifiable_type=,web=false,rss=true value=1')
+      allow(RabbitmqBus).to receive(:send_to_bus).with('metrics', 'notification,action=read value=1')
     end
 
     context 'if delivered change, we should track it' do
       before do
-        rss_notification.delivered = true
+        web_notification.delivered = true
       end
 
       it do
-        rss_notification.save
-        expect(RabbitmqBus).to have_received(:send_to_bus).with('metrics', 'notification.delivered,notifiable_type=,web=false,rss=true value=1')
+        web_notification.save
+        expect(RabbitmqBus).to have_received(:send_to_bus).with('metrics', 'notification,action=read value=1')
       end
     end
 
-    context 'if delivered doe not change, we should not track it' do
+    context 'if delivered does not change, we should not track it' do
       before do
-        rss_notification.title = 'FOO FOO'
+        web_notification.title = 'FOO FOO'
       end
 
       it do
-        rss_notification.save
-        expect(RabbitmqBus).not_to have_received(:send_to_bus).with('metrics', 'notification.delivered,notifiable_type=,web=false,rss=true value=1')
+        web_notification.save
+        expect(RabbitmqBus).not_to have_received(:send_to_bus).with('metrics', 'notification,action=read value=1')
       end
     end
   end