From 4a4a4050b9164dcd801d05dd20a66b8ff3f7de17 Mon Sep 17 00:00:00 2001
From: Piotr Szczepanik <piter75@gmail.com>
Date: Fri, 25 May 2018 12:40:06 +0200
Subject: [PATCH] Fixed a regression in table visualisation row selection
 (#125)

---
 src/common/models/essence/essence.mocha.ts    | 22 ++++++++
 src/common/models/essence/essence.ts          |  6 +--
 .../models/highlight/highlight.fixtures.ts    | 50 +++++++++++++++++++
 3 files changed, 75 insertions(+), 3 deletions(-)
 create mode 100644 src/common/models/highlight/highlight.fixtures.ts

diff --git a/src/common/models/essence/essence.mocha.ts b/src/common/models/essence/essence.mocha.ts
index d3349f847..ad2ae7239 100644
--- a/src/common/models/essence/essence.mocha.ts
+++ b/src/common/models/essence/essence.mocha.ts
@@ -19,9 +19,11 @@ import { expect } from 'chai';
 import { testImmutableClass } from 'immutable-class-tester';
 
 import { $ } from 'plywood';
+import { Highlight } from "..";
 import { MANIFESTS } from "../../manifests";
 import { LINE_CHART_MANIFEST } from "../../manifests/line-chart/line-chart";
 import { TABLE_MANIFEST } from "../../manifests/table/table";
+import { HighlightFixtures } from "../highlight/highlight.fixtures";
 import { MeasureFixtures } from "../measure/measure.fixtures";
 import { Essence, EssenceJS, VisStrategy } from './essence';
 import { DataCube, Introspection } from "../data-cube/data-cube";
@@ -105,6 +107,26 @@ describe('Essence', () => {
     ], { context });
   });
 
+  describe("removes highlight when necessary", () => {
+    const { lineChartWithAddedMeasure, lineChartWithAvgAddedMeasure, tableNoMeasure } = HighlightFixtures;
+
+    const tests: {highlight: Highlight, expected: Highlight, description: string}[] = [
+      { highlight: lineChartWithAddedMeasure(), expected: lineChartWithAddedMeasure(), description: "is kept when measure is selected" },
+      { highlight: tableNoMeasure(), expected: tableNoMeasure(), description: "is kept when contains no measure" },
+      { highlight: lineChartWithAvgAddedMeasure(), expected: null, description: "is removed when measure is not selected" }
+    ];
+
+    tests.forEach(({ highlight, expected, description }) => {
+      it(`highlight ${description}`, () => {
+        const essenceValue = EssenceMock.wikiTable().valueOf();
+        const essenceValueWithHighlight = { ...essenceValue, highlight };
+        const essenceWithHighlight = new Essence(essenceValueWithHighlight);
+
+        expect(essenceWithHighlight.highlight).to.deep.equal(expected);
+
+      });
+    });
+  });
 
   describe('errors', () => {
     it('must have context', () => {
diff --git a/src/common/models/essence/essence.ts b/src/common/models/essence/essence.ts
index 9fe9d8f8c..531ecf8b2 100644
--- a/src/common/models/essence/essence.ts
+++ b/src/common/models/essence/essence.ts
@@ -243,9 +243,9 @@ export class Essence implements Instance<EssenceValue, EssenceJS> {
 
     if (!dataCube) throw new Error('Essence must have a dataCube');
 
-    function isHighlightMeasureVisible(highlight: Highlight): boolean {
+    function hasNoMeasureOrMeasureIsSelected(highlight: Highlight): boolean {
       if (!highlight || !highlight.measure)
-        return false;
+        return true;
 
       const { measure } = highlight;
       return multiMeasureMode ? selectedMeasures.has(measure) : measure === singleMeasure;
@@ -292,7 +292,7 @@ export class Essence implements Instance<EssenceValue, EssenceJS> {
     this.pinnedDimensions = pinnedDimensions;
     this.colors = colors;
     this.pinnedSort = pinnedSort;
-    this.highlight = isHighlightMeasureVisible(highlight) ? highlight : null;
+    this.highlight = hasNoMeasureOrMeasureIsSelected(highlight) ? highlight : null;
     this.compare = compare;
     this.visResolve = visResolve;
   }
diff --git a/src/common/models/highlight/highlight.fixtures.ts b/src/common/models/highlight/highlight.fixtures.ts
new file mode 100644
index 000000000..9eb749926
--- /dev/null
+++ b/src/common/models/highlight/highlight.fixtures.ts
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2017-2018 Allegro.pl
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Filter } from "..";
+import { LineChart } from "../../../client/visualizations/line-chart/line-chart";
+import { Table } from "../../../client/visualizations/table/table";
+import { FilterClauseFixtures } from "../filter-clause/filter-clause.fixtures";
+import { Highlight } from "./highlight";
+
+export class HighlightFixtures {
+  static tableNoMeasure(): Highlight {
+    const channelInFilterClause = FilterClauseFixtures.stringIn("channel", ["en"]);
+    return new Highlight({
+      owner: Table.id,
+      measure: null,
+      delta: Filter.fromClause(channelInFilterClause)
+    });
+  }
+
+  static lineChartWithAvgAddedMeasure(): Highlight {
+    const channelInFilterClause = FilterClauseFixtures.stringIn("channel", ["en"]);
+    return new Highlight({
+      owner: LineChart.id,
+      measure: "avg_added",
+      delta: Filter.fromClause(channelInFilterClause)
+    });
+  }
+
+  static lineChartWithAddedMeasure(): Highlight {
+    const channelInFilterClause = FilterClauseFixtures.stringIn("channel", ["en"]);
+    return new Highlight({
+      owner: LineChart.id,
+      measure: "added",
+      delta: Filter.fromClause(channelInFilterClause)
+    });
+  }
+}