Removed need for filter.id in members filtering

refs https://github.com/TryGhost/Team/issues/1419

- the `id` property isn't necessary for any of our use-cases and adds extra complexity to the code as we need to keep track of it and apply+increment it manually each time we work with filter instances
  - dropped the property
  - switched actions to pass the Filter instance rather than just the id so we can do direct compares when working on the filters array and modify properties on the Filter instance directly
- part of cleanup to reduce the amount of code/complexity in filtering so we can more easily refactor
This commit is contained in:
Kevin Ansfield 2022-03-08 14:18:23 +00:00
parent 9bfc542400
commit 789d5b9395
4 changed files with 59 additions and 88 deletions

View File

@ -1,6 +1,6 @@
{{#if (eq @filter.type 'label')}} {{#if (eq @filter.type 'label')}}
<GhMemberLabelInput <GhMemberLabelInput
@onChange={{fn this.setLabelsFilterValue @filter.type @filter.id}} @onChange={{fn this.setLabelsFilterValue @filter.type @filter}}
@onLabelEdit={{@onLabelEdit}} @onLabelEdit={{@onLabelEdit}}
@triggerId="label-input" @triggerId="label-input"
@labels={{@filter.value}} @labels={{@filter.value}}
@ -11,7 +11,7 @@
{{else if (eq @filter.type 'product')}} {{else if (eq @filter.type 'product')}}
<div class="relative"> <div class="relative">
<Tiers::SegmentSelect <Tiers::SegmentSelect
@onChange={{fn this.setProductsFilterValue @filter.type @filter.id}} @onChange={{fn this.setProductsFilterValue @filter.type @filter}}
@tiers={{this.productFilterValue}} @tiers={{this.productFilterValue}}
@renderInPlace={{true}} @renderInPlace={{true}}
@hideOptionsWhenAllSelected={{true}} @hideOptionsWhenAllSelected={{true}}
@ -26,7 +26,7 @@
@optionValuePath="name" @optionValuePath="name"
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@update={{fn this.setFilterValue @filter.type @filter.id}} @update={{fn this.setFilterValue @filter.type @filter}}
data-test-select="members-filter-value" data-test-select="members-filter-value"
/> />
{{svg-jar "arrow-down-small"}} {{svg-jar "arrow-down-small"}}
@ -38,7 +38,7 @@
@value={{@filter.value}} @value={{@filter.value}}
@maxDate={{now}} @maxDate={{now}}
@maxDateError="Must be in the past" @maxDateError="Must be in the past"
@onChange={{fn @setFilterValue @filter.type @filter.id}} @onChange={{fn @setFilterValue @filter.type @filter}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
as |datepicker| as |datepicker|
> >
@ -65,7 +65,7 @@
@value={{@filter.value}} @value={{@filter.value}}
@maxDate={{now}} @maxDate={{now}}
@maxDateError="Must be in the past" @maxDateError="Must be in the past"
@onChange={{fn @setFilterValue @filter.type @filter.id}} @onChange={{fn @setFilterValue @filter.type @filter}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
/> />
@ -77,7 +77,7 @@
@optionValuePath="name" @optionValuePath="name"
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@update={{fn this.setFilterValue @filter.type @filter.id}} @update={{fn this.setFilterValue @filter.type @filter}}
data-test-select="members-filter-value" data-test-select="members-filter-value"
/> />
{{svg-jar "arrow-down-small"}} {{svg-jar "arrow-down-small"}}
@ -89,9 +89,9 @@
value={{@filter.value}} value={{@filter.value}}
class="gh-input" class="gh-input"
aria-label="Email count filter value" aria-label="Email count filter value"
{{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} {{on "input" (fn this.setInputFilterValue @filter.type @filter)}}
{{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}}
{{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
/> />
@ -101,9 +101,9 @@
value={{@filter.value}} value={{@filter.value}}
class="gh-input" class="gh-input"
aria-label="Email opened count filter value" aria-label="Email opened count filter value"
{{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} {{on "input" (fn this.setInputFilterValue @filter.type @filter)}}
{{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}}
{{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
/> />
@ -115,9 +115,9 @@
value={{@filter.value}} value={{@filter.value}}
class="gh-input" class="gh-input"
aria-label="Email open rate filter value" aria-label="Email open rate filter value"
{{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} {{on "input" (fn this.setInputFilterValue @filter.type @filter)}}
{{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}}
{{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
/> />
</div> </div>
@ -130,7 +130,7 @@
@optionValuePath="name" @optionValuePath="name"
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@update={{fn this.setFilterValue @filter.type @filter.id}} @update={{fn this.setFilterValue @filter.type @filter}}
data-test-select="members-filter-value" data-test-select="members-filter-value"
/> />
{{svg-jar "arrow-down-small"}} {{svg-jar "arrow-down-small"}}
@ -144,7 +144,7 @@
@optionValuePath="name" @optionValuePath="name"
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@update={{fn this.setFilterValue @filter.type @filter.id}} @update={{fn this.setFilterValue @filter.type @filter}}
data-test-select="members-filter-value" data-test-select="members-filter-value"
/> />
{{svg-jar "arrow-down-small"}} {{svg-jar "arrow-down-small"}}
@ -153,14 +153,14 @@
{{else if (eq @filter.type 'subscriptions.start_date')}} {{else if (eq @filter.type 'subscriptions.start_date')}}
<GhDatePicker <GhDatePicker
@value={{@filter.value}} @value={{@filter.value}}
@onChange={{fn @setFilterValue @filter.type @filter.id}} @onChange={{fn @setFilterValue @filter.type @filter}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
/> />
{{else if (eq @filter.type 'subscriptions.current_period_end')}} {{else if (eq @filter.type 'subscriptions.current_period_end')}}
<GhDatePicker <GhDatePicker
@value={{@filter.value}} @value={{@filter.value}}
@onChange={{fn @setFilterValue @filter.type @filter.id}} @onChange={{fn @setFilterValue @filter.type @filter}}
data-test-input="members-filter-value" data-test-input="members-filter-value"
/> />
@ -168,11 +168,11 @@
<input <input
type="text" type="text"
value={{@filter.value}} value={{@filter.value}}
name={{@filter.id}} name={{@index}}
class="gh-input" class="gh-input"
aria-label="{{@filter.id}} filter value" aria-label="filter value"
{{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} {{on "input" (fn this.setInputFilterValue @filter.type @filter)}}
{{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}}
{{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}}
/> />
{{/if}} {{/if}}

View File

@ -49,38 +49,38 @@ export default class MembersFilterValue extends Component {
} }
@action @action
setInputFilterValue(filterType, filterId, event) { setInputFilterValue(filterType, filter, event) {
this.filterValue = event.target.value; this.filterValue = event.target.value;
} }
@action @action
updateInputFilterValue(filterType, filterId, event) { updateInputFilterValue(filterType, filter, event) {
if (event.type === 'blur') { if (event.type === 'blur') {
this.filterValue = event.target.value; this.filterValue = event.target.value;
} }
this.args.setFilterValue(filterType, filterId, this.filterValue); this.args.setFilterValue(filterType, filter, this.filterValue);
} }
@action @action
updateInputFilterValueOnEnter(filterType, filterId, event) { updateInputFilterValueOnEnter(filterType, filter, event) {
if (event.key === 'Enter') { if (event.key === 'Enter') {
event.preventDefault(); event.preventDefault();
this.args.setFilterValue(filterType, filterId, this.filterValue); this.args.setFilterValue(filterType, filter, this.filterValue);
} }
} }
@action @action
setLabelsFilterValue(filterType, filterId, labels) { setLabelsFilterValue(filterType, filter, labels) {
this.args.setFilterValue(filterType, filterId, labels.map(label => label.slug)); this.args.setFilterValue(filterType, filter, labels.map(label => label.slug));
} }
@action @action
setProductsFilterValue(filterType, filterId, tiers) { setProductsFilterValue(filterType, filter, tiers) {
this.args.setFilterValue(filterType, filterId, tiers.map(tier => tier.slug)); this.args.setFilterValue(filterType, filter, tiers.map(tier => tier.slug));
} }
@action @action
setFilterValue(filterType, filterId, value) { setFilterValue(filterType, filter, value) {
this.args.setFilterValue(filterType, filterId, value); this.args.setFilterValue(filterType, filter, value);
} }
} }

View File

@ -18,7 +18,7 @@
<section class="gh-filters"> <section class="gh-filters">
{{#each this.filters as |filter index|}} {{#each this.filters as |filter index|}}
<div class="gh-filter-block"> <div class="gh-filter-block">
<GhFormGroup @property={{filter.id}} @classNames="max-width" data-test-members-filter={{index}}> <div class="form-group max-width" data-test-members-filter={{index}}>
<div class="gh-filter-inputgroup"> <div class="gh-filter-inputgroup">
<span class="gh-select"> <span class="gh-select">
<OneWaySelect <OneWaySelect
@ -28,7 +28,7 @@
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@groupLabelPath="group" @groupLabelPath="group"
@update={{fn this.setFilterType filter.id}} @update={{fn this.setFilterType filter}}
data-test-select="members-filter" data-test-select="members-filter"
/> />
{{svg-jar "arrow-down-small"}} {{svg-jar "arrow-down-small"}}
@ -40,12 +40,13 @@
@optionValuePath="name" @optionValuePath="name"
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@update={{fn this.setFilterRelation filter.id}} @update={{fn this.setFilterRelation filter}}
data-test-select="members-filter-operator" data-test-select="members-filter-operator"
/> />
{{svg-jar "arrow-down-small"}} {{svg-jar "arrow-down-small"}}
</span> </span>
<Members::FilterValue <Members::FilterValue
@index={{index}}
@filter={{filter}} @filter={{filter}}
@setFilterValue={{this.setFilterValue}} @setFilterValue={{this.setFilterValue}}
@onLabelEdit={{@onLabelEdit}} @onLabelEdit={{@onLabelEdit}}
@ -54,13 +55,13 @@
type="button" type="button"
class="gh-btn gh-btn-text gh-btn-link gh-btn-icon gh-delete-filter" class="gh-btn gh-btn-text gh-btn-link gh-btn-icon gh-delete-filter"
title="Delete filter" title="Delete filter"
{{on "click" (fn this.deleteFilter filter.id)}} {{on "click" (fn this.deleteFilter filter)}}
data-test-delete-members-filter={{index}} data-test-delete-members-filter={{index}}
> >
{{svg-jar "close"}} <span class="hidden">Delete filter</span> {{svg-jar "close"}} <span class="hidden">Delete filter</span>
</button> </button>
</div> </div>
</GhFormGroup> </div>
</div> </div>
{{/each}} {{/each}}
<div> <div>

View File

@ -135,7 +135,6 @@ class Filter {
@tracked relationOptions; @tracked relationOptions;
constructor(options) { constructor(options) {
this.id = options.id;
this.type = options.type; this.type = options.type;
this.relation = options.relation; this.relation = options.relation;
this.relationOptions = options.relationOptions; this.relationOptions = options.relationOptions;
@ -162,7 +161,6 @@ export default class MembersFilter extends Component {
@tracked filters = new TrackedArray([ @tracked filters = new TrackedArray([
new Filter({ new Filter({
id: `filter-0`,
type: 'label', type: 'label',
relation: 'is', relation: 'is',
value: [], value: [],
@ -172,7 +170,6 @@ export default class MembersFilter extends Component {
availableFilterRelationsOptions = FILTER_RELATIONS_OPTIONS; availableFilterRelationsOptions = FILTER_RELATIONS_OPTIONS;
availableFilterValueOptions = FILTER_VALUE_OPTIONS; availableFilterValueOptions = FILTER_VALUE_OPTIONS;
nextFilterId = 1;
get availableFilterProperties() { get availableFilterProperties() {
let availableFilters = FILTER_PROPERTIES; let availableFilters = FILTER_PROPERTIES;
@ -217,13 +214,11 @@ export default class MembersFilter extends Component {
@action @action
addFilter() { addFilter() {
this.filters.push(new Filter({ this.filters.push(new Filter({
id: `filter-${this.nextFilterId}`,
type: 'label', type: 'label',
relation: 'is', relation: 'is',
value: [], value: [],
relationOptions: FILTER_RELATIONS_OPTIONS.label relationOptions: FILTER_RELATIONS_OPTIONS.label
})); }));
this.nextFilterId = this.nextFilterId + 1;
this.applySoftFilter(); this.applySoftFilter();
} }
@ -297,13 +292,10 @@ export default class MembersFilter extends Component {
const keys = Object.keys(nqlFilter); const keys = Object.keys(nqlFilter);
const key = keys[0]; const key = keys[0];
const value = nqlFilter[key]; const value = nqlFilter[key];
const filterId = this.nextFilterId;
if (typeof value === 'object') { if (typeof value === 'object') {
if (value.$in !== undefined && ['label', 'product'].includes(key)) { if (value.$in !== undefined && ['label', 'product'].includes(key)) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is', relation: 'is',
value: value.$in, value: value.$in,
@ -313,9 +305,7 @@ export default class MembersFilter extends Component {
} }
if (value.$nin !== undefined && ['label', 'product'].includes(key)) { if (value.$nin !== undefined && ['label', 'product'].includes(key)) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is-not', relation: 'is-not',
value: value.$nin, value: value.$nin,
@ -325,9 +315,7 @@ export default class MembersFilter extends Component {
} }
if (value.$ne !== undefined) { if (value.$ne !== undefined) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is-not', relation: 'is-not',
value: value.$ne, value: value.$ne,
@ -337,10 +325,7 @@ export default class MembersFilter extends Component {
} }
if (value.$gt !== undefined) { if (value.$gt !== undefined) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is-greater', relation: 'is-greater',
value: value.$gt, value: value.$gt,
@ -350,10 +335,7 @@ export default class MembersFilter extends Component {
} }
if (value.$gte !== undefined) { if (value.$gte !== undefined) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is-or-greater', relation: 'is-or-greater',
value: value.$gte, value: value.$gte,
@ -363,9 +345,7 @@ export default class MembersFilter extends Component {
} }
if (value.$lt !== undefined) { if (value.$lt !== undefined) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is-less', relation: 'is-less',
value: value.$lt, value: value.$lt,
@ -375,9 +355,7 @@ export default class MembersFilter extends Component {
} }
if (value.$lte !== undefined) { if (value.$lte !== undefined) {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is-or-less', relation: 'is-or-less',
value: value.$lte, value: value.$lte,
@ -388,10 +366,7 @@ export default class MembersFilter extends Component {
return null; return null;
} else { } else {
this.nextFilterId = this.nextFilterId + 1;
return new Filter({ return new Filter({
id: `filter-${filterId}`,
type: key, type: key,
relation: 'is', relation: 'is',
value: value, value: value,
@ -440,20 +415,20 @@ export default class MembersFilter extends Component {
} }
@action @action
deleteFilter(filterId, event) { deleteFilter(filter, event) {
event.stopPropagation(); event.stopPropagation();
event.preventDefault(); event.preventDefault();
if (this.filters.length === 1) { if (this.filters.length === 1) {
this.resetFilter(); this.resetFilter();
} else { } else {
this.filters = new TrackedArray(this.filters.reject(f => f.id === filterId)); this.filters = new TrackedArray(this.filters.reject(f => f === filter));
this.applySoftFilter(); this.applySoftFilter();
} }
} }
@action @action
setFilterType(filterId, newType) { setFilterType(filter, newType) {
if (newType instanceof Event) { if (newType instanceof Event) {
newType = newType.target.value; newType = newType.target.value;
} }
@ -484,7 +459,6 @@ export default class MembersFilter extends Component {
} }
const newFilter = new Filter({ const newFilter = new Filter({
id: filterId,
type: newType, type: newType,
relation: defaultRelation, relation: defaultRelation,
relationOptions: this.availableFilterRelationsOptions[newType], relationOptions: this.availableFilterRelationsOptions[newType],
@ -492,7 +466,7 @@ export default class MembersFilter extends Component {
timezone: this.settings.get('timezone') timezone: this.settings.get('timezone')
}); });
const filterToSwap = this.filters.find(f => f.id === filterId); const filterToSwap = this.filters.find(f => f === filter);
this.filters[this.filters.indexOf(filterToSwap)] = newFilter; this.filters[this.filters.indexOf(filterToSwap)] = newFilter;
if (newType !== 'label' && defaultValue) { if (newType !== 'label' && defaultValue) {
@ -505,29 +479,27 @@ export default class MembersFilter extends Component {
} }
@action @action
setFilterRelation(filterId, newRelation) { setFilterRelation(filter, newRelation) {
const filterToEdit = this.filters.find(f => f.id === filterId); filter.relation = newRelation;
filterToEdit.relation = newRelation;
this.applySoftFilter(); this.applySoftFilter();
} }
@action @action
setFilterValue(filterType, filterId, filterValue) { setFilterValue(filterType, filter, filterValue) {
const filterToEdit = this.filters.find(f => f.id === filterId); filter.value = filterValue;
filterToEdit.value = filterValue;
this.applySoftFilter(); this.applySoftFilter();
} }
@action @action
applySoftFilter() { applySoftFilter() {
const validFilters = this.filters.filter((fil) => { const validFilters = this.filters.filter((filter) => {
if (fil.type === 'label') { if (filter.type === 'label') {
return fil.value?.length; return filter.value?.length;
} }
if (fil.type === 'product') { if (filter.type === 'product') {
return fil.value?.length; return filter.value?.length;
} }
return fil.value; return filter.value;
}); });
const query = this.generateNqlFilter(validFilters); const query = this.generateNqlFilter(validFilters);
this.args.onApplySoftFilter(query, validFilters); this.args.onApplySoftFilter(query, validFilters);
@ -535,11 +507,11 @@ export default class MembersFilter extends Component {
@action @action
applyFilter() { applyFilter() {
const validFilters = this.filters.filter((fil) => { const validFilters = this.filters.filter((filter) => {
if (['label', 'product'].includes(fil.type)) { if (['label', 'product'].includes(filter.type)) {
return fil.value?.length; return filter.value?.length;
} }
return fil.value; return filter.value;
}); });
const query = this.generateNqlFilter(validFilters); const query = this.generateNqlFilter(validFilters);
@ -548,10 +520,8 @@ export default class MembersFilter extends Component {
@action @action
resetFilter() { resetFilter() {
this.nextFilterId = 1;
this.filters = new TrackedArray([ this.filters = new TrackedArray([
new Filter({ new Filter({
id: `filter-0`,
type: 'label', type: 'label',
relation: 'is', relation: 'is',
value: [], value: [],