diff --git a/app/assets/javascripts/initializers/initNotifications.js b/app/assets/javascripts/initializers/initNotifications.js index 61c9f2750..7b65387dd 100644 --- a/app/assets/javascripts/initializers/initNotifications.js +++ b/app/assets/javascripts/initializers/initNotifications.js @@ -4,28 +4,35 @@ function initNotifications() { initReactions(); listenForNotificationsBellClick(); initPagination(); + initLoadMoreButton(); } function markNotificationsAsRead() { - setTimeout(function () { + setTimeout(function() { if (document.getElementById('notifications-container')) { var xmlhttp; - var locationAsArray = window.location.pathname.split("/"); + var locationAsArray = window.location.pathname.split('/'); // Use regex to ensure only numbers in the original string are converted to integers - var parsedLastParam = parseInt(locationAsArray[locationAsArray.length - 1].replace(/[^0-9]/g, ''), 10); + var parsedLastParam = parseInt( + locationAsArray[locationAsArray.length - 1].replace(/[^0-9]/g, ''), + 10, + ); if (window.XMLHttpRequest) { xmlhttp = new XMLHttpRequest(); } else { xmlhttp = new ActiveXObject('Microsoft.XMLHTTP'); } - xmlhttp.onreadystatechange = function () { - }; + xmlhttp.onreadystatechange = function() {}; var csrfToken = document.querySelector("meta[name='csrf-token']").content; - if(Number.isInteger(parsedLastParam)) { - xmlhttp.open('Post', '/notifications/reads?org_id=' + parsedLastParam, true); + if (Number.isInteger(parsedLastParam)) { + xmlhttp.open( + 'Post', + '/notifications/reads?org_id=' + parsedLastParam, + true, + ); } else { xmlhttp.open('Post', '/notifications/reads', true); } @@ -36,29 +43,42 @@ function markNotificationsAsRead() { } function fetchNotificationsCount() { - if (document.getElementById('notifications-container') == null && checkUserLoggedIn()) { + if ( + document.getElementById('notifications-container') == null && + checkUserLoggedIn() + ) { var xmlhttp; if (window.XMLHttpRequest) { xmlhttp = new XMLHttpRequest(); } else { xmlhttp = new ActiveXObject('Microsoft.XMLHTTP'); } - xmlhttp.onreadystatechange = function () { + xmlhttp.onreadystatechange = function() { if (xmlhttp.readyState == XMLHttpRequest.DONE) { var count = xmlhttp.response; if (isNaN(count)) { - document.getElementById('notifications-number').classList.remove('showing'); - } else if (count != '0' && count != undefined && count != "") { - document.getElementById('notifications-number').innerHTML = xmlhttp.response; - document.getElementById('notifications-number').classList.add('showing'); - if(instantClick){ - InstantClick.removeExpiredKeys("force"); - setTimeout(function(){ - InstantClick.preload(document.getElementById("notifications-link").href, "force"); - },30) + document + .getElementById('notifications-number') + .classList.remove('showing'); + } else if (count != '0' && count != undefined && count != '') { + document.getElementById('notifications-number').innerHTML = + xmlhttp.response; + document + .getElementById('notifications-number') + .classList.add('showing'); + if (instantClick) { + InstantClick.removeExpiredKeys('force'); + setTimeout(function() { + InstantClick.preload( + document.getElementById('notifications-link').href, + 'force', + ); + }, 30); } } else { - document.getElementById('notifications-number').classList.remove('showing'); + document + .getElementById('notifications-number') + .classList.remove('showing'); } } }; @@ -69,12 +89,12 @@ function fetchNotificationsCount() { } function initReactions() { - setTimeout(function () { + setTimeout(function() { if (document.getElementById('notifications-container')) { var butts = document.getElementsByClassName('reaction-button'); for (var i = 0; i < butts.length; i++) { var butt = butts[i]; - butt.onclick = function (event) { + butt.onclick = function(event) { event.preventDefault(); sendHapticMessage('medium'); var thisButt = this; @@ -95,7 +115,7 @@ function initReactions() { getCsrfToken() .then(sendFetch('reaction-creation', formData)) - .then(function (response) { + .then(function(response) { if (response.status === 200) { response.json().then(successCb); } @@ -105,13 +125,19 @@ function initReactions() { var butts = document.getElementsByClassName('toggle-reply-form'); for (var i = 0; i < butts.length; i++) { var butt = butts[i]; - butt.onclick = function (event) { + butt.onclick = function(event) { event.preventDefault(); var thisButt = this; - document.getElementById('comment-form-for-' + thisButt.dataset.reactableId).classList.add('showing'); + document + .getElementById('comment-form-for-' + thisButt.dataset.reactableId) + .classList.add('showing'); thisButt.innerHTML = ''; - setTimeout(function () { - document.getElementById('comment-textarea-for-' + thisButt.dataset.reactableId).focus(); + setTimeout(function() { + document + .getElementById( + 'comment-textarea-for-' + thisButt.dataset.reactableId, + ) + .focus(); }, 30); }; } @@ -120,26 +146,53 @@ function initReactions() { } function listenForNotificationsBellClick() { - setTimeout(function () { - document.getElementById('notifications-link').onclick = function () { - document.getElementById('notifications-number').classList.remove('showing'); + setTimeout(function() { + document.getElementById('notifications-link').onclick = function() { + document + .getElementById('notifications-number') + .classList.remove('showing'); }; }, 180); } function initPagination() { - var el = document.getElementById("notifications-pagination") - if (el) { - window.fetch(el.dataset.paginationPath, { - method: 'GET', - credentials: 'same-origin' - }).then(function (response) { - if (response.status === 200) { - response.text().then(function(html){ - el.innerHTML = html - initReactions(); + // 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 + const paginators = document.getElementsByClassName('notifications-paginator'); + if (paginators && paginators.length > 0) { + const paginator = paginators[paginators.length - 1]; + + if (paginator) { + window + .fetch(paginator.dataset.paginationPath, { + method: 'GET', + credentials: 'same-origin', + }) + .then(function(response) { + if (response.status === 200) { + response.text().then(function(html) { + const notificationsList = html.trim(); + + if (notificationsList) { + paginator.innerHTML = notificationsList; + initReactions(); + } else { + // no more notifications to load, we hide the load more wrapper + const button = document.getElementById('load-more-button'); + if (button) { + button.style.display = 'none'; + } + } + }); + } }); - } - }); + } + } +} + +function initLoadMoreButton() { + const button = document.getElementById('load-more-button'); + if (button) { + button.addEventListener('click', initPagination); } } diff --git a/app/assets/stylesheets/_mixins.scss b/app/assets/stylesheets/_mixins.scss index 019d469ce..badf661d2 100644 --- a/app/assets/stylesheets/_mixins.scss +++ b/app/assets/stylesheets/_mixins.scss @@ -25,3 +25,21 @@ #{$property}: #{$fallback} !important; #{$property}: var(--#{$cssVariable}, #{$fallback}) !important; } + + +/* Mixin for a load more wrapper, made by a container div and child button */ +@mixin load-more() { + text-align: center; + + button { + background: transparent; + @include themeable(border, theme-border, 1px solid $light-medium-gray); + font-size: 17px; + padding: 14px 5px; + margin: 40px auto 70px; + width: 320px; + max-width: 80%; + border-radius: 100px; + font-weight: bold; + } +} diff --git a/app/assets/stylesheets/item-list.scss b/app/assets/stylesheets/item-list.scss index 88ca4a1e6..a68eae750 100644 --- a/app/assets/stylesheets/item-list.scss +++ b/app/assets/stylesheets/item-list.scss @@ -198,19 +198,7 @@ } .load-more-wrapper { - text-align: center; - - button { - background: transparent; - @include themeable(border, theme-border, 1px solid $light-medium-gray); - font-size: 17px; - padding: 14px 5px; - margin: 40px auto 70px; - width: 320px; - max-width: 80%; - border-radius: 100px; - font-weight: bold; - } + @include load-more; } } } diff --git a/app/assets/stylesheets/notifications.scss b/app/assets/stylesheets/notifications.scss index 738ab3777..ae2c7ccb4 100644 --- a/app/assets/stylesheets/notifications.scss +++ b/app/assets/stylesheets/notifications.scss @@ -2,6 +2,7 @@ @import 'mixins'; .notifications-index { @include themeable(background, theme-background, $lightest-gray); + .home { .articles-list { .signup-cue { @@ -406,4 +407,8 @@ } } } + + .load-more-wrapper { + @include load-more; + } } diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index aa6e146df..555bfe674 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -5,12 +5,17 @@ class NotificationsController < ApplicationController @notifications_index = true @user = user_to_view - if params[:page] - num = 45 - notified_at_offset = Notification.find(params[:page])&.notified_at + + # 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 + if params[:offset] + num = 30 + notified_at_offset = Notification.find(params[:offset])&.notified_at else num = 8 end + @notifications = if (params[:org_id].present? || params[:filter] == "org") && allowed_user? organization_notifications elsif params[:org_id].blank? && params[:filter].present? @@ -18,11 +23,21 @@ class NotificationsController < ApplicationController else Notification.where(user_id: @user.id).order("notified_at DESC") end + + # 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 + + @notifications = NotificationDecorator.decorate_collection(@notifications.limit(num)) + @last_user_reaction = @user.reactions.last&.id @last_user_comment = @user.comments.last&.id - @notifications = @notifications.where("notified_at < ?", notified_at_offset) if notified_at_offset - @notifications = NotificationDecorator.decorate_collection(@notifications.limit(num)) + @organizations = @user.member_organizations if @user.organizations + + # The first call, the one coming from the browser URL bar will render the "index" view, which renders + # the first few notifications. After that the JS frontend code (see `initNotification.js`) + # will call this action again by sending the offset id for the last known notifications, the result + # will be the partial rendering of only the list of notifications that will be attached to the DOM by JS render partial: "notifications_list" if notified_at_offset end diff --git a/app/views/notifications/_comment.html.erb b/app/views/notifications/_comment.html.erb index fd25d7eb6..bf791e6da 100644 --- a/app/views/notifications/_comment.html.erb +++ b/app/views/notifications/_comment.html.erb @@ -25,9 +25,7 @@ <%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "default" %> <% elsif notification.action == "Moderation" %> - Hey there! <%= image_tag("emoji/apple-hugging-face.png", class: "reaction-image", alt: "Hugging face emoji") %>

">@<%= json_data["comment"]["path"].split("/")[1] %> just left a comment. Since they are new to the community, could you leave a nice reply to help them feel welcome? -

Thank you!

Alternatively, if this comment violates the code of conduct, please downvote/report as appropriate. @@ -38,7 +36,7 @@ <%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "moderation" %>
-
All negative reactions are 100% private. Thank you for being a trusted <%= ApplicationConfig["COMMUNITY_NAME"] %> member.
+
All negative reactions are 100% private.
<% elsif notification.action == "First" %> "><%= json_data["user"]["name"] %> wrote their first comment on: diff --git a/app/views/notifications/_notifications_list.html.erb b/app/views/notifications/_notifications_list.html.erb index 641f0df28..3ba382ebc 100644 --- a/app/views/notifications/_notifications_list.html.erb +++ b/app/views/notifications/_notifications_list.html.erb @@ -1,12 +1,25 @@ -<% notification_count = 0 %> -<% @notifications.each do |notification| %> +<% num_notifications = @notifications.size %> + +<% @notifications.each_with_index do |notification, index| %> <% next if notification.notified_at < 24.hours.ago && notification.aggregated? %> - <% notification_count += 1 %> -
" data-notification-id="<%= notification.id %>"> + +
" + data-notification-id="<%= notification.id %>"> <%= render notification.notifiable_type.downcase.to_s, notification: notification %>
+ + <%# Since pagination is offset based, the fastest way to retrieve the last known notification id is to %> + <%# ask Ruby when it is at the end of the loop %> + <% if index == num_notifications - 1 %> + <% sub_path = params[:org_id].present? ? "#{params[:filter]}/#{params[:org_id]}" : params[:filter].to_s %> +
+ <% end %> <% rescue => e %> - <% logger.error("Notifification error - #{e.message} - #{notification.id}") %> + <% logger.error("Notification error - #{e.message} - #{notification.id}") %> +
Sloan, the sloth mascot diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 40190df96..9519df54f 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -62,9 +62,16 @@
<% end %> <% end %> - <%= render "notifications_list" %> - <% if @notifications.any? %> -
?page=<%= @notifications.last.id %>">
+ + <%= render "notifications_list", params: params %> + + <%# not using "any?"" here because "notifications_list" already asks for ".size", a little optimization :) %> + <% if @notifications.size > 7 %> +
+ +
<% end %>
diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index ff7be318f..60ca205ad 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -391,5 +391,22 @@ RSpec.describe "NotificationsIndex", type: :request do expect(response.body).to include time_ago_in_words(article.published_at) end end + + context "when a user is an admin" do + let(:admin) { create(:user, :super_admin) } + let(:user2) { create(:user) } + let(:article) { create(:article, user_id: user.id) } + + before do + user2.follow(user) + Notification.send_to_followers_without_delay(article, "Published") + sign_in admin + end + + it "can view other people's notifications" do + get "/notifications?username=#{user2.username}" + expect(response.body).to include "made a new post:" + end + end end end