From 80d36848651a08c22c20f3a2f2c23fcbd5cd8604 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Ple=C5=A1ko?= Date: Thu, 27 Sep 2018 15:15:42 +0200 Subject: [PATCH] Show correct VMs upon Service retirement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this commit we make sure correct VMs are shown as part of service retirement request. Talking about "Affected VMs" section. Prior to this commit there was always only a single VM displayed in there, and it was not the right one. Happened to see AWS EC2 VM on a vCloud vApp retirement request details! The reason for all this was that ``` @miq_request.options[:src_ids] ``` were treated as it's a list of VM ids whereas it infact was a list of Service ids. If we were lucky enough, some Service id collided with random VM id and BUUUM, wrong VM was shown. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1632239 Signed-off-by: Miha Pleško --- .../miq_request_methods.rb | 5 ++ app/views/miq_request/_request.html.haml | 2 + .../_service_retire_show.html.haml | 7 +++ .../miq_request_controller_spec.rb | 55 +++++++++++++++++++ 4 files changed, 69 insertions(+) create mode 100644 app/views/miq_request/_service_retire_show.html.haml diff --git a/app/controllers/application_controller/miq_request_methods.rb b/app/controllers/application_controller/miq_request_methods.rb index 754cf565900..4c7cf7e627d 100644 --- a/app/controllers/application_controller/miq_request_methods.rb +++ b/app/controllers/application_controller/miq_request_methods.rb @@ -793,6 +793,11 @@ def prov_set_show_vars end @options[:wf] = options[:wf] end + elsif @miq_request.resource_type == 'ServiceRetireRequest' + @view, @pages = nil + if (service_id = @miq_request.options[:src_ids].first) && (service = Service.find_by(:id => service_id)) && Rbac.filtered_object(service) + @view, @pages = get_view(Vm, :parent => service, :view_suffix => 'OrchestrationStackRetireRequest') + end else @options = @miq_request.options @options[:memory], @options[:mem_typ] = reconfigure_calculations(@options[:vm_memory][0]) if @options[:vm_memory] diff --git a/app/views/miq_request/_request.html.haml b/app/views/miq_request/_request.html.haml index b23c80d1f8c..932d5cdaca5 100644 --- a/app/views/miq_request/_request.html.haml +++ b/app/views/miq_request/_request.html.haml @@ -123,6 +123,8 @@ = render :partial => "st_prov_show" - elsif @miq_request.type == 'ServiceReconfigureRequest' = render :partial => "service_reconfigure_show" + - elsif @miq_request.type == "ServiceRetireRequest" + = render :partial => "service_retire_show" - elsif @miq_request.type == "AutomationRequest" = render :partial => "ae_prov_show" - else diff --git a/app/views/miq_request/_service_retire_show.html.haml b/app/views/miq_request/_service_retire_show.html.haml new file mode 100644 index 00000000000..e3aa10c6bda --- /dev/null +++ b/app/views/miq_request/_service_retire_show.html.haml @@ -0,0 +1,7 @@ +#main_div + %h3 + = _("Affected VMs") + - if @view + - @embedded = true + - @gtl_type = "list" + = render :partial => "layouts/gtl", :locals => {:view => @view, :no_flash_div => true} diff --git a/spec/controllers/miq_request_controller_spec.rb b/spec/controllers/miq_request_controller_spec.rb index a794b1c69bc..a49491530bc 100644 --- a/spec/controllers/miq_request_controller_spec.rb +++ b/spec/controllers/miq_request_controller_spec.rb @@ -266,6 +266,61 @@ end end + context 'showing details of a retirement task' do + before do + @user = stub_admin + EvmSpecHelper.create_guid_miq_server_zone + end + + let(:vm1) { FactoryGirl.create(:vm_cloud) } + let(:vm2) { FactoryGirl.create(:vm_cloud) } # not part of the stack + let(:stack) { FactoryGirl.create(:orchestration_stack).tap { |stack| stack.direct_vms = [vm1] } } + let(:service) { FactoryGirl.create(:service_orchestration).tap { |service| service.add_resource!(stack) } } + let(:request) do + FactoryGirl.create(:service_retire_request, + :type => 'ServiceRetireRequest', + :requester => @user, + :options => {:src_ids => [service.id]}) + end + # let(:request) { FactoryGirl.create(:miq_service_retirement_request, :options => {:src_ids => [service.id]}) } + + let(:payload) { { :model_name => 'Vm', :parent_id => service.id.to_s, additional_key => additional_val } } + let(:additional_val) do + { + :model => 'Vm', + :parent_id => service.id.to_s, + :parent_class_name => 'ServiceOrchestration', + :view_suffix => 'OrchestrationStackRetireRequest', + :display => 'main', + :gtl_type => 'list' + } + end + + context 'angular for grid with affected VMs is properly initialized' do + let(:additional_key) { :report_data_additional_options } + it do + expect(controller).to receive(:prov_set_show_vars).once.and_call_original + + # Verify Rails correctly initializes Angular which will then perform POST /report_data to fetch the VMs. + expect_any_instance_of(GtlHelper).to receive(:render_gtl).with match_gtl_options(**payload) + + get :show, :params => {:id => request.id} + expect(response.status).to eq(200) + end + end + + context 'angular for grid with affected VMs gets the correct VMs with XHR call' do + let(:additional_key) { :additional_options } + it do + post :report_data, :params => payload + expect(response.status).to eq(200) + + # Verify Angular got correct VMs when sending the payload as set by previous test. + expect(JSON.parse(response.body).dig('data', 'rows').map { |row| row['id'] }).to eq([vm1.id.to_s]) + end + end + end + context "#edit_button" do before do stub_user(:features => :all)