From 9e8e047aae492d9c3bc85be6d88e754b5b646f90 Mon Sep 17 00:00:00 2001
From: Nicholas Fitton <9018929+NickFitton@users.noreply.github.com>
Date: Sat, 7 May 2022 18:12:31 +0100
Subject: [PATCH] fix: Chart display fix (#5154)

---
 src/charts.test.ts | 121 +++++++++++++++++++++++++++++++++++++++++++++
 src/charts.ts      |  23 ++++++---
 2 files changed, 137 insertions(+), 7 deletions(-)
 create mode 100644 src/charts.test.ts

diff --git a/src/charts.test.ts b/src/charts.test.ts
new file mode 100644
index 00000000..5c2cce70
--- /dev/null
+++ b/src/charts.test.ts
@@ -0,0 +1,121 @@
+import {
+  Spreadsheet,
+  tryParseCells,
+  tryParseNumber,
+  VALID_SPREADSHEET,
+} from "./charts";
+
+describe("charts", () => {
+  describe("tryParseNumber", () => {
+    it.each<[string, number]>([
+      ["1", 1],
+      ["0", 0],
+      ["-1", -1],
+      ["0.1", 0.1],
+      [".1", 0.1],
+      ["1.", 1],
+      ["424.", 424],
+      ["$1", 1],
+      ["-.1", -0.1],
+      ["-$1", -1],
+      ["$-1", -1],
+    ])("should correctly identify %s as numbers", (given, expected) => {
+      expect(tryParseNumber(given)).toEqual(expected);
+    });
+
+    it.each<[string]>([["a"], ["$"], ["$a"], ["-$a"]])(
+      "should correctly identify %s as not a number",
+      (given) => {
+        expect(tryParseNumber(given)).toBeNull();
+      },
+    );
+  });
+
+  describe("tryParseCells", () => {
+    it("Successfully parses a spreadsheet", () => {
+      const spreadsheet = [
+        ["time", "value"],
+        ["01:00", "61"],
+        ["02:00", "-60"],
+        ["03:00", "85"],
+        ["04:00", "-67"],
+        ["05:00", "54"],
+        ["06:00", "95"],
+      ];
+
+      const result = tryParseCells(spreadsheet);
+
+      expect(result.type).toBe(VALID_SPREADSHEET);
+
+      const { title, labels, values } = (
+        result as { type: typeof VALID_SPREADSHEET; spreadsheet: Spreadsheet }
+      ).spreadsheet;
+
+      expect(title).toEqual("value");
+      expect(labels).toEqual([
+        "01:00",
+        "02:00",
+        "03:00",
+        "04:00",
+        "05:00",
+        "06:00",
+      ]);
+      expect(values).toEqual([61, -60, 85, -67, 54, 95]);
+    });
+
+    it("Uses the second column as the label if it is not a number", () => {
+      const spreadsheet = [
+        ["time", "value"],
+        ["01:00", "61"],
+        ["02:00", "-60"],
+        ["03:00", "85"],
+        ["04:00", "-67"],
+        ["05:00", "54"],
+        ["06:00", "95"],
+      ];
+
+      const result = tryParseCells(spreadsheet);
+
+      expect(result.type).toBe(VALID_SPREADSHEET);
+
+      const { title, labels, values } = (
+        result as { type: typeof VALID_SPREADSHEET; spreadsheet: Spreadsheet }
+      ).spreadsheet;
+
+      expect(title).toEqual("value");
+      expect(labels).toEqual([
+        "01:00",
+        "02:00",
+        "03:00",
+        "04:00",
+        "05:00",
+        "06:00",
+      ]);
+      expect(values).toEqual([61, -60, 85, -67, 54, 95]);
+    });
+
+    it("treats the first column as labels if both columns are numbers", () => {
+      const spreadsheet = [
+        ["time", "value"],
+        ["01", "61"],
+        ["02", "-60"],
+        ["03", "85"],
+        ["04", "-67"],
+        ["05", "54"],
+        ["06", "95"],
+      ];
+
+      const result = tryParseCells(spreadsheet);
+
+      expect(result.type).toBe(VALID_SPREADSHEET);
+
+      const { title, labels, values } = (
+        result as { type: typeof VALID_SPREADSHEET; spreadsheet: Spreadsheet }
+      ).spreadsheet;
+
+      expect(title).toEqual("value");
+      expect(labels).toEqual(["01", "02", "03", "04", "05", "06"]);
+      expect(values).toEqual([61, -60, 85, -67, 54, 95]);
+    });
+  });
+});
diff --git a/src/charts.ts b/src/charts.ts
index 4e057b9d..5f911638 100644
--- a/src/charts.ts
+++ b/src/charts.ts
@@ -29,18 +29,24 @@ type ParseSpreadsheetResult =
   | { type: typeof NOT_SPREADSHEET; reason: string }
   | { type: typeof VALID_SPREADSHEET; spreadsheet: Spreadsheet };
 
-const tryParseNumber = (s: string): number | null => {
-  const match = /^[$€£¥₩]?([0-9,]+(\.[0-9]+)?)$/.exec(s);
+/**
+ * @private exported for testing
+ */
+export const tryParseNumber = (s: string): number | null => {
+  const match = /^([-+]?)[$€£¥₩]?([-+]?)([\d.,]+)[%]?$/.exec(s);
   if (!match) {
     return null;
   }
-  return parseFloat(match[1].replace(/,/g, ""));
+  return parseFloat(`${(match[1] || match[2]) + match[3]}`.replace(/,/g, ""));
 };
 
 const isNumericColumn = (lines: string[][], columnIndex: number) =>
   lines.slice(1).every((line) => tryParseNumber(line[columnIndex]) !== null);
 
-const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => {
+/**
+ * @private exported for testing
+ */
+export const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => {
   const numCols = cells[0].length;
 
   if (numCols > 2) {
@@ -71,13 +77,16 @@ const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => {
     };
   }
 
-  const valueColumnIndex = isNumericColumn(cells, 0) ? 0 : 1;
+  const labelColumnNumeric = isNumericColumn(cells, 0);
+  const valueColumnNumeric = isNumericColumn(cells, 1);
 
-  if (!isNumericColumn(cells, valueColumnIndex)) {
+  if (!labelColumnNumeric && !valueColumnNumeric) {
     return { type: NOT_SPREADSHEET, reason: "Value is not numeric" };
   }
 
-  const labelColumnIndex = (valueColumnIndex + 1) % 2;
+  const [labelColumnIndex, valueColumnIndex] = valueColumnNumeric
+    ? [0, 1]
+    : [1, 0];
   const hasHeader = tryParseNumber(cells[0][valueColumnIndex]) === null;
   const rows = hasHeader ? cells.slice(1) : cells;