FIX: Avoid marking notifications as seen in readonly mode.

This commit is contained in:
Guo Xiang Tan 2020-06-25 10:14:07 +08:00 committed by Robin Ward
parent 1b17482eab
commit f10f87cc68
2 changed files with 17 additions and 8 deletions

View File

@ -25,11 +25,12 @@ class NotificationsController < ApplicationController
notifications = Notification.recent_report(current_user, limit) notifications = Notification.recent_report(current_user, limit)
changed = false changed = false
if notifications.present? if notifications.present? && !(params.has_key?(:slient) || @readonly_mode)
# ordering can be off due to PMs # ordering can be off due to PMs
max_id = notifications.map(&:id).max max_id = notifications.map(&:id).max
changed = current_user.saw_notification_id(max_id) unless params.has_key?(:silent) changed = current_user.saw_notification_id(max_id)
end end
user.reload user.reload
user.publish_notifications_state if changed user.publish_notifications_state if changed

View File

@ -33,7 +33,8 @@ end
describe NotificationsController do describe NotificationsController do
context 'when logged in' do context 'when logged in' do
context 'as normal user' do context 'as normal user' do
let!(:user) { sign_in(Fabricate(:user)) } fab!(:user) { sign_in(Fabricate(:user)) }
fab!(:notification) { Fabricate(:notification, user: user) }
describe '#index' do describe '#index' do
it 'should succeed for recent' do it 'should succeed for recent' do
@ -47,7 +48,6 @@ describe NotificationsController do
end end
it 'should mark notifications as viewed' do it 'should mark notifications as viewed' do
Fabricate(:notification, user: user)
expect(user.reload.unread_notifications).to eq(1) expect(user.reload.unread_notifications).to eq(1)
expect(user.reload.total_unread_notifications).to eq(1) expect(user.reload.total_unread_notifications).to eq(1)
get "/notifications.json", params: { recent: true } get "/notifications.json", params: { recent: true }
@ -56,7 +56,6 @@ describe NotificationsController do
end end
it 'should not mark notifications as viewed if silent param is present' do it 'should not mark notifications as viewed if silent param is present' do
Fabricate(:notification, user: user)
expect(user.reload.unread_notifications).to eq(1) expect(user.reload.unread_notifications).to eq(1)
expect(user.reload.total_unread_notifications).to eq(1) expect(user.reload.total_unread_notifications).to eq(1)
get "/notifications", params: { recent: true, silent: true } get "/notifications", params: { recent: true, silent: true }
@ -64,6 +63,17 @@ describe NotificationsController do
expect(user.reload.total_unread_notifications).to eq(1) expect(user.reload.total_unread_notifications).to eq(1)
end end
it 'should not mark notifications as viewed in readonly mode' do
Discourse.received_redis_readonly!
expect(user.reload.unread_notifications).to eq(1)
expect(user.reload.total_unread_notifications).to eq(1)
get "/notifications", params: { recent: true, silent: true }
expect(user.reload.unread_notifications).to eq(1)
expect(user.reload.total_unread_notifications).to eq(1)
ensure
Discourse.clear_redis_readonly!
end
context 'when username params is not valid' do context 'when username params is not valid' do
it 'should raise the right error' do it 'should raise the right error' do
get "/notifications.json", params: { username: 'somedude' } get "/notifications.json", params: { username: 'somedude' }
@ -78,7 +88,6 @@ describe NotificationsController do
end end
it "can update a single notification" do it "can update a single notification" do
notification = Fabricate(:notification, user: user)
notification2 = Fabricate(:notification, user: user) notification2 = Fabricate(:notification, user: user)
put "/notifications/mark-read.json", params: { id: notification.id } put "/notifications/mark-read.json", params: { id: notification.id }
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -91,7 +100,6 @@ describe NotificationsController do
end end
it "updates the `read` status" do it "updates the `read` status" do
Fabricate(:notification, user: user)
expect(user.reload.unread_notifications).to eq(1) expect(user.reload.unread_notifications).to eq(1)
expect(user.reload.total_unread_notifications).to eq(1) expect(user.reload.total_unread_notifications).to eq(1)
put "/notifications/mark-read.json" put "/notifications/mark-read.json"
@ -120,7 +128,7 @@ describe NotificationsController do
end end
context 'as admin' do context 'as admin' do
let!(:admin) { sign_in(Fabricate(:admin)) } fab!(:admin) { sign_in(Fabricate(:admin)) }
describe '#create' do describe '#create' do
it "can create notification" do it "can create notification" do