Fix units formating for grouped charts #382
Conversation
lib/report_formatter/c3.rb
Outdated
| @@ -91,7 +91,8 @@ def build_document_header | |||
| unless mri.graph[:type] == 'Donut' || mri.graph[:type] == 'Pie' | |||
| mri.chart[:legend] = {:position => 'bottom'} | |||
| end | |||
| format, options = javascript_format(mri.graph[:columns][0], nil) | |||
| column = mri.performance&.dig(:group_by_category) ? mri.graph[:columns][0].split(/_+/)[0..-2].join('_') : mri.graph[:columns][0] | |||
There was a problem hiding this comment.
Don't use &. yet, for the same reason as here: ManageIQ/manageiq#10663
column = mri.performance.try(:dig, :group_by_category) ?
mri.graph[:columns][0].split(/_+/)[0..-2].join('_') :
mri.graph[:columns][0]
There was a problem hiding this comment.
don't use dig for the same reason
|
Maybe a simple test? I believe I have added a way to test the charts in the past so it should be possible to add a test for this particular feature. |
|
A small thing: my physics teacher used to say that if you have a chart X/Y vales with units, then you don't need the same unit in parentheses on the axis' label, or in this case in the chart's title. |
PanSpagetka
force-pushed
the
fix-units-in-CU-grouped
branch
from
acc4079
to
69dfff4
Compare
lib/report_formatter/c3.rb
Outdated
| @@ -91,7 +91,10 @@ def build_document_header | |||
| unless mri.graph[:type] == 'Donut' || mri.graph[:type] == 'Pie' | |||
| mri.chart[:legend] = {:position => 'bottom'} | |||
| end | |||
| format, options = javascript_format(mri.graph[:columns][0], nil) | |||
|
|
|||
| binding.pry | |||
|
The specs patch could use a less generic comment ;-) |
|
@PanSpagetka Please fix the rubocop warnings. |
PanSpagetka
force-pushed
the
fix-units-in-CU-grouped
branch
from
8219974
to
02b98dd
Compare
PanSpagetka
force-pushed
the
fix-units-in-CU-grouped
branch
from
02b98dd
to
d1b648b
Compare
|
Checked commits PanSpagetka/manageiq-ui-classic@2ccc5a6~...d1b648b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 lib/report_formatter/c3.rb
|
|
Euwe backport (to manageiq repo) details: |
Following PR for #335. Formatting function was not added to charts because of name of columns.
Specs depend on ManageIQ/manageiq#13998
Screenshots
Before:
After:
Links
https://bugzilla.redhat.com/show_bug.cgi?id=1367560