Skip to content

Commit

Permalink
Fix current/active server deletability validation
Browse files Browse the repository at this point in the history
https://bugzilla.redhat.com/show_bug.cgi?id=1575077

Note, the UI will need to check server.errors after attempting to
destroy the record to see any errors.
  • Loading branch information
jrafanie committed May 7, 2018
1 parent bf52707 commit 7040201
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
11 changes: 7 additions & 4 deletions app/models/miq_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,7 @@ def self.check_migrations_up_to_date

def validate_is_deleteable
unless self.is_deleteable?
_log.error(@error_message)
@error_message = nil
_log.error(@errors.full_messages)
throw :abort
end
end
Expand Down Expand Up @@ -492,13 +491,17 @@ def is_recently_active?

def is_deleteable?
if self.is_local?
@error_message = N_("Cannot delete currently used %{log_message}") % {:log_message => format_short_log_msg}
message = N_("Cannot delete currently used %{log_message}") % {:log_message => format_short_log_msg}
@errors ||= ActiveModel::Errors.new(self)
@errors.add(:base, message)
return false
end
return true if self.stopped?

if is_recently_active?
@error_message = N_("Cannot delete recently active %{log_message}") % {:log_message => format_short_log_msg}
message = N_("Cannot delete recently active %{log_message}") % {:log_message => format_short_log_msg}
@errors ||= ActiveModel::Errors.new(self)
@errors.add(:base, message)
return false
end

Expand Down
17 changes: 17 additions & 0 deletions spec/models/miq_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,23 @@
end
end

context "validate_is_deleteable before destroying" do
it "prevents deleting the current server" do
allow(@miq_server).to receive(:is_local?).and_return(true)
@miq_server.destroy

expect(@miq_server.errors.full_messages.first).to match(/current/)
end

it "prevents deleting recently active server" do
allow(@miq_server).to receive(:is_local?).and_return(false)
@miq_server.last_heartbeat = 2.minutes.ago.utc
@miq_server.destroy

expect(@miq_server.errors.full_messages.first).to match(/recently/)
end
end

context "#ntp_reload_queue" do
let(:queue_cond) { {:method_name => 'ntp_reload', :class_name => 'MiqServer', :instance_id => @miq_server.id, :server_guid => @miq_server.guid, :zone => @miq_server.zone.name} }
let(:message) { MiqQueue.where(queue_cond).first }
Expand Down

0 comments on commit 7040201

Please sign in to comment.