From af7bae48a0fe4b67f531158d4792ac85d8a03a26 Mon Sep 17 00:00:00 2001 From: Martin Artnik Date: Fri, 8 Dec 2023 13:57:43 +0100 Subject: [PATCH] Implement delivery notifications, fix specs [SCI-9856] --- .../failed_delivery_notifiable_job.rb | 13 +++++---- app/jobs/protocols/docx_import_job.rb | 18 ++++++++----- app/jobs/reports/docx_job.rb | 15 ++++++----- app/jobs/reports/pdf_job.rb | 15 ++++++----- app/jobs/repositories_export_job.rb | 23 +++++++++------- app/jobs/zip_export_job.rb | 23 +++++++++------- app/notifications/delivery_notification.rb | 27 +++---------------- .../extends/notification_extends.rb | 7 ++++- .../user_my_modules_controller_spec.rb | 3 ++- spec/factories/notifications.rb | 2 +- spec/models/user_spec.rb | 5 ---- 11 files changed, 75 insertions(+), 76 deletions(-) diff --git a/app/jobs/concerns/failed_delivery_notifiable_job.rb b/app/jobs/concerns/failed_delivery_notifiable_job.rb index caae13da9..78acba052 100644 --- a/app/jobs/concerns/failed_delivery_notifiable_job.rb +++ b/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( - title: failed_notification_title, - message: failed_notification_message, - error: true - ).deliver(@user) + DeliveryNotification.send_notifications( + { + title: failed_notification_title, + message: failed_notification_message, + error: true, + user: @user + } + ) end def failed_notification_title diff --git a/app/jobs/protocols/docx_import_job.rb b/app/jobs/protocols/docx_import_job.rb index f9809fe9e..709d5cb43 100644 --- a/app/jobs/protocols/docx_import_job.rb +++ b/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}" - DeliveryNotification.with( - 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')} " \ - "" \ - "#{@protocol.name}" - ).deliver(@user) + DeliveryNotification.send_notifications( + { + 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')} " \ + "" \ + "#{@protocol.name}", + user: @user + } + ) end # Overrides method from FailedDeliveryNotifiableJob concern diff --git a/app/jobs/reports/docx_job.rb b/app/jobs/reports/docx_job.rb index cb8395ca7..8c4af0ac4 100644 --- a/app/jobs/reports/docx_job.rb +++ b/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( - title: I18n.t('projects.reports.index.generation.completed_docx_notification_title'), - message: I18n.t('projects.reports.index.generation.completed_notification_message', - report_link: "#{escape_input(report.name)}", - team_name: escape_input(report.team.name)) - ).deliver(user) + 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: "#{escape_input(report.name)}", + team_name: escape_input(report.team.name)), + user: @user + } + ) Reports::DocxPreviewJob.perform_now(report.id) ensure diff --git a/app/jobs/reports/pdf_job.rb b/app/jobs/reports/pdf_job.rb index 6517a0e35..a122c1891 100644 --- a/app/jobs/reports/pdf_job.rb +++ b/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( - title: I18n.t('projects.reports.index.generation.completed_pdf_notification_title'), - message: I18n.t('projects.reports.index.generation.completed_notification_message', - report_link: "#{escape_input(@report.name)}", - team_name: escape_input(@report.team.name)) - ).deliver(@user) + 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: "#{escape_input(@report.name)}", + team_name: escape_input(@report.team.name)), + user: @user + } + ) end def append_result_asset_previews diff --git a/app/jobs/repositories_export_job.rb b/app/jobs/repositories_export_job.rb index f45d8d79a..4f13d8be1 100644 --- a/app/jobs/repositories_export_job.rb +++ b/app/jobs/repositories_export_job.rb @@ -83,16 +83,19 @@ class RepositoriesExportJob < ApplicationJob end def generate_notification - DeliveryNotification.with( - title: I18n.t('zip_export.notification_title'), - message: "" \ - "#{@zip_export.zip_file_name}" - ).deliver(@user) + DeliveryNotification.send_notifications( + { + title: I18n.t('zip_export.notification_title'), + message: "" \ + "#{@zip_export.zip_file_name}", + user: @user + } + ) end # Overrides method from FailedDeliveryNotifiableJob concern diff --git a/app/jobs/zip_export_job.rb b/app/jobs/zip_export_job.rb index a2e829026..a64d03b28 100644 --- a/app/jobs/zip_export_job.rb +++ b/app/jobs/zip_export_job.rb @@ -34,15 +34,18 @@ class ZipExportJob < ApplicationJob end def generate_notification! - DeliveryNotification.with( - title: I18n.t('zip_export.notification_title'), - message: "" \ - "#{@zip_export.zip_file_name}" - ).deliver(@user) + DeliveryNotification.send_notifications( + { + title: I18n.t('zip_export.notification_title'), + message: "" \ + "#{@zip_export.zip_file_name}", + user: @user + } + ) end end diff --git a/app/notifications/delivery_notification.rb b/app/notifications/delivery_notification.rb index 9df109db5..9af4ef3a5 100644 --- a/app/notifications/delivery_notification.rb +++ b/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 - # - # deliver_by :email, mailer: "UserMailer" - # deliver_by :slack - # deliver_by :custom, class: "MyDeliveryMethod" + def self.subtype + :delivery + end - # 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 diff --git a/config/initializers/extends/notification_extends.rb b/config/initializers/extends/notification_extends.rb index 1d05b2203..8afefad16 100644 --- a/config/initializers/extends/notification_extends.rb +++ b/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 diff --git a/spec/controllers/user_my_modules_controller_spec.rb b/spec/controllers/user_my_modules_controller_spec.rb index 3664c11ff..65116d1b8 100644 --- a/spec/controllers/user_my_modules_controller_spec.rb +++ b/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 diff --git a/spec/factories/notifications.rb b/spec/factories/notifications.rb index 9776c1f97..29b648cb1 100644 --- a/spec/factories/notifications.rb +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d63effe31..926811786 100644 --- a/spec/models/user_spec.rb +++ b/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