Changing status from unread to read system notification is not correct one [SCI-3077 and SCI-3075] (#1513)

* fix dropdown apperance

* Remove search and fix duplicate bug

* Fix specs for new method

app/assets/javascripts/navigation.js

@@ -113,6 +113,7 @@
     button
       .on('click', function() {
         noRecentText.hide();
+        $('.dropdown-system-notifications .system-notification').remove();
         $.ajax({
           url: button.attr('data-href'),
           type: 'GET',
@@ -120,12 +121,6 @@
           beforeSend: animateSpinner($('.system-notifications-dropdown-header'), true),
           success: function(data) {
             var ul = $('.dropdown-system-notifications');
-            // After closing system notification modal release system notifications dropdown
-            $('#manage-module-system-notification-modal').on('hidden.bs.modal', function() {
-              setTimeout(function() {
-                $('.dropdown.system-notifications')[0].dataset.closable = true;
-              }, 100);
-            });
             $('.system-notifications-dropdown-header')
               .nextAll('.system-notification')
               .remove();
@@ -136,7 +131,7 @@
               noRecentText.show();
             }
             bindSystemNotificationAjax();
-            SystemNotificationsMarkAsSeen('.dropdown-system-notifications');
+            SystemNotificationsMarkAsSeen();
           }
         });
         $('#count-system-notifications').hide();
@@ -146,15 +141,5 @@
 
   // init
   loadDropdownSystemNotifications();
-  $('.dropdown-system-notifications').scroll(function() {
-    SystemNotificationsMarkAsSeen('.dropdown-system-notifications');
-  });
   loadUnseenNotificationsNumber('system-notifications', '.fa-gift');
-  // Override dropdown menu closing action while system notification modal open
-  $('.dropdown.system-notifications').on('hide.bs.dropdown', function() {
-    if (this.dataset.closable === 'false') {
-      return false;
-    }
-    return true;
-  });
 })();

app/assets/javascripts/system_notifications/index.js

@@ -1,18 +1,9 @@
 'use strict';
 
 // update selected notiifcations
-function SystemNotificationsMarkAsSeen(container = window) {
-  var WindowSize = $(container).height();
-  var NotificationsToUpdate = [];
-  _.each($('.system-notification[data-new="1"]'), function(el) {
-    var NotificationTopPosition = el.getBoundingClientRect().top;
-    if (NotificationTopPosition > 0 && NotificationTopPosition < WindowSize) {
-      NotificationsToUpdate.push(el.dataset.systemNotificationId);
-      el.dataset.new = 0;
-    }
-  });
-  if (NotificationsToUpdate.length > 0) {
-    $.post('/system_notifications/mark_as_seen', { notifications: JSON.stringify(NotificationsToUpdate) });
+function SystemNotificationsMarkAsSeen() {
+  if ($('.system-notification[data-new="1"]').length > 0) {
+    $.post('/system_notifications/mark_as_seen');
   }
 }
 
@@ -27,13 +18,15 @@ function bindSystemNotificationAjax() {
 
   $('.modal-system-notification')
     .on('ajax:success', function(ev, data) {
-      var SystemNotification = $('.system-notification[data-system-notification-id=' + data.id + ']')[0];
+      var SystemNotification = $('.system-notification[data-system-notification-id=' + data.id + ']');
       SystemNotificationModalBody.html(data.modal_body);
       SystemNotificationModalTitle.text(data.modal_title);
-      $('.dropdown.system-notifications')[0].dataset.closable = false;
+      $('.dropdown.system-notifications').removeClass('open');
       // Open modal
       SystemNotificationModal.modal('show');
-      if (SystemNotification.dataset.unread === '1') {
+      if (SystemNotification[0].dataset.unread === '1') {
+        $.each(SystemNotification, (index, e) => { e.dataset.unread = '0'; });
+        SystemNotification.find('.status-icon').addClass('seen');
         $.post('/system_notifications/' + data.id + '/mark_as_read');
       }
     });
@@ -42,7 +35,7 @@ function bindSystemNotificationAjax() {
 function initSystemNotificationsButton() {
   $('.btn-more-notifications')
     .on('ajax:success', function(e, data) {
-      $(data.html).insertAfter($('.notifications-container .system-notification').last());
+      $(data.html).insertAfter($('.system-notifications-container .system-notification').last());
       bindSystemNotificationAjax();
       if (data.more_url) {
         $(this).attr('href', data.more_url);
@@ -52,9 +45,6 @@ function initSystemNotificationsButton() {
     });
 }
 
-$(window).scroll(function() {
-  SystemNotificationsMarkAsSeen();
-});
 initSystemNotificationsButton();
 SystemNotificationsMarkAsSeen();
 bindSystemNotificationAjax();

app/assets/stylesheets/system_notifications.scss

@@ -13,6 +13,16 @@
   position: relative;
   width: 100%;
 
+  &[data-unread="0"] {
+
+    .body-block {
+
+      .title {
+        font-weight: normal;
+      }
+    }
+  }
+
   &:hover {
     background: $color-concrete;
     opacity: .7;
@@ -55,6 +65,7 @@
     }
 
     .title {
+      font-weight: bold;
       line-height: 20px;
       margin: 0;
     }
@@ -71,9 +82,11 @@
 #system-notifications-index {
   display: inline-block;
   position: relative;
+  width: 100%;
 
   #search-bar-notifications {
     border-bottom: 1px solid $color-gainsboro;
+    display: none;
     margin: 10px 0 0;
     padding: 0 0 10px;
     width: 100%;

app/controllers/system_notifications_controller.rb

@@ -24,8 +24,7 @@ class SystemNotificationsController < ApplicationController
 
   # Update seen_at parameter for system notifications
   def mark_as_seen
-    notifications = JSON.parse(params[:notifications])
-    current_user.user_system_notifications.mark_as_seen(notifications)
+    current_user.user_system_notifications.mark_as_seen
     render json: { result: 'ok' }
   rescue StandardError
     render json: { result: 'failed' }

app/models/user_system_notification.rb

@@ -9,9 +9,8 @@ class UserSystemNotification < ApplicationRecord
 
   scope :unseen, -> { where(seen_at: nil) }
 
-  def self.mark_as_seen(notifications_id)
-    where(system_notification_id: notifications_id)
-      .update_all(seen_at: Time.now)
+  def self.mark_as_seen
+    unseen.update_all(seen_at: Time.now)
   end
 
   def self.mark_as_read(notification_id)

app/views/system_notifications/_notification.html.erb

@@ -1,12 +1,12 @@
-<%= link_to system_notification_path(notification.id, format: :json), class: "modal-system-notification", remote: true do %>
-  <div
-    class="system-notification"
-    data-new="<%= notification.seen_at ? 0 : 1 %>"
-    data-unread="<%= notification.read_at ? 0 : 1 %>"
-    data-system-notification-id="<%= notification.id %>"
-  >
+<%= link_to system_notification_path(notification.id, format: :json), {
+    class: "modal-system-notification system-notification",
+    'data-new': notification.seen_at ? 0 : 1,
+    'data-unread': notification.read_at ? 0 : 1,
+    'data-system-notification-id': notification.id,
+     remote: true
+   } do %>
   <div class="status-block">
-      <div class="status-icon <%= notification.seen_at ? "seen" : "" %>">
+    <div class="status-icon <%= notification.read_at ? "seen" : "" %>">
       <i class="fas fa-gift"></i>
     </div>
   </div>
@@ -15,11 +15,10 @@
       <span><%= l(notification.last_time_changed_at, format: :full) %></span>
     </div>
     <h5 class="title">
-        <strong><%= notification.title %></strong>
+      <%= notification.title %>
     </h5>
     <div class="message">
       <span><%= notification.description %></span>
     </div>
   </div>
-  </div>
 <% end %>

app/views/system_notifications/index.html.erb

@@ -17,7 +17,7 @@
     </div>
   </div>
   <% end %>
-  <div class="notifications-container">
+  <div class="system-notifications-container">
     <%= render partial: "list", locals: { notifications: @system_notifications[:notifications] } %>
   </div>
     <div class="text-center">

config/locales/en.yml

@@ -1297,10 +1297,10 @@ en:
       intro_paragraph: "Hi %{user_name}, you've received What's new notification in SciNote:"
     index:
       whats_new: "What's New"
-      more_notifications: "More system notifications"
-      no_notifications: "No more system notifications"
+      more_notifications: "Show more notifications"
+      no_notifications: "No more notifications"
       settings: "Settings"
-      see_all: "See all"
+      see_all: "show all"
   activities:
     index:
       today: "Today"

spec/controllers/system_notifications_controller_spec.rb

@@ -41,14 +41,6 @@ describe SystemNotificationsController, type: :controller do
       expect(user.user_system_notifications.where(seen_at: nil).count).to eq 0
     end
 
-    it '#mark_as_seen response failed on wrong id\'s format' do
-      params = { notifications: 'wrong format' }
-      get :mark_as_seen, format: :json, params: params
-      expect(response).to have_http_status(:ok)
-      body = JSON.parse(response.body)
-      expect(body['result']).to eq 'failed'
-    end
-
     it '#mark_as_read correctly set read_at' do
       params = {
         id: user.user_system_notifications.first.system_notification_id

spec/models/user_system_notification_spec.rb

@@ -55,7 +55,7 @@ describe UserSystemNotification do
                    user: user,
                    system_notification: notifcation_one
       notifications_to_update = [usn.system_notification_id]
-      user.user_system_notifications.mark_as_seen(notifications_to_update)
+      user.user_system_notifications.mark_as_seen
       expect(UserSystemNotification.find(usn.id).seen_at).not_to be_nil
     end