Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Manage policies and Check compliance buttons for Container Images and others #4206

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 74 additions & 21 deletions app/controllers/ems_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,37 +307,57 @@ def button
end
else
@refresh_div = "main_div" # Default div for button.rjs to refresh
redirect_to :action => "new" if params[:pressed] == "new"
deleteemss if params[:pressed] == "#{table_name}_delete"
refresh_or_capture_emss("refresh_ems", _("Refresh")) if params[:pressed] == "#{table_name}_refresh"
refresh_or_capture_emss("capture_ems", _("Capture Metrics")) if params[:pressed] == "#{table_name}_capture_metrics"
pause_or_resume_emss(:pause => true) if params[:pressed] == "#{table_name}_pause"
pause_or_resume_emss(:resume => true) if params[:pressed] == "#{table_name}_resume"
tag(model) if params[:pressed] == "#{table_name}_tag"

# Edit Tags for Middleware Manager Relationship pages
tag(@display.camelize.singularize) if @display && @display != 'main' &&
params[:pressed] == "#{@display.singularize}_tag"
assign_policies(model) if params[:pressed] == "#{table_name}_protect"
check_compliance(model) if params[:pressed] == "#{table_name}_check_compliance"
edit_record if params[:pressed] == "#{table_name}_edit"
if params[:pressed] == "#{table_name}_timeline"

case params[:pressed]
when 'new'
redirect_to(:action => 'new')
when "#{table_name}_delete"
deleteemss
when "#{table_name}_refresh"
refresh_or_capture_emss("refresh_ems", _("Refresh"))
when "#{table_name}_capture_metrics"
refresh_or_capture_emss("capture_ems", _("Capture Metrics"))
when "#{table_name}_pause"
pause_or_resume_emss(:pause => true)
when "#{table_name}_resume"
pause_or_resume_emss(:resume => true)
when "#{table_name}_tag"
tag(model)
when "#{table_name}_protect"
assign_policies(model)
when "#{table_name}_check_compliance"
check_compliance(model)
when "#{table_name}_edit"
edit_record
when "#{table_name}_timeline"
timeline_pressed
return
end
if params[:pressed] == "#{table_name}_perf"
when "#{table_name}_perf"
performance_pressed
return
end
if params[:pressed] == "#{table_name}_ad_hoc_metrics"
when "#{table_name}_ad_hoc_metrics"
ad_hoc_metrics_pressed
return
end
if params[:pressed] == "refresh_server_summary"
when 'refresh_server_summary'
javascript_redirect :back
return
end

if @display && @display != 'main'
model_class = @display.camelize.singularize.safe_constantize
display_s = @display.singularize

case params[:pressed]
when "#{display_s}_tag"
tag(model_class)
when "#{display_s}_protect"
assign_policies(model_class)
when "#{display_s}_check_compliance"
check_compliance_nested(model_class)
return
end
end

if params[:pressed] == "ems_cloud_recheck_auth_status" ||
params[:pressed] == "ems_infra_recheck_auth_status" ||
params[:pressed] == "ems_physical_infra_recheck_auth_status" ||
Expand Down Expand Up @@ -456,6 +476,39 @@ def check_compliance(model)

private ############################

# Check compliance of Last Known Configuration for items displayed in nested lists
def check_compliance_nested(model)
assert_privileges("#{model.name.underscore}_check_compliance")
ids = find_checked_ids_with_rbac(model)

if ids.empty?
add_flash(_("No %{model} were selected for %{task}") % {:model => ui_lookup(:models => model.to_s),
:task => "Compliance Check"}, :error)
else
process_check_compliance(model, ids)
end

show_list
ids.count
end

def process_check_compliance(model, ids)
model.where(:id => ids).order("lower(name)").each do |entity|
begin
entity.check_compliance
rescue StandardError => bang
add_flash(_("%{model} \"%{name}\": Error during 'Check Compliance': %{error}") %
{:model => ui_lookup(:model => model.to_s),
:name => entity.name,
:error => bang.message},
:error) # Push msg and error flag
else
add_flash(_("\"%{record}\": Compliance check successfully initiated") % {:record => entity.name})
end
end
javascript_flash
end

# Set form variables for edit
def set_form_vars
form_instance_vars
Expand Down
59 changes: 55 additions & 4 deletions spec/controllers/ems_common_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ def test_creating(emstype)
end
end

context "#update" do
describe "#update" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 😍 😍

context "updates provider with new token" do
before :each do
before do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 😍 😍

stub_user(:features => :all)
session[:edit] = assigns(:edit)
end
Expand Down Expand Up @@ -202,8 +202,8 @@ def test_setting_few_fields
end
end

context "#button" do
before(:each) do
describe "#button" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 😍 😍

before do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 😍 😍

stub_user(:features => :all)
EvmSpecHelper.create_guid_miq_server_zone
end
Expand Down Expand Up @@ -232,6 +232,57 @@ def test_setting_few_fields
post :button, :params => { :pressed => "vm_edit", :format => :js, "check_#{vm.id}" => "1" }
expect(controller.send(:flash_errors?)).not_to be_truthy
end

context 'displaying nested lists from summary page of container provider' do
let(:provider) { ManageIQ::Providers::ContainerManager.new }

before do
allow(controller).to receive(:javascript_redirect)
allow(controller).to receive(:performed?).and_return(true)
controller.instance_variable_set(:@display, display)
controller.instance_variable_set(:@_params, :pressed => press, :miq_grid_checks => item.id.to_s, :id => provider.id)
controller.instance_variable_set(:@breadcrumbs, [])
end

{
'container_image' => 'Container Images',
'container_replicator' => 'Container Replicators',
'container_node' => 'Container Nodes',
'container_group' => 'Container Pods'
}.each do |display_s, items|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might not need the hash values, just use an array and then call humanize.pluralize on each item.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But.. what about Container Pods?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need it in the testing, or just in the describe/context/it strings?

Copy link
Author

@hstastna hstastna Jun 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I removed the hash values and replaced them by an array of four items, I don't know how to set the string for context block properly and nicely (to see there for example 'tagging selected Container Pods' and not 'tagging selected Container Groups'). It's only for this there - for info what is being tested (context).

I consider this solution as the easiest and the most readable (at least for now). If something went wrong in some of the specs, we can easily recognize, what went wrong: we will not need to figure out that container_group is related to Container Pods because it's already there. And I need all of the contexts there to set params[:pressed] properly for each situation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

context "displaying #{items}" do
let(:item) { FactoryGirl.create(display_s.to_sym) }
let(:display) { display_s.pluralize }

context "tagging selected #{items}" do
let(:press) { "#{display_s}_tag" }

it 'calls tag method with proper model class' do
expect(controller).to receive(:tag).with(display_s.classify.safe_constantize)
controller.send(:button)
end
end

context "managing policies of selected #{items}" do
let(:press) { "#{display_s}_protect" }

it 'calls assign_policies method with proper model class' do
expect(controller).to receive(:assign_policies).with(display_s.classify.safe_constantize)
controller.send(:button)
end
end

context "checking compliance of selected #{items}" do
let(:press) { "#{display_s}_check_compliance" }

it 'calls check_compliance_nested method with proper model class' do
expect(controller).to receive(:check_compliance_nested).with(display_s.classify.safe_constantize)
controller.send(:button)
end
end
end
end
end
end

describe "#download_summary_pdf" do
Expand Down