diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f7565912e20..892e927c0f6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -56,7 +56,7 @@ Metrics/BlockNesting: # Offense count: 23 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 342 + Max: 343 # Offense count: 72 # Configuration parameters: AllowedMethods, AllowedPatterns. diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 4962f8a8c70..eca83efa855 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -41,6 +41,7 @@ def initialize(user) can :update, :account_terms can :create, :account_pd_declaration can :read, :dashboard + can :index, :notification can [:read, :update], [:preferences, :profile] can [:create, :subscribe, :unsubscribe], DiaryEntry can :update, DiaryEntry, :user => user diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb new file mode 100644 index 00000000000..b056b8d28f6 --- /dev/null +++ b/app/controllers/notifications_controller.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class NotificationsController < ApplicationController + layout :site_layout + + before_action :authorize_web + before_action :set_locale + + authorize_resource :class => false + + before_action :check_database_readable + + def index + @notifications = UserNotifications.new(current_user).visible + end +end diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index f27772892b0..a8be3a7f0b1 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -6,13 +6,15 @@ module UserHelper def user_image(user, options = {}) options[:class] ||= "user_image border border-secondary-subtle bg-body" options[:alt] ||= "" + width = options.fetch(:width, 100) + height = options.fetch(:height, 100) if user.image_use_gravatar user_gravatar_tag(user, options) elsif user.avatar.attached? - user_avatar_variant_tag(user, { :resize_to_limit => [100, 100] }, options) + user_avatar_variant_tag(user, { :resize_to_limit => [width, height] }, options) else - image_tag "avatar.svg", options.merge(:width => 100, :height => 100) + image_tag "avatar.svg", options.merge(:width => width, :height => height) end end diff --git a/app/models/user.rb b/app/models/user.rb index ceee49b5561..15f7f9fa60f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -91,6 +91,8 @@ class User < ApplicationRecord has_many :reports + has_many :notifications, :as => :recipient, :class_name => "Noticed::Notification" + has_many :social_links accepts_nested_attributes_for :social_links, :allow_destroy => true diff --git a/app/models/user_notifications.rb b/app/models/user_notifications.rb new file mode 100644 index 00000000000..902b1827544 --- /dev/null +++ b/app/models/user_notifications.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +class UserNotifications + class Notification + def self.from(notification) + partial_classname = notification.class.name.sub("Notifier::Notification", "") + klass = "UserNotifications::#{partial_classname}Notification".constantize + klass.new(notification) + end + + def initialize(notification) + @notification = notification + end + + def visible? + true + end + + def to_partial_path + partial_basename = @notification.class.name.sub("Notifier::Notification", "").underscore + "notifications/#{partial_basename}" + end + + def timestamp + record.created_at + end + + def record + @notification.record + end + end + + class ChangesetCommentNotification < Notification + delegate :changeset, :to => :record + delegate :visible?, :to => :record + + def commenter + record.author + end + + def comment_id + record.id + end + + def comment_body + record.body + end + end + + class GpxImportFailureNotification < Notification + def trace_filename + @notification.params[:trace_name] + end + + def trace_description + @notification.params[:trace_description] + end + + def trace_tags + @notification.params[:trace_tags] + end + + def trace_possible_points + nil + end + + def trace_points + nil + end + end + + class GpxImportSuccessNotification < Notification + def trace_filename + record.file.name + end + + def trace_description + record.description + end + + def trace_tags + record.tags.map(&:tag) + end + + def trace_possible_points + @notification.params[:possible_points] + end + + def trace_points + record.size + end + + def user + record.user + end + end + + class NewFollowerNotification < Notification + delegate :follower, :to => :record + end + + include Enumerable + + LISTABLE_NOTIFICATIONS = %w[ + ChangesetCommentNotifier::Notification + GpxImportFailureNotifier::Notification + GpxImportSuccessNotifier::Notification + NewFollowerNotifier::Notification + ].freeze + + def initialize(user) + @user = user + end + + def visible + select(&:visible?) + end + + def each(&) + @user + .notifications + .where(:type => LISTABLE_NOTIFICATIONS) + .newest_first + .map { |instance| Notification.from(instance) } + .each(&) + end +end diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index 3d16fbb85ed..ad0d27934f9 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -65,6 +65,7 @@ <%= number_with_delimiter(current_user.new_messages.size) %> <% end %> <%= link_to t("users.show.my profile"), current_user, :class => "dropdown-item" %> + <%= link_to t("users.show.my_notifications"), notifications_path, :class => "dropdown-item" %> <%= link_to t("users.show.my_account"), account_path, :class => "dropdown-item" %> <%= link_to t("users.show.my_preferences"), basic_preferences_path, :class => "dropdown-item" %> diff --git a/app/views/notifications/_changeset_comment.html.erb b/app/views/notifications/_changeset_comment.html.erb new file mode 100644 index 00000000000..2182672480d --- /dev/null +++ b/app/views/notifications/_changeset_comment.html.erb @@ -0,0 +1,47 @@ +<%# locals: (notification:) %> + +
+
+

+ <%= link_to( + notification.commenter.display_name, + notification.commenter + ) %> + left a comment + <%= friendly_date_ago(notification.timestamp) %> + on a changeset you are watching, created by + <%= link_to( + notification.changeset.user.display_name, + notification.changeset.user + ) %> + with comment + <%= notification.changeset.comment %> +

+
+
+ <%= link_to( + user_image(notification.commenter, :width => 50, :height => 50, :class => "img-fluid"), + user_url(notification.commenter), + :target => "_blank", :rel => "noopener" + ) %> +
+
+ <%= notification.comment_body.to_html %> +
+
+
+ +

+ <%= + link_to( + t(".view_changeset_comment"), + changeset_path( + notification.changeset, + :anchor => "c#{notification.comment_id}" + ) + ) + %> +

+
+ +
diff --git a/app/views/notifications/_gpx_import_failure.html.erb b/app/views/notifications/_gpx_import_failure.html.erb new file mode 100644 index 00000000000..a5aa31b6770 --- /dev/null +++ b/app/views/notifications/_gpx_import_failure.html.erb @@ -0,0 +1,44 @@ +<%# locals: (notification:) %> + +
+
+

+ It looks like your file failed to be imported as a GPS trace. +

+
+
+ <%# This space intentionally left blank, as there is no other user %> +
+
+
+
Filename
+
<%= notification.trace_filename %>
+ +
Description
+
<%= notification.trace_description %>
+ + <% if notification.trace_tags.length.positive? %> +
Tags
+
<%= notification.trace_tags.join(", ") %>
+ <% end %> + + <% if notification.trace_possible_points %> +
Possible points
+
<%= notification.trace_possible_points %>
+ <% end %> + + <% if notification.trace_points %> +
Imported points
+
<%= notification.trace_points %>
+ <% end %> +
+
+
+
+ +

+ <%# This space intentionally left blank, as there is no obvious action %> +

+
+ +
diff --git a/app/views/notifications/_gpx_import_success.html.erb b/app/views/notifications/_gpx_import_success.html.erb new file mode 100644 index 00000000000..ac01c28a012 --- /dev/null +++ b/app/views/notifications/_gpx_import_success.html.erb @@ -0,0 +1,44 @@ +<%# locals: (notification:) %> + +
+
+

+ It looks like your file was imported successfully as a GPS trace. +

+
+
+ <%# This space intentionally left blank, as there is no other user %> +
+
+
+
Filename
+
<%= notification.trace_filename %>
+ +
Description
+
<%= notification.trace_description %>
+ + <% if notification.trace_tags.length.positive? %> +
Tags
+
<%= notification.trace_tags.join(", ") %>
+ <% end %> + + <% if notification.trace_possible_points %> +
Possible points
+
<%= notification.trace_possible_points %>
+ <% end %> + + <% if notification.trace_points %> +
Imported points
+
<%= notification.trace_points %>
+ <% end %> +
+
+
+
+ +

+ <%= link_to "View", show_trace_path(notification.user, notification.record) %> +

+
+ +
diff --git a/app/views/notifications/_new_follower.html.erb b/app/views/notifications/_new_follower.html.erb new file mode 100644 index 00000000000..80efffac892 --- /dev/null +++ b/app/views/notifications/_new_follower.html.erb @@ -0,0 +1,32 @@ +<%# locals: (notification:) %> + +
+
+

+ <%= link_to( + notification.follower.display_name, + notification.follower + ) %> + started following you + <%= friendly_date_ago(notification.timestamp) %> +

+
+
+ <%= link_to( + user_image(notification.follower, :width => 50, :height => 50, :class => "img-fluid"), + user_url(notification.follower), + :target => "_blank", :rel => "noopener" + ) %> +
+
+

You can <%= link_to "follow them back", follow_url(notification.follower) %> if you wish.

+
+
+
+ +

+ <%# This space intentionally left blank, as there is no obvious action %> +

+
+ +
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb new file mode 100644 index 00000000000..6c25c8e8042 --- /dev/null +++ b/app/views/notifications/index.html.erb @@ -0,0 +1,12 @@ +<% content_for :heading do %> +

<%= t ".title" %>

+<% end %> + +<% if @notifications.empty? %> +

You have no notifications at the moment.

+<% end %> +
+ <% @notifications.each do |notification| %> + <%= render :partial => notification.to_partial_path, :object => notification, :as => :notification %> + <% end %> +
diff --git a/config/locales/en.yml b/config/locales/en.yml index 050e9c9b2a9..5188384518f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -601,6 +601,13 @@ en: followed_diaries: "diary entries" nearby_changesets: "nearby user changesets" nearby_diaries: "nearby user diary entries" + notifications: + index: + title: Notifications + changeset_comment: + notification_header_html: "%{changeset_comment_link} from %{commenter_name_link} %{time_ago}" + changeset_comment: "Changeset comment" + view_changeset_comment: View diary_entries: new: title: New Diary Entry @@ -3217,6 +3224,7 @@ en: my traces: My Traces my notes: My Notes my messages: My Messages + my_notifications: My Notifications my profile: My Profile my_account: My Account my comments: My Comments diff --git a/config/routes.rb b/config/routes.rb index e63bed58e68..4f62110b700 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -345,6 +345,8 @@ get "/preferences", :to => redirect(:path => "/preferences/basic"), :as => nil get "/preferences/edit", :to => redirect(:path => "/preferences/basic"), :as => nil + resources :notifications, :only => [:index] + # friendships scope "/user/:display_name" do resource :follow, :only => [:create, :destroy, :show], :path => "follow" diff --git a/test/controllers/notifications/changeset_comment_view_test.rb b/test/controllers/notifications/changeset_comment_view_test.rb new file mode 100644 index 00000000000..795f66b6dba --- /dev/null +++ b/test/controllers/notifications/changeset_comment_view_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "test_helper" + +class Notifications + class ChangesetCommentViewTest < ActionView::TestCase + def test_basic + comment_author = build_stubbed( + :user, + :display_name => "Helpful Commenter" + ) + changeset = build_stubbed(:changeset) + changeset_comment = build_stubbed( + :changeset_comment, + :author => comment_author, + :changeset => changeset + ) + notification = Struct.new(:record).new(changeset_comment) + notification_wrapper = UserNotifications::ChangesetCommentNotification.new(notification) + + render "notifications/changeset_comment", :notification => notification_wrapper + + assert_dom ".user-notification p", "Changeset comment from Helpful Commenter less than 1 minute ago" + assert_dom ".user-notification blockquote", changeset.comment + end + end +end diff --git a/test/controllers/notifications_controller_test.rb b/test/controllers/notifications_controller_test.rb new file mode 100644 index 00000000000..7a74465b11f --- /dev/null +++ b/test/controllers/notifications_controller_test.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "test_helper" + +class NotificationsControllerTest < ActionDispatch::IntegrationTest + def test_index + session_for(create(:user)) + get notifications_path + + assert_response :success + assert_template "index" + end + + def test_index_unauthorized + get notifications_path + + assert_redirected_to login_path(:referer => notifications_path) + end +end diff --git a/test/factories/notifications.rb b/test/factories/notifications.rb new file mode 100644 index 00000000000..58410206166 --- /dev/null +++ b/test/factories/notifications.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :changeset_comment_notification, :class => "ChangesetCommentNotifier::Notification" do + event :factory => :changeset_comment_notifier + recipient :factory => :user + end + + factory :changeset_comment_notifier, :class => "ChangesetCommentNotifier" do + record :factory => :changeset_comment + end +end diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb index 6734db45deb..6b6b9a8f6d0 100644 --- a/test/mailers/previews/user_mailer_preview.rb +++ b/test/mailers/previews/user_mailer_preview.rb @@ -87,7 +87,21 @@ def note_comment_notification def changeset_comment_notification recipient = create(:user, :languages => [I18n.locale]) - comment = create(:changeset_comment) + + changeset_description_text = <<~TEXT + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Praesent vel vehicula leo. In felis metus, faucibus quis iaculis at, pulvinar ac lacus.Morbi mollis mattis lacus, eu ultricies libero fermentum eget. + TEXT + changeset_description_tag = create(:changeset_tag, :k => "comment", :v => changeset_description_text) + + comment_text = <<~TEXT + Sed ut est laoreet leo blandit vestibulum ut in lorem. Pellentesque ut turpis pellentesque, tincidunt ipsum non, iaculis quam. Maecenas varius, lorem et maximus bibendum, lacus urna pharetra arcu, eget vestibulum sapien massa eget augue. + + Aenean maximus mollis diam, sit amet sodales ipsum ultrices sed. Duis quis sapien mattis, commodo eros eget, condimentum sapien. Donec cursus risus id diam facilisis venenatis. Duis hendrerit eget massa non dictum. Vivamus sed purus sit amet neque laoreet gravida. Integer mi mauris, dictum rutrum lorem at, euismod placerat eros. Duis lorem odio, porta vitae vestibulum ut, faucibus eget quam. Proin feugiat dui vel lacus tristique rutrum. + + Nulla eu tellus in nunc vehicula vehicula at at erat. + TEXT + comment = create(:changeset_comment, :changeset => changeset_description_tag.changeset, :body => comment_text) + UserMailer.with(:record => comment, :recipient => recipient).changeset_comment_notification end end diff --git a/test/models/user_notifications_test.rb b/test/models/user_notifications_test.rb new file mode 100644 index 00000000000..853e0b500e7 --- /dev/null +++ b/test/models/user_notifications_test.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require "test_helper" + +class UserNotificationsTest < ActiveSupport::TestCase + def test_visible_skips_hidden_changeset_comments + create(:language, :code => "en") + changeset_author = create(:user) + changeset = create(:changeset, :user => changeset_author) + comments = create_list(:changeset_comment, 3, :changeset => changeset) + comments.each do |comment| + event = create(:changeset_comment_notifier, :record => comment) + create(:changeset_comment_notification, :event => event, :recipient => changeset_author) + end + + assert_equal 3, UserNotifications.new(changeset_author).visible.count + + comments.first.update(:visible => false) + assert_equal 2, UserNotifications.new(changeset_author).visible.count + end +end diff --git a/test/system/onsite_notifications_test.rb b/test/system/onsite_notifications_test.rb new file mode 100644 index 00000000000..351a9f59dc0 --- /dev/null +++ b/test/system/onsite_notifications_test.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "application_system_test_case" + +class OnsiteNotificationsTest < ApplicationSystemTestCase + test "read latest notifications" do + changeset_author = create(:user) + commenter = create(:user, :display_name => "Commenter") + setup_changeset_comment( + :changeset_author => changeset_author, + :commenter => commenter + ) + + sign_in_as(changeset_author) + + click_on changeset_author.display_name + click_on "My Notifications" + + assert_text "Notifications" + assert_text "Changeset comment from Commenter" + end + + private + + def setup_changeset_comment(changeset_author:, commenter:) + changeset = create(:changeset, :user => changeset_author) + create(:changeset_subscription, :changeset => changeset, :subscriber => changeset_author) + + comment = create(:changeset_comment, :changeset => changeset, :author => commenter) + create(:changeset_subscription, :changeset => changeset, :subscriber => commenter) + ChangesetCommentNotifier.with(:record => comment).deliver + end +end