From b0c196aea065c4cafcf550750d7c434b759e33ba Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 11 Nov 2023 03:17:42 +0100 Subject: [PATCH] Add notifications for likes on comments --- .rubocop.yml | 5 ++ app/controllers/notifications_controller.rb | 58 +++++++------------ app/helpers/notifications_helper.rb | 13 ++++- .../notification_mailers/liked_comment.rb | 16 +++++ app/models/notifications/liked_comment.rb | 26 +++++++++ app/models/user_preference.rb | 23 ++++---- app/services/notification_service.rb | 3 +- app/views/notifications/_notification.haml | 7 ++- .../notifications/_notification.mobile.haml | 7 ++- app/views/notifications/index.html.haml | 2 +- app/views/notifier/liked.html.haml | 4 +- app/views/notifier/liked.text.erb | 2 +- app/views/notifier/liked_comment.html.haml | 10 ++++ app/views/notifier/liked_comment.text.erb | 10 ++++ app/views/users/_edit.haml | 5 ++ app/workers/mail/liked.rb | 1 - app/workers/mail/liked_comment.rb | 8 +++ config/locales/diaspora/en.yml | 30 +++++----- .../step_definitions/notifications_steps.rb | 2 +- lib/schemas/api_v1.json | 1 + .../notifications_controller_spec.rb | 1 + spec/mailers/notifier_spec.rb | 57 ++++++++++++++++++ 22 files changed, 218 insertions(+), 73 deletions(-) create mode 100644 app/mailers/notification_mailers/liked_comment.rb create mode 100644 app/models/notifications/liked_comment.rb create mode 100644 app/views/notifier/liked_comment.html.haml create mode 100644 app/views/notifier/liked_comment.text.erb create mode 100644 app/workers/mail/liked_comment.rb diff --git a/.rubocop.yml b/.rubocop.yml index e3306ee3d..0f69beccf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -73,6 +73,11 @@ Layout/HashAlignment: EnforcedHashRocketStyle: table EnforcedColonStyle: table +# This rule makes haml files less readable, as there is no 'end' there. +Layout/CaseIndentation: + Exclude: + - "app/views/**/*" + # Mixing the styles looks just silly. Style/HashSyntax: EnforcedStyle: ruby19_no_mixed_keys diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 31fb8aa38..120d5dcd6 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -7,24 +7,9 @@ class NotificationsController < ApplicationController before_action :authenticate_user! - def update - note = Notification.where(:recipient_id => current_user.id, :id => params[:id]).first - if note - note.set_read_state(params[:set_unread] != "true" ) - - respond_to do |format| - format.json { render :json => { :guid => note.id, :unread => note.unread } } - end - - else - respond_to do |format| - format.json { render :json => {}.to_json } - end - end - end - def index - conditions = {:recipient_id => current_user.id} + conditions = {recipient_id: current_user.id} + types = NotificationService::NOTIFICATIONS_JSON_TYPES if params[:type] && types.has_key?(params[:type]) conditions[:type] = types[params[:type]] end @@ -33,7 +18,7 @@ class NotificationsController < ApplicationController per_page = params[:per_page] || 25 @notifications = WillPaginate::Collection.create(page, per_page, Notification.where(conditions).count ) do |pager| result = Notification.where(conditions) - .includes(:target, :actors => :profile) + .includes(:target, actors: :profile) .order("updated_at desc") .limit(pager.per_page) .offset(pager.offset) @@ -52,13 +37,28 @@ class NotificationsController < ApplicationController respond_to do |format| format.html - format.xml { render :xml => @notifications.to_xml } + format.xml { render xml: @notifications.to_xml } format.json { render json: render_as_json(@unread_notification_count, @grouped_unread_notification_counts, @notifications) } end end + def update + note = Notification.where(recipient_id: current_user.id, id: params[:id]).first + if note + note.set_read_state(params[:set_unread] != "true") + + respond_to do |format| + format.json { render json: {guid: note.id, unread: note.unread} } + end + else + respond_to do |format| + format.json { render json: {}.to_json } + end + end + end + def default_serializer_options { context: self, @@ -67,7 +67,7 @@ class NotificationsController < ApplicationController end def read_all - current_type = types[params[:type]] + current_type = NotificationService::NOTIFICATIONS_JSON_TYPES[params[:type]] notifications = Notification.where(recipient_id: current_user.id, unread: true) notifications = notifications.where(type: current_type) if params[:type] notifications.update_all(unread: false) @@ -79,8 +79,8 @@ class NotificationsController < ApplicationController format.html { redirect_to stream_path } format.mobile { redirect_to stream_path } end - format.xml { render :xml => {}.to_xml } - format.json { render :json => {}.to_json } + format.xml { render xml: {}.to_xml } + format.json { render json: {}.to_json } end end @@ -95,18 +95,4 @@ class NotificationsController < ApplicationController } }.as_json end - - def types - { - "also_commented" => "Notifications::AlsoCommented", - "comment_on_post" => "Notifications::CommentOnPost", - "liked" => "Notifications::Liked", - "mentioned" => "Notifications::MentionedInPost", - "mentioned_in_comment" => "Notifications::MentionedInComment", - "reshared" => "Notifications::Reshared", - "started_sharing" => "Notifications::StartedSharing", - "contacts_birthday" => "Notifications::ContactsBirthday" - } - end - helper_method :types end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 6972d811c..5252dd4fc 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -16,6 +16,8 @@ module NotificationsHelper elsif %w(Notifications::CommentOnPost Notifications::AlsoCommented Notifications::Reshared Notifications::Liked) .include?(note.type) opts.merge!(opts_for_post(note.linked_object)) + elsif note.is_a?(Notifications::LikedComment) + opts.merge!(opts_for_comment(note.linked_object)) elsif note.is_a?(Notifications::ContactsBirthday) opts.merge!(opts_for_birthday(note)) end @@ -33,7 +35,16 @@ module NotificationsHelper post_link: link_to(post_page_title(post), post_path(post), data: {ref: post.id}, - class: "hard_object_link").html_safe + class: "hard_object_link") + } + end + + def opts_for_comment(comment) + { + comment_link: link_to(comment.message.title, + post_path(comment.post, anchor: comment.guid), + data: {ref: comment.id}, + class: "hard_object_link") } end diff --git a/app/mailers/notification_mailers/liked_comment.rb b/app/mailers/notification_mailers/liked_comment.rb new file mode 100644 index 000000000..4da96c2ba --- /dev/null +++ b/app/mailers/notification_mailers/liked_comment.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module NotificationMailers + class LikedComment < NotificationMailers::Base + attr_accessor :like + + delegate :target, to: :like, prefix: true + + def set_headers(like_id) # rubocop:disable Naming/AccessorMethodName + @like = Like.find(like_id) + + @headers[:subject] = I18n.t("notifier.liked_comment.liked", name: @sender.name) + @headers[:in_reply_to] = @headers[:references] = "<#{@like.parent.commentable.guid}@#{AppConfig.pod_uri.host}>" + end + end +end diff --git a/app/models/notifications/liked_comment.rb b/app/models/notifications/liked_comment.rb new file mode 100644 index 000000000..478b6ad81 --- /dev/null +++ b/app/models/notifications/liked_comment.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Notifications + class LikedComment < Notification + def mail_job + Workers::Mail::LikedComment + end + + def popup_translation_key + "notifications.liked_comment" + end + + def deleted_translation_key + "notifications.liked_comment_deleted" + end + + def self.notify(like, _recipient_user_ids) + actor = like.author + target_author = like.target.author + + return unless like.target_type == "Comment" && target_author.local? && actor != target_author + + concatenate_or_create(target_author.owner, like.target, actor).email_the_user(like, actor) + end + end +end diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index 4f6b5f9aa..9f338dd76 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -6,16 +6,19 @@ class UserPreference < ApplicationRecord validate :must_be_valid_email_type VALID_EMAIL_TYPES = - ["someone_reported", - "mentioned", - "mentioned_in_comment", - "comment_on_post", - "private_message", - "started_sharing", - "also_commented", - "liked", - "reshared", - "contacts_birthday"] + %w[ + someone_reported + mentioned + mentioned_in_comment + comment_on_post + private_message + started_sharing + also_commented + liked + liked_comment + reshared + contacts_birthday + ].freeze def must_be_valid_email_type unless VALID_EMAIL_TYPES.include?(self.email_type) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 755ae7b5c..211590d79 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -3,7 +3,7 @@ class NotificationService NOTIFICATION_TYPES = { Comment => [Notifications::MentionedInComment, Notifications::CommentOnPost, Notifications::AlsoCommented], - Like => [Notifications::Liked], + Like => [Notifications::Liked, Notifications::LikedComment], StatusMessage => [Notifications::MentionedInPost], Conversation => [Notifications::PrivateMessage], Message => [Notifications::PrivateMessage], @@ -15,6 +15,7 @@ class NotificationService "also_commented" => "Notifications::AlsoCommented", "comment_on_post" => "Notifications::CommentOnPost", "liked" => "Notifications::Liked", + "liked_comment" => "Notifications::LikedComment", "mentioned" => "Notifications::MentionedInPost", "mentioned_in_comment" => "Notifications::MentionedInComment", "reshared" => "Notifications::Reshared", diff --git a/app/views/notifications/_notification.haml b/app/views/notifications/_notification.haml index cba9824c4..059cdb6fc 100644 --- a/app/views/notifications/_notification.haml +++ b/app/views/notifications/_notification.haml @@ -1,5 +1,8 @@ -.media.stream-element{data: {guid: note.id, type: (types.key(note.type) || "")}, - class: (note.unread ? "unread" : "read")} +.media.stream-element{data: { + guid: note.id, + type: (NotificationService::NOTIFICATIONS_JSON_TYPES.key(note.type) || "") + }, + class: (note.unread ? "unread" : "read")} .unread-toggle.pull-right %i.entypo-eye{title: (note.unread ? t("notifications.index.mark_read") : t("notifications.index.mark_unread"))} - if note.type == "Notifications::StartedSharing" && (!defined?(no_aspect_dropdown) || !no_aspect_dropdown) diff --git a/app/views/notifications/_notification.mobile.haml b/app/views/notifications/_notification.mobile.haml index b38977152..918379a3f 100644 --- a/app/views/notifications/_notification.mobile.haml +++ b/app/views/notifications/_notification.mobile.haml @@ -1,5 +1,8 @@ -.notification_element{data: {guid: note.id, type: (types.key(note.type) || "")}, - class: (note.unread ? "unread" : "read")} +.notification_element{data: { + guid: note.id, + type: (NotificationService::NOTIFICATIONS_JSON_TYPES.key(note.type) || "") + }, + class: (note.unread ? "unread" : "read")} .pull-right.unread-toggle %i.entypo-eye{title: (note.unread ? t("notifications.index.mark_read") : t("notifications.index.mark_unread"))} = person_image_tag note.actors.first, :thumb_small diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 283eca5a3..92b2ddbfe 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -21,7 +21,7 @@ - case key - when "also_commented", "comment_on_post" %i.entypo-comment - - when "liked" + - when "liked", "liked_comment" %i.entypo-heart - when "mentioned", "mentioned_in_comment" %span.mentionIcon diff --git a/app/views/notifier/liked.html.haml b/app/views/notifier/liked.html.haml index 96cf3770f..6aeda8848 100644 --- a/app/views/notifier/liked.html.haml +++ b/app/views/notifier/liked.html.haml @@ -1,10 +1,10 @@ - if @notification.like_target.public? %p - #{t('.liked', name: @notification.sender_name)}: + #{t(".liked", name: @notification.sender_name)}: = post_message(@notification.like_target, html: true) - else %p - #{t('notifier.liked.limited_post', name: @notification.sender_name)}. + #{t(".limited_post", name: @notification.sender_name)}. %p = link_to t(".view_post"), post_url(@notification.like_target) diff --git a/app/views/notifier/liked.text.erb b/app/views/notifier/liked.text.erb index ce743ade3..4036c2d2e 100644 --- a/app/views/notifier/liked.text.erb +++ b/app/views/notifier/liked.text.erb @@ -3,7 +3,7 @@ <%= post_message(@notification.like_target) %> <% else %> -<%= "#{t("notifier.liked.limited_post", name: @notification.sender_name)}." %> +<%= "#{t(".limited_post", name: @notification.sender_name)}." %> <% end %> <%= t(".view_post") %> diff --git a/app/views/notifier/liked_comment.html.haml b/app/views/notifier/liked_comment.html.haml new file mode 100644 index 000000000..04805d615 --- /dev/null +++ b/app/views/notifier/liked_comment.html.haml @@ -0,0 +1,10 @@ +- if @notification.like_target.public? + %p + #{t(".liked", name: @notification.sender_name)}: + = post_message(@notification.like_target, html: true) +- else + %p + #{t(".limited_post", name: @notification.sender_name)}. + +%p + = link_to t(".view_comment"), post_url(@notification.like_target.root, anchor: @notification.like_target.guid) diff --git a/app/views/notifier/liked_comment.text.erb b/app/views/notifier/liked_comment.text.erb new file mode 100644 index 000000000..f05118959 --- /dev/null +++ b/app/views/notifier/liked_comment.text.erb @@ -0,0 +1,10 @@ +<% if @notification.like_target.public? %> +<%= "#{t(".liked", name: @notification.sender_name)}:" %> + +<%= post_message(@notification.like_target) %> +<% else %> +<%= "#{t(".limited_post", name: @notification.sender_name)}." %> +<% end %> + +<%= t(".view_comment") %> +<%= post_url(@notification.like_target.root, anchor: @notification.like_target.guid) %> diff --git a/app/views/users/_edit.haml b/app/views/users/_edit.haml index e20bcff44..ce94d8a99 100644 --- a/app/views/users/_edit.haml +++ b/app/views/users/_edit.haml @@ -153,6 +153,11 @@ = t(".liked") .small-horizontal-spacer + = type.label :liked_comment, class: "checkbox-inline" do + = type.check_box :liked_comment, {checked: email_prefs["liked_comment"]}, false, true + = t(".liked_comment") + .small-horizontal-spacer + = type.label :reshared, class: "checkbox-inline" do = type.check_box :reshared, {checked: email_prefs["reshared"]}, false, true = t(".reshared") diff --git a/app/workers/mail/liked.rb b/app/workers/mail/liked.rb index 8d5ca0db2..4d1ca86e5 100644 --- a/app/workers/mail/liked.rb +++ b/app/workers/mail/liked.rb @@ -12,4 +12,3 @@ module Workers end end end - diff --git a/app/workers/mail/liked_comment.rb b/app/workers/mail/liked_comment.rb new file mode 100644 index 000000000..fa00a0f24 --- /dev/null +++ b/app/workers/mail/liked_comment.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Workers + module Mail + class LikedComment < Liked + end + end +end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index fea1bf946..9f719ef2f 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -632,22 +632,8 @@ en: likes: create: error: "Failed to like." - fail: "Like creation has failed" destroy: error: "Failed to unlike." - people_like_this: - zero: "No likes" - one: "%{count} like" - other: "%{count} likes" - people_like_this_comment: - zero: "No likes" - one: "%{count} like" - other: "%{count} likes" - people_dislike_this: - zero: "No dislikes" - one: "%{count} dislike" - other: "%{count} dislikes" - not_found: "Post or like not found" notifications: started_sharing: @@ -676,6 +662,10 @@ en: zero: "%{actors} have liked your post %{post_link}." one: "%{actors} has liked your post %{post_link}." other: "%{actors} have liked your post %{post_link}." + liked_comment: + zero: "%{actors} have liked your comment %{comment_link}." + one: "%{actors} has liked your comment %{comment_link}." + other: "%{actors} have liked your comment %{comment_link}." reshared: zero: "%{actors} have reshared your post %{post_link}." one: "%{actors} has reshared your post %{post_link}." @@ -688,6 +678,10 @@ en: zero: "%{actors} commented on a deleted post." one: "%{actors} commented on a deleted post." other: "%{actors} commented on a deleted post." + liked_comment_deleted: + zero: "%{actors} liked your deleted comment." + one: "%{actors} liked your deleted comment." + other: "%{actors} liked your deleted comment." liked_post_deleted: zero: "%{actors} liked your deleted post." one: "%{actors} liked your deleted post." @@ -713,7 +707,8 @@ en: all_notifications: "All notifications" also_commented: "Also commented" comment_on_post: "Comment on post" - liked: "Liked" + liked: "Liked post" + liked_comment: "Liked comment" mentioned: "Mentioned in post" mentioned_in_comment: "Mentioned in comment" reshared: "Reshared" @@ -760,6 +755,10 @@ en: liked: "%{name} liked your post" limited_post: "%{name} liked your limited post" view_post: "View post >" + liked_comment: + liked: "%{name} liked your comment" + limited_post: "%{name} liked your comment on a limited post" + view_comment: "View comment >" reshared: reshared: "%{name} reshared your post" view_post: "View post >" @@ -1298,6 +1297,7 @@ en: mentioned: "you are mentioned in a post" mentioned_in_comment: "you are mentioned in a comment" liked: "someone likes your post" + liked_comment: "someone likes your comment" reshared: "someone reshares your post" comment_on_post: "someone comments on your post" also_commented: "someone comments on a post you’ve commented on" diff --git a/features/step_definitions/notifications_steps.rb b/features/step_definitions/notifications_steps.rb index 9c521a908..5e5ffab9d 100644 --- a/features/step_definitions/notifications_steps.rb +++ b/features/step_definitions/notifications_steps.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true When "I filter notifications by likes" do - step %(I follow "Liked" within "#notifications_container .list-group") + step %(I follow "Liked post" within "#notifications_container .list-group") end When "I filter notifications by mentions" do diff --git a/lib/schemas/api_v1.json b/lib/schemas/api_v1.json index d46eeea9e..2a0014391 100644 --- a/lib/schemas/api_v1.json +++ b/lib/schemas/api_v1.json @@ -201,6 +201,7 @@ "also_commented", "comment_on_post", "liked", + "liked_comment", "mentioned", "mentioned_in_comment", "reshared", diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index 079089edc..9fe300b6b 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -83,6 +83,7 @@ describe NotificationsController, type: :controller do "comment_on_post" => 0, "contacts_birthday" => 0, "liked" => 0, + "liked_comment" => 0, "mentioned" => 0, "mentioned_in_comment" => 0, "reshared" => 0, diff --git a/spec/mailers/notifier_spec.rb b/spec/mailers/notifier_spec.rb index ead4a115a..3928d34a0 100644 --- a/spec/mailers/notifier_spec.rb +++ b/spec/mailers/notifier_spec.rb @@ -212,6 +212,27 @@ describe Notifier, type: :mailer do end end + describe ".liked_comment" do + before do + @post = FactoryBot.create(:status_message, author: alice.person, public: true) + @comment = FactoryBot.create(:comment, author: alice.person, post: @post) + @like = @comment.likes.create!(author: bob.person) + @mail = Notifier.send_notification("liked_comment", alice.id, @like.author.id, @like.id) + end + + it "TO: goes to the right person" do + expect(@mail.to).to eq([alice.email]) + end + + it "BODY: contains the original comment" do + expect(@mail.body.encoded).to include(@comment.message.plain_text) + end + + it "BODY: contains the name of person liking" do + expect(@mail.body.encoded).to include(@like.author.name) + end + end + describe ".reshared" do before do @post = FactoryBot.create(:status_message, author: alice.person, public: true) @@ -489,6 +510,42 @@ describe Notifier, type: :mailer do expect(mail.body.encoded).to include(bob.name) end end + + describe ".liked_comment" do + let(:comment) { alice.comment!(limited_post, "Totally is") } + let(:like) { bob.like_comment!(comment) } + let(:mail) { Notifier.send_notification("liked_comment", alice.id, bob.person.id, like.id) } + + it "TO: goes to the right person" do + expect(mail.to).to eq([alice.email]) + end + + it "FROM: contains the sender's name" do + expect(mail["From"].to_s).to eq("\"#{pod_name} (#{bob.name})\" <#{AppConfig.mail.sender_address}>") + end + + it "FROM: removes emojis from sender's name" do + bob.person.profile.update!(first_name: "1️⃣2️3️⃣ Numbers 123", last_name: "👍✅👍🏻Emojis😀😇❄️") + expect(mail["From"].to_s).to eq("\"#{pod_name} (Numbers 123 Emojis)\" <#{AppConfig.mail.sender_address}>") + end + + it "SUBJECT: does not show the limited comment" do + expect(mail.subject).not_to include("Totally is") + end + + it "IN-REPLY-TO and REFERENCES: references the liked post" do + expect(mail.in_reply_to).to eq("#{limited_post.guid}@#{AppConfig.pod_uri.host}") + expect(mail.references).to eq("#{limited_post.guid}@#{AppConfig.pod_uri.host}") + end + + it "BODY: does not show the limited post" do + expect(mail.body.encoded).not_to include("Totally is") + end + + it "BODY: contains the name of person liking" do + expect(mail.body.encoded).to include(bob.name) + end + end end describe ".confirm_email" do