From ff7ee24f4d50b38e96d5c0eccd8ca7398d6d310e Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 14 Jun 2018 16:43:10 -0500 Subject: [PATCH] Prefer valid_report_column? and contains_records? from MiqReportResult https://bugzilla.redhat.com/show_bug.cgi?id=1590908 These are new methods added in: https://github.com/ManageIQ/manageiq/pull/17590 And are far more efficient for the use cases of ChargebackController and ReportController::SavedReports. --- app/controllers/chargeback_controller.rb | 5 ++--- .../report_controller/saved_reports.rb | 5 ++--- spec/controllers/chargeback_controller_spec.rb | 3 ++- .../miq_report_controller/trees_spec.rb | 16 ++++++++++------ spec/controllers/report_controller_spec.rb | 10 ++++++++-- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/app/controllers/chargeback_controller.rb b/app/controllers/chargeback_controller.rb index 1e22d52c0a0..a7cac960d8b 100644 --- a/app/controllers/chargeback_controller.rb +++ b/app/controllers/chargeback_controller.rb @@ -345,9 +345,8 @@ def cb_rpts_fetch_saved_report(id) @report_result_id = session[:report_result_id] = rr.id session[:report_result_runtime] = rr.last_run_on if rr.status.downcase == "complete" - @report = rr.report_results session[:rpt_task_id] = nil - if @report.blank? + unless rr.valid_report_column? @saved_reports = cb_rpts_get_all_reps(rr.miq_report_id.to_s) rep = MiqReport.find_by_id(rr.miq_report_id) if x_active_tree == :cb_reports_tree @@ -355,7 +354,7 @@ def cb_rpts_fetch_saved_report(id) end return else - if @report.contains_records? + if rr.contains_records? @html = report_first_page(rr) # Get the first page of the results unless @report.graph.blank? @render_chart = true diff --git a/app/controllers/report_controller/saved_reports.rb b/app/controllers/report_controller/saved_reports.rb index 4e1ab8e02af..ff615a349fd 100644 --- a/app/controllers/report_controller/saved_reports.rb +++ b/app/controllers/report_controller/saved_reports.rb @@ -35,10 +35,9 @@ def fetch_saved_report(id) return unless rr.status.downcase == "complete" - @report = rr.report_results session[:rpt_task_id] = nil - if @report.blank? + unless rr.valid_report_column? add_flash(_("Saved Report \"%{time}\" not found, Schedule may have failed") % {:time => format_timezone(rr.created_on, Time.zone, "gtl")}, :error) @@ -56,7 +55,7 @@ def fetch_saved_report(id) return end - unless @report.contains_records? + unless rr.contains_records? add_flash(_("No records found for this report"), :warning) return end diff --git a/spec/controllers/chargeback_controller_spec.rb b/spec/controllers/chargeback_controller_spec.rb index 7a44f8012f6..30b96a27435 100644 --- a/spec/controllers/chargeback_controller_spec.rb +++ b/spec/controllers/chargeback_controller_spec.rb @@ -581,7 +581,8 @@ def convert_chargeback_rate_to_hash(rate) miq_task.state_finished miq_report_result.report = chargeback_report.to_hash.merge(:extras=> {:total_html_rows => 100}) miq_report_result.save - allow(controller).to receive(:report_first_page) + controller.instance_variable_set(:@sb, {}) + controller.instance_variable_set(:@settings, :perpage => { :reports => 20 }) end it "fetch existing report" do diff --git a/spec/controllers/miq_report_controller/trees_spec.rb b/spec/controllers/miq_report_controller/trees_spec.rb index c9a182284d5..e1c1142aedd 100644 --- a/spec/controllers/miq_report_controller/trees_spec.rb +++ b/spec/controllers/miq_report_controller/trees_spec.rb @@ -22,17 +22,21 @@ end it 'renders show' do + session[:settings] = {:perpage => {:reports => 20}} controller.instance_variable_set(:@html, "

Test

") - allow(controller).to receive(:report_first_page) + report = FactoryGirl.create(:miq_report_with_results) - allow(report).to receive(:contains_records?).and_return(true) task = FactoryGirl.create(:miq_task) task.update_attributes(:state => "Finished") task.reload - report_result = FactoryGirl.create(:miq_report_result, + + report_result = FactoryGirl.build(:miq_report_result, :miq_group_id => user.current_group.id, - :miq_task_id => task.id) - allow_any_instance_of(MiqReportResult).to receive(:report_results).and_return(report) + :miq_task_id => task.id, + :miq_report => report) + report_result.report = report.to_hash.merge(:extras=> {:total_html_rows => 100}) + report_result.save + binary_blob = FactoryGirl.create(:binary_blob, :resource_type => "MiqReportResult", :resource_id => report_result.id) @@ -40,7 +44,7 @@ :data => "--- Quota \xE2\x80\x93 Max CPUs\n...\n", :binary_blob_id => binary_blob.id) - post :tree_select, :params => { :id => "rr-#{report_result.id}", :format => :js, :accord => 'savedreports' } + post :tree_select, :params => { :id => "rr-#{report_result.id}", :ppsetting => 20, :format => :js, :accord => 'savedreports' } expect(response).to render_template('shared/_report_chart_and_html') end end diff --git a/spec/controllers/report_controller_spec.rb b/spec/controllers/report_controller_spec.rb index 2ce0fe61243..c31acb75d24 100644 --- a/spec/controllers/report_controller_spec.rb +++ b/spec/controllers/report_controller_spec.rb @@ -1296,6 +1296,8 @@ context "User2 generates report under Group1" do before :each do + os = OperatingSystem.create(:name => "RHEL 7", :product_name => "RHEL7") + FactoryGirl.create(:vm_vmware, :operating_system => os) @rpt = create_and_generate_report_for_user("Vendor and Guest OS", "User2") end @@ -1320,12 +1322,16 @@ controller.instance_variable_set(:@_params, :id => report_result_id, :controller => "report", :action => "explorer") controller.instance_variable_set(:@sb, :last_savedreports_id => nil) + controller.instance_variable_set(:@settings, :perpage => { :reports => 20 }) allow(controller).to receive(:get_all_reps) controller.send(:show_saved_report) + fetched_report_result_id = controller.instance_variable_get(:@report_result_id) expect(fetched_report_result_id).to eq(@rpt.miq_report_results.first.id) - fetched_report = controller.instance_variable_get(:@report) - expect(fetched_report.id).to eq(@rpt.id) + + fetched_report = controller.instance_variable_get(:@report) + fetched_report.id = @rpt.id # Reports serialized into the report column don't have ids + expect(fetched_report).to eq(@rpt) end end end