XWIKI-18661: Impossible to filter panel type or category with empty value (#1639)

manuelleduc · web-flow · commit a5469d5ef822 · 2021-06-16T16:10:42.000+02:00
- Introduce a new 'empty' filter on Live Data
- Introduce a new 'empty' matcher on Livetable
- Make the conversion for the new filter on Live Data - Live Table Connector
- Hides the select field in the advanced panel when the filter type is 'empty'
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api/src/main/resources/ApplicationResources.properties b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api/src/main/resources/ApplicationResources.properties
@@ -24,6 +24,7 @@ liveData.layout.cards=Cards
 liveData.operator.contains=Contains
 liveData.operator.startsWith=Starts with
 liveData.operator.equals=Equals
+liveData.operator.empty=Empty
 liveData.operator.less=Less than
 liveData.operator.greater=Greater than
 liveData.operator.between=Between
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api/src/main/resources/liveDataConfiguration.json b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api/src/main/resources/liveDataConfiguration.json
@@ -35,7 +35,7 @@
       },
       {
         "id": "list",
-        "operators": ["equals", "startsWith", "contains"]
+        "operators": ["equals", "startsWith", "contains", "empty"]
       }
     ],
 
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api/src/test/resources/DefaultLiveDataConfigurationResolver.test b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api/src/test/resources/DefaultLiveDataConfigurationResolver.test
@@ -139,7 +139,8 @@
         "operators":[
           {"id":"equals","name":"Equals"},
           {"id":"startsWith","name":"startsWith"},
-          {"id":"contains","name":"contains"}
+          {"id":"contains","name":"contains"},
+          {"id":"empty","name":"empty"}
         ]
       }
     ],
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/main/java/org/xwiki/livedata/internal/livetable/LiveTableLiveDataPropertyStore.java b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/main/java/org/xwiki/livedata/internal/livetable/LiveTableLiveDataPropertyStore.java
@@ -41,7 +41,6 @@
 import org.xwiki.livedata.LiveDataPropertyDescriptor;
 import org.xwiki.livedata.LiveDataPropertyDescriptor.DisplayerDescriptor;
 import org.xwiki.livedata.LiveDataPropertyDescriptor.FilterDescriptor;
-import org.xwiki.livedata.LiveDataPropertyDescriptor.OperatorDescriptor;
 import org.xwiki.livedata.LiveDataPropertyDescriptorStore;
 import org.xwiki.livedata.WithParameters;
 import org.xwiki.model.reference.DocumentReference;
@@ -146,6 +145,7 @@ private LiveDataPropertyDescriptor getLiveDataPropertyDescriptor(PropertyClass x
         descriptor.setDisplayer(new DisplayerDescriptor("xObjectProperty"));
         if (xproperty instanceof ListClass) {
             descriptor.setFilter(new FilterDescriptor("list"));
+            descriptor.getFilter().addOperator("empty", null);
             if (xproperty instanceof LevelsClass) {
                 descriptor.getFilter().setParameter("options", ((LevelsClass) xproperty).getList(xcontext));
             } else {
@@ -155,7 +155,7 @@ private LiveDataPropertyDescriptor getLiveDataPropertyDescriptor(PropertyClass x
                 // The default live table results page currently supports only exact matching for list properties with
                 // multiple selection and no relational storage (selected values are stored concatenated on a single
                 // database column).
-                descriptor.getFilter().setOperators(Collections.singletonList(new OperatorDescriptor("equals", null)));
+                descriptor.getFilter().addOperator("equals", null);
             }
         } else if (xproperty instanceof DateClass) {
             String dateFormat = ((DateClass) xproperty).getDateFormat();
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/main/java/org/xwiki/livedata/internal/livetable/LiveTableRequestHandler.java b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/main/java/org/xwiki/livedata/internal/livetable/LiveTableRequestHandler.java
@@ -243,6 +243,15 @@ private void addFilterRequestParameters(Filter filter, Map<String, String[]> req
                 .map(operator -> MATCH_TYPE.getOrDefault(operator, StringUtils.defaultString(operator)))
                 .collect(Collectors.toList());
             requestParams.put(filter.getProperty() + "_match", matchType.toArray(new String[matchType.size()]));
+
+            // Add a value to the empty fields since otherwise they are dismissed by LiveTableResultMacros.
+            // Changing the handling of empty values in the result templates is dangerous and could lead to regressions 
+            // when the livetable clients expected empty values to simply be dismissed.
+            for (int i = 0; i < matchType.size(); i++) {
+                if (Objects.equals(matchType.get(i), "empty")) {
+                    requestParams.get(filter.getProperty())[i] = "-";
+                }
+            }
         }
     }
 }
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/test/java/org/xwiki/livedata/internal/livetable/LiveTableLiveDataPropertyStoreTest.java b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/test/java/org/xwiki/livedata/internal/livetable/LiveTableLiveDataPropertyStoreTest.java
@@ -172,10 +172,10 @@ void getAll() throws Exception
             + "'type':'Computed','displayer':{'id':'xObjectProperty'}},");
         expectedClassProps.append("{'id':'status','name':'Status','description':'The status.',"
             + "'type':'List','sortable':false,'displayer':{'id':'xObjectProperty'},"
-            + "'filter':{'id':'list','operators':[{'id':'equals'}],"
+            + "'filter':{'id':'list','operators':[{'id':'empty'},{'id':'equals'}],"
             + "'searchURL':'/xwiki/rest/wikis/wiki/classes/Some.Class/properties/status/values?fp={encodedQuery}'}},");
         expectedClassProps.append("{'id':'levels','type':'Levels','displayer':{'id':'xObjectProperty'},"
-            + "'filter':{'id':'list','options':['edit','delete']}}");
+            + "'filter':{'id':'list','operators':[{'id':'empty'}],'options':['edit','delete']}}");
 
         Collection<LiveDataPropertyDescriptor> properties = this.propertyStore.get();
 
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/test/java/org/xwiki/livedata/internal/livetable/LiveTableRequestHandlerTest.java b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-livetable/src/test/java/org/xwiki/livedata/internal/livetable/LiveTableRequestHandlerTest.java
@@ -64,7 +64,7 @@
  * @since 12.10
  */
 @ComponentTest
-public class LiveTableRequestHandlerTest
+class LiveTableRequestHandlerTest
 {
     @InjectMockComponents
     private LiveTableRequestHandler handler;
@@ -197,6 +197,28 @@ void getLiveTableResultsFromResponse() throws Exception
         verify(this.xcontext).setFinished(false);
     }
 
+    @Test
+    void getLiveTableResultsEmptyFilter()
+    {
+        Map<String, String[]> expectedRequestParams = new HashMap<>();
+        expectedRequestParams.put("title/join_mode", new String[] { "OR" });
+        expectedRequestParams.put("reqNo", new String[] { "1" });
+        expectedRequestParams.put("outputSyntax", new String[] { "plain" });
+        expectedRequestParams.put("title", new String[] { "-" });
+        expectedRequestParams.put("title_match", new String[] { "empty" });
+
+        LiveDataQuery liveDataQuery = new LiveDataQuery();
+        liveDataQuery.setFilters(Collections.singletonList(new Filter("title", "empty", "")));
+        this.handler.getLiveTableResults(liveDataQuery, () -> null);
+
+        ArgumentCaptor<XWikiRequest> requestCaptor = ArgumentCaptor.forClass(XWikiRequest.class);
+        verify(this.xcontext, times(2)).setRequest(requestCaptor.capture());
+        List<XWikiRequest> requests = requestCaptor.getAllValues();
+
+        assertRequestParameters(expectedRequestParams, requests.get(0).getParameterMap());
+        assertSame(this.originalRequest, requests.get(1));
+    }
+
     private void assertRequestParameters(Map<String, String[]> expectedParams, Map<String, String[]> actualParams)
     {
         assertEquals(expectedParams.size(), actualParams.size());
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-macro/src/main/resources/ApplicationResources.properties b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-macro/src/main/resources/ApplicationResources.properties
@@ -112,4 +112,6 @@ livedata.displayer.boolean.true=True
 livedata.displayer.boolean.false=False
 
 livedata.displayer.xObjectProperty.missingDocumentName.errorMessage=Can't save an XObject property with a missing document name.
-livedata.displayer.xObjectProperty.failedToRetrieveField.errorMessage=Failed to retrieve the {0} field.
+livedata.displayer.xObjectProperty.failedToRetrieveField.errorMessage=Failed to retrieve the {0} field.
+
+livedata.filter.list.emptyLabel=---
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-test/xwiki-platform-livedata-test-pageobjects/src/main/java/org/xwiki/livedata/test/po/TableLayoutElement.java b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-test/xwiki-platform-livedata-test-pageobjects/src/main/java/org/xwiki/livedata/test/po/TableLayoutElement.java
@@ -423,7 +423,7 @@ public void filterColumn(String columnLabel, String content, boolean wait)
     {
         int columnIndex = findColumnIndex(columnLabel);
         WebElement element = getRoot()
-            .findElement(By.cssSelector(String.format(".column-filters > th:nth-child(%d) > input", columnIndex)));
+            .findElement(By.cssSelector(String.format(".column-filters > th:nth-child(%d) input", columnIndex)));
 
         List<String> classes = Arrays.asList(getClasses(element));
         if (classes.contains("filter-list")) {
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/tests/mocks/xwiki-selectize.js b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/tests/mocks/xwiki-selectize.js
@@ -0,0 +1,19 @@
+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/tests/unit/filters/FilterList.spec.js b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/tests/unit/filters/FilterList.spec.js
@@ -0,0 +1,92 @@
+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+import FilterList from "../../../filters/FilterList.vue";
+import {mount} from '@vue/test-utils';
+import _ from "lodash";
+import $ from "jquery";
+
+/**
+ * Initialize a FilterList component using default values vue test-utils `mount` parameters.
+ * The default parameters can be overridden using the `mountConfiguration` parameter.
+ *
+ * The default configuration is:
+ * ```
+ * {
+ *   provide: {
+ *     logic: {
+ *       getQueryFilterGroup() {
+ *         return {};
+ *       },
+ *       onEventWhere() {
+ *       },
+ *       getFilterDescriptor() {
+ *         return {options: ''};
+ *       }
+ *     }
+ *   }
+ * }
+ * ```
+ *
+ * @param mountConfiguration mount parameters merged over the default configuration
+ * @returns {{options: string}|{}|*} an initialized FilterList Vue component
+ */
+function initWrapper(mountConfiguration = {}) {
+  // Define an empty xwikiSelectize to prevent the component mount to fail.
+  $.fn.xwikiSelectize = () => {
+  };
+
+  return mount(FilterList, _.merge({
+    provide: {
+      logic: {
+        getQueryFilterGroup() {
+          return {};
+        },
+        onEventWhere() {
+        },
+        getFilterDescriptor() {
+          return {options: ''};
+        }
+      }
+    }
+  }, mountConfiguration));
+}
+
+describe('FilterList.vue', () => {
+  it('Render the filter list when visible', () => {
+    const wrapper = initWrapper();
+    expect(wrapper.html()).toBe("<span><input class=\"filter-list livedata-filter\"></span>")
+  })
+
+  it('Render the filter list when Empty filter and advanced', () => {
+    const wrapper = initWrapper({
+      propsData: {index: 0, isAdvanced: true},
+      provide: {
+        logic: {
+          getQueryFilterGroup() {
+            return {
+              constraints: [{value: undefined, operator: 'empty'}]
+            }
+          }
+        }
+      }
+    });
+    expect(wrapper.html()).toBe('<span style="display: none;"><input class="filter-list livedata-filter"></span>')
+  })
+})
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/filters/FilterList.vue b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/filters/FilterList.vue
@@ -23,7 +23,9 @@
 -->
 <template>
   <!-- A simple text input that will be enhanced by the selectize widget. -->
-  <input :value="value" class="filter-list" ref="input"/>
+  <span v-show="isVisible">
+    <input :value="value" class="filter-list livedata-filter" ref="input"/>
+  </span>
 </template>
 
 <script>
@@ -38,6 +40,13 @@ export default {
   // Add the filterMixin to get access to all the generic filter methods and computed properties inside this component.
   mixins: [filterMixin],
 
+  props: {
+    isAdvanced: {
+      type: Boolean,
+      default: false
+    }
+  },
+
   computed: {
     // Current value of the filter entry.
     value () {
@@ -60,8 +69,12 @@ export default {
         // * the user can still add more values by adding more constraints from the advanced filtering panel
         maxItems: 1,
         onChange: value => {
-          if (value !== this.value) {
-            this.applyFilter(value);
+          if (this.$refs.input.selectize.items.length === 0) {
+            // When no values are selected, simply remove the filter.
+            this.removeFilter();
+          } else if (value !== this.value) {
+            // If the selected value has an empty value, than use the empty filter, otherwise use the contains filter.
+            this.applyFilter(value, value === '' ? 'empty' : 'contains');
           }
         },
       };
@@ -73,6 +86,14 @@ export default {
       }
       return settings;
     },
+
+    isVisible() {
+      // We do not show this component when the type of filter is 'Empty' and we are in the advanced filtering panel.  
+      return this.filterEntry.operator !== 'empty' || !this.isAdvanced
+    },
+    hasEmptyOperator() {
+      return this.config.operators.some(it => it.id === 'empty');
+    }
   },
 
   methods: {
@@ -81,20 +102,30 @@ export default {
       return (text, callback) => {
         // TODO: Support multiple search URLs (sources). See suggestUsersAndGroups.js for an example.
         const searchURL = this.config.searchURL.replace('{encodedQuery}', encodeURIComponent(text));
-        $.getJSON(searchURL, searchParams).then(this.getResultsAdapter()).done(callback).fail(callback);
+        $.getJSON(searchURL, searchParams)
+          .done((results) => callback(this.getResultsAdapter(results)))
+          .fail(() => callback(this.getResultsAdapter()));
       };
     },
 
-    getResultsAdapter () {
-      return results => {
-        if (Array.isArray(results)) {
-          return results;
-        } else if (Array.isArray(results?.propertyValues)) {
-          return results.propertyValues.map(this.getSuggestion);
-        } else {
-          return [];
-        }
-      };
+    getResultsAdapter(results) {
+      let adaptedResults = [];
+      if (Array.isArray(results)) {
+        adaptedResults = results;
+      } else if (Array.isArray(results?.propertyValues)) {
+        adaptedResults = results.propertyValues.map(this.getSuggestion);
+      }
+
+      // An empty option is automatically added to the results only when hasEmptyOperator is true, no empty 
+      // option is already found, and we are not in an advanced filter panel.
+      if (!this.isAdvanced && this.hasEmptyOperator && !adaptedResults.some((value) => value.value === '')) {
+        adaptedResults.unshift({
+          value: '',
+          label: this.$t('livedata.filter.list.emptyLabel')
+        })
+      }
+
+      return adaptedResults;
     },
 
     // Convert the fetched data to the format expected by the selectize widget.
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/filters/LivedataFilter.vue b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/filters/LivedataFilter.vue
@@ -48,6 +48,7 @@
     :property-id="propertyId"
     :index="index"
     :is-filtering.sync="isFiltering"
+    :is-advanced="isAdvanced"
   ></component>
 
   <!--
@@ -77,6 +78,10 @@ export default {
   props: {
     propertyId: String,
     index: Number,
+    isAdvanced: {
+      type: Boolean,
+      default: false
+    }
   },
 
   data () {
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/filters/filterMixin.js b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/filters/filterMixin.js
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/panels/LivedataAdvancedPanelFilterEntry.vue b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/panels/LivedataAdvancedPanelFilterEntry.vue
diff --git a/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/webjar/Logic.js b/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/webjar/Logic.js
diff --git a/xwiki-platform-core/xwiki-platform-livetable/xwiki-platform-livetable-ui/src/main/resources/XWiki/LiveTableResultsMacros.xml b/xwiki-platform-core/xwiki-platform-livetable/xwiki-platform-livetable-ui/src/main/resources/XWiki/LiveTableResultsMacros.xml
diff --git a/xwiki-platform-core/xwiki-platform-livetable/xwiki-platform-livetable-ui/src/test/java/org/xwiki/livetable/LiveTableResultsTest.java b/xwiki-platform-core/xwiki-platform-livetable/xwiki-platform-livetable-ui/src/test/java/org/xwiki/livetable/LiveTableResultsTest.java

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@`
`35`	`35`	`},`
`36`	`36`	`{`
`37`	`37`	`"id": "list",`
`38`		`- "operators": ["equals", "startsWith", "contains"]`
	`38`	`+ "operators": ["equals", "startsWith", "contains", "empty"]`
`39`	`39`	`}`
`40`	`40`	`],`
`41`	`41`
Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,8 @@`
`139`	`139`	`"operators":[`
`140`	`140`	`{"id":"equals","name":"Equals"},`
`141`	`141`	`{"id":"startsWith","name":"startsWith"},`
`142`		`- {"id":"contains","name":"contains"}`
	`142`	`+ {"id":"contains","name":"contains"},`
	`143`	`+ {"id":"empty","name":"empty"}`
`143`	`144`	`]`
`144`	`145`	`}`
`145`	`146`	`],`
Original file line number	Diff line number	Diff line change
`@@ -243,6 +243,15 @@ private void addFilterRequestParameters(Filter filter, Map<String, String[]> req`
`243`	`243`	`.map(operator -> MATCH_TYPE.getOrDefault(operator, StringUtils.defaultString(operator)))`
`244`	`244`	`.collect(Collectors.toList());`
`245`	`245`	`requestParams.put(filter.getProperty() + "_match", matchType.toArray(new String[matchType.size()]));`
	`246`	`+`
	`247`	`+ // Add a value to the empty fields since otherwise they are dismissed by LiveTableResultMacros.`
	`248`	`+ // Changing the handling of empty values in the result templates is dangerous and could lead to regressions`
	`249`	`+ // when the livetable clients expected empty values to simply be dismissed.`
	`250`	`+ for (int i = 0; i < matchType.size(); i++) {`
	`251`	`+ if (Objects.equals(matchType.get(i), "empty")) {`
	`252`	`+ requestParams.get(filter.getProperty())[i] = "-";`
	`253`	`+ }`
	`254`	`+ }`
`246`	`255`	`}`
`247`	`256`	`}`
`248`	`257`	`}`
Original file line number	Diff line number	Diff line change
`@@ -423,7 +423,7 @@ public void filterColumn(String columnLabel, String content, boolean wait)`
`423`	`423`	`{`
`424`	`424`	`int columnIndex = findColumnIndex(columnLabel);`
`425`	`425`	`WebElement element = getRoot()`
`426`		`- .findElement(By.cssSelector(String.format(".column-filters > th:nth-child(%d) > input", columnIndex)));`
	`426`	`+ .findElement(By.cssSelector(String.format(".column-filters > th:nth-child(%d) input", columnIndex)));`
`427`	`427`
`428`	`428`	`List<String> classes = Arrays.asList(getClasses(element));`
`429`	`429`	`if (classes.contains("filter-list")) {`