From 75f3f7fcbd37deea794e077253d5eb1a0d20f356 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 16 Sep 2016 16:14:00 +1000
Subject: [PATCH] FEATURE: clean API method for reading a single notification

---
 app/controllers/application_controller.rb         |  5 +----
 app/controllers/notifications_controller.rb       | 13 ++++++++-----
 app/models/notification.rb                        |  9 +++++++++
 config/routes.rb                                  |  3 +++
 spec/controllers/notifications_controller_spec.rb | 13 +++++++++++++
 5 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1e1f5322d91..263377d8a97 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -172,10 +172,7 @@ class ApplicationController < ActionController::Base
 
       if notifications.present?
         notification_ids = notifications.split(",").map(&:to_i)
-        count = Notification.where(user_id: current_user.id, id: notification_ids, read: false).update_all(read: true)
-        if count > 0
-          current_user.publish_notifications_state
-        end
+        Notification.read(current_user, notification_ids)
         cookies.delete('cn')
       end
     end
diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index da047671b79..7d2f0da89bb 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -46,11 +46,14 @@ class NotificationsController < ApplicationController
   end
 
   def mark_read
-    Notification.where(user_id: current_user.id).includes(:topic).where(read: false).update_all(read: true)
-
-    current_user.saw_notification_id(Notification.recent_report(current_user, 1).max.try(:id))
-    current_user.reload
-    current_user.publish_notifications_state
+    if params[:id]
+      Notification.read(current_user, [params[:id].to_i])
+    else
+      Notification.where(user_id: current_user.id).includes(:topic).where(read: false).update_all(read: true)
+      current_user.saw_notification_id(Notification.recent_report(current_user, 1).max.try(:id))
+      current_user.reload
+      current_user.publish_notifications_state
+    end
 
     render json: success_json
   end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index b9c05f24a65..40470325483 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -60,6 +60,15 @@ class Notification < ActiveRecord::Base
     count
   end
 
+  def self.read(user, notification_ids)
+    count = Notification.where(user_id: user.id,
+                               id: notification_ids,
+                               read: false).update_all(read: true)
+    if count > 0
+      user.publish_notifications_state
+    end
+  end
+
   def self.interesting_after(min_date)
     result =  where("created_at > ?", min_date)
               .includes(:topic)
diff --git a/config/routes.rb b/config/routes.rb
index ca9e1d2df5d..45cf5a871df 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -426,6 +426,9 @@ Discourse::Application.routes.draw do
 
   get 'notifications' => 'notifications#index'
   put 'notifications/mark-read' => 'notifications#mark_read'
+  # creating an alias cause the api was extended to mark a single notification
+  # this allows us to cleanly target it
+  put 'notifications/read' => 'notifications#mark_read'
 
   match "/auth/:provider/callback", to: "users/omniauth_callbacks#complete", via: [:get, :post]
   match "/auth/failure", to: "users/omniauth_callbacks#failure", via: [:get, :post]
diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb
index da58fc6e6c3..dbf80666390 100644
--- a/spec/controllers/notifications_controller_spec.rb
+++ b/spec/controllers/notifications_controller_spec.rb
@@ -38,6 +38,19 @@ describe NotificationsController do
       expect(user.reload.total_unread_notifications).to eq(1)
     end
 
+    it "can update a single notification" do
+      notification = Fabricate(:notification, user: user)
+      notification2 = Fabricate(:notification, user: user)
+      xhr :put, :mark_read, id: notification.id
+      expect(response).to be_success
+
+      notification.reload
+      notification2.reload
+
+      expect(notification.read).to eq(true)
+      expect(notification2.read).to eq(false)
+    end
+
     it "updates the `read` status" do
       notification = Fabricate(:notification, user: user)
       expect(user.reload.unread_notifications).to eq(1)