Add bulk insert for UserSystemNotifications, New service for sending emails

app/models/user_system_notification.rb

@@ -4,8 +4,7 @@ class UserSystemNotification < ApplicationRecord
   belongs_to :user
   belongs_to :system_notification
 
-  after_create :send_email,
+  validates :system_notification, uniqueness: { scope: :user }
-               if: proc { |sn| sn.user.system_message_email_notification }
 
   scope :unseen, -> { where(seen_at: nil) }
 
@@ -15,9 +14,7 @@ class UserSystemNotification < ApplicationRecord
 
   def self.mark_as_read(notification_id)
     notification = find_by_system_notification_id(notification_id)
-    if notification && notification.read_at.nil?
+    notification.update(read_at: Time.now) if notification && notification.read_at.nil?
-      notification.update(read_at: Time.now)
-    end
   end
 
   def self.show_on_login(update_read_time = false)
@@ -45,10 +42,4 @@ class UserSystemNotification < ApplicationRecord
       notification
     end
   end
-
-  private
-
-  def send_email
-    AppMailer.delay.system_notification(user, system_notification)
-  end
 end

app/services/notifications/handle_system_notification_in_communication_channel_service.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Notifications
+  class HandleSystemNotificationInCommunicationChannelService
+    extend Service
+
+    attr_reader :errors
+
+    def initialize(system_notification)
+      @system_notification = system_notification
+      @errors = {}
+    end
+
+    def call
+      @system_notification.user_system_notifications.find_each do |usn|
+        user = usn.user
+        AppMailer.delay.system_notification(user, @system_notification) if user.system_message_email_notification
+      end
+
+      self
+    end
+
+    def succeed?
+      @errors.none?
+    end
+  end
+end

app/services/notifications/push_to_communication_channel_service.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Notifications
+  class PushToCommunicationChannelService
+    extend Service
+
+    WHITELISTED_ITEM_TYPES = %w(SystemNotification).freeze
+
+    attr_reader :errors
+
+    def initialize(item_id:, item_type:)
+      @item_type = item_type
+      @item = item_type.constantize.find item_id
+      @errors = {}
+    end
+
+    def call
+      return self unless valid?
+
+      "Notifications::Handle#{@item_type}InCommunicationChannelService".constantize.call(@item)
+      self
+    end
+
+    def succeed?
+      @errors.none?
+    end
+
+    private
+
+    def valid?
+      raise 'Dont know how to handle this type of items' unless WHITELISTED_ITEM_TYPES.include?(@item_type)
+
+      if @item.nil?
+        @errors[:invalid_arguments] = 'Can\'t find item' if @item.nil?
+        return false
+      end
+      true
+    end
+  end
+end

app/services/notifications/sync_system_notifications_service.rb

@@ -81,12 +81,27 @@ module Notifications
             .where(source_id: attrs[:source_id]).first_or_initialize(attrs)
 
         if n.new_record?
-          n.users = User.all
+          save_notification n
-          n.save!
         elsif n.last_time_changed_at < attrs[:last_time_changed_at]
           n.update_attributes!(attrs)
         end
       end
     end
+
+    def save_notification(notification)
+      ActiveRecord::Base.transaction do
+        notification.save!
+
+        User.find_in_batches do |user_ids|
+          user_system_notifications = user_ids.pluck(:id).collect do |item|
+            Hash[:user_id, item, :system_notification_id, notification.id]
+          end
+          UserSystemNotification.import user_system_notifications, validate: false
+        end
+      end
+
+      Notifications::PushToCommunicationChannelService.delay.call(item_id: notification.id,
+                                                            item_type: notification.class.name)
+    end
   end
 end

db/migrate/20190424113216_add_unique_index_to_system_notifications.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AddUniqueIndexToSystemNotifications < ActiveRecord::Migration[5.1]
+  def change
+    # remove not unique index and add new with uniq
+    remove_index :system_notifications, :source_id
+    add_index :system_notifications, :source_id, unique: true
+
+    add_index :user_system_notifications, %i(user_id system_notification_id), unique: true,
+              name: 'index_user_system_notifications_on_user_and_notification_id'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20190410110605) do
+ActiveRecord::Schema.define(version: 20190424113216) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -684,7 +684,7 @@ ActiveRecord::Schema.define(version: 20190410110605) do
     t.datetime "updated_at", null: false
     t.index ["last_time_changed_at"], name: "index_system_notifications_on_last_time_changed_at"
     t.index ["source_created_at"], name: "index_system_notifications_on_source_created_at"
-    t.index ["source_id"], name: "index_system_notifications_on_source_id"
+    t.index ["source_id"], name: "index_system_notifications_on_source_id", unique: true
   end
 
   create_table "tables", force: :cascade do |t|
@@ -819,6 +819,7 @@ ActiveRecord::Schema.define(version: 20190410110605) do
     t.index ["read_at"], name: "index_user_system_notifications_on_read_at"
     t.index ["seen_at"], name: "index_user_system_notifications_on_seen_at"
     t.index ["system_notification_id"], name: "index_user_system_notifications_on_system_notification_id"
+    t.index ["user_id", "system_notification_id"], name: "index_user_system_notifications_on_user_and_notification_id", unique: true
     t.index ["user_id"], name: "index_user_system_notifications_on_user_id"
   end
 

spec/factories/system_notifications.rb

@@ -7,7 +7,7 @@ FactoryBot.define do
     modal_title { Faker::Name.first_name }
     modal_body { Faker::Lorem.paragraphs(4).map { |pr| "<p>#{pr}</p>" }.join }
     source_created_at { Faker::Time.between(3.days.ago, Date.today) }
-    source_id { Faker::Number.between(1, 1000) }
+    source_id { SystemNotification.order(source_id: :desc).first&.source_id.to_i + 1 }
     last_time_changed_at { Time.now }
     trait :show_on_login do
       show_on_login { true }

spec/models/user_system_notification_spec.rb

@@ -15,36 +15,6 @@ describe UserSystemNotification do
     it { is_expected.to belong_to(:system_notification) }
   end
 
-  describe '.create' do
-    before do
-      Delayed::Worker.delay_jobs = false
-    end
-
-    after do
-      Delayed::Worker.delay_jobs = true
-    end
-
-    context 'when user has enabled notifications' do
-      it 'calls send an email on creation' do
-        allow(user_system_notification.user)
-          .to receive(:system_message_email_notification).and_return(true)
-
-        expect(user_system_notification).to receive(:send_email)
-        user_system_notification.save
-      end
-    end
-
-    context 'when user has disabled notifications' do
-      it 'doesn\'t call send an email on createion' do
-        allow(user_system_notification.user)
-          .to receive(:system_message_email_notification).and_return(false)
-
-        expect(user_system_notification).not_to receive(:send_email)
-        user_system_notification.save
-      end
-    end
-  end
-
   describe 'Methods' do
     let(:notifcation_one) { create :system_notification }
     let(:notifcation_two) { create :system_notification }

spec/services/notifications/handle_system_notification_in_communication_channel_service_spec.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Notifications::HandleSystemNotificationInCommunicationChannelService do
+  let(:system_notification) { create :system_notification }
+  let!(:user_system_notification) do
+    create :user_system_notification, user: user, system_notification: system_notification
+  end
+  let(:user) { create :user }
+  let(:service_call) do
+    Notifications::HandleSystemNotificationInCommunicationChannelService.call(system_notification)
+  end
+
+  before do
+    Delayed::Worker.delay_jobs = false
+  end
+
+  after do
+    Delayed::Worker.delay_jobs = true
+  end
+
+  context 'when user has enabled notifications' do
+    it 'calls AppMailer' do
+      allow_any_instance_of(User).to receive(:system_message_email_notification).and_return(true)
+
+      expect(AppMailer).to receive(:system_notification).and_return(double('Mailer', deliver: true))
+
+      service_call
+    end
+  end
+
+  context 'when user has disabled notifications' do
+    it 'does not call AppMailer' do
+      allow_any_instance_of(User).to receive(:system_message_email_notification).and_return(false)
+
+      expect(AppMailer).not_to receive(:system_notification)
+
+      service_call
+    end
+  end
+end

spec/services/notifications/push_to_communication_channel_service_spec.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Notifications::PushToCommunicationChannelService do
+  let(:system_notification) { create :system_notification }
+  let(:service_call) do
+    Notifications::PushToCommunicationChannelService.call(item_id: system_notification.id,
+                                                          item_type: system_notification.class.name)
+  end
+
+  context 'when call with valid items' do
+    it 'call service to to handle sending out' do
+      expect(Notifications::HandleSystemNotificationInCommunicationChannelService)
+        .to receive(:call).with(system_notification)
+
+      service_call
+    end
+  end
+
+  context 'when call with not valid items' do
+    it 'returns error with key invalid_arguments when system notification not exists' do
+      allow(SystemNotification).to receive(:find).and_return(nil)
+
+      expect(service_call.errors).to have_key(:invalid_arguments)
+    end
+
+    it 'raise error when have not listed object' do
+      u = create :user
+
+      expect do
+        Notifications::PushToCommunicationChannelService.call(item_id: u.id, item_type: 'User')
+      end.to(raise_error('Dont know how to handle this type of items'))
+    end
+  end
+end

spec/services/notifications/sync_system_notifications_service_spec.rb

@@ -6,6 +6,8 @@ describe Notifications::SyncSystemNotificationsService do
   url = 'http://system-notifications-service.test/api/system_notifications'
   let!(:user) { create :user }
   let(:service_call) do
+    allow_any_instance_of(Notifications::PushToCommunicationChannelService).to receive(:call).and_return(nil)
+
     Notifications::SyncSystemNotificationsService.call
   end
 
@@ -80,6 +82,14 @@ describe Notifications::SyncSystemNotificationsService do
 
       expect { service_call }.to change { UserSystemNotification.count }.by(20)
     end
+
+    it 'calls service to notify users about notification' do
+      Delayed::Worker.delay_jobs = false
+      expect(Notifications::PushToCommunicationChannelService).to receive(:call).exactly(10)
+
+      service_call
+      Delayed::Worker.delay_jobs = true
+    end
   end
 
   context 'when request is unsuccessful' do
@@ -101,5 +111,13 @@ describe Notifications::SyncSystemNotificationsService do
 
       expect(service_call.errors).to have_key(:socketerror)
     end
+
+    it 'does not call service to notify users about notification' do
+      Delayed::Worker.delay_jobs = false
+      expect(Notifications::PushToCommunicationChannelService).to_not receive(:call)
+
+      service_call
+      Delayed::Worker.delay_jobs = true
+    end
   end
 end