From bc82745d0372212da6d1befcf056f2011b7c0943 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 2 Nov 2022 14:53:42 -0700 Subject: [PATCH 01/21] Don't send empty sort or fields params in PPS requests --- src/search-param-url-generator.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/search-param-url-generator.ts b/src/search-param-url-generator.ts index 6ba9cf6..6ee49fe 100644 --- a/src/search-param-url-generator.ts +++ b/src/search-param-url-generator.ts @@ -74,11 +74,11 @@ export class SearchParamURLGenerator { params.append('page', String(searchParams.page)); } - if (searchParams.fields) { + if (searchParams.fields && searchParams.fields.length > 0) { params.append('fields', searchParams.fields.join(',')); } - if (searchParams.sort) { + if (searchParams.sort && searchParams.sort.length > 0) { const sortStrings = searchParams.sort.map(sort => this.sortParamsAsString(sort) ); From 422f771ba178c07127ab73f4d57962dd43eaef43 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Thu, 3 Nov 2022 17:41:03 -0700 Subject: [PATCH 02/21] Add param for filter_map to external interface --- index.ts | 1 + src/search-params.ts | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/index.ts b/index.ts index 290e177..e07284e 100644 --- a/index.ts +++ b/index.ts @@ -46,5 +46,6 @@ export { SortDirection, AggregateSearchParams, AggregateSearchParam, + FilterParam, } from './src/search-params'; export { SearchServiceError } from './src/search-service-error'; diff --git a/src/search-params.ts b/src/search-params.ts index 4d6040e..c9928ed 100644 --- a/src/search-params.ts +++ b/src/search-params.ts @@ -48,6 +48,16 @@ export interface SortParam { direction: SortDirection; } +export interface FilterParam { + key: string, + include: boolean, + exclude: boolean, + gt: number, + gte: number, + lt: number, + lte: number, +} + /** * SearchParams provides an encapsulation to all of the search parameters * available for searching. @@ -105,6 +115,26 @@ export interface SearchParams { */ fields?: string[]; + /** + * An array of filter params that can be used to narrow the result set. + * Each filter param is an object with a string `key` identifying what + * attribute to filter on (e.g., `'year'`, `'subject'`, etc.) and one or more + * additional properties identifying what filter to apply. The filters + * allowed are: + * - `include` (string, results must include the given value for this `key`) + * - `exclude` (string, results must exclude the given value for this `key`) + * - `gt` (number, results must have a value for `key` greater than the given value) + * - `gte` (number, as above but greater than _or equal_) + * - `lt` (number, results must have a value for `key` less than the given value) + * - `lte` (number, as above but less than _or equal_) + * + * So a filter like `{ key: 'creator', include: 'Cicero' }` will produce + * search results that include `Cicero` as a creator, and a filter like + * `{ key: 'year', gte: 2000, lte: 2005 }` will produce search results whose + * `year` field is between 2000 and 2005 (inclusive). + */ + filters?: FilterParam[]; + /** * An object specifying which aggregation types should be returned with * a search query. From dad5a336c34c673f166982ba74e516b90a64ce9a Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Fri, 4 Nov 2022 15:44:38 -0700 Subject: [PATCH 03/21] Rethink search service filters interface, and document more clearly --- src/search-params.ts | 81 +++++++++++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/src/search-params.ts b/src/search-params.ts index c9928ed..ddb5dab 100644 --- a/src/search-params.ts +++ b/src/search-params.ts @@ -48,14 +48,55 @@ export interface SortParam { direction: SortDirection; } +/** + * A parameter specifying what filters to apply to the returned hits for a specific field. + * This filter may represent a selected/hidden facet, a value range (e.g., date picker), + * or some other type of restriction on the results. + */ export interface FilterParam { - key: string, - include: boolean, - exclude: boolean, - gt: number, - gte: number, - lt: number, - lte: number, + /** + * (Optional) One or more values for the current field, which all results must include _at least one of_. + * + * For instance, `{ field: 'subject', include: ['baseball', 'basketball'] }` specifies that only results + * that contain _either_ `baseball` _or_ `basketball` as a subject should be returned. + */ + include?: string[], + + /** + * (Optional) One or more values for the current field which must _not_ be included in _any_ of the results. + * + * For instance, `{ field: 'subject', exclude: ['baseball', 'basketball'] }` specifies that only results + * that contain _neither_ `baseball` _nor_ `basketball` as a subject should be returned. + */ + exclude?: string[], + + /** + * (Optional) A strict lower bound on numeric values for the current field. + * All returned hits must have a `field` value greater than the one specified here. + * This only makes sense for numeric fields. + */ + gt?: number, + + /** + * (Optional) A non-strict lower bound on numeric values for the current field. + * All returned hits must have a `field` value greater than or equal to the one specified here. + * This only makes sense for numeric fields. + */ + gte?: number, + + /** + * (Optional) A strict upper bound on numeric values for the current field. + * All returned hits must have a `field` value less than the one specified here. + * This only makes sense for numeric fields. + */ + lt?: number, + + /** + * (Optional) A non-strict upper bound on numeric values for the current field. + * All returned hits must have a `field` value less than or equal to the one specified here. + * This only makes sense for numeric fields. + */ + lte?: number, } /** @@ -116,24 +157,24 @@ export interface SearchParams { fields?: string[]; /** - * An array of filter params that can be used to narrow the result set. - * Each filter param is an object with a string `key` identifying what - * attribute to filter on (e.g., `'year'`, `'subject'`, etc.) and one or more - * additional properties identifying what filter to apply. The filters - * allowed are: - * - `include` (string, results must include the given value for this `key`) - * - `exclude` (string, results must exclude the given value for this `key`) - * - `gt` (number, results must have a value for `key` greater than the given value) + * A map from field names to filter params that can be used to narrow the result set. + * The keys identify what field to filter on (e.g., `'year'`, `'subject'`, etc.), + * and the values provide one or more constraints to apply to that field. + * + * The constraints allowed are: + * - `include` (string[], results must include at least one of the given values for this field) + * - `exclude` (string[], results must *not* include *any* of the given values for this field) + * - `gt` (number, results must have a value greater than the given one for this field) * - `gte` (number, as above but greater than _or equal_) - * - `lt` (number, results must have a value for `key` less than the given value) + * - `lt` (number, results must have a value less than the given one for this field) * - `lte` (number, as above but less than _or equal_) * - * So a filter like `{ key: 'creator', include: 'Cicero' }` will produce - * search results that include `Cicero` as a creator, and a filter like - * `{ key: 'year', gte: 2000, lte: 2005 }` will produce search results whose + * So filters like `{ creator: { include: ['Cicero'] } }` will produce + * search results that all include `Cicero` as a creator, while filters like + * `{ year: { gte: 2000, lte: 2005 } }` will produce search results whose * `year` field is between 2000 and 2005 (inclusive). */ - filters?: FilterParam[]; + filters?: Record; /** * An object specifying which aggregation types should be returned with From 9d90811fa239e35497d76811e6e35f497edd039c Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Fri, 4 Nov 2022 18:20:13 -0700 Subject: [PATCH 04/21] Further refine filter map interface/docs, and include it in URL --- index.ts | 4 +- src/search-param-url-generator.ts | 14 ++++ src/search-params.ts | 129 ++++++++++++++++++++---------- 3 files changed, 104 insertions(+), 43 deletions(-) diff --git a/index.ts b/index.ts index e07284e..e71b620 100644 --- a/index.ts +++ b/index.ts @@ -46,6 +46,8 @@ export { SortDirection, AggregateSearchParams, AggregateSearchParam, - FilterParam, + FilterMap, + FieldFilter, + FilterConstraint, } from './src/search-params'; export { SearchServiceError } from './src/search-service-error'; diff --git a/src/search-param-url-generator.ts b/src/search-param-url-generator.ts index 6ee49fe..d3b16fa 100644 --- a/src/search-param-url-generator.ts +++ b/src/search-param-url-generator.ts @@ -1,5 +1,7 @@ import { AggregateSearchParams, + FieldFilter, + FilterMap, SearchParams, SortParam, } from './search-params'; @@ -54,6 +56,11 @@ export class SearchParamURLGenerator { return `${sortParams.field}:${sortParams.direction}`; } + static filterParamsAsString(filters: FilterMap): string { + // FilterMap already has the correct shape, we just need to stringify it + return JSON.stringify(filters); + } + static generateURLSearchParams(searchParams: SearchParams): URLSearchParams { const params: URLSearchParams = new URLSearchParams(); params.append('user_query', searchParams.query); @@ -78,6 +85,13 @@ export class SearchParamURLGenerator { params.append('fields', searchParams.fields.join(',')); } + if (searchParams.filters && Object.keys(searchParams.filters).length > 0) { + const filterMapString = this.filterParamsAsString(searchParams.filters); + if (filterMapString && filterMapString !== '{}') { // Don't send an empty map + params.append('filter_map', filterMapString); + } + } + if (searchParams.sort && searchParams.sort.length > 0) { const sortStrings = searchParams.sort.map(sort => this.sortParamsAsString(sort) diff --git a/src/search-params.ts b/src/search-params.ts index ddb5dab..0679039 100644 --- a/src/search-params.ts +++ b/src/search-params.ts @@ -49,55 +49,100 @@ export interface SortParam { } /** - * A parameter specifying what filters to apply to the returned hits for a specific field. - * This filter may represent a selected/hidden facet, a value range (e.g., date picker), - * or some other type of restriction on the results. + * Enumerates the possible contraints that may be imposed on search results + * by filter params. */ -export interface FilterParam { +export enum FilterConstraint { /** - * (Optional) One or more values for the current field, which all results must include _at least one of_. + * Specifies that all results must include _at least one of_ the values constrained + * with INCLUDE for this field. * - * For instance, `{ field: 'subject', include: ['baseball', 'basketball'] }` specifies that only results - * that contain _either_ `baseball` _or_ `basketball` as a subject should be returned. + * For instance, `{ subject: { baseball: INCLUDE, basketball: INCLUDE } }` specifies + * that only results containing _either_ `baseball` _or_ `basketball` as a subject + * should be returned. */ - include?: string[], + INCLUDE = 'inc', /** - * (Optional) One or more values for the current field which must _not_ be included in _any_ of the results. + * Specifies that all results must _not_ include the given value for this field. * - * For instance, `{ field: 'subject', exclude: ['baseball', 'basketball'] }` specifies that only results - * that contain _neither_ `baseball` _nor_ `basketball` as a subject should be returned. + * For instance, `{ subject: { baseball: EXCLUDE, basketball: EXCLUDE } }` specifies + * that only results containing _neither_ `baseball` _nor_ `basketball` as a subject + * should be returned. */ - exclude?: string[], + EXCLUDE = 'exc', /** - * (Optional) A strict lower bound on numeric values for the current field. - * All returned hits must have a `field` value greater than the one specified here. - * This only makes sense for numeric fields. + * Imposes a strict lower bound on numeric values for the current field. + * All returned hits must have a value for this field that is greater than the one + * specified by this filter. + * + * This only makes sense for numeric fields like `year`. */ - gt?: number, + GREATER_THAN = 'gt', /** - * (Optional) A non-strict lower bound on numeric values for the current field. - * All returned hits must have a `field` value greater than or equal to the one specified here. - * This only makes sense for numeric fields. + * Imposes a non-strict lower bound on numeric values for the current field. + * All returned hits must have a value for this field that is greater than or equal + * to the one specified by this filter. + * + * This only makes sense for numeric fields like `year`. */ - gte?: number, + GREATER_OR_EQUAL = 'gte', /** - * (Optional) A strict upper bound on numeric values for the current field. - * All returned hits must have a `field` value less than the one specified here. - * This only makes sense for numeric fields. + * Imposes a strict upper bound on numeric values for the current field. + * All returned hits must have a value for this field that is less than the one + * specified by this filter. + * + * This only makes sense for numeric fields like `year`. */ - lt?: number, + LESS_THAN = 'lt', /** - * (Optional) A non-strict upper bound on numeric values for the current field. - * All returned hits must have a `field` value less than or equal to the one specified here. - * This only makes sense for numeric fields. + * Imposes a non-strict upper bound on numeric values for the current field. + * All returned hits must have a value for this field that is less than or equal + * to the one specified by this filter. + * + * This only makes sense for numeric fields like `year`. */ - lte?: number, -} + LESS_OR_EQUAL = 'lte', +}; + +/** + * A filter mapping a field value to the type of constraint that it should impose on results. + * + * Some examples (where the values are members of `FilterConstraint`): + * - `{ 'puppies': INCLUDE }` + * - `{ '1950': GREATER_OR_EQUAL, '1970': LESS_OR_EQUAL }` + */ +export type FieldFilter = Record; + +/** + * A map of fields (e.g., 'year', 'subject', ...) to the filters that should be + * applied to them when retrieving search results. + * + * These filters may represent selected/hidden facets, value ranges (e.g., date picker), + * or other types of restrictions on the result set. + * + * An example of a valid FilterMap: + * ``` + * { + * 'subject': { + * 'dogs': INCLUDE, + * 'puppies': EXCLUDE, + * }, + * 'year': { + * '1990': GREATER_OR_EQUAL, + * '2010': LESS_OR_EQUAL, + * '2003': EXCLUDE, + * '2004': EXCLUDE, + * }, + * // ... + * } + * ``` + */ +export type FilterMap = Record; /** * SearchParams provides an encapsulation to all of the search parameters @@ -157,24 +202,24 @@ export interface SearchParams { fields?: string[]; /** - * A map from field names to filter params that can be used to narrow the result set. + * A map from field names to filters that can be used to shape the result set. * The keys identify what field to filter on (e.g., `'year'`, `'subject'`, etc.), - * and the values provide one or more constraints to apply to that field. + * and the values identify what filters to apply for that field. * - * The constraints allowed are: - * - `include` (string[], results must include at least one of the given values for this field) - * - `exclude` (string[], results must *not* include *any* of the given values for this field) - * - `gt` (number, results must have a value greater than the given one for this field) - * - `gte` (number, as above but greater than _or equal_) - * - `lt` (number, results must have a value less than the given one for this field) - * - `lte` (number, as above but less than _or equal_) + * The constraints allowed are the members of `FilterContraint`: + * - `INCLUDE` (at least one of these values must be present) + * - `EXCLUDE` (none of these values may be present) + * - `GREATER_THAN` (result values must be strictly greater than the one specified) + * - `GREATER_OR_EQUAL` (result values must be greater than or equal to than the one specified) + * - `LESS_THAN` (result values must be strictly less than the one specified) + * - `LESS_OR_EQUAL` (result values must be less than or equal to the one specified) * - * So filters like `{ creator: { include: ['Cicero'] } }` will produce + * So filters like `{ creator: { 'Cicero': INCLUDE } }` will produce * search results that all include `Cicero` as a creator, while filters like - * `{ year: { gte: 2000, lte: 2005 } }` will produce search results whose - * `year` field is between 2000 and 2005 (inclusive). + * `{ year: { '2000': GREATER_THAN, '2005': LESS_THAN } }` will produce search results whose + * `year` field is between 2000 and 2005 (exclusive). */ - filters?: Record; + filters?: FilterMap; /** * An object specifying which aggregation types should be returned with From f5df3110bc9d7c90e5ceb9e61e58ca1478a763be Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Mon, 7 Nov 2022 10:22:47 -0800 Subject: [PATCH 05/21] Add tests for filter_map --- src/search-param-url-generator.ts | 4 +- src/search-params.ts | 26 +++++------ test/search-params.test.ts | 78 ++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/src/search-param-url-generator.ts b/src/search-param-url-generator.ts index d3b16fa..15ac225 100644 --- a/src/search-param-url-generator.ts +++ b/src/search-param-url-generator.ts @@ -1,6 +1,5 @@ import { AggregateSearchParams, - FieldFilter, FilterMap, SearchParams, SortParam, @@ -87,7 +86,8 @@ export class SearchParamURLGenerator { if (searchParams.filters && Object.keys(searchParams.filters).length > 0) { const filterMapString = this.filterParamsAsString(searchParams.filters); - if (filterMapString && filterMapString !== '{}') { // Don't send an empty map + if (filterMapString && filterMapString !== '{}') { + // Don't send an empty map params.append('filter_map', filterMapString); } } diff --git a/src/search-params.ts b/src/search-params.ts index 0679039..7415e86 100644 --- a/src/search-params.ts +++ b/src/search-params.ts @@ -49,14 +49,14 @@ export interface SortParam { } /** - * Enumerates the possible contraints that may be imposed on search results + * Enumerates the possible contraints that may be imposed on search results * by filter params. */ export enum FilterConstraint { /** * Specifies that all results must include _at least one of_ the values constrained * with INCLUDE for this field. - * + * * For instance, `{ subject: { baseball: INCLUDE, basketball: INCLUDE } }` specifies * that only results containing _either_ `baseball` _or_ `basketball` as a subject * should be returned. @@ -65,7 +65,7 @@ export enum FilterConstraint { /** * Specifies that all results must _not_ include the given value for this field. - * + * * For instance, `{ subject: { baseball: EXCLUDE, basketball: EXCLUDE } }` specifies * that only results containing _neither_ `baseball` _nor_ `basketball` as a subject * should be returned. @@ -76,7 +76,7 @@ export enum FilterConstraint { * Imposes a strict lower bound on numeric values for the current field. * All returned hits must have a value for this field that is greater than the one * specified by this filter. - * + * * This only makes sense for numeric fields like `year`. */ GREATER_THAN = 'gt', @@ -85,7 +85,7 @@ export enum FilterConstraint { * Imposes a non-strict lower bound on numeric values for the current field. * All returned hits must have a value for this field that is greater than or equal * to the one specified by this filter. - * + * * This only makes sense for numeric fields like `year`. */ GREATER_OR_EQUAL = 'gte', @@ -94,7 +94,7 @@ export enum FilterConstraint { * Imposes a strict upper bound on numeric values for the current field. * All returned hits must have a value for this field that is less than the one * specified by this filter. - * + * * This only makes sense for numeric fields like `year`. */ LESS_THAN = 'lt', @@ -103,15 +103,15 @@ export enum FilterConstraint { * Imposes a non-strict upper bound on numeric values for the current field. * All returned hits must have a value for this field that is less than or equal * to the one specified by this filter. - * + * * This only makes sense for numeric fields like `year`. */ LESS_OR_EQUAL = 'lte', -}; +} /** * A filter mapping a field value to the type of constraint that it should impose on results. - * + * * Some examples (where the values are members of `FilterConstraint`): * - `{ 'puppies': INCLUDE }` * - `{ '1950': GREATER_OR_EQUAL, '1970': LESS_OR_EQUAL }` @@ -121,10 +121,10 @@ export type FieldFilter = Record; /** * A map of fields (e.g., 'year', 'subject', ...) to the filters that should be * applied to them when retrieving search results. - * + * * These filters may represent selected/hidden facets, value ranges (e.g., date picker), * or other types of restrictions on the result set. - * + * * An example of a valid FilterMap: * ``` * { @@ -205,7 +205,7 @@ export interface SearchParams { * A map from field names to filters that can be used to shape the result set. * The keys identify what field to filter on (e.g., `'year'`, `'subject'`, etc.), * and the values identify what filters to apply for that field. - * + * * The constraints allowed are the members of `FilterContraint`: * - `INCLUDE` (at least one of these values must be present) * - `EXCLUDE` (none of these values may be present) @@ -213,7 +213,7 @@ export interface SearchParams { * - `GREATER_OR_EQUAL` (result values must be greater than or equal to than the one specified) * - `LESS_THAN` (result values must be strictly less than the one specified) * - `LESS_OR_EQUAL` (result values must be less than or equal to the one specified) - * + * * So filters like `{ creator: { 'Cicero': INCLUDE } }` will produce * search results that all include `Cicero` as a creator, while filters like * `{ year: { '2000': GREATER_THAN, '2005': LESS_THAN } }` will produce search results whose diff --git a/test/search-params.test.ts b/test/search-params.test.ts index eef0e1d..723f435 100644 --- a/test/search-params.test.ts +++ b/test/search-params.test.ts @@ -1,7 +1,11 @@ import { expect } from '@open-wc/testing'; import { SearchParamURLGenerator } from '../src/search-param-url-generator'; -import { SortParam } from '../src/search-params'; +import { + FilterConstraint, + SearchParams, + SortParam, +} from '../src/search-params'; describe('SearchParams', () => { it('can be instantiated with just a query', async () => { @@ -252,6 +256,78 @@ describe('SearchParams', () => { expect(queryParams.get('aggregations')).to.equal(expectedAggregationsParam); }); + it('properly generates a URLSearchParam with simple filter map', async () => { + const query = 'title:foo'; + const params: SearchParams = { + query, + filters: { + subject: { + foo: FilterConstraint.INCLUDE, + }, + }, + }; + const urlSearchParam = SearchParamURLGenerator.generateURLSearchParams( + params + ); + const queryAsString = urlSearchParam.toString(); + const queryParams = new URL(`https://foo.bar/?${queryAsString}`) + .searchParams; + expect(queryParams.get('user_query')).to.equal('title:foo'); + expect(queryParams.get('filter_map')).to.equal('{"subject":{"foo":"inc"}}'); + }); + + it('properly generates a URLSearchParam with complex filter map', async () => { + const query = 'title:foo'; + const params: SearchParams = { + query, + filters: { + subject: { + foo: FilterConstraint.INCLUDE, + bar: FilterConstraint.EXCLUDE, + }, + year: { + 1950: FilterConstraint.GREATER_OR_EQUAL, + 2000: FilterConstraint.LESS_OR_EQUAL, + 1980: FilterConstraint.EXCLUDE, + 1990: FilterConstraint.EXCLUDE, + }, + }, + }; + const urlSearchParam = SearchParamURLGenerator.generateURLSearchParams( + params + ); + const queryAsString = urlSearchParam.toString(); + const queryParams = new URL(`https://foo.bar/?${queryAsString}`) + .searchParams; + expect(queryParams.get('user_query')).to.equal('title:foo'); + expect(queryParams.get('filter_map')).to.equal( + '{"subject":{"foo":"inc","bar":"exc"},"year":{"1950":"gte","1980":"exc","1990":"exc","2000":"lte"}}' + ); + }); + + it('properly generates a URLSearchParam with gt/lt pairs', async () => { + const query = 'title:foo'; + const params: SearchParams = { + query, + filters: { + year: { + 1950: FilterConstraint.GREATER_THAN, + 2000: FilterConstraint.LESS_THAN, + }, + }, + }; + const urlSearchParam = SearchParamURLGenerator.generateURLSearchParams( + params + ); + const queryAsString = urlSearchParam.toString(); + const queryParams = new URL(`https://foo.bar/?${queryAsString}`) + .searchParams; + expect(queryParams.get('user_query')).to.equal('title:foo'); + expect(queryParams.get('filter_map')).to.equal( + '{"year":{"1950":"gt","2000":"lt"}}' + ); + }); + it('properly generates a URLSearchParam with debugging enabled', async () => { const query = 'title:foo'; const params = { From 0583afefd3be3ea8cf52f155b5c581c3e2af1bef Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Mon, 7 Nov 2022 10:39:26 -0800 Subject: [PATCH 06/21] Add tests for empty fields/sort params --- test/search-params.test.ts | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/search-params.test.ts b/test/search-params.test.ts index 723f435..f0e1514 100644 --- a/test/search-params.test.ts +++ b/test/search-params.test.ts @@ -101,6 +101,40 @@ describe('SearchParams', () => { expect(queryParams.get('sort')).to.equal('downloads:desc,foo:asc'); }); + it('does not include fields param in URL for empty fields array', async () => { + const query = 'title:foo AND collection:bar'; + const fields: string[] = []; + const params = { + query, + fields, + }; + const urlSearchParam = SearchParamURLGenerator.generateURLSearchParams( + params + ); + const queryAsString = urlSearchParam.toString(); + const queryParams = new URL(`https://foo.bar/?${queryAsString}`) + .searchParams; + expect(queryParams.get('user_query')).to.equal(query); + expect(queryParams.get('fields')).to.be.null; + }); + + it('does not include sort param in URL for empty sort array', async () => { + const query = 'title:foo AND collection:bar'; + const sort: SortParam[] = []; + const params = { + query, + sort, + }; + const urlSearchParam = SearchParamURLGenerator.generateURLSearchParams( + params + ); + const queryAsString = urlSearchParam.toString(); + const queryParams = new URL(`https://foo.bar/?${queryAsString}`) + .searchParams; + expect(queryParams.get('user_query')).to.equal(query); + expect(queryParams.get('sort')).to.be.null; + }); + it('properly generates a URLSearchParam with a page_type', async () => { const query = 'title:foo AND collection:bar'; const params = { From d3a0963e54e2a0c9be6b5ad14a5a59fc3cbfb6e2 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Mon, 7 Nov 2022 16:44:11 -0800 Subject: [PATCH 07/21] Collapsible URL params in demo app + refactoring --- demo/app-root.ts | 47 ++++++++++++++++++++++++++++------------------- demo/index.html | 3 ++- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index b7951ba..f04736d 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -213,24 +213,27 @@ export class AppRoot extends LitElement { - ${this.searchResults ? this.resultsTemplate : nothing} + ${this.searchResults || this.loadingSearchResults ? this.resultsTemplate : nothing} `; } private get resultsTemplate(): TemplateResult { return html` - ${this.lastSearchParams - ? html`
- Last search params: - 
${this.lastSearchParams}
-
` - : nothing} - ${this.lastAggregationParams - ? html`
- Last aggregation params: - 
${this.lastAggregationParams}
-
` - : nothing} +
+ PPS URL params + ${this.lastSearchParams + ? html`
+ Last search params: + 
${this.lastSearchParams}
+
` + : nothing} + ${this.lastAggregationParams + ? html`
+ Last aggregation params: + 
${this.lastAggregationParams}
+
` + : nothing} +
${this.loadingSearchResults ? html`

Loading search results...

` : [this.minimalSearchResultsTemplate, this.fullSearchResultsTemplate]} @@ -370,15 +373,16 @@ export class AppRoot extends LitElement { uid: 'demo', }; + this.lastSearchParams = SearchParamURLGenerator.generateURLSearchParams( + searchParams + ).toString(); + this.loadingSearchResults = true; const result = await this.searchService.search(searchParams, searchType); this.loadingSearchResults = false; if (result?.success) { this.searchResponse = result?.success; - this.lastSearchParams = SearchParamURLGenerator.generateURLSearchParams( - searchParams - ).toString(); } else { alert(`Oh noes: ${result?.error?.message}`); console.error('Error searching', result?.error); @@ -413,15 +417,16 @@ export class AppRoot extends LitElement { searchParams.aggregations = aggregations; } + this.lastAggregationParams = SearchParamURLGenerator.generateURLSearchParams( + searchParams + ).toString(); + this.loadingAggregations = true; const result = await this.searchService.search(searchParams, searchType); this.loadingAggregations = false; if (result?.success) { this.aggregationsResponse = result?.success; - this.lastAggregationParams = SearchParamURLGenerator.generateURLSearchParams( - searchParams - ).toString(); } else { alert(`Oh noes: ${result?.error?.message}`); console.error('Error searching', result?.error); @@ -430,6 +435,10 @@ export class AppRoot extends LitElement { static get styles(): CSSResult { return css` + :host { + font-size: 1.3rem; + } + .search-options { margin-top: 0.6rem; } diff --git a/demo/index.html b/demo/index.html index a6378b3..7108b5c 100644 --- a/demo/index.html +++ b/demo/index.html @@ -3,9 +3,10 @@ From 66a35afba40d2e820660a85a368c0748ad3a9cb6 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 12:35:34 -0800 Subject: [PATCH 08/21] Add FilterMapBuilder utility class --- index.ts | 1 + src/filter-map-builder.ts | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 src/filter-map-builder.ts diff --git a/index.ts b/index.ts index e71b620..4d4b727 100644 --- a/index.ts +++ b/index.ts @@ -50,4 +50,5 @@ export { FieldFilter, FilterConstraint, } from './src/search-params'; +export { FilterMapBuilder } from './src/filter-map-builder'; export { SearchServiceError } from './src/search-service-error'; diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts new file mode 100644 index 0000000..355d562 --- /dev/null +++ b/src/filter-map-builder.ts @@ -0,0 +1,94 @@ +import { FilterConstraint, FilterMap } from "./search-params"; + +const NUMERIC_CONSTRAINTS = [ + FilterConstraint.GREATER_OR_EQUAL, + FilterConstraint.GREATER_THAN, + FilterConstraint.LESS_OR_EQUAL, + FilterConstraint.LESS_THAN +]; + +/** + * A utility class for building filter maps + */ + export class FilterMapBuilder { + private filterMap: FilterMap = {}; + + /** + * Adds a filter to the FilterMap under construction. + * The new filter may overwrite or modify existing filters already added to the builder. + * @param field The field to filter on (e.g., 'subject', 'year', ...) + * @param value The value of the field to filter on (e.g., 'Cicero', '1920', ...) + * @param constraint The constraint to apply to the `field`, with respect to the given `value`. + * Allowed values are the enum members of `FilterConstraint`. + */ + addFilter(field: string, value: string, constraint: FilterConstraint): this { + if (!this.filterMap[field]) { + this.filterMap[field] = {}; + } + + // Edge case for numeric fields (i.e., year) + // Logically it would be valid for excluded values to overlap with gt/gte/lt/lte values. + // For instance, specifying 1950 <= year <= 2000, but also excluding year = 1950. + // But since we can only have one constraint for a given value, these would overwrite each other. + // To work around this, we adjust the resulting map to maintain the correct logic. + const currentConstraint = this.filterMap[field][value]; + if (currentConstraint && constraint === FilterConstraint.EXCLUDE) { + if (currentConstraint === FilterConstraint.GREATER_OR_EQUAL) { + // gte and exclude on the same value -> gt + this.filterMap[field][value] = FilterConstraint.GREATER_THAN; + return this; + } else if (currentConstraint === FilterConstraint.LESS_OR_EQUAL) { + // lte and exclude on the same value -> lt + this.filterMap[field][value] = FilterConstraint.LESS_THAN; + return this; + } else if (currentConstraint === FilterConstraint.GREATER_THAN || currentConstraint === FilterConstraint.LESS_THAN) { + // gt/lt and exclude -> no additional filter + return this; + } + } + + // An 'include' constraint should always take precedence over a gt/gte/lt/lte constraint on the same value. + if (currentConstraint === FilterConstraint.INCLUDE) { + if (NUMERIC_CONSTRAINTS.includes(constraint)) { + // Don't overwrite the existing 'include' constraint + return this; + } + } + + // Otherwise, overwrite any existing filter for this value + this.filterMap[field][value] = constraint; + return this; + } + + /** + * Initializes the filter map under construction to have filters exactly equal to the given one. + * This will overwrite *all* existing filters already added to the builder. + * @param map The FilterMap to set this builder's state to. + */ + setFilterMap(map: FilterMap): this { + this.filterMap = {...map}; + return this; + } + + /** + * Adds all filters from an existing filter map to the one being built. + * Filters from the provided map may overwrite or modify existing filters already added to the builder. + * @param map The FilterMap to merge into the one being built. + */ + mergeFilterMap(map: FilterMap): this { + for (const [field, filters] of Object.entries(map)) { + for (const [value, constraint] of Object.entries(filters)) { + this.addFilter(field, value, constraint); + } + } + return this; + } + + /** + * + * @returns A FilterMap that includes the filters applied to + */ + build(): FilterMap { + return this.filterMap; + } +} \ No newline at end of file From 9d77e88ac432340b646b435f37b244634373a149 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 12:36:56 -0800 Subject: [PATCH 09/21] Fix demo app alignment of radio/checkboxes --- demo/app-root.ts | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index f04736d..e602c4d 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -88,25 +88,31 @@ export class AppRoot extends LitElement { - - + + + +
Search type: - - - - + + + + + + + +
@@ -450,6 +456,12 @@ export class AppRoot extends LitElement { fieldset { margin-bottom: 0.5rem; } + + .input-with-label { + display: inline-flex; + align-items: center; + margin-right: 8px; + } `; } } From 80deb9f7bbafaf41ada97c858855bd60cf015352 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 12:37:08 -0800 Subject: [PATCH 10/21] Add filters to demo app --- demo/app-root.ts | 177 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 160 insertions(+), 17 deletions(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index e602c4d..fe70610 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -1,14 +1,18 @@ import { html, css, LitElement, TemplateResult, CSSResult, nothing } from 'lit'; import { customElement, query, state } from 'lit/decorators.js'; +import { map } from 'lit/directives/map.js'; import { SearchResponse } from '../src/responses/search-response'; import { SearchService } from '../src/search-service'; import { SearchServiceInterface } from '../src/search-service-interface'; import { SearchResult } from '../src/models/hit-types/hit'; import { SearchType } from '../src/search-type'; -import { SearchParams, SortDirection } from '../src/search-params'; +import { FilterConstraint, FilterMap, SearchParams, SortDirection } from '../src/search-params'; import { Aggregation, Bucket } from '../src/models/aggregation'; import { SearchBackendOptionsInterface } from '../src/search-backend/search-backend-options'; import { SearchParamURLGenerator } from '../src/search-param-url-generator'; +import { FilterMapBuilder } from '../src/filter-map-builder'; + +type SingleFilter = { field: string, value: string, constraint: FilterConstraint }; @customElement('app-root') export class AppRoot extends LitElement { @@ -33,6 +37,9 @@ export class AppRoot extends LitElement { @state() private searchServiceUrlOptions?: SearchBackendOptionsInterface = this.initSearchServiceUrlOptions(); + @state() + private filterMap: FilterMap = {}; + @state() private searchResponse?: SearchResponse; @@ -129,18 +136,48 @@ export class AppRoot extends LitElement {
Sort by title: - - - - - - + + + + + + + + + + + + +
+ +
+ Filters: + + + + +
${this.appliedFiltersTemplate}
@@ -223,6 +260,80 @@ export class AppRoot extends LitElement { `; } + private filterFieldChanged(e: Event) { + const target = e.target as HTMLSelectElement; + const fieldIsNumeric = !!target.selectedOptions[0].dataset.numeric; + const constraints = (this.shadowRoot?.querySelectorAll('#filter-constraint option') ?? []) as HTMLOptionElement[]; + for (const constraint of constraints) { + constraint.toggleAttribute('hidden', !fieldIsNumeric && !!constraint.dataset.numeric); + } + } + + private addFilterClicked() { + const filterFieldInput = this.shadowRoot?.getElementById('filter-field') as HTMLSelectElement | null; + const filterField = filterFieldInput?.selectedOptions[0]?.value; + + const filterConstraintInput = this.shadowRoot?.getElementById('filter-constraint') as HTMLSelectElement | null; + const filterConstraint = filterConstraintInput?.selectedOptions[0]?.value as FilterConstraint; + + const filterValueInput = this.shadowRoot?.getElementById('filter-value') as HTMLInputElement | null; + const filterValue = filterValueInput?.value; + + if (!filterField || !filterConstraint || !filterValue) { + return; + } + + this.filterMap = new FilterMapBuilder() + .setFilterMap(this.filterMap) + .addFilter(filterField, filterValue, filterConstraint) + .build(); + + if (filterValueInput) filterValueInput.value = ''; + } + + private removeFilterClicked(e: Event) { + const target = e.target as HTMLButtonElement; + const { field, value } = target.dataset; + + if (field && value) { + this.filterMap = {...this.filterMap}; + delete this.filterMap[field][value]; + + if (Object.keys(this.filterMap[field]).length === 0) { + delete this.filterMap[field]; + } + } + } + + private get appliedFiltersTemplate() { + const filtersArray: SingleFilter[] = []; + for (const [field, filters] of Object.entries(this.filterMap)) { + for (const [value, constraint] of Object.entries(filters)) { + filtersArray.push({ field, value, constraint }); + } + } + + if (filtersArray.length === 0) return html`(no filters applied)`; + + const readableConstraints: Record = { + 'inc': 'includes', + 'exc': 'excludes', + 'gt': '>', + 'gte': '>=', + 'lt': '<', + 'lte': '<=', + }; + + return map(filtersArray, ({ field, value, constraint }) => { + return html` + + '${field}' ${readableConstraints[constraint]} '${value}' + + `; + }); + } + private get resultsTemplate(): TemplateResult { return html`
@@ -374,14 +485,15 @@ export class AppRoot extends LitElement { query, rows: numRows, sort: sortParam, + filters: this.filterMap, aggregations: { omit: true }, debugging: includeDebugging, uid: 'demo', }; - this.lastSearchParams = SearchParamURLGenerator.generateURLSearchParams( + this.lastSearchParams = decodeURIComponent(SearchParamURLGenerator.generateURLSearchParams( searchParams - ).toString(); + ).toString()); this.loadingSearchResults = true; const result = await this.searchService.search(searchParams, searchType); @@ -414,6 +526,7 @@ export class AppRoot extends LitElement { const searchParams: SearchParams = { query, rows: 0, + filters: this.filterMap, aggregationsSize: numAggs, debugging: includeDebugging, uid: 'demo', @@ -423,9 +536,9 @@ export class AppRoot extends LitElement { searchParams.aggregations = aggregations; } - this.lastAggregationParams = SearchParamURLGenerator.generateURLSearchParams( + this.lastAggregationParams = decodeURIComponent(SearchParamURLGenerator.generateURLSearchParams( searchParams - ).toString(); + ).toString()); this.loadingAggregations = true; const result = await this.searchService.search(searchParams, searchType); @@ -457,6 +570,36 @@ export class AppRoot extends LitElement { margin-bottom: 0.5rem; } + #applied-filters { + margin-top: 6px; + } + + .filter { + display: inline-block; + margin-bottom: 3px; + font-size: 1.1rem; + font-family: sans-serif; + } + + .filter-text { + padding: 3px 3px 3px 6px; + border-radius: 3px 0 0 3px; + background: #ccc; + } + + .remove-filter { + all: unset; + padding: 3px 6px; + border-radius: 0 3px 3px 0; + background: #ccc; + } + .remove-filter:hover { + background: #999; + } + .remove-filter:active { + background: #888; + } + .input-with-label { display: inline-flex; align-items: center; From 8791c9598cdbc9b4cb6e088aab1cbd7aee94a159 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 13:05:55 -0800 Subject: [PATCH 11/21] Add removeFilter method to filter map builder --- src/filter-map-builder.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts index 355d562..0f935fa 100644 --- a/src/filter-map-builder.ts +++ b/src/filter-map-builder.ts @@ -60,6 +60,24 @@ const NUMERIC_CONSTRAINTS = [ return this; } + /** + * Removes any filter currently associated with the given field and value. + * @param field The field to remove a filter for + * @param value The value to remove the filter for + */ + removeFilter(field: string, value: string): this { + if (this.filterMap[field]) { + delete this.filterMap[field][value]; + + // If there are no remaining filters for this field, delete the whole field object. + if (Object.keys(this.filterMap[field]).length === 0) { + delete this.filterMap[field]; + } + } + + return this; + } + /** * Initializes the filter map under construction to have filters exactly equal to the given one. * This will overwrite *all* existing filters already added to the builder. From 0c9ed482313810675f552330a690065726cf5286 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 13:24:36 -0800 Subject: [PATCH 12/21] Add tests for filter map builder and fix formatting --- demo/app-root.ts | 90 ++++++++++++++------ src/filter-map-builder.ts | 21 +++-- test/filter-map-builder.test.ts | 142 ++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 36 deletions(-) create mode 100644 test/filter-map-builder.test.ts diff --git a/demo/app-root.ts b/demo/app-root.ts index fe70610..96bd5c9 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -6,13 +6,22 @@ import { SearchService } from '../src/search-service'; import { SearchServiceInterface } from '../src/search-service-interface'; import { SearchResult } from '../src/models/hit-types/hit'; import { SearchType } from '../src/search-type'; -import { FilterConstraint, FilterMap, SearchParams, SortDirection } from '../src/search-params'; +import { + FilterConstraint, + FilterMap, + SearchParams, + SortDirection, +} from '../src/search-params'; import { Aggregation, Bucket } from '../src/models/aggregation'; import { SearchBackendOptionsInterface } from '../src/search-backend/search-backend-options'; import { SearchParamURLGenerator } from '../src/search-param-url-generator'; import { FilterMapBuilder } from '../src/filter-map-builder'; -type SingleFilter = { field: string, value: string, constraint: FilterConstraint }; +type SingleFilter = { + field: string; + value: string; + constraint: FilterConstraint; +}; @customElement('app-root') export class AppRoot extends LitElement { @@ -175,8 +184,10 @@ export class AppRoot extends LitElement { - - + +
${this.appliedFiltersTemplate}
@@ -256,27 +267,41 @@ export class AppRoot extends LitElement {
- ${this.searchResults || this.loadingSearchResults ? this.resultsTemplate : nothing} + ${this.searchResults || this.loadingSearchResults + ? this.resultsTemplate + : nothing} `; } private filterFieldChanged(e: Event) { const target = e.target as HTMLSelectElement; const fieldIsNumeric = !!target.selectedOptions[0].dataset.numeric; - const constraints = (this.shadowRoot?.querySelectorAll('#filter-constraint option') ?? []) as HTMLOptionElement[]; + const constraints = (this.shadowRoot?.querySelectorAll( + '#filter-constraint option' + ) ?? []) as HTMLOptionElement[]; for (const constraint of constraints) { - constraint.toggleAttribute('hidden', !fieldIsNumeric && !!constraint.dataset.numeric); + constraint.toggleAttribute( + 'hidden', + !fieldIsNumeric && !!constraint.dataset.numeric + ); } } private addFilterClicked() { - const filterFieldInput = this.shadowRoot?.getElementById('filter-field') as HTMLSelectElement | null; + const filterFieldInput = this.shadowRoot?.getElementById( + 'filter-field' + ) as HTMLSelectElement | null; const filterField = filterFieldInput?.selectedOptions[0]?.value; - const filterConstraintInput = this.shadowRoot?.getElementById('filter-constraint') as HTMLSelectElement | null; - const filterConstraint = filterConstraintInput?.selectedOptions[0]?.value as FilterConstraint; + const filterConstraintInput = this.shadowRoot?.getElementById( + 'filter-constraint' + ) as HTMLSelectElement | null; + const filterConstraint = filterConstraintInput?.selectedOptions[0] + ?.value as FilterConstraint; - const filterValueInput = this.shadowRoot?.getElementById('filter-value') as HTMLInputElement | null; + const filterValueInput = this.shadowRoot?.getElementById( + 'filter-value' + ) as HTMLInputElement | null; const filterValue = filterValueInput?.value; if (!filterField || !filterConstraint || !filterValue) { @@ -296,7 +321,7 @@ export class AppRoot extends LitElement { const { field, value } = target.dataset; if (field && value) { - this.filterMap = {...this.filterMap}; + this.filterMap = { ...this.filterMap }; delete this.filterMap[field][value]; if (Object.keys(this.filterMap[field]).length === 0) { @@ -313,22 +338,33 @@ export class AppRoot extends LitElement { } } - if (filtersArray.length === 0) return html`(no filters applied)`; + if (filtersArray.length === 0) + return html`(no filters applied)`; const readableConstraints: Record = { - 'inc': 'includes', - 'exc': 'excludes', - 'gt': '>', - 'gte': '>=', - 'lt': '<', - 'lte': '<=', + inc: 'includes', + exc: 'excludes', + gt: '>', + gte: '>=', + lt: '<', + lte: '<=', }; return map(filtersArray, ({ field, value, constraint }) => { return html` - '${field}' ${readableConstraints[constraint]} '${value}' + '${field}' ${readableConstraints[constraint]} '${value}' `; }); @@ -491,9 +527,9 @@ export class AppRoot extends LitElement { uid: 'demo', }; - this.lastSearchParams = decodeURIComponent(SearchParamURLGenerator.generateURLSearchParams( - searchParams - ).toString()); + this.lastSearchParams = decodeURIComponent( + SearchParamURLGenerator.generateURLSearchParams(searchParams).toString() + ); this.loadingSearchResults = true; const result = await this.searchService.search(searchParams, searchType); @@ -536,9 +572,9 @@ export class AppRoot extends LitElement { searchParams.aggregations = aggregations; } - this.lastAggregationParams = decodeURIComponent(SearchParamURLGenerator.generateURLSearchParams( - searchParams - ).toString()); + this.lastAggregationParams = decodeURIComponent( + SearchParamURLGenerator.generateURLSearchParams(searchParams).toString() + ); this.loadingAggregations = true; const result = await this.searchService.search(searchParams, searchType); diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts index 0f935fa..e438dc7 100644 --- a/src/filter-map-builder.ts +++ b/src/filter-map-builder.ts @@ -1,16 +1,16 @@ -import { FilterConstraint, FilterMap } from "./search-params"; +import { FilterConstraint, FilterMap } from './search-params'; const NUMERIC_CONSTRAINTS = [ FilterConstraint.GREATER_OR_EQUAL, FilterConstraint.GREATER_THAN, FilterConstraint.LESS_OR_EQUAL, - FilterConstraint.LESS_THAN + FilterConstraint.LESS_THAN, ]; /** * A utility class for building filter maps */ - export class FilterMapBuilder { +export class FilterMapBuilder { private filterMap: FilterMap = {}; /** @@ -25,7 +25,7 @@ const NUMERIC_CONSTRAINTS = [ if (!this.filterMap[field]) { this.filterMap[field] = {}; } - + // Edge case for numeric fields (i.e., year) // Logically it would be valid for excluded values to overlap with gt/gte/lt/lte values. // For instance, specifying 1950 <= year <= 2000, but also excluding year = 1950. @@ -41,7 +41,10 @@ const NUMERIC_CONSTRAINTS = [ // lte and exclude on the same value -> lt this.filterMap[field][value] = FilterConstraint.LESS_THAN; return this; - } else if (currentConstraint === FilterConstraint.GREATER_THAN || currentConstraint === FilterConstraint.LESS_THAN) { + } else if ( + currentConstraint === FilterConstraint.GREATER_THAN || + currentConstraint === FilterConstraint.LESS_THAN + ) { // gt/lt and exclude -> no additional filter return this; } @@ -84,7 +87,7 @@ const NUMERIC_CONSTRAINTS = [ * @param map The FilterMap to set this builder's state to. */ setFilterMap(map: FilterMap): this { - this.filterMap = {...map}; + this.filterMap = { ...map }; return this; } @@ -103,10 +106,10 @@ const NUMERIC_CONSTRAINTS = [ } /** - * - * @returns A FilterMap that includes the filters applied to + * + * @returns A FilterMap that includes the filters applied to */ build(): FilterMap { return this.filterMap; } -} \ No newline at end of file +} diff --git a/test/filter-map-builder.test.ts b/test/filter-map-builder.test.ts new file mode 100644 index 0000000..4157a10 --- /dev/null +++ b/test/filter-map-builder.test.ts @@ -0,0 +1,142 @@ +import { expect } from '@open-wc/testing'; +import { FilterMapBuilder } from '../src/filter-map-builder'; +import { FilterConstraint, FilterMap } from '../src/search-params'; + +describe('filter map builder', () => { + it('initializes with empty filter map', () => { + expect(new FilterMapBuilder().build()).to.deep.equal({}); + }); + + it('can add filters', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); + expect(builder.build()).to.deep.equal({ foo: { bar: 'inc' } }); + + builder.addFilter('baz', 'boop', FilterConstraint.EXCLUDE); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc' }, + baz: { boop: 'exc' }, + }); + + builder.addFilter('foo', 'beep', FilterConstraint.GREATER_THAN); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc', beep: 'gt' }, + baz: { boop: 'exc' }, + }); + }); + + it('can remove filters', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); + builder.addFilter('baz', 'boop', FilterConstraint.EXCLUDE); + builder.addFilter('foo', 'beep', FilterConstraint.GREATER_THAN); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc', beep: 'gt' }, + baz: { boop: 'exc' }, + }); + + builder.removeFilter('foo', 'bar'); + expect(builder.build()).to.deep.equal({ + foo: { beep: 'gt' }, + baz: { boop: 'exc' }, + }); + + builder.removeFilter('foo', 'beep'); + expect(builder.build()).to.deep.equal({ baz: { boop: 'exc' } }); + + builder.removeFilter('not', 'exist'); + expect(builder.build()).to.deep.equal({ baz: { boop: 'exc' } }); + + builder.removeFilter('baz', 'boop'); + expect(builder.build()).to.deep.equal({}); + }); + + it('can be initialized with an existing filter map', () => { + const builder = new FilterMapBuilder(); + const filterMap: FilterMap = { + foo: { + bar: FilterConstraint.INCLUDE, + }, + baz: { + boop: FilterConstraint.EXCLUDE, + }, + }; + + builder.setFilterMap(filterMap); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc' }, + baz: { boop: 'exc' }, + }); + }); + + it('can be merged with an existing filter map', () => { + const builder = new FilterMapBuilder(); + const filterMap: FilterMap = { + foo: { + bar: FilterConstraint.INCLUDE, + }, + baz: { + boop: FilterConstraint.EXCLUDE, + }, + }; + + builder.addFilter('foo', 'beep', FilterConstraint.LESS_OR_EQUAL); + expect(builder.build()).to.deep.equal({ foo: { beep: 'lte' } }); + + builder.mergeFilterMap(filterMap); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc', beep: 'lte' }, + baz: { boop: 'exc' }, + }); + }); + + it('correctly resolves overlap between exclude and gte constraints', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('year', '1950', FilterConstraint.GREATER_OR_EQUAL); + expect(builder.build()).to.deep.equal({ year: { 1950: 'gte' } }); + + builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); + // Overwrites constraint with GREATER_THAN + expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } }); + }); + + it('correctly resolves overlap between exclude and lte constraints', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('year', '1950', FilterConstraint.LESS_OR_EQUAL); + expect(builder.build()).to.deep.equal({ year: { 1950: 'lte' } }); + + builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); + // Overwrites constraint with LESS_THAN + expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } }); + }); + + it('correctly resolves overlap between exclude and gt constraints', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('year', '1950', FilterConstraint.GREATER_THAN); + expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } }); + + builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); + // Leaves constraint unchanged + expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } }); + }); + + it('correctly resolves overlap between exclude and lt constraints', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('year', '1950', FilterConstraint.LESS_THAN); + expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } }); + + builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); + // Leaves constraint unchanged + expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } }); + }); + + it('correctly resolves overlap between include and numeric constraints', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('year', '1950', FilterConstraint.INCLUDE); + expect(builder.build()).to.deep.equal({ year: { 1950: 'inc' } }); + + builder.addFilter('year', '1950', FilterConstraint.GREATER_THAN); + // Existing include constraint takes precedence + expect(builder.build()).to.deep.equal({ year: { 1950: 'inc' } }); + }); +}); From 4f2cf739bb531c0ed556fe16c9ac0b2cdb6d9a78 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 14:50:39 -0800 Subject: [PATCH 13/21] Clean up demo app queries --- demo/app-root.ts | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index 96bd5c9..d325d6d 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -40,6 +40,15 @@ export class AppRoot extends LitElement { @query(`input[name='sort']:checked`) private checkedSort!: HTMLInputElement; + @query('#filter-field') + private filterFieldInput!: HTMLSelectElement; + + @query('#filter-constraint') + private filterConstraintInput!: HTMLSelectElement; + + @query('#filter-value') + private filterValueInput!: HTMLInputElement; + @query('#aggs-default') private defaultAggregationsCheckbox!: HTMLInputElement; @@ -288,21 +297,10 @@ export class AppRoot extends LitElement { } private addFilterClicked() { - const filterFieldInput = this.shadowRoot?.getElementById( - 'filter-field' - ) as HTMLSelectElement | null; - const filterField = filterFieldInput?.selectedOptions[0]?.value; - - const filterConstraintInput = this.shadowRoot?.getElementById( - 'filter-constraint' - ) as HTMLSelectElement | null; - const filterConstraint = filterConstraintInput?.selectedOptions[0] - ?.value as FilterConstraint; - - const filterValueInput = this.shadowRoot?.getElementById( - 'filter-value' - ) as HTMLInputElement | null; - const filterValue = filterValueInput?.value; + const filterField = this.filterFieldInput.selectedOptions[0].value; + const filterValue = this.filterValueInput.value; + const filterConstraint = this.filterConstraintInput.selectedOptions[0] + .value as FilterConstraint; if (!filterField || !filterConstraint || !filterValue) { return; @@ -313,7 +311,7 @@ export class AppRoot extends LitElement { .addFilter(filterField, filterValue, filterConstraint) .build(); - if (filterValueInput) filterValueInput.value = ''; + this.filterValueInput.value = ''; } private removeFilterClicked(e: Event) { @@ -628,6 +626,7 @@ export class AppRoot extends LitElement { padding: 3px 6px; border-radius: 0 3px 3px 0; background: #ccc; + cursor: pointer; } .remove-filter:hover { background: #999; From bc0f49ab62a559d9112ea2cf088f499237645d24 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 15:05:11 -0800 Subject: [PATCH 14/21] Fill in missing doc comment on builder --- src/filter-map-builder.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts index e438dc7..388959c 100644 --- a/src/filter-map-builder.ts +++ b/src/filter-map-builder.ts @@ -106,8 +106,8 @@ export class FilterMapBuilder { } /** - * - * @returns A FilterMap that includes the filters applied to + * Produces a `FilterMap` including all the filters that have been applied to + * this builder. */ build(): FilterMap { return this.filterMap; From b41482a5e081067bcd075bcfe3038f33f876f412 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 18:22:18 -0800 Subject: [PATCH 15/21] Add notes to gt/lt constraint docs (they are unsupported by FTS) --- src/search-params.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/search-params.ts b/src/search-params.ts index 7415e86..4266b97 100644 --- a/src/search-params.ts +++ b/src/search-params.ts @@ -78,6 +78,8 @@ export enum FilterConstraint { * specified by this filter. * * This only makes sense for numeric fields like `year`. + * Note that `GREATER_THAN` is not supported by the FTS engine, for which it is + * coerced to `GREATER_OR_EQUAL`. */ GREATER_THAN = 'gt', @@ -96,6 +98,8 @@ export enum FilterConstraint { * specified by this filter. * * This only makes sense for numeric fields like `year`. + * Note that `LESS_THAN` is not supported by the FTS engine, for which it is + * coerced to `LESS_OR_EQUAL`. */ LESS_THAN = 'lt', From a2d8b48c613cf3f56999834b16f7ac5a5e0e3f7c Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 19:01:28 -0800 Subject: [PATCH 16/21] Further streamline demo app with common template for aggregation inputs --- demo/app-root.ts | 111 +++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 72 deletions(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index d325d6d..534d96c 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -107,9 +107,9 @@ export class AppRoot extends LitElement { render(): TemplateResult { return html`
- Search + Search options
- + @@ -202,76 +202,22 @@ export class AppRoot extends LitElement {
Include aggregations for: - - - - - - - - - - - - - - - - + + + + + ${this.aggregationCheckboxTemplate('mediatype', 'Mediatype')} + ${this.aggregationCheckboxTemplate('year', 'Year')} + ${this.aggregationCheckboxTemplate('subject', 'Subject')} + ${this.aggregationCheckboxTemplate('language', 'Language')} + ${this.aggregationCheckboxTemplate('creator', 'Creator')} + ${this.aggregationCheckboxTemplate('collection', 'Collection')} + ${this.aggregationCheckboxTemplate('lending___status', 'Lending')}
@@ -368,6 +314,23 @@ export class AppRoot extends LitElement { }); } + private aggregationCheckboxTemplate(value: string, label?: string) { + const id = `aggs-${value}`; + return html` + + + + + `; + } + private get resultsTemplate(): TemplateResult { return html`
@@ -604,6 +567,10 @@ export class AppRoot extends LitElement { margin-bottom: 0.5rem; } + #search-input { + min-width: 220px; + } + #applied-filters { margin-top: 6px; } From c023aedec2c74dea03a6fb35a315fd8e69fdffc2 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Wed, 9 Nov 2022 19:02:15 -0800 Subject: [PATCH 17/21] Make aggregation label param required --- demo/app-root.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index 534d96c..77bf12d 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -314,7 +314,7 @@ export class AppRoot extends LitElement { }); } - private aggregationCheckboxTemplate(value: string, label?: string) { + private aggregationCheckboxTemplate(value: string, label: string) { const id = `aggs-${value}`; return html` From 907911e716d5b4a135928eca2f746f18bc3a0987 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Thu, 10 Nov 2022 17:58:04 -0800 Subject: [PATCH 18/21] Remove filter overlap handling -- PPS doesn't actually impl gt/lt so it's pointless --- src/filter-map-builder.ts | 45 ++--------------------------- test/filter-map-builder.test.ts | 50 --------------------------------- 2 files changed, 3 insertions(+), 92 deletions(-) diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts index 388959c..a30101c 100644 --- a/src/filter-map-builder.ts +++ b/src/filter-map-builder.ts @@ -1,12 +1,5 @@ import { FilterConstraint, FilterMap } from './search-params'; -const NUMERIC_CONSTRAINTS = [ - FilterConstraint.GREATER_OR_EQUAL, - FilterConstraint.GREATER_THAN, - FilterConstraint.LESS_OR_EQUAL, - FilterConstraint.LESS_THAN, -]; - /** * A utility class for building filter maps */ @@ -15,7 +8,7 @@ export class FilterMapBuilder { /** * Adds a filter to the FilterMap under construction. - * The new filter may overwrite or modify existing filters already added to the builder. + * The new filter may overwrite existing filters already added to the builder. * @param field The field to filter on (e.g., 'subject', 'year', ...) * @param value The value of the field to filter on (e.g., 'Cicero', '1920', ...) * @param constraint The constraint to apply to the `field`, with respect to the given `value`. @@ -26,39 +19,7 @@ export class FilterMapBuilder { this.filterMap[field] = {}; } - // Edge case for numeric fields (i.e., year) - // Logically it would be valid for excluded values to overlap with gt/gte/lt/lte values. - // For instance, specifying 1950 <= year <= 2000, but also excluding year = 1950. - // But since we can only have one constraint for a given value, these would overwrite each other. - // To work around this, we adjust the resulting map to maintain the correct logic. - const currentConstraint = this.filterMap[field][value]; - if (currentConstraint && constraint === FilterConstraint.EXCLUDE) { - if (currentConstraint === FilterConstraint.GREATER_OR_EQUAL) { - // gte and exclude on the same value -> gt - this.filterMap[field][value] = FilterConstraint.GREATER_THAN; - return this; - } else if (currentConstraint === FilterConstraint.LESS_OR_EQUAL) { - // lte and exclude on the same value -> lt - this.filterMap[field][value] = FilterConstraint.LESS_THAN; - return this; - } else if ( - currentConstraint === FilterConstraint.GREATER_THAN || - currentConstraint === FilterConstraint.LESS_THAN - ) { - // gt/lt and exclude -> no additional filter - return this; - } - } - - // An 'include' constraint should always take precedence over a gt/gte/lt/lte constraint on the same value. - if (currentConstraint === FilterConstraint.INCLUDE) { - if (NUMERIC_CONSTRAINTS.includes(constraint)) { - // Don't overwrite the existing 'include' constraint - return this; - } - } - - // Otherwise, overwrite any existing filter for this value + // Overwrite any existing filter for this value this.filterMap[field][value] = constraint; return this; } @@ -93,7 +54,7 @@ export class FilterMapBuilder { /** * Adds all filters from an existing filter map to the one being built. - * Filters from the provided map may overwrite or modify existing filters already added to the builder. + * Filters from the provided map may overwrite existing filters already added to the builder. * @param map The FilterMap to merge into the one being built. */ mergeFilterMap(map: FilterMap): this { diff --git a/test/filter-map-builder.test.ts b/test/filter-map-builder.test.ts index 4157a10..b00f81c 100644 --- a/test/filter-map-builder.test.ts +++ b/test/filter-map-builder.test.ts @@ -89,54 +89,4 @@ describe('filter map builder', () => { baz: { boop: 'exc' }, }); }); - - it('correctly resolves overlap between exclude and gte constraints', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('year', '1950', FilterConstraint.GREATER_OR_EQUAL); - expect(builder.build()).to.deep.equal({ year: { 1950: 'gte' } }); - - builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); - // Overwrites constraint with GREATER_THAN - expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } }); - }); - - it('correctly resolves overlap between exclude and lte constraints', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('year', '1950', FilterConstraint.LESS_OR_EQUAL); - expect(builder.build()).to.deep.equal({ year: { 1950: 'lte' } }); - - builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); - // Overwrites constraint with LESS_THAN - expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } }); - }); - - it('correctly resolves overlap between exclude and gt constraints', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('year', '1950', FilterConstraint.GREATER_THAN); - expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } }); - - builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); - // Leaves constraint unchanged - expect(builder.build()).to.deep.equal({ year: { 1950: 'gt' } }); - }); - - it('correctly resolves overlap between exclude and lt constraints', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('year', '1950', FilterConstraint.LESS_THAN); - expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } }); - - builder.addFilter('year', '1950', FilterConstraint.EXCLUDE); - // Leaves constraint unchanged - expect(builder.build()).to.deep.equal({ year: { 1950: 'lt' } }); - }); - - it('correctly resolves overlap between include and numeric constraints', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('year', '1950', FilterConstraint.INCLUDE); - expect(builder.build()).to.deep.equal({ year: { 1950: 'inc' } }); - - builder.addFilter('year', '1950', FilterConstraint.GREATER_THAN); - // Existing include constraint takes precedence - expect(builder.build()).to.deep.equal({ year: { 1950: 'inc' } }); - }); }); From e0422f18b3baa858d3882fdd5b6f6e6ee6affa16 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Fri, 18 Nov 2022 17:50:05 -0800 Subject: [PATCH 19/21] Support filter constraints being arrays or single values --- demo/app-root.ts | 24 ++++++---- src/filter-map-builder.ts | 80 ++++++++++++++++++++++++++++----- src/search-params.ts | 8 ++-- test/filter-map-builder.test.ts | 64 ++++++++++++++++++++++---- 4 files changed, 145 insertions(+), 31 deletions(-) diff --git a/demo/app-root.ts b/demo/app-root.ts index 77bf12d..46edb7a 100644 --- a/demo/app-root.ts +++ b/demo/app-root.ts @@ -262,15 +262,13 @@ export class AppRoot extends LitElement { private removeFilterClicked(e: Event) { const target = e.target as HTMLButtonElement; - const { field, value } = target.dataset; + const { field, value, constraint } = target.dataset; - if (field && value) { - this.filterMap = { ...this.filterMap }; - delete this.filterMap[field][value]; - - if (Object.keys(this.filterMap[field]).length === 0) { - delete this.filterMap[field]; - } + if (field && value && constraint) { + this.filterMap = new FilterMapBuilder() + .setFilterMap(this.filterMap) + .removeSingleFilter(field, value, constraint as FilterConstraint) + .build(); } } @@ -278,7 +276,14 @@ export class AppRoot extends LitElement { const filtersArray: SingleFilter[] = []; for (const [field, filters] of Object.entries(this.filterMap)) { for (const [value, constraint] of Object.entries(filters)) { - filtersArray.push({ field, value, constraint }); + // The constraint may be either a single item or an array + if (Array.isArray(constraint)) { + for (const subConstraint of constraint) { + filtersArray.push({ field, value, constraint: subConstraint }); + } + } else { + filtersArray.push({ field, value, constraint }); + } } } @@ -305,6 +310,7 @@ export class AppRoot extends LitElement { class="remove-filter" data-field=${field} data-value=${value} + data-constraint=${constraint} @click=${this.removeFilterClicked} > x diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts index a30101c..1187814 100644 --- a/src/filter-map-builder.ts +++ b/src/filter-map-builder.ts @@ -8,7 +8,8 @@ export class FilterMapBuilder { /** * Adds a filter to the FilterMap under construction. - * The new filter may overwrite existing filters already added to the builder. + * If existing constraint(s) already exist for this field and value, then the old and new constraints + * will be joined into a single array. * @param field The field to filter on (e.g., 'subject', 'year', ...) * @param value The value of the field to filter on (e.g., 'Cicero', '1920', ...) * @param constraint The constraint to apply to the `field`, with respect to the given `value`. @@ -19,24 +20,74 @@ export class FilterMapBuilder { this.filterMap[field] = {}; } - // Overwrite any existing filter for this value - this.filterMap[field][value] = constraint; + // If there are already constraints for this value, concat them into an array + if (this.filterMap[field][value]) { + const mergedConstraints = ([] as FilterConstraint[]).concat( + this.filterMap[field][value], + constraint + ); + + // Ensure there are no duplicate constraints in the array + this.filterMap[field][value] = Array.from(new Set(mergedConstraints)); + } else { + // Otherwise just use the provided value + this.filterMap[field][value] = constraint; + } + return this; } /** - * Removes any filter currently associated with the given field and value. + * Removes a single filter currently associated with the given field, value, and constraint type. * @param field The field to remove a filter for * @param value The value to remove the filter for + * @param constraint The constraint type to remove for this field and value */ - removeFilter(field: string, value: string): this { - if (this.filterMap[field]) { + removeSingleFilter( + field: string, + value: string, + constraint: FilterConstraint + ): this { + if (!this.filterMap[field]?.[value]) return this; + + const constraints = ([] as FilterConstraint[]).concat( + this.filterMap[field][value] + ); + const constraintIndex = constraints.indexOf(constraint); + if (constraintIndex >= 0) { + constraints.splice(constraintIndex, 1); + } + + // 2 or more constraints -> leave as array + // 1 constraint -> pull out single constraint + // 0 constraints -> delete the value entirely + this.filterMap[field][value] = + constraints.length === 1 ? constraints[0] : constraints; + if (constraints.length === 0) { delete this.filterMap[field][value]; + } - // If there are no remaining filters for this field, delete the whole field object. - if (Object.keys(this.filterMap[field]).length === 0) { - delete this.filterMap[field]; - } + // If there are no remaining filters for this field, delete the whole field object. + if (Object.keys(this.filterMap[field]).length === 0) { + delete this.filterMap[field]; + } + + return this; + } + + /** + * Removes any filters currently associated with the given field and value. + * @param field The field to remove a filter for + * @param value The value to remove the filter for + */ + removeFilters(field: string, value: string): this { + if (!this.filterMap[field]) return this; + + delete this.filterMap[field][value]; + + // If there are no remaining filters for this field, delete the whole field object. + if (Object.keys(this.filterMap[field]).length === 0) { + delete this.filterMap[field]; } return this; @@ -60,7 +111,14 @@ export class FilterMapBuilder { mergeFilterMap(map: FilterMap): this { for (const [field, filters] of Object.entries(map)) { for (const [value, constraint] of Object.entries(filters)) { - this.addFilter(field, value, constraint); + // There may be either a single constraint or an array of them + if (Array.isArray(constraint)) { + for (const subConstraint of constraint) { + this.addFilter(field, value, subConstraint); + } + } else { + this.addFilter(field, value, constraint); + } } } return this; diff --git a/src/search-params.ts b/src/search-params.ts index 4266b97..d0326bf 100644 --- a/src/search-params.ts +++ b/src/search-params.ts @@ -114,13 +114,15 @@ export enum FilterConstraint { } /** - * A filter mapping a field value to the type of constraint that it should impose on results. + * A filter mapping a field value to the type of constraint(s) that it should impose on results. + * Multiple constraints for the same value may be provided as an array. * - * Some examples (where the values are members of `FilterConstraint`): + * Some examples (where the property values are members of `FilterConstraint`): * - `{ 'puppies': INCLUDE }` * - `{ '1950': GREATER_OR_EQUAL, '1970': LESS_OR_EQUAL }` + * - `{ '1950': [ GREATER_OR_EQUAL, EXCLUDE ] }` */ -export type FieldFilter = Record; +export type FieldFilter = Record; /** * A map of fields (e.g., 'year', 'subject', ...) to the filters that should be diff --git a/test/filter-map-builder.test.ts b/test/filter-map-builder.test.ts index b00f81c..0341179 100644 --- a/test/filter-map-builder.test.ts +++ b/test/filter-map-builder.test.ts @@ -25,6 +25,17 @@ describe('filter map builder', () => { }); }); + it('can add multiple constraints for one value', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); + expect(builder.build()).to.deep.equal({ foo: { bar: 'inc' } }); + + builder.addFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); + expect(builder.build()).to.deep.equal({ + foo: { bar: ['inc', 'gte'] }, + }); + }); + it('can remove filters', () => { const builder = new FilterMapBuilder(); builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); @@ -35,19 +46,53 @@ describe('filter map builder', () => { baz: { boop: 'exc' }, }); - builder.removeFilter('foo', 'bar'); + builder.removeFilters('foo', 'bar'); expect(builder.build()).to.deep.equal({ foo: { beep: 'gt' }, baz: { boop: 'exc' }, }); - builder.removeFilter('foo', 'beep'); + builder.removeFilters('foo', 'beep'); expect(builder.build()).to.deep.equal({ baz: { boop: 'exc' } }); - builder.removeFilter('not', 'exist'); + builder.removeFilters('not', 'exist'); expect(builder.build()).to.deep.equal({ baz: { boop: 'exc' } }); - builder.removeFilter('baz', 'boop'); + builder.removeFilters('baz', 'boop'); + expect(builder.build()).to.deep.equal({}); + }); + + it('can remove single filters by constraint type', () => { + const builder = new FilterMapBuilder(); + builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); + builder.addFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); + builder.addFilter('baz', 'boop', FilterConstraint.EXCLUDE); + expect(builder.build()).to.deep.equal({ + foo: { bar: ['inc', 'gte'] }, + baz: { boop: 'exc' }, + }); + + builder.removeSingleFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc' }, + baz: { boop: 'exc' }, + }); + + builder.removeSingleFilter('foo', 'bar', FilterConstraint.EXCLUDE); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'inc' }, + baz: { boop: 'exc' }, + }); + + builder.removeSingleFilter('foo', 'bar', FilterConstraint.INCLUDE); + expect(builder.build()).to.deep.equal({ + baz: { boop: 'exc' }, + }); + + builder.removeSingleFilter('baz', 'boop', FilterConstraint.EXCLUDE); + expect(builder.build()).to.deep.equal({}); + + builder.removeSingleFilter('not', 'exist', FilterConstraint.INCLUDE); expect(builder.build()).to.deep.equal({}); }); @@ -76,17 +121,20 @@ describe('filter map builder', () => { bar: FilterConstraint.INCLUDE, }, baz: { - boop: FilterConstraint.EXCLUDE, + boop: [FilterConstraint.EXCLUDE, FilterConstraint.LESS_OR_EQUAL], }, }; + builder.addFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); builder.addFilter('foo', 'beep', FilterConstraint.LESS_OR_EQUAL); - expect(builder.build()).to.deep.equal({ foo: { beep: 'lte' } }); + expect(builder.build()).to.deep.equal({ + foo: { bar: 'gte', beep: 'lte' }, + }); builder.mergeFilterMap(filterMap); expect(builder.build()).to.deep.equal({ - foo: { bar: 'inc', beep: 'lte' }, - baz: { boop: 'exc' }, + foo: { bar: ['gte', 'inc'], beep: 'lte' }, + baz: { boop: ['exc', 'lte'] }, }); }); }); From 5cedba433c620268dec47c470e4ded181cc6d3d2 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Mon, 21 Nov 2022 10:56:34 -0800 Subject: [PATCH 20/21] Extract duplicate filter map builder logic into helper method --- src/filter-map-builder.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/filter-map-builder.ts b/src/filter-map-builder.ts index 1187814..1495fd7 100644 --- a/src/filter-map-builder.ts +++ b/src/filter-map-builder.ts @@ -67,11 +67,7 @@ export class FilterMapBuilder { delete this.filterMap[field][value]; } - // If there are no remaining filters for this field, delete the whole field object. - if (Object.keys(this.filterMap[field]).length === 0) { - delete this.filterMap[field]; - } - + this.deleteFieldIfEmpty(field); return this; } @@ -84,13 +80,16 @@ export class FilterMapBuilder { if (!this.filterMap[field]) return this; delete this.filterMap[field][value]; + this.deleteFieldIfEmpty(field); + return this; + } - // If there are no remaining filters for this field, delete the whole field object. - if (Object.keys(this.filterMap[field]).length === 0) { + /** If there are no remaining filters for this field, deletes the whole field object. */ + private deleteFieldIfEmpty(field: string): void { + const filters = this.filterMap[field]; + if (filters && Object.keys(filters).length === 0) { delete this.filterMap[field]; } - - return this; } /** From 563a8954b99b79b6c807c82dcef2da1505902806 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Mon, 21 Nov 2022 12:21:26 -0800 Subject: [PATCH 21/21] Extract repeated filter map builder test prep into helper --- test/filter-map-builder.test.ts | 49 +++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/test/filter-map-builder.test.ts b/test/filter-map-builder.test.ts index 0341179..711e1b8 100644 --- a/test/filter-map-builder.test.ts +++ b/test/filter-map-builder.test.ts @@ -2,6 +2,29 @@ import { expect } from '@open-wc/testing'; import { FilterMapBuilder } from '../src/filter-map-builder'; import { FilterConstraint, FilterMap } from '../src/search-params'; +/** + * Creates a filter map builder with the following structure: + * ``` + * { + * 'foo': { + * 'bar': [INCLUDE, GREATER_OR_EQUAL], + * 'beep': GREATER_THAN, + * }, + * 'baz': { + * 'boop': EXCLUDE + * }, + * } + * ``` + */ +const getComplexFilterMapBuilder = (): FilterMapBuilder => { + const builder = new FilterMapBuilder(); + builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); + builder.addFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); + builder.addFilter('baz', 'boop', FilterConstraint.EXCLUDE); + builder.addFilter('foo', 'beep', FilterConstraint.GREATER_THAN); + return builder; +}; + describe('filter map builder', () => { it('initializes with empty filter map', () => { expect(new FilterMapBuilder().build()).to.deep.equal({}); @@ -37,15 +60,7 @@ describe('filter map builder', () => { }); it('can remove filters', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); - builder.addFilter('baz', 'boop', FilterConstraint.EXCLUDE); - builder.addFilter('foo', 'beep', FilterConstraint.GREATER_THAN); - expect(builder.build()).to.deep.equal({ - foo: { bar: 'inc', beep: 'gt' }, - baz: { boop: 'exc' }, - }); - + const builder = getComplexFilterMapBuilder(); builder.removeFilters('foo', 'bar'); expect(builder.build()).to.deep.equal({ foo: { beep: 'gt' }, @@ -63,32 +78,26 @@ describe('filter map builder', () => { }); it('can remove single filters by constraint type', () => { - const builder = new FilterMapBuilder(); - builder.addFilter('foo', 'bar', FilterConstraint.INCLUDE); - builder.addFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); - builder.addFilter('baz', 'boop', FilterConstraint.EXCLUDE); - expect(builder.build()).to.deep.equal({ - foo: { bar: ['inc', 'gte'] }, - baz: { boop: 'exc' }, - }); - + const builder = getComplexFilterMapBuilder(); builder.removeSingleFilter('foo', 'bar', FilterConstraint.GREATER_OR_EQUAL); expect(builder.build()).to.deep.equal({ - foo: { bar: 'inc' }, + foo: { bar: 'inc', beep: 'gt' }, baz: { boop: 'exc' }, }); builder.removeSingleFilter('foo', 'bar', FilterConstraint.EXCLUDE); expect(builder.build()).to.deep.equal({ - foo: { bar: 'inc' }, + foo: { bar: 'inc', beep: 'gt' }, baz: { boop: 'exc' }, }); builder.removeSingleFilter('foo', 'bar', FilterConstraint.INCLUDE); expect(builder.build()).to.deep.equal({ + foo: { beep: 'gt' }, baz: { boop: 'exc' }, }); + builder.removeSingleFilter('foo', 'beep', FilterConstraint.GREATER_THAN); builder.removeSingleFilter('baz', 'boop', FilterConstraint.EXCLUDE); expect(builder.build()).to.deep.equal({});