diff --git a/app/models/embedded_ansible_worker/runner.rb b/app/models/embedded_ansible_worker/runner.rb index e0bae3ca0e1..9c6f545078d 100644 --- a/app/models/embedded_ansible_worker/runner.rb +++ b/app/models/embedded_ansible_worker/runner.rb @@ -97,7 +97,12 @@ def raise_role_notification(notification_type) :role_name => ServerRole.find_by(:name => worker.class.required_roles.first).description, :server_name => MiqServer.my_server.name } - Notification.create(:type => notification_type, :options => notification_options) + + # Create the notification only if there are no existing ones for embedded ansible which are unseen + Notification.create(:type => notification_type, :options => notification_options) if Notification.of_type(notification_type).none? do |n| + correct_notification = n.options == notification_options + correct_notification && !n.seen_by_all_recipients? + end end def embedded_ansible diff --git a/app/models/notification.rb b/app/models/notification.rb index ebd1b224c2c..17d665b6d2e 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -16,6 +16,8 @@ class Notification < ApplicationRecord serialize :options, Hash default_value_for(:options) { Hash.new } + scope :of_type, ->(notification_type) { joins(:notification_type).where(:notification_types => {:name => notification_type}) } + def type=(typ) self.notification_type = NotificationType.find_by!(:name => typ) end @@ -35,6 +37,10 @@ def to_h } end + def seen_by_all_recipients? + notification_recipients.unseen.empty? + end + private def emit_message diff --git a/spec/models/embedded_ansible_worker/runner_spec.rb b/spec/models/embedded_ansible_worker/runner_spec.rb index 852e3c6ab2a..36aac12e9ab 100644 --- a/spec/models/embedded_ansible_worker/runner_spec.rb +++ b/spec/models/embedded_ansible_worker/runner_spec.rb @@ -21,37 +21,8 @@ } context "#do_before_work_loop" do - let(:start_notification_id) { NotificationType.find_by(:name => "role_activate_start").id } - let(:success_notification_id) { NotificationType.find_by(:name => "role_activate_success").id } - - before do - ServerRole.seed - NotificationType.seed - end - - it "creates a notification to inform the user that the service has started" do - expect(runner).to receive(:setup_ansible) - expect(runner).to receive(:update_embedded_ansible_provider) - - runner.do_before_work_loop - - note = Notification.find_by(:notification_type_id => success_notification_id) - expect(note.options[:role_name]).to eq("Embedded Ansible") - expect(note.options.keys).to include(:server_name) - end - - it "creates a notification to inform the user that the role has been assigned" do - expect(runner).to receive(:setup_ansible) - expect(runner).to receive(:update_embedded_ansible_provider) - - runner.do_before_work_loop - - note = Notification.find_by(:notification_type_id => start_notification_id) - expect(note.options[:role_name]).to eq("Embedded Ansible") - expect(note.options.keys).to include(:server_name) - end - it "exits on exceptions" do + allow(runner).to receive(:raise_role_notification) expect(runner).to receive(:setup_ansible) expect(runner).to receive(:update_embedded_ansible_provider).and_raise(StandardError) expect(runner).to receive(:do_exit) @@ -170,5 +141,61 @@ end end end + + context "#raise_role_notification (private)" do + let(:start_notification_id) { NotificationType.find_by(:name => "role_activate_start").id } + let(:success_notification_id) { NotificationType.find_by(:name => "role_activate_success").id } + + before do + ServerRole.seed + NotificationType.seed + FactoryGirl.create(:user_admin) + end + + it "creates a notification to inform the user that the service has started" do + runner.send(:raise_role_notification, :role_activate_start) + + note = Notification.find_by(:notification_type_id => start_notification_id) + expect(note.options[:role_name]).to eq("Embedded Ansible") + expect(note.options.keys).to include(:server_name) + end + + it "creates a notification to inform the user that the role has been assigned" do + runner.send(:raise_role_notification, :role_activate_success) + + note = Notification.find_by(:notification_type_id => success_notification_id) + expect(note.options[:role_name]).to eq("Embedded Ansible") + expect(note.options.keys).to include(:server_name) + end + + it "doesn't create additional notifications if an unread one exists" do + runner.send(:raise_role_notification, :role_activate_start) + expect(Notification.count).to eq(1) + + runner.send(:raise_role_notification, :role_activate_start) + expect(Notification.count).to eq(1) + end + + it "creates a new notification if the existing one was read" do + runner.send(:raise_role_notification, :role_activate_start) + expect(Notification.count).to eq(1) + + Notification.first.notification_recipients.each { |r| r.update_attributes(:seen => true) } + runner.send(:raise_role_notification, :role_activate_start) + expect(Notification.count).to eq(2) + end + + it "creates a new notification if there is one for a different role" do + Notification.create(:type => :role_activate_start, :options => {:role_name => "someotherrole", :server_name => miq_server.name}) + runner.send(:raise_role_notification, :role_activate_start) + expect(Notification.count).to eq(2) + end + + it "creates a new notification if there is one for a different server" do + Notification.create(:type => :role_activate_start, :options => {:role_name => "Embedded Ansible", :server_name => "#{miq_server.name}somenonsense"}) + runner.send(:raise_role_notification, :role_activate_start) + expect(Notification.count).to eq(2) + end + end end end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 7dab6354208..f324289d263 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -4,6 +4,23 @@ let(:tenant) { FactoryGirl.create(:tenant) } let!(:user) { FactoryGirl.create(:user_with_group, :tenant => tenant) } + describe '.of_type' do + it "only returns notification of the given type" do + types = NotificationType.all + type_name_one = types[0].name + type_name_two = types[1].name + type_name_three = types[2].name + + Notification.create(:type => type_name_one, :initiator => user) + Notification.create(:type => type_name_one, :initiator => user) + Notification.create(:type => type_name_two, :initiator => user) + + expect(Notification.of_type(type_name_one).count).to eq(2) + expect(Notification.of_type(type_name_two).count).to eq(1) + expect(Notification.of_type(type_name_three).count).to eq(0) + end + end + describe '.emit' do context 'successful' do let(:vm) { FactoryGirl.create(:vm, :tenant => tenant) } @@ -96,4 +113,22 @@ ) end end + + describe "#seen_by_all_recipients?" do + let(:notification) { FactoryGirl.create(:notification, :initiator => user) } + + it "is false if a user has not seen the notification" do + expect(notification.seen_by_all_recipients?).to be_falsey + end + + it "is true when all recipients have seen the notification" do + notification.notification_recipients.each { |r| r.update_attributes(:seen => true) } + expect(notification.seen_by_all_recipients?).to be_truthy + end + + it "is true when there are no recipients" do + notification.notification_recipients.destroy_all + expect(notification.seen_by_all_recipients?).to be_truthy + end + end end