Fix autocomplete error when filtering string column w/null values

* Factor out a function for filtering out null values before feeding them into the autocomplete choices * Add unit testing config to perspective-viewer package * Add a unit test to cover this bugfix
finos · Jul 23, 2019 · 10eeba5 · 10eeba5
1 parent 8360a64
commit 10eeba5
Show file tree

Hide file tree

Showing 9 changed files with 112 additions and 15 deletions.
diff --git a/.gitignore b/.gitignore
@@ -116,6 +116,7 @@ website/i18n/*
 .idea
 cmake-build-debug
 .ipynb_checkpoints
+.python-version
 
 package.json.lerna_backup
 website/static/css/material.dark.css

diff --git a/packages/perspective-viewer-d3fc/package.json b/packages/perspective-viewer-d3fc/package.json
@@ -30,7 +30,7 @@
         "build:webpack:cjs": "webpack --color --config src/config/cjs.config.js",
         "build:webpack:umd": "webpack --color --config src/config/umd.config.js",
         "build:webpack": "npm-run-all build:webpack:cjs build:webpack:umd",
-        "build": "npm-run-all --silent build:babel build:webpack",
+        "build": "npm-run-all --silent build:babel build:webpack:cjs build:webpack:umd",
         "test:build": "cpx \"test/html/*\" dist/umd",
         "watch": "webpack --color --watch --config src/config/d3fc.watch.config.js",
         "test:run": "jest --silent --color 2>&1",

diff --git a/packages/perspective-viewer/package.json b/packages/perspective-viewer/package.json
@@ -22,7 +22,8 @@
         "build": "npm-run-all --silent build:babel build:webpack:cjs build:webpack:umd",
         "watch": "webpack --color --watch --config src/config/view.config.js",
         "test:build": "cpx \"test/html/*\" dist/umd && cpx \"test/csv/*\" dist/umd && cpx \"test/css/*\" dist/umd",
-        "test:run": "jest --silent --color",
+        "test:run": "jest --silent --color 2>&1",
+        "test:unit": "jest --config=test/js/jest.unit.config.js --color --watch",
         "test": "npm-run-all test:build test:run",
         "clean": "rimraf dist",
         "clean:screenshots": "rimraf \"screenshots/**/*.@(failed|diff).png\"",
@@ -40,7 +41,14 @@
         "verbose": true,
         "testURL": "http://localhost/",
         "transform": {},
-        "automock": false
+        "automock": false,
+        "transform": {
+            ".js$": "./test/js/transform.js",
+            ".html$": "html-loader-jest"
+        },
+        "setupFiles": [
+            "./test/js/beforeEachSpec.js"
+        ]
     },
     "repository": {
         "type": "git",

diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js
@@ -138,8 +138,8 @@ class ComputedColumn extends HTMLElement {
         this.state.swap_target = false;
 
         for (let i = 0; i < computation.num_params; i++) {
-            this._input_columns.innerHTML += `<div class="psp-cc-computation__input-column" 
-                      data-index="${i}" 
+            this._input_columns.innerHTML += `<div class="psp-cc-computation__input-column"
+                      data-index="${i}"
                       drop-target>
                       <span class="psp-label__requiredType ${input_type}"></span>
                       <span class="psp-label__placeholder">Param ${i + 1}</span>

diff --git a/packages/perspective-viewer/src/js/viewer/dom_element.js b/packages/perspective-viewer/src/js/viewer/dom_element.js
@@ -65,17 +65,13 @@ export class DomElement extends PerspectiveElement {
 
         if (filter) {
             row.setAttribute("filter", filter);
+
             if (type === "string") {
-                const v = this._table.view({row_pivots: [name], aggregates: {}});
-                v.to_json().then(json => {
-                    row.choices(
-                        json
-                            .slice(1, json.length)
-                            .map(x => x.__ROW_PATH__)
-                            .filter(x => (Array.isArray(x) ? x.filter(v => !!v).length > 0 : !!x))
-                    );
-                });
-                v.delete();
+                const view = this._table.view({row_pivots: [name], aggregates: {}});
+                view.to_json().then(json => {
+                    row.choices(this._autocomplete_choices(json)) }
+                );
+                view.delete();
             }
         }
 
@@ -299,4 +295,11 @@ export class DomElement extends PerspectiveElement {
             this.load(data);
         }
     }
+
+    _autocomplete_choices(json) {
+        return json
+            .slice(1, json.length)
+            .map(x => x.__ROW_PATH__)
+            .filter(x => (Array.isArray(x) ? x.filter(v => !!v).length > 0 : !!x));
+    }
 }
diff --git a/packages/perspective-viewer/test/js/beforeEachSpec.js b/packages/perspective-viewer/test/js/beforeEachSpec.js
@@ -0,0 +1,17 @@
+/******************************************************************************
+ *
+ * Copyright (c) 2017, the Perspective Authors.
+ *
+ * This file is part of the Perspective library, distributed under the terms of
+ * the Apache License 2.0.  The full license can be found in the LICENSE file.
+ *
+ */
+
+global.CustomEvent = () => {};
+global.customElements = {define: () => {}};
+global.HTMLElement = class {
+    getAttribute() {}
+    hasAttribute() {}
+    removeAttribute() {}
+    setAttribute() {}
+};
diff --git a/packages/perspective-viewer/test/js/jest.unit.config.js b/packages/perspective-viewer/test/js/jest.unit.config.js
@@ -0,0 +1,20 @@
+/******************************************************************************
+ *
+ * Copyright (c) 2017, the Perspective Authors.
+ *
+ * This file is part of the Perspective library, distributed under the terms of
+ * the Apache License 2.0.  The full license can be found in the LICENSE file.
+ *
+ */
+
+module.exports = {
+    roots: ["unit"],
+    verbose: true,
+    testURL: "http://localhost/",
+    automock: false,
+    transform: {
+        ".js$": "./transform.js",
+        ".html$": "html-loader-jest"
+    },
+    setupFiles: ["./beforeEachSpec.js"]
+};
diff --git a/packages/perspective-viewer/test/js/transform.js b/packages/perspective-viewer/test/js/transform.js
@@ -0,0 +1,13 @@
+/******************************************************************************
+ *
+ * Copyright (c) 2017, the Perspective Authors.
+ *
+ * This file is part of the Perspective library, distributed under the terms of
+ * the Apache License 2.0.  The full license can be found in the LICENSE file.
+ *
+ */
+
+const config = require("../../babel.config.js");
+config.presets[0][1].modules = "auto";
+
+module.exports = require("babel-jest").createTransformer(config);
diff --git a/packages/perspective-viewer/test/js/unit/dom_element.spec.js b/packages/perspective-viewer/test/js/unit/dom_element.spec.js
@@ -0,0 +1,35 @@
+/******************************************************************************
+ *
+ * Copyright (c) 2018, the Perspective Authors.
+ *
+ * This file is part of the Perspective library, distributed under the terms of
+ * the Apache License 2.0.  The full license can be found in the LICENSE file.
+ *
+ */
+
+import {DomElement} from "../../../src/js/viewer/dom_element";
+
+jest.mock("../../../src/less/computed_column.less", () => jest.fn());
+
+describe(DomElement, () => {
+    describe("._autocomplete_choices", () => {
+        let dom_element, json_choices;
+
+        beforeEach(() => {
+            dom_element = new DomElement();
+            json_choices = [
+                {__ROW_PATH__: [], foo: 2},
+                {__ROW_PATH__: [null], foo: 25},
+                {__ROW_PATH__: ["somestring"], foo: 3},
+                {__ROW_PATH__: ["otherstring"], foo: 3}
+            ];
+        });
+
+        test("the first value and null values are filtered out", () => {
+            expect(dom_element._autocomplete_choices(json_choices)).toEqual([
+                ["somestring"],
+                ["otherstring"]
+            ]);
+        });
+    });
+});