Skip to content

Commit

Permalink
Prefer valid_report_column? and contains_records? from MiqReportResult
Browse files Browse the repository at this point in the history
https://bugzilla.redhat.com/show_bug.cgi?id=1590908

These are new methods added in:

ManageIQ/manageiq#17590

And are far more efficient for the use cases of ChargebackController and
ReportController::SavedReports.
  • Loading branch information
NickLaMuro committed Jun 19, 2018
1 parent e3c4096 commit ff7ee24
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 15 deletions.
5 changes: 2 additions & 3 deletions app/controllers/chargeback_controller.rb
Expand Up @@ -345,17 +345,16 @@ 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
self.x_node = "reports-#{rep.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
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/report_controller/saved_reports.rb
Expand Up @@ -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)
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/controllers/chargeback_controller_spec.rb
Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions spec/controllers/miq_report_controller/trees_spec.rb
Expand Up @@ -22,25 +22,29 @@
end

it 'renders show' do
session[:settings] = {:perpage => {:reports => 20}}
controller.instance_variable_set(:@html, "<h1>Test</h1>")
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)
FactoryGirl.create(:binary_blob_part,
: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
Expand Down
10 changes: 8 additions & 2 deletions spec/controllers/report_controller_spec.rb
Expand Up @@ -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

Expand All @@ -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
Expand Down

0 comments on commit ff7ee24

Please sign in to comment.