Skip to content

Commit

Permalink
Backup subject's name in case subject is removed
Browse files Browse the repository at this point in the history
Fixes a bug where notifications look like the following if the subject
vm has been removed:

'Virtual Machine %{subject} has been provisioned.'

https://bugzilla.redhat.com/show_bug.cgi?id=1469372
  • Loading branch information
jrafanie committed Aug 17, 2018
1 parent 1ef29e6 commit 3f10f63
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
8 changes: 8 additions & 0 deletions app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ class Notification < ApplicationRecord
# Do not emit notifications if they are not enabled for the server
after_commit :emit_message, :on => :create

before_save :backup_subject_name

serialize :options, Hash
default_value_for(:options) { Hash.new }

Expand Down Expand Up @@ -40,6 +42,12 @@ def to_h
}
end

def backup_subject_name
return unless subject
backup_name = (subject.try(:name) || subject.try(:description))
self.options[:subject] = backup_name if backup_name
end

def seen_by_all_recipients?
notification_recipients.unseen.empty?
end
Expand Down
38 changes: 38 additions & 0 deletions spec/models/notification_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,44 @@
)
)
end

context "subject text" do
let(:vm) { FactoryGirl.create(:vm, :tenant => tenant) }
subject { Notification.create(:type => :vm_snapshot_failure, :subject => vm, :options => {:error => "oops", :snapshot_op => "create"}) }

it "stuffs subject into options hash in case the subject is destroyed" do
expect(subject.options[:subject]).to eql(subject.subject.name)
end

it "detects a rename of the subject" do
subject
vm_name = "#{vm.name}_1"
vm.update(:name => vm_name)

note = Notification.first
expect(note.to_h.fetch_path(:bindings, :subject, :text)).to eql(vm_name)
end

it "retains name incase subject is destroyed" do
subject
vm_name = vm.name
vm.destroy

note = Notification.first
expect(note.to_h.fetch_path(:bindings, :subject, :text)).to eql(vm_name)
end

it "doesn't detect subject rename if subject is destroyed" do
subject
original_name = vm.name
vm_name = "#{vm.name}_1"
vm.update(:name => vm_name)
vm.destroy

note = Notification.first
expect(note.to_h.fetch_path(:bindings, :subject, :text)).to eql(original_name)
end
end
end

describe "#seen_by_all_recipients?" do
Expand Down

0 comments on commit 3f10f63

Please sign in to comment.