From 789d5b939550eb9d3912f2aa4940626213ccb467 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 8 Mar 2022 14:18:23 +0000 Subject: [PATCH] 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 --- .../app/components/members/filter-value.hbs | 48 +++++++------- .../app/components/members/filter-value.js | 22 +++---- ghost/admin/app/components/members/filter.hbs | 11 ++-- ghost/admin/app/components/members/filter.js | 66 +++++-------------- 4 files changed, 59 insertions(+), 88 deletions(-) diff --git a/ghost/admin/app/components/members/filter-value.hbs b/ghost/admin/app/components/members/filter-value.hbs index 112005dacb..9e6c7a2418 100644 --- a/ghost/admin/app/components/members/filter-value.hbs +++ b/ghost/admin/app/components/members/filter-value.hbs @@ -1,6 +1,6 @@ {{#if (eq @filter.type 'label')}} {{svg-jar "arrow-down-small"}} @@ -38,7 +38,7 @@ @value={{@filter.value}} @maxDate={{now}} @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" as |datepicker| > @@ -65,7 +65,7 @@ @value={{@filter.value}} @maxDate={{now}} @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" /> @@ -77,7 +77,7 @@ @optionValuePath="name" @optionLabelPath="label" @optionTargetPath="name" - @update={{fn this.setFilterValue @filter.type @filter.id}} + @update={{fn this.setFilterValue @filter.type @filter}} data-test-select="members-filter-value" /> {{svg-jar "arrow-down-small"}} @@ -89,9 +89,9 @@ value={{@filter.value}} class="gh-input" aria-label="Email count filter value" - {{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} - {{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} - {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} + {{on "input" (fn this.setInputFilterValue @filter.type @filter)}} + {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}} + {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}} data-test-input="members-filter-value" /> @@ -101,9 +101,9 @@ value={{@filter.value}} class="gh-input" aria-label="Email opened count filter value" - {{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} - {{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} - {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} + {{on "input" (fn this.setInputFilterValue @filter.type @filter)}} + {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}} + {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}} data-test-input="members-filter-value" /> @@ -115,9 +115,9 @@ value={{@filter.value}} class="gh-input" aria-label="Email open rate filter value" - {{on "input" (fn this.setInputFilterValue @filter.type @filter.id)}} - {{on "blur" (fn this.updateInputFilterValue @filter.type @filter.id)}} - {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter.id)}} + {{on "input" (fn this.setInputFilterValue @filter.type @filter)}} + {{on "blur" (fn this.updateInputFilterValue @filter.type @filter)}} + {{on "keypress" (fn this.updateInputFilterValueOnEnter @filter.type @filter)}} data-test-input="members-filter-value" /> @@ -130,7 +130,7 @@ @optionValuePath="name" @optionLabelPath="label" @optionTargetPath="name" - @update={{fn this.setFilterValue @filter.type @filter.id}} + @update={{fn this.setFilterValue @filter.type @filter}} data-test-select="members-filter-value" /> {{svg-jar "arrow-down-small"}} @@ -144,7 +144,7 @@ @optionValuePath="name" @optionLabelPath="label" @optionTargetPath="name" - @update={{fn this.setFilterValue @filter.type @filter.id}} + @update={{fn this.setFilterValue @filter.type @filter}} data-test-select="members-filter-value" /> {{svg-jar "arrow-down-small"}} @@ -153,14 +153,14 @@ {{else if (eq @filter.type 'subscriptions.start_date')}} {{else if (eq @filter.type 'subscriptions.current_period_end')}} @@ -168,11 +168,11 @@ {{/if}} diff --git a/ghost/admin/app/components/members/filter-value.js b/ghost/admin/app/components/members/filter-value.js index e58c096477..d20ecad880 100644 --- a/ghost/admin/app/components/members/filter-value.js +++ b/ghost/admin/app/components/members/filter-value.js @@ -49,38 +49,38 @@ export default class MembersFilterValue extends Component { } @action - setInputFilterValue(filterType, filterId, event) { + setInputFilterValue(filterType, filter, event) { this.filterValue = event.target.value; } @action - updateInputFilterValue(filterType, filterId, event) { + updateInputFilterValue(filterType, filter, event) { if (event.type === 'blur') { this.filterValue = event.target.value; } - this.args.setFilterValue(filterType, filterId, this.filterValue); + this.args.setFilterValue(filterType, filter, this.filterValue); } @action - updateInputFilterValueOnEnter(filterType, filterId, event) { + updateInputFilterValueOnEnter(filterType, filter, event) { if (event.key === 'Enter') { event.preventDefault(); - this.args.setFilterValue(filterType, filterId, this.filterValue); + this.args.setFilterValue(filterType, filter, this.filterValue); } } @action - setLabelsFilterValue(filterType, filterId, labels) { - this.args.setFilterValue(filterType, filterId, labels.map(label => label.slug)); + setLabelsFilterValue(filterType, filter, labels) { + this.args.setFilterValue(filterType, filter, labels.map(label => label.slug)); } @action - setProductsFilterValue(filterType, filterId, tiers) { - this.args.setFilterValue(filterType, filterId, tiers.map(tier => tier.slug)); + setProductsFilterValue(filterType, filter, tiers) { + this.args.setFilterValue(filterType, filter, tiers.map(tier => tier.slug)); } @action - setFilterValue(filterType, filterId, value) { - this.args.setFilterValue(filterType, filterId, value); + setFilterValue(filterType, filter, value) { + this.args.setFilterValue(filterType, filter, value); } } diff --git a/ghost/admin/app/components/members/filter.hbs b/ghost/admin/app/components/members/filter.hbs index 213b278a59..5d042b7e15 100644 --- a/ghost/admin/app/components/members/filter.hbs +++ b/ghost/admin/app/components/members/filter.hbs @@ -18,7 +18,7 @@
{{#each this.filters as |filter index|}}
- +
{{svg-jar "arrow-down-small"}} @@ -40,12 +40,13 @@ @optionValuePath="name" @optionLabelPath="label" @optionTargetPath="name" - @update={{fn this.setFilterRelation filter.id}} + @update={{fn this.setFilterRelation filter}} data-test-select="members-filter-operator" /> {{svg-jar "arrow-down-small"}} {{svg-jar "close"}}
- +
{{/each}}
diff --git a/ghost/admin/app/components/members/filter.js b/ghost/admin/app/components/members/filter.js index e8df54cfa0..eb7251b14c 100644 --- a/ghost/admin/app/components/members/filter.js +++ b/ghost/admin/app/components/members/filter.js @@ -135,7 +135,6 @@ class Filter { @tracked relationOptions; constructor(options) { - this.id = options.id; this.type = options.type; this.relation = options.relation; this.relationOptions = options.relationOptions; @@ -162,7 +161,6 @@ export default class MembersFilter extends Component { @tracked filters = new TrackedArray([ new Filter({ - id: `filter-0`, type: 'label', relation: 'is', value: [], @@ -172,7 +170,6 @@ export default class MembersFilter extends Component { availableFilterRelationsOptions = FILTER_RELATIONS_OPTIONS; availableFilterValueOptions = FILTER_VALUE_OPTIONS; - nextFilterId = 1; get availableFilterProperties() { let availableFilters = FILTER_PROPERTIES; @@ -217,13 +214,11 @@ export default class MembersFilter extends Component { @action addFilter() { this.filters.push(new Filter({ - id: `filter-${this.nextFilterId}`, type: 'label', relation: 'is', value: [], relationOptions: FILTER_RELATIONS_OPTIONS.label })); - this.nextFilterId = this.nextFilterId + 1; this.applySoftFilter(); } @@ -297,13 +292,10 @@ export default class MembersFilter extends Component { const keys = Object.keys(nqlFilter); const key = keys[0]; const value = nqlFilter[key]; - const filterId = this.nextFilterId; if (typeof value === 'object') { if (value.$in !== undefined && ['label', 'product'].includes(key)) { - this.nextFilterId = this.nextFilterId + 1; return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is', value: value.$in, @@ -313,9 +305,7 @@ export default class MembersFilter extends Component { } if (value.$nin !== undefined && ['label', 'product'].includes(key)) { - this.nextFilterId = this.nextFilterId + 1; return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is-not', value: value.$nin, @@ -325,9 +315,7 @@ export default class MembersFilter extends Component { } if (value.$ne !== undefined) { - this.nextFilterId = this.nextFilterId + 1; return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is-not', value: value.$ne, @@ -337,10 +325,7 @@ export default class MembersFilter extends Component { } if (value.$gt !== undefined) { - this.nextFilterId = this.nextFilterId + 1; - return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is-greater', value: value.$gt, @@ -350,10 +335,7 @@ export default class MembersFilter extends Component { } if (value.$gte !== undefined) { - this.nextFilterId = this.nextFilterId + 1; - return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is-or-greater', value: value.$gte, @@ -363,9 +345,7 @@ export default class MembersFilter extends Component { } if (value.$lt !== undefined) { - this.nextFilterId = this.nextFilterId + 1; return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is-less', value: value.$lt, @@ -375,9 +355,7 @@ export default class MembersFilter extends Component { } if (value.$lte !== undefined) { - this.nextFilterId = this.nextFilterId + 1; return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is-or-less', value: value.$lte, @@ -388,10 +366,7 @@ export default class MembersFilter extends Component { return null; } else { - this.nextFilterId = this.nextFilterId + 1; - return new Filter({ - id: `filter-${filterId}`, type: key, relation: 'is', value: value, @@ -440,20 +415,20 @@ export default class MembersFilter extends Component { } @action - deleteFilter(filterId, event) { + deleteFilter(filter, event) { event.stopPropagation(); event.preventDefault(); if (this.filters.length === 1) { this.resetFilter(); } else { - this.filters = new TrackedArray(this.filters.reject(f => f.id === filterId)); + this.filters = new TrackedArray(this.filters.reject(f => f === filter)); this.applySoftFilter(); } } @action - setFilterType(filterId, newType) { + setFilterType(filter, newType) { if (newType instanceof Event) { newType = newType.target.value; } @@ -484,7 +459,6 @@ export default class MembersFilter extends Component { } const newFilter = new Filter({ - id: filterId, type: newType, relation: defaultRelation, relationOptions: this.availableFilterRelationsOptions[newType], @@ -492,7 +466,7 @@ export default class MembersFilter extends Component { 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; if (newType !== 'label' && defaultValue) { @@ -505,29 +479,27 @@ export default class MembersFilter extends Component { } @action - setFilterRelation(filterId, newRelation) { - const filterToEdit = this.filters.find(f => f.id === filterId); - filterToEdit.relation = newRelation; + setFilterRelation(filter, newRelation) { + filter.relation = newRelation; this.applySoftFilter(); } @action - setFilterValue(filterType, filterId, filterValue) { - const filterToEdit = this.filters.find(f => f.id === filterId); - filterToEdit.value = filterValue; + setFilterValue(filterType, filter, filterValue) { + filter.value = filterValue; this.applySoftFilter(); } @action applySoftFilter() { - const validFilters = this.filters.filter((fil) => { - if (fil.type === 'label') { - return fil.value?.length; + const validFilters = this.filters.filter((filter) => { + if (filter.type === 'label') { + return filter.value?.length; } - if (fil.type === 'product') { - return fil.value?.length; + if (filter.type === 'product') { + return filter.value?.length; } - return fil.value; + return filter.value; }); const query = this.generateNqlFilter(validFilters); this.args.onApplySoftFilter(query, validFilters); @@ -535,11 +507,11 @@ export default class MembersFilter extends Component { @action applyFilter() { - const validFilters = this.filters.filter((fil) => { - if (['label', 'product'].includes(fil.type)) { - return fil.value?.length; + const validFilters = this.filters.filter((filter) => { + if (['label', 'product'].includes(filter.type)) { + return filter.value?.length; } - return fil.value; + return filter.value; }); const query = this.generateNqlFilter(validFilters); @@ -548,10 +520,8 @@ export default class MembersFilter extends Component { @action resetFilter() { - this.nextFilterId = 1; this.filters = new TrackedArray([ new Filter({ - id: `filter-0`, type: 'label', relation: 'is', value: [],