Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve filtering dates for specific operators #3362

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
* New `XH.pageState` provides observable access to the current lifecycle state of the app, allowing
apps to react to changes in page visibility and focus, as well as detecting when the browser has
frozen a tab due to inactivity or navigation.
* Improved filtering of fields with `type: date` to use the end of day when evaluating `>` or `<=`
operators, resulting in more intuitive results when a user inputs or selects a `yyyy-mm-dd`
comparison value. Spec `fieldSpec.fieldType: 'localDate'` to opt-in to this behavior.

## 57.0.0 - 2023-06-20

Expand Down
18 changes: 2 additions & 16 deletions admin/tabs/activity/clienterrors/ClientErrorsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import {FilterChooserModel} from '@xh/hoist/cmp/filter';
import {FormModel} from '@xh/hoist/cmp/form';
import {GridModel} from '@xh/hoist/cmp/grid';
import {HoistModel, LoadSpec, managed, XH} from '@xh/hoist/core';
import {fmtDate, fmtJson} from '@xh/hoist/format';
import {fmtJson} from '@xh/hoist/format';
import {action, bindable, observable, makeObservable, comparer} from '@xh/hoist/mobx';
import {LocalDate} from '@xh/hoist/utils/datetime';
import * as Col from '@xh/hoist/admin/columns';
import moment from 'moment';

export class ClientErrorsModel extends HoistModel {
override persistWith = {localStorageKey: 'xhAdminClientErrorsState'};
Expand Down Expand Up @@ -93,20 +92,7 @@ export class ClientErrorsModel extends HoistModel {
},
{
field: 'dateCreated',
example: 'YYYY-MM-DD',
valueParser: (v, op) => {
let ret = moment(v, ['YYYY-MM-DD', 'YYYYMMDD'], true);
if (!ret.isValid()) return null;

// Note special handling for '>' & '<=' queries.
if (['>', '<='].includes(op)) {
ret = moment(ret).endOf('day');
}

return ret.toDate();
},
valueRenderer: v => fmtDate(v),
ops: ['>', '>=', '<', '<=']
fieldType: 'localDate'
}
],
persistWith: this.persistWith
Expand Down
17 changes: 2 additions & 15 deletions admin/tabs/activity/tracking/ActivityTrackingModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {FormModel} from '@xh/hoist/cmp/form';
import {GridModel, TreeStyle} from '@xh/hoist/cmp/grid';
import {HoistModel, LoadSpec, managed, XH} from '@xh/hoist/core';
import {Cube, CubeFieldSpec, FieldSpec} from '@xh/hoist/data';
import {fmtDate, fmtNumber} from '@xh/hoist/format';
import {fmtNumber} from '@xh/hoist/format';
import {action, computed, makeObservable} from '@xh/hoist/mobx';
import {LocalDate} from '@xh/hoist/utils/datetime';
import * as Col from '@xh/hoist/admin/columns';
Expand Down Expand Up @@ -129,20 +129,7 @@ export class ActivityTrackingModel extends HoistModel {
},
{
field: 'dateCreated',
example: 'YYYY-MM-DD',
valueParser: (v, op) => {
let ret = moment(v, ['YYYY-MM-DD', 'YYYYMMDD'], true);
if (!ret.isValid()) return null;

// Note special handling for '>' & '<=' queries.
if (['>', '<='].includes(op)) {
ret = moment(ret).endOf('day');
}

return ret.toDate();
},
valueRenderer: v => fmtDate(v),
ops: ['>', '>=', '<', '<=']
fieldType: 'localDate'
}
],
persistWith: this.persistWith
Expand Down
16 changes: 15 additions & 1 deletion admin/tabs/activity/tracking/detail/ActivityDetailModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,21 @@ export class ActivityDetailModel extends HoistModel {
sortBy: 'dateCreated|desc',
colChooserModel: true,
enableExport: true,
filterModel: true,
filterModel: {
fieldSpecs: [
'username',
'category',
'msg',
'device',
'browser',
'userAgent',
'elapsed',
{
field: 'dateCreated',
fieldType: 'localDate'
}
]
},
exportOptions: {
columns: 'ALL',
filename: `${XH.appCode}-activity-detail`
Expand Down
5 changes: 5 additions & 0 deletions cmp/filter/FilterChooserFieldSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ export class FilterChooserFieldSpec extends BaseFilterFieldSpec {
if (!valueParser && (this.fieldType === 'int' || this.fieldType === 'number')) {
return input => parseNumber(input);
}

// Default date parser if fieldSpec's fieldType is localDate and record's field type is date.
if (!valueParser && this.filterDateAsLocalDate) {
return (v, op) => this.parseLocalDateAsDate(v, op);
}
return valueParser;
}

Expand Down
5 changes: 3 additions & 2 deletions cmp/filter/impl/Option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function minimalFieldOption({fieldSpec}): FilterChooserOption {
* Create an option representing an existing or suggested FieldFilter.
*/
export function fieldFilterOption({filter, fieldSpec, isExact = false}): FilterChooserOption {
let {fieldType, displayName} = fieldSpec as FilterChooserFieldSpec,
let {fieldType, displayName, filterDateAsLocalDate} = fieldSpec as FilterChooserFieldSpec,
displayOp,
displayValue;

Expand All @@ -70,7 +70,8 @@ export function fieldFilterOption({filter, fieldSpec, isExact = false}): FilterC
displayValue = filter.op === '!=' ? 'not blank' : 'blank';
} else {
displayOp = filter.op;
fieldType = fieldType === 'tags' ? 'string' : fieldType;
if (fieldType === 'tags') fieldType = 'string';
if (filterDateAsLocalDate) fieldType = 'date';
displayValue = fieldSpec.renderValue(
parseFieldValue(filter.value, fieldType, null),
filter.op
Expand Down
39 changes: 34 additions & 5 deletions data/filter/BaseFilterFieldSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import {HoistBase} from '@xh/hoist/core';
import {Field, Store, FieldFilter, FieldType, genDisplayName, View} from '@xh/hoist/data';
import {isEmpty} from 'lodash';
import moment from 'moment';
import {FieldFilterOperator} from './Types';

export interface BaseFilterFieldSpecConfig {
Expand Down Expand Up @@ -91,6 +92,9 @@ export abstract class BaseFilterFieldSpec extends HoistBase {
* Type 'value' indicates the field should use equality operators:
* `(=, !=, like, not like, begins, ends)`
* against a suggested exact value or user-provided input.
*
* Type 'collection' indicates the field should use:
* `(includes, excludes)`
*/
get filterType(): 'range' | 'value' | 'collection' {
switch (this.fieldType) {
Expand All @@ -109,18 +113,15 @@ export abstract class BaseFilterFieldSpec extends HoistBase {
get isRangeType(): boolean {
return this.filterType === 'range';
}

get isValueType(): boolean {
return this.filterType === 'value';
}

get isCollectionType(): boolean {
return this.filterType === 'collection';
}

get isDateBasedFieldType(): boolean {
const {fieldType} = this;
return fieldType === 'date' || fieldType === 'localDate';
}

get isNumericFieldType(): boolean {
const {fieldType} = this;
return fieldType === 'int' || fieldType === 'number';
Expand All @@ -130,6 +131,20 @@ export abstract class BaseFilterFieldSpec extends HoistBase {
return this.fieldType === 'bool';
}

get isDateBasedFieldType(): boolean {
const {fieldType} = this;
return fieldType === 'date' || fieldType === 'localDate';
}

/**
* Convenience mode where a Date field can be filtered as if it is a LocalDate, to
* support comparative operators without time precision.
*/
get filterDateAsLocalDate(): boolean {
const sourceField = this.source.fields.find(f => f.name === this.field);
return this.fieldType === 'localDate' && sourceField.type === 'date';
}

loadValues() {
if (!this.hasExplicitValues && this.enableValues) {
this.loadValuesFromSource();
Expand All @@ -149,6 +164,18 @@ export abstract class BaseFilterFieldSpec extends HoistBase {
);
}

parseLocalDateAsDate(v: any, op: FieldFilterOperator): Date {
let ret = moment(v, ['YYYY-MM-DD', 'YYYYMMDD'], true);
if (!ret.isValid()) return null;

// Note special handling for '>' & '<=' queries.
if (['>', '<='].includes(op)) {
ret = ret.endOf('day');
}

return ret.toDate();
}

//------------------------
// Abstract
//------------------------
Expand All @@ -165,12 +192,14 @@ export abstract class BaseFilterFieldSpec extends HoistBase {
private getDefaultOperators(): FieldFilterOperator[] {
if (this.isBoolFieldType) return ['='];
if (this.isCollectionType) return ['includes', 'excludes'];
if (this.filterDateAsLocalDate) return ['>', '>=', '<', '<='];
return this.isValueType
? ['=', '!=', 'like', 'not like', 'begins', 'ends']
: ['>', '>=', '<', '<=', '=', '!='];
}

private get isEnumerableByDefault(): boolean {
if (this.filterDateAsLocalDate) return false;
switch (this.fieldType) {
case 'int':
case 'number':
Expand Down
3 changes: 2 additions & 1 deletion desktop/cmp/grid/impl/filter/custom/CustomRow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ const inputField = hoistCmp.factory<CustomRowModel>(({model}) => {
enableShorthandUnits: true
});
} else if (fieldSpec.isDateBasedFieldType) {
const fieldType = fieldSpec.fieldType as 'localDate' | 'date';
return dateInput({
...props,
valueType: fieldSpec.fieldType as 'localDate' | 'date'
valueType: fieldSpec.filterDateAsLocalDate ? 'date' : fieldType
});
} else if (fieldSpec.supportsSuggestions(op as FieldFilterOperator)) {
return select({
Expand Down
4 changes: 3 additions & 1 deletion desktop/cmp/grid/impl/filter/custom/CustomRowModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class CustomRowModel extends HoistModel {
/** FieldFilter config output of this row. */
@computed.struct
get value(): FieldFilterSpec {
const {field} = this.fieldSpec;
const {field, filterDateAsLocalDate} = this.fieldSpec;

let op = this.op,
value = this.inputVal;
Expand All @@ -39,6 +39,8 @@ export class CustomRowModel extends HoistModel {
} else if (op === 'not blank') {
op = '!=';
value = null;
} else if (filterDateAsLocalDate) {
value = this.fieldSpec.parseLocalDateAsDate(value, op);
} else if (isNil(value)) {
return null;
}
Expand Down
Loading