diff --git a/app/assets/javascripts/initializers/initNotifications.js b/app/assets/javascripts/initializers/initNotifications.js index 7b65387dd..ab7de6e1e 100644 --- a/app/assets/javascripts/initializers/initNotifications.js +++ b/app/assets/javascripts/initializers/initNotifications.js @@ -156,8 +156,7 @@ function listenForNotificationsBellClick() { } function initPagination() { - // paginators appear at the end of each block of HTML notifications sent by - // the server, each time we paginate we're only interested in the last one + // paginators appear after each block of HTML notifications sent by the server const paginators = document.getElementsByClassName('notifications-paginator'); if (paginators && paginators.length > 0) { const paginator = paginators[paginators.length - 1]; @@ -171,10 +170,17 @@ function initPagination() { .then(function(response) { if (response.status === 200) { response.text().then(function(html) { - const notificationsList = html.trim(); + const markup = html.trim(); + + if (markup) { + const container = document.getElementById('articles-list'); + + const newNotifications = document.createElement('div'); + newNotifications.innerHTML = markup; + + paginator.remove(); + container.append(newNotifications); - if (notificationsList) { - paginator.innerHTML = notificationsList; initReactions(); } else { // no more notifications to load, we hide the load more wrapper @@ -182,6 +188,7 @@ function initPagination() { if (button) { button.style.display = 'none'; } + paginator.remove(); } }); } diff --git a/app/assets/stylesheets/notifications.scss b/app/assets/stylesheets/notifications.scss index ae2c7ccb4..d4c7a18a0 100644 --- a/app/assets/stylesheets/notifications.scss +++ b/app/assets/stylesheets/notifications.scss @@ -410,5 +410,9 @@ .load-more-wrapper { @include load-more; + + button { + margin-top: 0; + } } } diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 555bfe674..f82deeefc 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -6,6 +6,8 @@ class NotificationsController < ApplicationController @notifications_index = true @user = user_to_view + @initial_page_size = 8 + # NOTE: this controller is using offset based pagination by assuming that # the id of the last notification also corresponds to the newest `notified_at` # this might not be forever true but it's good enough for now @@ -13,7 +15,7 @@ class NotificationsController < ApplicationController num = 30 notified_at_offset = Notification.find(params[:offset])&.notified_at else - num = 8 + num = @initial_page_size end @notifications = if (params[:org_id].present? || params[:filter] == "org") && allowed_user? @@ -21,9 +23,11 @@ class NotificationsController < ApplicationController elsif params[:org_id].blank? && params[:filter].present? filtered_notifications else - Notification.where(user_id: @user.id).order("notified_at DESC") + @user.notifications end + @notifications = @notifications.without_recently_aggregated_reactions_and_follows.order(notified_at: :desc) + # if offset based pagination is invoked by the frontend code, we filter out all earlier ones @notifications = @notifications.where("notified_at < ?", notified_at_offset) if notified_at_offset @@ -53,23 +57,19 @@ class NotificationsController < ApplicationController def filtered_notifications if params[:filter].to_s.casecmp("posts").zero? - Notification.where(user_id: @user.id, notifiable_type: "Article", action: "Published"). - order("notified_at DESC") + @user.notifications.for_published_articles elsif params[:filter].to_s.casecmp("comments").zero? - Notification.where(user_id: @user.id, notifiable_type: "Comment", action: nil). # Nil action means not reaction in this context - or(Notification.where(user_id: @user.id, notifiable_type: "Mention")). - order("notified_at DESC") + @user.notifications.for_comments.or(@user.notifications.for_mentions) end end def organization_notifications + org_id = params[:org_id] + if params[:filter].to_s.casecmp("comments").zero? - Notification.where(organization_id: params[:org_id], notifiable_type: "Comment", action: nil, user_id: nil). # Nil action means not reaction in this context - or(Notification.where(organization_id: params[:org_id], notifiable_type: "Mention", user_id: nil)). - order("notified_at DESC") + Notification.for_organization_comments(org_id).or(Notification.for_organization_mentions(org_id)) else - Notification.where(organization_id: params[:org_id], user_id: nil). - order("notified_at DESC") + Notification.for_organization(org_id) end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 052475a84..b0f2c5aae 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -3,12 +3,32 @@ class Notification < ApplicationRecord belongs_to :user, optional: true belongs_to :organization, optional: true - validates :user_id, presence: true, if: proc { |notifcation| notifcation.organization_id.nil? } - validates :organization_id, presence: true, if: proc { |notifcation| notifcation.user_id.nil? } + validates :user_id, presence: true, if: proc { |notification| notification.organization_id.nil? } + validates :organization_id, presence: true, if: proc { |notification| notification.user_id.nil? } + validates :user_id, uniqueness: { scope: %i[organization_id notifiable_id notifiable_type action] } before_create :mark_notified_at_time - validates :user_id, uniqueness: { scope: %i[organization_id notifiable_id notifiable_type action] } + scope :for_published_articles, -> { where(notifiable_type: "Article", action: "Published") } + scope :for_comments, -> { where(notifiable_type: "Comment", action: nil) } # nil action means "not a reaction" + scope :for_mentions, -> { where(notifiable_type: "Mention") } + + scope :for_organization, ->(org_id) { where(organization_id: org_id, user_id: nil) } + scope :for_organization_comments, lambda { |org_id| + # nil action means "not a reaction" + where(organization_id: org_id, notifiable_type: "Comment", action: nil, user_id: nil) + } + scope :for_organization_mentions, lambda { |org_id| + where(organization_id: org_id, notifiable_type: "Mention", user_id: nil) + } + + scope :with_recently_aggregated_reactions_and_follows, lambda { + where("notified_at < ?", 24.hours.ago). + where(action: %w[Reaction Follow]) + } + scope :without_recently_aggregated_reactions_and_follows, lambda { + where.not(with_recently_aggregated_reactions_and_follows.arel.exists) + } class << self def send_new_follower_notification(follow, is_read = false) diff --git a/app/views/notifications/_notifications_list.html.erb b/app/views/notifications/_notifications_list.html.erb index 3ba382ebc..7924d6196 100644 --- a/app/views/notifications/_notifications_list.html.erb +++ b/app/views/notifications/_notifications_list.html.erb @@ -1,22 +1,9 @@ -<% num_notifications = @notifications.size %> - -<% @notifications.each_with_index do |notification, index| %> - <% next if notification.notified_at < 24.hours.ago && notification.aggregated? %> - +<% @notifications.each do |notification| %>