Customizable chart top values (dropdown choices)

[miyamotoh]

Depends on

• externalizing GRAPH_MAX_COUNT to settings.yaml manageiq#22632

Proposing to make the "chart top values" (currently hardcoded to 10) externalized to settings.yaml and thus customizable. It'd read the below setting, if present, and use it. If this is good, I'm happy to create a PR for the Core with the default value (10).

:reporting
  :chart_top_values: 20

This controls how long/big this dropdown goes;

[miyamotoh]

Noticed that the graph field in miq_reports table shows the chosen value as a text, instead of an integer. Put this in WIP until a fix is found...

[miyamotoh]

I added a .to_i upon reading in the form selected value, which seems to be a sensible thing to do, though I don't know how it was "working" without before. Maybe .to_i is also needed for GRAPH_MAX_COUNT initialization, too, and now my local server/db is somehow plagued??

Anyway, I'm ready for bringing this forward, and thus will try to remove the Draft state (like last night)...

[Fryguy]

This is happening at constant time, so if the user changes it you'd have to reboot the worker/server. Instead of a constant, this should probably move to a method to allow the runtime lookup.

Additionally, we try to avoid having "hidden" values in the settings. Instead, it's probably preferable to only use settings, and then just set the value in the settings to 10.

[miyamotoh]

@Fryguy I'm afraid that I'm not versed to come up with the best way to do what you are suggesting, but PTAL at my latest force-push. It appears to achieve the dynamic behavior on UI.

In the meantime, I opened this PR on Core to support this logic.

[Fryguy]

I think this needs to be

Suggested change

	@edit[:new][:graph_count] = chart_top_values
	@edit[:new][:graph_count] = self.class.chart_top_values

[miyamotoh]

In my tests, chart_top_values didn't work, nor did self.class.chart_top_values. The only one I got working is ReportController::Reports::Editor.chart_top_values, which looks odd right in the module defining file itself, but... anyway, PTAL at the latest force-push.

[Fryguy]

Please also include a default value hardcoded in the settings and set to 10 (I think it needs to be a core PR and then make this PR dependent on that one)

[Fryguy]

@miyamotoh Just for my clarity, what does this value represent? Is this a dashboard top 10 thing only, or is this any chart report? If the former, I wonder if we should more clearly name the option :dashboard_chart_top_values, or something. On that note, does this also affect dashboard tables (that is, things that aren't charts)?

[miyamotoh]

@Fryguy PTAL the opening description of this PR, as I added a little screenshot.

[Fryguy]

ohhh thanks...that's much clearer now

[Fryguy]

updated the OP to show it depends on ManageIQ/manageiq#22632

[Fryguy]

Weird question. If someone picks a large number, say 10, and saves the report, then an admin changes the settings to a smaller number, say 5, then what happens to existing reports. I'm ok if they continue to show 10, but we need to make sure the report editor doesn't blow up.

[miyamotoh]

It jumps back to "3." If I set the max to 2, you'd get the below, can't change the previous selection, Save button even grayed out. Even if you get the Save button enabled by changing the display-other selection, the subsequent Save won't change the top-values in the DB.

[jeffibm]

Hey @miyamotoh , could you please check with the failing specs..

[miq-bot]

Checked commit miyamotoh@93fc67c with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 🏆

[miyamotoh]

@jeffibm PTAL at the latest force-push. Thanks!