Merge pull request #9387 from dmarcoux/notifications

Mark notification as `unread`

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

@@ -4,23 +4,23 @@ class Webui::Users::NotificationsController < Webui::WebuiController
   def index
     notification_type = params[:type]
     case notification_type
-    when 'done'
+    when 'read'
       @notifications = Notification.with_notifiable.where(delivered: true)
                                    .for_subscribed_user(User.session)
     when 'reviews'
-      @notifications = Notification.with_notifiable.not_marked_as_done
+      @notifications = Notification.with_notifiable.unread
                                    .where(notifiable_type: 'Review')
                                    .for_subscribed_user(User.session)
     when 'comments'
-      @notifications = Notification.with_notifiable.not_marked_as_done
+      @notifications = Notification.with_notifiable.unread
                                    .where(notifiable_type: 'Comment')
                                    .for_subscribed_user(User.session)
     when 'requests'
-      @notifications = Notification.with_notifiable.not_marked_as_done
+      @notifications = Notification.with_notifiable.unread
                                    .where(notifiable_type: 'BsRequest')
                                    .for_subscribed_user(User.session)
     else
-      @notifications = Notification.with_notifiable.not_marked_as_done
+      @notifications = Notification.with_notifiable.unread
                                    .for_subscribed_user(User.session)
     end
   end
@@ -29,10 +29,10 @@ def update
     notification = User.session.notifications.find(params[:id])
     authorize notification, policy_class: NotificationPolicy
 
-    if notification.update(delivered: true)
-      flash[:success] = 'Successfully marked the notification as done'
+    if notification.toggle(:delivered).save
+      flash[:success] = "Successfully marked the notification as #{notification.unread? ? 'unread' : 'read'}"
     else
-      flash[:error] = "Couldn't mark the notification as done"
+      flash[:error] = "Couldn't mark the notification as #{notification.unread? ? 'read' : 'unread'}"
     end
     redirect_back(fallback_location: root_path)
   end

src/api/app/models/notification.rb

@@ -9,7 +9,7 @@ class Notification < ApplicationRecord
     where("(subscriber_type = 'User' AND subscriber_id = ?) OR (subscriber_type = 'Group' AND subscriber_id IN (?))",
           user, user.groups.map(&:id))
   }
-  scope :not_marked_as_done, -> { where(delivered: false) }
+  scope :unread, -> { where(delivered: false) }
   default_scope -> { order(created_at: :desc) }
 
   serialize :event_payload, JSON
@@ -35,4 +35,8 @@ def any_user_in_group_active?
   def template_name
     event_type.gsub('Event::', '').underscore
   end
+
+  def unread?
+    !delivered?
+  end
 end

src/api/app/models/user.rb

@@ -812,7 +812,7 @@ def tasks
   end
 
   def unread_notifications
-    notifications.not_marked_as_done.size
+    notifications.unread.size
   end
 
   def watched_project_names

src/api/app/views/webui/users/notifications/index.html.haml

@@ -13,8 +13,8 @@
 
       .card-body.collapse#filters
         .row.list-group-flush
-          = user_notification_link('Inbox', 'inbox', filter: params[:type])
-          = user_notification_link('Done', 'done', filter: params[:type])
+          = user_notification_link('Unread', 'unread', filter: params[:type])
+          = user_notification_link('Read', 'read', filter: params[:type])
         .row.list-group-flush.mt-5
           %h5.ml-3 Filter
           = user_notification_link('Reviews', 'reviews', filter: params[:type])
@@ -43,8 +43,13 @@
                     %span.badge.badge-secondary= notification.notification_badge
                     %small.text-muted= time_ago_in_words(notification.created_at)
                   = link_to(n.title, notification.link_to_notification_target, class: 'text-word-break-all')
-                - unless params[:type] == 'done'
+                - unless params[:type] == 'read'
                   .actions.align-self-end.align-self-sm-start.pl-3.pt-3.pt-sm-0
                     = link_to(my_notification_path(id: notification),
-                                      method: :put, class: 'btn btn-sm btn-outline-success px-4', title: 'Mark as "Done"') do
+                                      method: :put, class: 'btn btn-sm btn-outline-success px-4', title: 'Mark as "Read"') do
                       %i.fas.fa-check
+                - unless notification.unread?
+                  .actions.align-self-end.align-self-sm-start.pl-3.pt-3.pt-sm-0
+                    = link_to(my_notification_path(id: notification),
+                                      method: :put, class: 'btn btn-sm btn-outline-success px-4', title: 'Mark as "Unread"') do
+                      %i.fas.fa-undo

src/api/spec/controllers/webui/users/notifications_controller_spec.rb

@@ -10,7 +10,7 @@
   let(:comment_for_project_notification) { create(:notification, :comment_for_project, subscriber: user) }
   let(:comment_for_package_notification) { create(:notification, :comment_for_package, subscriber: user) }
   let(:comment_for_request_notification) { create(:notification, :comment_for_request, subscriber: user) }
-  let(:done_notification) { create(:notification, :request_state_change, subscriber: user, delivered: true) }
+  let(:read_notification) { create(:notification, :request_state_change, subscriber: user, delivered: true) }
   let(:notifications_for_other_users) { create(:notification, :request_state_change, subscriber: other_user) }
 
   shared_examples 'returning success' do
@@ -50,13 +50,13 @@
       end
     end
 
-    context "when param type is 'done'" do
-      let(:params) { default_params.merge(type: 'done') }
+    context "when param type is 'read'" do
+      let(:params) { default_params.merge(type: 'read') }
 
       it_behaves_like 'returning success'
 
       it 'sets @notifications to all delivered notifications regardless of type' do
-        expect(assigns[:notifications]).to include(done_notification)
+        expect(assigns[:notifications]).to include(read_notification)
       end
     end
 
@@ -95,24 +95,45 @@
   end
 
   describe 'PUT #update' do
-    subject! do
-      put :update, params: { id: state_change_notification.id, user_login: user_to_log_in.login }
-    end
+    context 'when a user marks one of his unread notifications as read' do
+      subject! do
+        put :update, params: { id: state_change_notification.id, user_login: user_to_log_in.login }
+      end
 
-    context 'when the user updates its own notifications' do
       let(:user_to_log_in) { user }
 
       it 'redirects back' do
         expect(response).to redirect_to(root_path)
       end
 
       it 'flashes a success message' do
-        expect(flash[:success]).to eql('Successfully marked the notification as done')
+        expect(flash[:success]).to eql('Successfully marked the notification as read')
       end
 
       it 'sets the notification as delivered' do
         expect(state_change_notification.reload.delivered).to be true
       end
     end
+
+    context 'when a user marks one of his read notifications as unread' do
+      subject! do
+        put :update, params: { id: read_notification.id, user_login: user_to_log_in.login }
+      end
+
+      let(:read_notification) { create(:notification, :request_state_change, subscriber: user, delivered: true) }
+      let(:user_to_log_in) { user }
+
+      it 'redirects back' do
+        expect(response).to redirect_to(root_path)
+      end
+
+      it 'flashes a success message' do
+        expect(flash[:success]).to eql('Successfully marked the notification as unread')
+      end
+
+      it 'sets the notification as not delivered' do
+        expect(read_notification.reload.delivered).to be false
+      end
+    end
   end
 end