[pve-devel] [PATCH v4 widget-toolkit 2/3] Allow searching for multiple columns in ComboGrid

[Thomas Lamprecht]

On 9/10/19 7:11 PM, Stefan Reiter wrote:
> Uses code from ExtJS 6.0.1 ComboBox.js to overwrite 'doLocalQuery',
> adding a new property 'searchFields' which allows to specify fields from
> the bound store which will all be searched (OR filter, i.e. the searched
> value can appear in any column specified).
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  form/ComboGrid.js | 82 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 

some style comments inline, did not tested it at all and would like
to get also a ack from Dominik for this feature.

> diff --git a/form/ComboGrid.js b/form/ComboGrid.js
> index c3e7782..d470a98 100644
> --- a/form/ComboGrid.js
> +++ b/form/ComboGrid.js
> @@ -310,6 +310,88 @@ Ext.define('Proxmox.form.ComboGrid', {
>          return picker;
>      },
>  
> +    createStoreFilter: function(property, queryString, suffix) {
> +	var me = this;
> +
> +	return new Ext.util.Filter({
> +	    id: me.id + '-filter-' + suffix,
> +	    anyMatch: me.anyMatch,
> +	    caseSensitive: me.caseSensitive,
> +	    root: 'data',
> +	    property: property,
> +	    value: me.enableRegEx ? new RegExp(queryString) : queryString
> +	});
> +    },
> +
> +    doLocalQuery: function(queryPlan) {
> +	var me = this,
> +	    queryString = queryPlan.query,
> +	    store = me.getStore(),
> +	    filter = me.queryFilter,
> +	    multiFilters;
> +
> +	me.queryFilter = null;
> +	// Must set changingFilters flag for this.checkValueOnChange.
> +	// the suppressEvents flag does not affect the filterchange event
> +	me.changingFilters = true;
> +	if (filter) {
> +	    store.removeFilter(filter, true);
> +	}
> +
> +	// Querying by a string...
> +	if (queryString) {
> +	    // proxmox overrides: allow filtering multiple fields
> +	    if (Ext.isArray(me.searchFields)) {
> +		// Since me.queryFilter is used elsewhere as a single filter, we
> +		// cannot assign an array. Instead, we create a function
> +		// manually applying the different filters OR'd together and set
> +		// that as the filterFn of a single, combined filter.
> +		multiFilters = [];
> +	        Ext.Array.each(me.searchFields, function (v) {
> +		    multiFilters.push(me.createStoreFilter(
> +			v,
> +			queryString,
> +			'multi-' + v
> +		    ))

Do not like the split over many newlines syntax to much, doesn't really fits
Proxmox's or ExtJS unwritten syntax style.. Nice and OK for configuration
objects, but not so much for a simple function parameter list.

Either:
let filter = me.createStoreFilter(v, queryString, 'multi-' + v);
multiFilters.push(filter);

or just use a map, below an (untested) example which uses map +
arrow function replacing 8 lines with 1:

multiFilter = Ext.Array.map(me.searchFields, (v) => me.createStoreFilter(v, queryString, 'multi-' + v));

or
multiFilter = Ext.Array.map(me.searchFields, (v) => {
    return me.createStoreFilter(v, queryString, 'multi-' + v)
});


> +		});
> +
> +		filter = me.queryFilter = new Ext.util.Filter({
> +		    id: me.id + '-filter-multi',
> +		    root: 'data',
> +		    filterFn: function (item) {
> +			for (var i = 0; i < multiFilters.length; i++) {
> +			    if (multiFilters[i].getFilterFn()(item)) {
> +				return true;
> +			    }
> +			}
> +			return false;
> +		    }
> +		})
> +	    } else {
> +		filter = me.queryFilter = me.createStoreFilter(
> +		    me.displayField,
> +		    queryString,
> +		    'single'
> +		);

see above

> +	    }
> +
> +	    store.addFilter(filter, true);
> +	}
> +	me.changingFilters = false;
> +
> +	// Expand after adjusting the filter if there are records or if emptyText is configured.
> +	if (me.store.getCount() || me.getPicker().emptyText) {
> +	    // The filter changing was done with events suppressed, so
> +	    // refresh the picker DOM while hidden and it will layout on show.
> +	    me.getPicker().refresh();
> +	    me.expand();
> +	} else {
> +	    me.collapse();
> +	}
> +
> +	me.afterQuery(queryPlan);
> +    },
> +
>      isValueInStore: function(value) {
>  	var me = this;
>  	var store = me.store;
>