Add notifications when a Result is commented (decidim#1543)

* Add Notifiable concern

* Send notifications to users to notify

* Add some tests

* Fix rubocop issues

* Add missing params to CommentNotificationMailer specs

* Fix specs

* Fix specs

* Add some feedback

* Specs: add process admin to test results notifications

decidim-budgets/app/models/decidim/budgets/project.rb

@@ -36,6 +36,11 @@ def comments_have_votes?
       def confirmed_orders_count
         orders.finished.count
       end
+
+      # Public: Overrides the `notifiable?` Notifiable concern method.
+      def notifiable?(_context)
+        false
+      end
     end
   end
 end

decidim-comments/app/commands/decidim/comments/create_comment.rb

@@ -23,7 +23,7 @@ def call
         return broadcast(:invalid) if form.invalid?
 
         create_comment
-        send_notification_to_author if has_author? && !same_author?
+        send_notification if @commentable.notifiable?(author: @author)
 
         broadcast(:ok, @comment)
       end
@@ -41,22 +41,18 @@ def create_comment
                                    decidim_user_group_id: form.user_group_id)
       end
 
-      def send_notification_to_author
-        if @comment.depth.positive? && @commentable.author.replies_notifications?
-          CommentNotificationMailer.reply_created(@comment, @commentable, @comment.root_commentable).deliver_later
-        elsif @comment.depth.zero? && @commentable.author.comments_notifications?
-          CommentNotificationMailer.comment_created(@comment, @commentable).deliver_later
+      def send_notification
+        if @comment.depth.positive?
+          @commentable.users_to_notify.each do |user|
+            CommentNotificationMailer.reply_created(user, @comment, @commentable, @comment.root_commentable).deliver_later
+          end
+        elsif @comment.depth.zero?
+          @commentable.users_to_notify.each do |user|
+            CommentNotificationMailer.comment_created(user, @comment, @commentable).deliver_later
+          end
         end
       end
 
-      def has_author?
-        @commentable.respond_to?(:author) && @commentable.author.present?
-      end
-
-      def same_author?
-        @author == @commentable.author
-      end
-
       def root_commentable(commentable)
         return commentable.root_commentable if commentable.is_a? Decidim::Comments::Comment
         commentable

decidim-comments/app/mailers/decidim/comments/comment_notification_mailer.rb

@@ -9,26 +9,28 @@ class CommentNotificationMailer < Decidim::ApplicationMailer
 
       helper_method :commentable_title
 
-      def comment_created(comment, commentable)
-        with_user(commentable.author) do
+      def comment_created(user, comment, commentable)
+        with_user(user) do
+          @user = user
           @comment = comment
           @commentable = commentable
           @locator = Decidim::ResourceLocatorPresenter.new(@commentable)
           @organization = commentable.organization
           subject = I18n.t("comment_created.subject", scope: "decidim.comments.mailer.comment_notification")
-          mail(to: commentable.author.email, subject: subject)
+          mail(to: user.email, subject: subject)
         end
       end
 
-      def reply_created(reply, comment, commentable)
-        with_user(comment.author) do
+      def reply_created(user, reply, comment, commentable)
+        with_user(user) do
+          @user = user
           @reply = reply
           @comment = comment
           @commentable = commentable
           @locator = Decidim::ResourceLocatorPresenter.new(@commentable)
           @organization = commentable.organization
           subject = I18n.t("reply_created.subject", scope: "decidim.comments.mailer.comment_notification")
-          mail(to: comment.author.email, subject: subject)
+          mail(to: user.email, subject: subject)
         end
       end
 

decidim-comments/app/models/decidim/comments/comment.rb

@@ -6,7 +6,7 @@ module Comments
     # comment on them. The will be able to create conversations between users
     # to discuss or share their thoughts about the resource.
     class Comment < ApplicationRecord
-      include Reportable
+      include Decidim::Reportable
       include Decidim::Authorable
       include Decidim::Comments::Commentable
 
@@ -59,6 +59,18 @@ def reported_content_url
         ResourceLocatorPresenter.new(root_commentable).url(anchor: "comment_#{id}")
       end
 
+      # Public: Overrides the `notifiable?` Notifiable concern method.
+      # When a comment is commented the comment's author is notified if it is not the same
+      # who has replied the comment and if the comment's author has replied notifiations enabled.
+      def notifiable?(context)
+        context[:author] != author && author.replies_notifications?
+      end
+
+      # Public: Overrides the `users_to_notify` Notifiable concern method.
+      def users_to_notify
+        [author]
+      end
+
       private
 
       # Private: Check if commentable can have comments and if not adds

decidim-comments/app/views/decidim/comments/comment_notification_mailer/comment_created.html.erb

@@ -1,4 +1,4 @@
-<p><%= t("decidim.comments.comment_notification_mailer.hello", name: @commentable.author.name) %></p>
+<p><%= t("decidim.comments.comment_notification_mailer.hello", name: @user.name) %></p>
 
 <p>
   <%=

decidim-comments/app/views/decidim/comments/comment_notification_mailer/reply_created.html.erb

@@ -1,4 +1,4 @@
-<p><%= t("decidim.comments.comment_notification_mailer.hello", name: @comment.author.name) %></p>
+<p><%= t("decidim.comments.comment_notification_mailer.hello", name: @user.name) %></p>
 
 <p>
   <%=

decidim-comments/lib/decidim/comments/commentable.rb

@@ -7,6 +7,7 @@ module Comments
     # Shared behaviour for commentable models.
     module Commentable
       extend ActiveSupport::Concern
+      include Decidim::Notifiable
 
       included do
         has_many :comments, as: :commentable, foreign_key: "decidim_commentable_id", foreign_type: "decidim_commentable_type", class_name: "Decidim::Comments::Comment"

decidim-comments/spec/commands/create_comment_spec.rb

@@ -9,6 +9,7 @@ module Comments
         let(:organization) { create(:organization) }
         let(:participatory_process) { create(:participatory_process, organization: organization) }
         let(:feature) { create(:feature, participatory_process: participatory_process) }
+        let(:user) { create(:user, organization: organization) }
         let(:author) { create(:user, organization: organization) }
         let(:dummy_resource) { create :dummy_resource, feature: feature }
         let(:commentable) { dummy_resource }
@@ -67,9 +68,9 @@ module Comments
             end.to change { Comment.count }.by(1)
           end
 
-          context "and the commentable doesn't have an author" do
+          context "and the commentable is not notifiable" do
             before do
-              commentable.author = nil
+              expect(commentable).to receive(:notifiable?).and_return(false)
             end
 
             it "doesn't send an email" do
@@ -78,70 +79,37 @@ module Comments
             end
           end
 
-          context "and the comment is a root comment" do
-            it "sends an email to the author of the commentable" do
-              expect(CommentNotificationMailer)
-                .to receive(:comment_created)
-                .with(an_instance_of(Comment), commentable)
-                .and_call_original
-
-              command.call
-            end
-
-            context "and I am the author of the commentable" do
-              let(:dummy_resource) { create :dummy_resource, feature: feature, author: author }
-
-              it "doesn't send an email" do
-                expect(CommentNotificationMailer).not_to receive(:comment_created)
-                command.call
-              end
+          context "and the commentable is notifiable" do
+            before do
+              expect(commentable).to receive(:notifiable?).and_return(true)
+              expect(commentable).to receive(:users_to_notify).and_return([user])
             end
 
-            context "and the author has comment notifications disabled" do
-              before do
-                commentable.author.update_attributes!(comments_notifications: false)
-              end
+            context "and the comment is a root comment" do
+              it "sends an email to the author of the commentable" do
+                expect(CommentNotificationMailer)
+                  .to receive(:comment_created)
+                  .with(user, an_instance_of(Comment), commentable)
+                  .and_call_original
 
-              it "doesn't send an email" do
-                expect(CommentNotificationMailer).not_to receive(:comment_created)
                 command.call
               end
             end
-          end
-
-          context "and the comment is a reply" do
-            let(:commentable) { create(:comment, commentable: dummy_resource) }
-
-            it "stores the root commentable" do
-              command.call
-              expect(Comment.last.root_commentable).to eq(dummy_resource)
-            end
-
-            it "sends an email to the author of the parent comment" do
-              expect(CommentNotificationMailer)
-                .to receive(:reply_created)
-                .with(an_instance_of(Comment), commentable, commentable.root_commentable)
-                .and_call_original
-
-              command.call
-            end
 
-            context "and I am the author of the parent comment" do
-              let(:commentable) { create(:comment, author: author, commentable: dummy_resource) }
+            context "and the comment is a reply" do
+              let(:commentable) { create(:comment, commentable: dummy_resource) }
 
-              it "doesn't send an email" do
-                expect(CommentNotificationMailer).not_to receive(:reply_created)
+              it "stores the root commentable" do
                 command.call
+                expect(Comment.last.root_commentable).to eq(dummy_resource)
               end
-            end
 
-            context "and the author has reply notifications disabled" do
-              before do
-                commentable.author.update_attribute(:replies_notifications, false)
-              end
+              it "sends an email to the author of the parent comment" do
+                expect(CommentNotificationMailer)
+                  .to receive(:reply_created)
+                  .with(user, an_instance_of(Comment), commentable, commentable.root_commentable)
+                  .and_call_original
 
-              it "doesn't send an email" do
-                expect(CommentNotificationMailer).not_to receive(:reply_created)
                 command.call
               end
             end

decidim-comments/spec/mailers/comment_notification_mailer_spec.rb

@@ -11,32 +11,33 @@ module Comments
       let(:feature) { create(:feature, participatory_process: participatory_process) }
       let(:commentable_author) { create(:user, organization: organization) }
       let(:commentable) { create(:dummy_resource, author: commentable_author, feature: feature) }
+      let(:user) { create(:user, organization: organization) }
       let(:author) { create(:user, organization: organization) }
       let(:comment) { create(:comment, author: author, commentable: commentable) }
       let(:reply) { create(:comment, author: author, commentable: comment) }
 
       describe "comment_created" do
-        let(:mail) { described_class.comment_created(comment, commentable) }
+        let(:mail) { described_class.comment_created(user, comment, commentable) }
 
         let(:subject) { "Tens un nou comentari" }
         let(:default_subject) { "You have a new comment" }
 
         let(:body) { "Hi ha un nou comentari d" }
         let(:default_body) { "There is a new comment" }
 
-        include_examples "author localised email"
+        include_examples "user localised email"
       end
 
       describe "reply_created" do
-        let(:mail) { described_class.reply_created(reply, comment, commentable) }
+        let(:mail) { described_class.reply_created(user, reply, comment, commentable) }
 
         let(:subject) { "Tens una nova resposta del teu comentari" }
         let(:default_subject) { "You have a new reply of your comment" }
 
         let(:body) { "Hi ha una nova resposta de" }
         let(:default_body) { "There is a new reply of your comment" }
 
-        include_examples "author localised email"
+        include_examples "user localised email"
       end
     end
   end

decidim-comments/spec/models/comment_spec.rb

@@ -6,7 +6,9 @@ module Decidim
   module Comments
     describe Comment do
       let!(:commentable) { create(:dummy_resource) }
-      let!(:comment) { create(:comment, commentable: commentable) }
+      let!(:replies_notifications) { true }
+      let!(:author) { create(:user, organization: commentable.organization, replies_notifications: replies_notifications) }
+      let!(:comment) { create(:comment, commentable: commentable, author: author) }
       let!(:replies) { create_list(:comment, 3, commentable: comment, root_commentable: commentable) }
       let!(:up_vote) { create(:comment_vote, :up_vote, comment: comment) }
       let!(:down_vote) { create(:comment_vote, :down_vote, comment: comment) }
@@ -88,6 +90,34 @@ module Comments
           expect(subject.down_voted_by?(user)).to be_falsy
         end
       end
+
+      describe "#notifiable?" do
+        let(:context_author) { create(:user, organization: subject.author.organization) }
+
+        context "when the context author is the same as the comment's author" do
+          let(:context_author) { subject.author }
+
+          it "is not notifiable" do
+            expect(subject.notifiable?(author: context_author)).to be_falsy
+          end
+        end
+
+        context "when the context author is not the same as the comment's author" do
+          context "when the comment's author has not replies notifications enabled" do
+            let(:replies_notifications) { false }
+
+            it "is not notifiable" do
+              expect(subject.notifiable?(author: context_author)).to be_falsy
+            end
+          end
+
+          context "when the comment's author has replies notifications enabled" do
+            it "is not notifiable" do
+              expect(subject.notifiable?(author: context_author)).to be_truthy
+            end
+          end
+        end
+      end
     end
   end
 end

decidim-comments/spec/shared/author_localised_email.rb

@@ -2,9 +2,8 @@
 
 require "spec_helper"
 
-shared_examples "author localised email" do
-  let(:author) { create(:user, locale: locale, organization: organization) }
-  let(:commentable_author) { create(:user, locale: locale, organization: organization) }
+shared_examples "user localised email" do
+  let(:user) { create(:user, locale: locale, organization: organization) }
 
   context "when the user has a custom locale" do
     let(:locale) { "ca" }

decidim-core/lib/decidim/core.rb

@@ -17,6 +17,7 @@ module Decidim
   autoload :Resourceable, "decidim/resourceable"
   autoload :Reportable, "decidim/reportable"
   autoload :Authorable, "decidim/authorable"
+  autoload :Notifiable, "decidim/notifiable"
   autoload :Features, "decidim/features"
   autoload :HasAttachments, "decidim/has_attachments"
   autoload :FeatureValidator, "decidim/feature_validator"

decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb

@@ -73,10 +73,10 @@
         end
       end
 
-      it "sends notifications received by commentable's author" do
-        if commentable.respond_to? :author
+      it "send notifications" do
+        if commentable.notifiable?(author: user)
           wait_for_email subject: "new comment"
-          relogin_as commentable.author, scope: :user
+          relogin_as commentable.users_to_notify.first, scope: :user
           visit last_email_first_link
 
           within "#comments" do

decidim-core/lib/decidim/notifiable.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require "active_support/concern"
+
+module Decidim
+  # Shared behaviour for notifiable models.
+  module Notifiable
+    extend ActiveSupport::Concern
+
+    included do
+      # Public: Whether the object is notifiable or not.
+      def notifiable?(_context)
+        true
+      end
+
+      # Public: A collection of users to send the notifications.
+      def users_to_notify
+        []
+      end
+    end
+  end
+end