diff --git a/CHANGELOG.md b/CHANGELOG.md index 087274526..26d835953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ See `HoistAuthModel` for more info. * Updated mobile `TabContainer` to flex properly within flexbox containers. * Fixed timing issue with missing validation for records added immediately to new store. * Fixed CSS bug in which date picker dates wrapped when `dateEditor` used in a grid in a dialog. +* Fixed several issues with the grid column filter when used on a "tags" field. ## 65.0.0 - 2024-06-26 @@ -230,6 +231,7 @@ for more details. * `Store` now supports multiple `summaryRecords`, displayed if so configured as multiple pinned rows within a bound grid. + ## 63.0.3 - 2024-04-16 ### 🐞 Bug Fixes diff --git a/cmp/grid/filter/GridFilterFieldSpec.ts b/cmp/grid/filter/GridFilterFieldSpec.ts index b5901c26e..0254d773c 100644 --- a/cmp/grid/filter/GridFilterFieldSpec.ts +++ b/cmp/grid/filter/GridFilterFieldSpec.ts @@ -12,7 +12,7 @@ import { BaseFilterFieldSpec, BaseFilterFieldSpecConfig } from '@xh/hoist/data/filter/BaseFilterFieldSpec'; -import {castArray, compact, flatten, isDate, isEmpty, uniqBy} from 'lodash'; +import {castArray, compact, flatten, isDate, isEmpty, isEqual, uniqBy} from 'lodash'; import {GridFilterModel} from './GridFilterModel'; export interface GridFilterFieldSpecConfig extends BaseFilterFieldSpecConfig { @@ -82,10 +82,17 @@ export class GridFilterFieldSpec extends BaseFilterFieldSpec { // Get values from current column filter const filterValues = []; columnFilters.forEach(filter => { - const newValues = castArray(filter.value).map(value => { - value = sourceField.parseVal(value); - return filterModel.toDisplayValue(value); - }); + // The parseVal of tag will castArray the value a second time, so make sure to flatten at the end. + const newValues = flatten( + castArray(filter.value) + .map(value => { + value = sourceField.parseVal(value); + return filterModel.toDisplayValue(value); + }) + // Filter out the null tag value as it isn't a valid value to display in value lists. + // It is set by 'is blank'/'is not blank' filters. + .filter(it => isEqual(it, [null])) + ); filterValues.push(...newValues); }); diff --git a/data/filter/FieldFilter.ts b/data/filter/FieldFilter.ts index 9d083383a..5d38bb743 100644 --- a/data/filter/FieldFilter.ts +++ b/data/filter/FieldFilter.ts @@ -14,6 +14,7 @@ import { escapeRegExp, first, isArray, + isEmpty, isEqual, isNil, isString, @@ -114,10 +115,11 @@ export class FieldFilter extends Filter { const storeField = store.getField(field); if (!storeField) return () => true; // Ignore (do not filter out) if field not in store - const fieldType = storeField.type === 'tags' ? 'string' : storeField.type; - value = isArray(value) - ? value.map(v => parseFieldValue(v, fieldType)) - : parseFieldValue(value, fieldType); + const fieldType = storeField.type; + value = + isArray(value) && storeField.type !== 'tags' + ? value.map(v => parseFieldValue(v, fieldType)) + : parseFieldValue(value, fieldType); } if (FieldFilter.ARRAY_OPERATORS.includes(op)) { @@ -128,14 +130,14 @@ export class FieldFilter extends Filter { switch (op) { case '=': opFn = v => { - if (isNil(v) || v === '') v = null; - return value.some(it => isEqual(v, it)); + if (isNil(v) || v === '' || (isArray(v) && isEmpty(v))) v = null; + return (v == null && isEmpty(value)) || value.some(it => isEqual(v, it)); }; break; case '!=': opFn = v => { - if (isNil(v) || v === '') v = null; - return !value.some(it => isEqual(v, it)); + if (isNil(v) || v === '' || (isArray(v) && isEmpty(v))) v = null; + return (v != null || !isEmpty(value)) && !value.some(it => isEqual(v, it)); }; break; case '>': diff --git a/desktop/cmp/grid/impl/filter/ColumnHeaderFilter.ts b/desktop/cmp/grid/impl/filter/ColumnHeaderFilter.ts index fde75fe11..c620a2e9b 100644 --- a/desktop/cmp/grid/impl/filter/ColumnHeaderFilter.ts +++ b/desktop/cmp/grid/impl/filter/ColumnHeaderFilter.ts @@ -118,6 +118,6 @@ const switcherButton = hoistCmp.factory(({model, id, ti text: title, active: activeTabId === id, outlined: true, - onClick: () => tabContainerModel.activateTab(id) + onClick: () => model.activateTab(id) }); }); diff --git a/desktop/cmp/grid/impl/filter/ColumnHeaderFilterModel.ts b/desktop/cmp/grid/impl/filter/ColumnHeaderFilterModel.ts index 985be5acb..609f50cb6 100644 --- a/desktop/cmp/grid/impl/filter/ColumnHeaderFilterModel.ts +++ b/desktop/cmp/grid/impl/filter/ColumnHeaderFilterModel.ts @@ -67,10 +67,15 @@ export class ColumnHeaderFilterModel extends HoistModel { @computed get isCustomFilter() { - const {columnCompoundFilter, columnFilters} = this; + const {columnCompoundFilter, columnFilters, fieldType} = this; if (columnCompoundFilter) return true; if (isEmpty(columnFilters)) return false; - return columnFilters.some(it => !['=', '!=', 'includes'].includes(it.op)); + return columnFilters.some( + it => + !['=', '!=', 'includes'].includes(it.op) || + // The is blank / is not blank filter on tags is only supported by custom filter. + (fieldType === 'tags' && ['=', '!='].includes(it.op) && it.value == null) + ); } get commitOnChange() { @@ -158,6 +163,12 @@ export class ColumnHeaderFilterModel extends HoistModel { this.isOpen = false; } + activateTab(tabId) { + const tabModel = tabId === 'valuesFilter' ? this.valuesTabModel : this.customTabModel; + tabModel.syncWithFilter(); + this.tabContainerModel.activateTab(tabId); + } + //------------------- // Implementation //------------------- diff --git a/desktop/cmp/grid/impl/filter/custom/CustomRowModel.ts b/desktop/cmp/grid/impl/filter/custom/CustomRowModel.ts index 50b26b225..e04bd419f 100644 --- a/desktop/cmp/grid/impl/filter/custom/CustomRowModel.ts +++ b/desktop/cmp/grid/impl/filter/custom/CustomRowModel.ts @@ -8,7 +8,7 @@ import {HoistModel} from '@xh/hoist/core'; import {FieldFilterOperator, FieldFilterSpec} from '@xh/hoist/data'; import {ColumnHeaderFilterModel} from '@xh/hoist/desktop/cmp/grid/impl/filter/ColumnHeaderFilterModel'; import {bindable, computed, makeObservable} from '@xh/hoist/mobx'; -import {isArray, isNil} from 'lodash'; +import {isArray, isEmpty, isNil} from 'lodash'; import {CustomTabModel} from './CustomTabModel'; type OperatorOptionValue = 'blank' | 'not blank' | FieldFilterOperator; @@ -82,7 +82,7 @@ export class CustomRowModel extends HoistModel { makeObservable(this); let newOp = op as OperatorOptionValue; - if (isNil(value)) { + if (isNil(value) || (isArray(value) && isEmpty(value))) { if (op === '=') newOp = 'blank'; if (op === '!=') newOp = 'not blank'; } diff --git a/desktop/cmp/grid/impl/filter/values/ValuesTabModel.ts b/desktop/cmp/grid/impl/filter/values/ValuesTabModel.ts index 6902d45ba..81f1eeb31 100644 --- a/desktop/cmp/grid/impl/filter/values/ValuesTabModel.ts +++ b/desktop/cmp/grid/impl/filter/values/ValuesTabModel.ts @@ -10,7 +10,7 @@ import {FieldFilterSpec} from '@xh/hoist/data'; import {ColumnHeaderFilterModel} from '../ColumnHeaderFilterModel'; import {checkbox} from '@xh/hoist/desktop/cmp/input'; import {action, bindable, computed, makeObservable, observable} from '@xh/hoist/mobx'; -import {castArray, difference, isEmpty, partition, uniq, without} from 'lodash'; +import {castArray, difference, flatten, isEmpty, isEqual, partition, uniq, without} from 'lodash'; export class ValuesTabModel extends HoistModel { override xhImpl = true; @@ -162,10 +162,17 @@ export class ValuesTabModel extends HoistModel { filterValues = []; arr.forEach(filter => { - const newValues = castArray(filter.value).map(value => { - value = fieldSpec.sourceField.parseVal(value); - return gridFilterModel.toDisplayValue(value); - }); + // The parseVal of tag will castArray the value a second time, so make sure to flatten at the end. + const newValues = flatten( + castArray(filter.value) + .map(value => { + value = fieldSpec.sourceField.parseVal(value); + return gridFilterModel.toDisplayValue(value); + }) + // Filter out the null tag value as it isn't a valid value to display in value lists. + // It is set by 'is blank'/'is not blank' filters. + .filter(it => isEqual(it, [null])) + ); filterValues.push(...newValues); // Todo: Is this safe? });