From a0c53f2b61b67476e2b1b72bed5f26a345264211 Mon Sep 17 00:00:00 2001
From: Cyprien P <cyprien.pindat@gmail.com>
Date: Thu, 30 Sep 2021 09:41:36 +0200
Subject: [PATCH] (core) Fix chart when x axis is set to a choice list column

Summary:
Bug reported by user: https://gristlabs.getgrist.com/doc/check-ins/p/3#a1.s7.r1183.c19p

Setting x axis to a column of type ChoiceList was breaking chart.

This diff fixes that by splitting the record into several records: one for each choice.

`test/nbrowser/ChartView1.ts` was becoming too big and long to run, so this diff introduces `test/nbrowser/ChartView2.ts` to add more test and `test/nbrowser/chartViewTestUtils.ts` to put all utilities or testing charts.

Test Plan: Adds new test.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3041
---
 app/client/components/ChartView.ts | 24 ++++++++++++++++++++++--
 app/client/lib/chartUtil.ts        | 29 +++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts
index 52935cba..81ab27e0 100644
--- a/app/client/components/ChartView.ts
+++ b/app/client/components/ChartView.ts
@@ -1,6 +1,6 @@
 import * as BaseView from 'app/client/components/BaseView';
 import {GristDoc} from 'app/client/components/GristDoc';
-import {sortByXValues, uniqXValues} from 'app/client/lib/chartUtil';
+import {sortByXValues, splitValuesByIndex, uniqXValues} from 'app/client/lib/chartUtil';
 import {Delay} from 'app/client/lib/Delay';
 import {Disposable} from 'app/client/lib/dispose';
 import {fromKoSave} from 'app/client/lib/fromKoSave';
@@ -18,6 +18,7 @@ import {cssDragger} from 'app/client/ui2018/draggableList';
 import {icon} from 'app/client/ui2018/icons';
 import {linkSelect, menu, menuItem, select} from 'app/client/ui2018/menus';
 import {nativeCompare} from 'app/common/gutil';
+import {decodeObject} from 'app/plugin/objtypes';
 import {Events as BackboneEvents} from 'backbone';
 import {Computed, dom, DomElementArg, fromKo, Disposable as GrainJSDisposable, IOption,
   makeTestId, Observable, styled} from 'grainjs';
@@ -94,6 +95,11 @@ function dateGetter(getter: RowPropGetter): RowPropGetter {
   };
 }
 
+
+// List of column types whose values are encoded has list, ie: ['L', 'foo', ...]. Such values
+// require special treatment to show correctly in charts.
+const LIST_TYPES = ['ChoiceList', 'RefList'];
+
 /**
  * ChartView component displays created charts.
  */
@@ -163,7 +169,7 @@ export class ChartView extends Disposable {
 
     const fields: ViewFieldRec[] = this.viewSection.viewFields().all();
     const rowIds: number[] = this.sortedRows.getKoArray().peek() as number[];
-    const series: Series[] = fields.map((field) => {
+    let series: Series[] = fields.map((field) => {
       // Use the colId of the displayCol, which may be different in case of Reference columns.
       const colId: string = field.displayColModel.peek().colId.peek();
       const getter = this.tableModel.tableData.getRowPropFunc(colId) as RowPropGetter;
@@ -175,6 +181,20 @@ export class ChartView extends Disposable {
       };
     });
 
+    const startIndexForYAxis = this._options.prop('multiseries') ? 2 : 1;
+    for (let i = 0; i < series.length; ++i) {
+      if (i < fields.length && LIST_TYPES.includes(fields[i].column.peek().pureType.peek())) {
+        if (i < startIndexForYAxis) {
+          // For x-axis and group column data, split series we should split records.
+          series = splitValuesByIndex(series, i);
+        } else {
+          // For all y-axis, it's not sure what would be a sensible representation for choice list,
+          // simply stringify choice list values seems reasonable.
+          series[i].values = series[i].values.map((v) => String(decodeObject(v as any)));
+        }
+      }
+    }
+
     const options: ChartOptions = this._options.peek() || {};
     let plotData: PlotData = {data: []};
 
diff --git a/app/client/lib/chartUtil.ts b/app/client/lib/chartUtil.ts
index 130841ca..b18c0ede 100644
--- a/app/client/lib/chartUtil.ts
+++ b/app/client/lib/chartUtil.ts
@@ -1,7 +1,9 @@
 import {typedCompare} from 'app/common/SortFunc';
+import {decodeObject} from 'app/plugin/objtypes';
 import {Datum} from 'plotly.js';
 import range = require('lodash/range');
 import uniqBy = require('lodash/uniqBy');
+import flatten = require('lodash/flatten');
 
 /**
  * Sort all values in a list of series according to the values in the first one.
@@ -31,3 +33,30 @@ export function uniqXValues<T extends {values: Datum[]}>(series: Array<T>): Arra
     values: line.values.filter((_val, i) => indexToKeep.has(i))
   }));
 }
+
+// Creates new version of series that split any entry whose value in the first series is a list into
+// multiple entries, one entry for each list's item. For all other series, newly created entries have
+// the same value as the original.
+export function splitValues<T extends {values: Datum[]}>(series: Array<T>): Array<T> {
+  return splitValuesByIndex(series, 0);
+}
+
+// This method is like splitValues except it splits according to the values of the series at position index.
+export function splitValuesByIndex<T extends {values: Datum[]}>(series: Array<T>, index: number): Array<T> {
+  const decoded = (series[index].values as any[]).map(decodeObject);
+
+  return series.map((s, si) => {
+    if (si === index) {
+      return {...series[index], values: flatten(decoded)};
+    }
+    let values: Datum[] = [];
+    for (const [i, splitByValue] of decoded.entries()) {
+      if (Array.isArray(splitByValue)) {
+        values = values.concat(Array(splitByValue.length).fill(s.values[i]));
+      } else {
+        values.push(s.values[i]);
+      }
+    }
+    return {...s, values};
+  });
+}