Merge pull request #6785 from artoscinote/ma_SCI_9856

Implement delivery notifications, fix specs [SCI-9856]

app/jobs/concerns/failed_delivery_notifiable_job.rb

@@ -24,11 +24,14 @@ module FailedDeliveryNotifiableJob
     @user = User.find_by(id: arguments.last[:user_id])
     return if @user.blank?
 
-    DeliveryNotification.with(
+    DeliveryNotification.send_notifications(
+      {
         title: failed_notification_title,
         message: failed_notification_message,
-      error: true
+        error: true,
-    ).deliver(@user)
+        user: @user
+      }
+    )
   end
 
   def failed_notification_title

app/jobs/protocols/docx_import_job.rb

@@ -136,13 +136,17 @@ module Protocols
         "href='#{Rails.application.routes.url_helpers.rails_blob_path(@tmp_files.take.file)}'>" \
         "#{@tmp_files.take.file.filename}</a>"
 
-      DeliveryNotification.with(
+      DeliveryNotification.send_notifications(
-        title: I18n.t('protocols.import_export.import_protocol_notification.title', link: original_file_download_link),
+        {
+          title:
+            I18n.t('protocols.import_export.import_protocol_notification.title', link: original_file_download_link),
           message: "#{I18n.t('protocols.import_export.import_protocol_notification.message')} " \
                    "<a data-id='#{@protocol.id}'  data-turbolinks='false' " \
                    "href='#{Rails.application.routes.url_helpers.protocol_path(@protocol)}'>" \
-                 "#{@protocol.name}</a>"
+                   "#{@protocol.name}</a>",
-      ).deliver(@user)
+          user: @user
+        }
+      )
     end
 
     # Overrides method from FailedDeliveryNotifiableJob concern

app/jobs/reports/docx_job.rb

@@ -22,12 +22,15 @@ module Reports
         report_path = Rails.application.routes.url_helpers
                            .reports_path(team: report.team.id, preview_report_id: report.id, preview_type: :docx)
 
-        DeliveryNotification.with(
+        DeliveryNotification.send_notifications(
+          {
             title: I18n.t('projects.reports.index.generation.completed_docx_notification_title'),
             message: I18n.t('projects.reports.index.generation.completed_notification_message',
                             report_link: "<a href='#{report_path}'>#{escape_input(report.name)}</a>",
-                          team_name: escape_input(report.team.name))
+                            team_name: escape_input(report.team.name)),
-        ).deliver(user)
+            user: @user
+          }
+        )
 
         Reports::DocxPreviewJob.perform_now(report.id)
       ensure

app/jobs/reports/pdf_job.rb

@@ -162,12 +162,15 @@ module Reports
     def create_notification_for_user
       report_path = Rails.application.routes.url_helpers
                          .reports_path(team: @report.team.id, preview_report_id: @report.id, preview_type: :pdf)
-      DeliveryNotification.with(
+      DeliveryNotification.send_notifications(
+        {
           title: I18n.t('projects.reports.index.generation.completed_pdf_notification_title'),
           message: I18n.t('projects.reports.index.generation.completed_notification_message',
                           report_link: "<a href='#{report_path}'>#{escape_input(@report.name)}</a>",
-                      team_name: escape_input(@report.team.name))
+                          team_name: escape_input(@report.team.name)),
-      ).deliver(@user)
+          user: @user
+        }
+      )
     end
 
     def append_result_asset_previews

app/jobs/repositories_export_job.rb

@@ -83,7 +83,8 @@ class RepositoriesExportJob < ApplicationJob
   end
 
   def generate_notification
-    DeliveryNotification.with(
+    DeliveryNotification.send_notifications(
+      {
         title: I18n.t('zip_export.notification_title'),
         message: "<a data-id='#{@zip_export.id}' " \
                       "data-turbolinks='false' " \
@@ -91,8 +92,10 @@ class RepositoriesExportJob < ApplicationJob
                                     .routes
                                     .url_helpers
                                     .zip_exports_download_export_all_path(@zip_export)}'>" \
-                    "#{@zip_export.zip_file_name}</a>"
+                      "#{@zip_export.zip_file_name}</a>",
-    ).deliver(@user)
+        user: @user
+      }
+    )
   end
 
   # Overrides method from FailedDeliveryNotifiableJob concern

app/jobs/zip_export_job.rb

@@ -34,7 +34,8 @@ class ZipExportJob < ApplicationJob
   end
 
   def generate_notification!
-    DeliveryNotification.with(
+    DeliveryNotification.send_notifications(
+      {
         title: I18n.t('zip_export.notification_title'),
         message: "<a data-id='#{@zip_export.id}' " \
                   "data-turbolinks='false' " \
@@ -42,7 +43,9 @@ class ZipExportJob < ApplicationJob
                                 .routes
                                 .url_helpers
                                 .zip_exports_download_path(@zip_export)}'>" \
-                "#{@zip_export.zip_file_name}</a>"
+                  "#{@zip_export.zip_file_name}</a>",
-    ).deliver(@user)
+        user: @user
+      }
+    )
   end
 end

app/notifications/delivery_notification.rb

@@ -1,36 +1,15 @@
 # frozen_string_literal: true
 
-# To deliver this notification:
-#
-# DeliveryNotification.with(post: @post).deliver_later(current_user)
-# DeliveryNotification.with(post: @post).deliver(current_user)
-
 class DeliveryNotification < BaseNotification
-  # Add your delivery methods
+  def self.subtype
-  #
+    :delivery
-  # deliver_by :email, mailer: "UserMailer"
+  end
-  # deliver_by :slack
-  # deliver_by :custom, class: "MyDeliveryMethod"
 
-  # Add required params
-  #
-  # param :post
-
-  # Define helper methods to make rendering easier.
-  #
   def message
-    # if params[:legacy]
     params[:message]
-    # else
-    # new logic
-    # end
   end
 
   def title
-    # if params[:legacy]
     params[:title]
-    # else
-    # new logic
-    # end
   end
 end

config/initializers/extends/notification_extends.rb

@@ -97,6 +97,9 @@ class NotificationExtends
     change_users_role_on_team_activity: {
       code: 94,
       recipients_module: :UserChangedRecipient
+    },
+    delivery: {
+      recipients_module: :DirectRecipient
     }
   }
 
@@ -151,7 +154,9 @@ class NotificationExtends
         remove_user_from_team
         change_users_role_on_team_activity
       ],
-      always_on: []
+      always_on: %I[
+        delivery
+      ]
     }
   }
 end

spec/controllers/user_my_modules_controller_spec.rb

@@ -6,11 +6,12 @@ describe UserMyModulesController, type: :controller do
   login_user
 
   include_context 'reference_project_structure'
+  let(:other_user) { create :user }
 
   describe 'POST create' do
     let(:action) { post :create, params: params, format: :json }
     let(:params) do
-      { my_module_id: my_module.id, user_my_module: { user_id: user.id } }
+      { my_module_id: my_module.id, user_my_module: { user_id: other_user.id } }
     end
 
     it 'calls create activity for assigning user to task' do

spec/factories/notifications.rb

@@ -3,7 +3,7 @@
 FactoryBot.define do
   factory :notification do
     recipient_type { 'User' }
-    recipient_id { 1 }
+    recipient_id { FactoryBot.create(:user).id }
     read_at { Time.now }
   end
 end

spec/models/user_spec.rb

@@ -89,7 +89,6 @@ describe User, type: :model do
     it { should have_many :archived_protocols }
     it { should have_many :restored_protocols }
     it { should have_many :assigned_my_module_repository_rows }
-    it { should have_many :user_notifications }
     it { should have_many :notifications }
     it { should have_many :zip_exports }
     it { should have_many(:shareable_links).dependent(:destroy) }
@@ -150,10 +149,6 @@ describe User, type: :model do
 
   describe 'user settings' do
     it { is_expected.to respond_to(:time_zone) }
-    it { is_expected.to respond_to(:assignments_notification) }
-    it { is_expected.to respond_to(:assignments_email_notification) }
-    it { is_expected.to respond_to(:recent_notification) }
-    it { is_expected.to respond_to(:recent_email_notification) }
   end
 
   describe 'user variables' do