From 920d90beb163bf9ae042c97e83da8bd84fc28115 Mon Sep 17 00:00:00 2001
From: "Bland, Mike" <mbland@cvent.com>
Date: Sat, 3 Mar 2018 04:11:54 -0500
Subject: [PATCH] Stop filter from matching invisible properties

Closes #725. The previous `if (filterable === false)` condition meant
that `filterable={false}` must be explicitly set on a ColumnDefinition
in order for any property returned by row.keySeq() to be ignored. This
resulted in the filter matching invisible row properties such as
griddleKey and any properties in the data not covered by a
ColumnDefinition.

I confirmed this by locally changing the expression at the end of this
filter from:

  return value &&
    value.toString().toLowerCase().indexOf(filterToLower) > -1;

to:

  if (value && value.toString().toLowerCase().indexOf(filterToLower) > -1) {
    console.log('KEY:', key, 'VALUE:', value.toString().toLowerCase());
    return true;
  }
  return false;

and observed the unexpected filter matches on invisible row properties
in the console.

This fixes the problem by updating the condition to exclude griddleKey
and any values not covered by a ColumnDefinition if any
ColumnDefinitions are present. I've added new tests and validated the
filter behavior in the Storybook.

Bear in mind that columns corresponding to other React components may
still match the filter based on generated HTML properties rendered as
part of the value.toString().toLowerCase() expression. At this point the
best course of action may be to recommend that such ColumnDefinitions
are tagged with `filterable={false}` if it's a significant problem for
the user.
---
 .../selectors/__tests__/localSelectorsTest.js | 36 ++++++++++++++++++-
 src/plugins/local/selectors/localSelectors.js |  8 +++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/src/plugins/local/selectors/__tests__/localSelectorsTest.js b/src/plugins/local/selectors/__tests__/localSelectorsTest.js
index 28b84b12..51978194 100644
--- a/src/plugins/local/selectors/__tests__/localSelectorsTest.js
+++ b/src/plugins/local/selectors/__tests__/localSelectorsTest.js
@@ -249,12 +249,34 @@ test('filteredDataSelector filters data respecting filterable', test => {
         name: {
           filterable: false,
         },
+        weapon: {
+          filterable: true,
+        }
+      }
+    },
+    filter: 'H',
+    data: [
+      { id: '1', name: 'luke skywalker', weapon: 'light saber' },
+      { id: '2', name: 'han solo', weapon: 'blaster' },
+    ]
+  });
+
+  test.deepEqual(selectors.filteredDataSelector(state).toJSON(), [
+    { id: '1', name: 'luke skywalker', weapon: 'light saber' }
+  ]);
+});
+
+test('filteredDataSelector matches ColumnDefinition fields only', test => {
+  const state = new Immutable.fromJS({
+    renderProperties: {
+      columnProperties: {
+        weapon: null,
       }
     },
     filter: 'H',
     data: [
       { id: '1', name: 'luke skywalker', weapon: 'light saber' },
-      { id: '2', name: 'han solo', weapon: 'blaster' }
+      { id: '2', name: 'han solo', weapon: 'blaster' },
     ]
   });
 
@@ -263,6 +285,18 @@ test('filteredDataSelector filters data respecting filterable', test => {
   ]);
 });
 
+test('filteredDataSelector ignores griddleKey matches', test => {
+  const state = new Immutable.fromJS({
+    filter: '1',
+    data: [
+      { griddleKey: '11', name: 'luke skywalker' },
+      { griddleKey: '12', name: 'han solo' }
+    ]
+  });
+
+  test.deepEqual(selectors.filteredDataSelector(state).toJSON(), []);
+});
+
 test('sortedDataSelector uses default sort if no sort method specifed for column', test => {
   const state = new Immutable.fromJS({
     data: [
diff --git a/src/plugins/local/selectors/localSelectors.js b/src/plugins/local/selectors/localSelectors.js
index be77f0d5..41d9e35b 100644
--- a/src/plugins/local/selectors/localSelectors.js
+++ b/src/plugins/local/selectors/localSelectors.js
@@ -52,9 +52,13 @@ export const filteredDataSelector = createSelector(
     return data.filter(row =>
       row.keySeq()
         .some((key) => {
-          const filterable = columnProperties && columnProperties.getIn([key, 'filterable']);
-          if (filterable === false) {
+          if (key === 'griddleKey') {
             return false;
+          } else if (columnProperties) {
+            if (columnProperties.get(key) === undefined ||
+              columnProperties.getIn([key, 'filterable']) === false) {
+              return false;
+            }
           }
           const value = row.get(key);
           return value &&