Fix for charts with side-by-side grouping (Bar/Area plots)

[aestoltm]

Risk: MEDIUM
Notes on Risk: These changes affect Area/Bar plots due to both of these making use of the stacking feature. Minor changes to trace ordering which affects every plot. The change on trace ordering should be lower risk because the zIndex is not being used due to layering being currently disabled.

Severity: MEDIUM/HIGH
Notes on Severity: Depending on the units of the data and if the data sources are from the same realm or not the plot has misplaced traces, due to wrongful stacking (instead of side-by-side). See attached plots.

Description

Side by side bar charts were broken when comparing time-series data from two different realms.

Before Fix:

After Fix:

Found an additional bug with legend ordering w/ trendline and error bars enabled:
Before:

After:

Motivation and Context

Fixes time-series grouped (side by side) bar charts.

Tests performed

Tested on dev and tested grouped bar charts with data from the same realm to check for regressions

Checklist:

• The pull request description is suitable for a Changelog entry
• The milestone is set correctly on the pull request
• The appropriate labels have been added to the pull request

[aestoltm]

Comment to explain some of the proposed changes:

Note: The term trace and data series are used interchangeably.

classes/DataWarehouse/Visualization/TimeseriesChart.php: There are two changes made within this file. First, updating the offsetgroup to correctly have different offset values for data series within the same yAxis object. The second change made was to use a counter value to determine the data series position within the plot and legend. Previously, the data series location was dependent on $traceIndex which is a data series index of a $yAxisDataObject. There a few issues with this approach. We want to put supplemental data series behind the primary data series within the data series array passed to Plotly (due to Plotly drawing objects based on the array ordering) therefore, $traceIndex does not work because supplement traces will be placed before the next series' primary trace. Additionally, I discovered that multi axis plots had incorrect ordering due to each yAxis having the same ordering due to different $yAxisDataObject's for each yAxis. Therefore, I decided it would be better to use a counter called $legendRank. This name may not be the most accurate due to it also being used for the data series location within both the plot and the legend. However, this counter increments by 2 each trace independent of which $yAxisDataObject the trace is assoicated with. The reason this counter increments by 2 is to accommodate the error and trendline supplemental data series. The error trace will be behind the primary trace and the trendline is two spots behind the primary trace. You can view that without these proposed changes that multi axis plots have incorrect ordering and the legend ranking when error and trendline series are enabled is incorrect. With the proposed changes, these are corrected. Also if you view the ordering of the data array passed to Plotly in PlotlyChartWrapper.js you can see that the order is how I described above (primary trace in front with supplemental behind) with the addition that null traces are at the beginning of the data array because I chose to put them behind every other trace.

classes/DataWarehouse/Access/Usage.php: I removed the previous code which added a counter within the array_walk. I noticed that because $legendRank is incremented by two we can calculate the trace's rank within the Usage tab by dividing by 2. I also changed how we check if we are looking at a primary trace or supplemental trace by adding a metadata flag as @jpwhite4 suggested.

[aestoltm]

Note there is no trendline series for aggregate charts therefore this was supposed to increment by 1.

[jpwhite4]

We've had problems in the past using floats because their representation changes between php and js versions.

Is $rank supposed to be an int? is it guaranteed to be an int?

[aestoltm]

I need to take another look at this. It doesn't seem to be guaranteed to be an int at the moment.

[aestoltm]

$rank here is guaranteed to be an int after taking a closer look. This is only run when the series is a part of a timeseries plot and it is based off a variable we control, $legendRank.

During my investigation I noticed I was incrementing the $legendRank off by one. However, it was not noticeable because any series with a "tied" $legendRank would be broken by the fact that one of the traces came later in the data array passed with the plot. Therefore, I reverted the refactor to the Aggregate chart $legendRank I did last week and I put the timeseries $legendRank to increment by 3 (because we have the primary trace and two possible supplemental traces).

[jpwhite4]

Please change this if to explicitly check for a column bar chart (rather than not == group which will match scatter plots and pie charts too).

[aestoltm]

This should be addressed.

For the trendline code that contained the ternaries, it seems that having the valid check was unnecessary. Nothing breaks when the log scale and std error bars are enabled with the trendline series. I need to start working on a few refactor PRs for Plotly because I saw a few cases where we can refactor barmode (only need for bar charts shouldn't be default on every chart) and allowing log scale with error bars (haven't found any reason why it shouldn't be allowed, I can look back on the git blame and try to find a reason)

[aestoltm]

Correcting the order of the trendline with the other series (primary and error) exposes the out-of-order timeseries bug that was introduced in PR #1899 . Therefore, I may need to get PR #1960 to merged first to fix that issue before merging this PR.