Skip to content

Commit

Permalink
Add save hooks on MiqReportResult to remove groupings
Browse files Browse the repository at this point in the history
https://bugzilla.redhat.com/show_bug.cgi?id=1590908

The `report.extras[:groupings]` on MiqReportResult were being
serialized to the `report` column, and on certain chargeback reports,
can get quite large.  This data is not needed, so we remove it prior to
saving.

The `after_commit` exists in case there is a use for the saved data after
it has been saved and still used within the original instance of the
MiqReportResult for building the result.  Most likely this is overkill,
but for now it is in place as a safety measure.

Also, `after_commit` is used because a `after_save` will cause an
"stack level too deep" error since editing the report value causes the
record to be put into a `dirty` state, and the ActiveRecord internals
don't like that.
  • Loading branch information
NickLaMuro committed Jun 16, 2018
1 parent 511cdae commit 4cd1f28
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
3 changes: 3 additions & 0 deletions app/models/miq_report_result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ class MiqReportResult < ApplicationRecord
end
end

before_save { @_extra_groupings = report.extras.delete(:grouping) if report.kind_of?(MiqReport) }
after_commit { report.extras[:grouping] = @_extra_groupings if report.kind_of?(MiqReport) }

delegate :table, :to => :report_results, :allow_nil => true

def result_set
Expand Down
17 changes: 17 additions & 0 deletions spec/models/miq_report_result_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,23 @@
expect(report_result.report_results.table.data).not_to be_nil
end

it "should not include `extras[:grouing]` in the report column" do
MiqReport.seed_report(name = "Vendor and Guest OS")
rpt = MiqReport.where(:name => name).last
rpt.generate_table(:userid => "test")
report_result = rpt.build_create_results(:userid => "test")

report_result_report = report_result.report
report_result.report.extras[:grouping] = {"extra data" => "not saved" }
report_result.save

result_reload = MiqReportResult.last

expect(report_result.report.kind_of?(MiqReport)).to be_truthy
expect(result_reload.report.extras[:grouping]).to be_nil
expect(report_result.report.extras[:grouping]).to eq({"extra data" => "not saved" })
end

context "for miq_report_result is used different miq_group_id than user's current id" do
before do
MiqUserRole.seed
Expand Down

0 comments on commit 4cd1f28

Please sign in to comment.